feat(variety): implement C3 warning cards with recipe names and swap links (V1) #51

New Issue

marcel · 2026-04-09T16:31:58+02:00

marcel commented

2026-04-09 16:31:58 +02:00

Überblick

Die /planner/variety Seite zeigt derzeit technische Tag-Codes in Warnkarten (MON, WED — erwäge einen Tausch). Der Planer muss selbst nachschlagen, welches Gericht betroffen ist, und dann manuell zum Planer navigieren.

V1 "Erweiterte Karten" löst dies mit minimalem Aufwand: Warnkarten erhalten strukturierte Zeilen pro betroffenem Tag — mit Wochentag-Abkürzung, Rezeptname und direktem "Tauschen →" Link.

Spec: specs/frontend/c3-variety-rework-v1-spec.html (auf master)
Mockups: specs/frontend/c3-variety-rework.html (auf master)

Scope

Kein neues Backend-Endpoint — alle Daten sind im bestehenden weekPlan-Load
Kein Layout-Umbau — Score-Hero, Bewertungsdetails und Protein-Grid bleiben unverändert
VarietyWarningCards.svelte ist bereits auf das neue ActionWarning-Format aktualisiert

Betroffene Dateien

Datei	Änderung
`frontend/src/routes/(app)/planner/variety/+page.svelte`	`DAY_SHORT`-Konstante, `slotsByDay`-Derived, `actionWarnings`-Derived, Template-Update (2×), Import-Bereinigung
`frontend/src/lib/planner/VarietyWarningCards.svelte`	Bereits aktualisiert — keine Änderung
`frontend/src/lib/planner/variety.ts`	Keine — `computeWarnings` bleibt, aber Import wird entfernt

Implementierungsschritte

Schritt 1 — DAY_SHORT-Konstante in +page.svelte ergänzen:

const DAY_SHORT: Record<string, string> = {
  MON: 'Mo', TUE: 'Di', WED: 'Mi',
  THU: 'Do', FRI: 'Fr', SAT: 'Sa', SUN: 'So'
};

Schritt 2 — slotsByDay-Derived aufbauen (nach bestehenden $derived-Deklarationen):

let slotsByDay = $derived.by(() => {
  const map: Record<string, { slotId: number; recipeName: string }> = {};
  for (const slot of weekPlan?.slots ?? []) {
    if (slot.dayOfWeek && slot.recipe?.name && slot.id) {
      map[slot.dayOfWeek] = { slotId: slot.id, recipeName: slot.recipe.name };
    }
  }
  return map;
});

Schritt 3 — actionWarnings-Derived ersetzen computeWarnings():

interface WarningItem { dayShort: string; recipeName: string; slotId: number; }
interface ActionWarning { title: string; items: WarningItem[]; }

let actionWarnings = $derived.by((): ActionWarning[] => {
  const result: ActionWarning[] = [];
  const vs = varietyScore;
  if (!vs) return result;

  for (const repeat of vs.tagRepeats ?? []) {
    if ((repeat.days?.length ?? 0) < 2) continue;
    const items: WarningItem[] = (repeat.days ?? [])
      .map((day) => {
        const slot = slotsByDay[day];
        return slot ? { dayShort: DAY_SHORT[day] ?? day, recipeName: slot.recipeName, slotId: slot.slotId } : null;
      })
      .filter((x): x is WarningItem => x !== null);
    if (items.length > 0) result.push({ title: `${repeat.tagName} mehrfach diese Woche`, items });
  }

  for (const overlap of vs.ingredientOverlaps ?? []) {
    if ((overlap.days?.length ?? 0) < 2) continue;
    const items: WarningItem[] = (overlap.days ?? [])
      .map((day) => {
        const slot = slotsByDay[day];
        return slot ? { dayShort: DAY_SHORT[day] ?? day, recipeName: slot.recipeName, slotId: slot.slotId } : null;
      })
      .filter((x): x is WarningItem => x !== null);
    if (items.length > 0) result.push({ title: `${overlap.ingredientName} in mehreren Gerichten`, items });
  }

  for (const name of vs.duplicatesInPlan ?? []) {
    const items: WarningItem[] = Object.entries(slotsByDay)
      .filter(([, s]) => s.recipeName === name)
      .map(([day, s]) => ({ dayShort: DAY_SHORT[day] ?? day, recipeName: s.recipeName, slotId: s.slotId }));
    if (items.length > 0) result.push({ title: `${name} doppelt geplant`, items });
  }

  return result;
});

Schritt 4 — Template (Mobile + Desktop, je 2 Stellen):

- {#if warnings.length > 0}
-   <VarietyWarningCards {warnings} />
+ {#if actionWarnings.length > 0}
+   <VarietyWarningCards warnings={actionWarnings} {weekStart} />

Schritt 5 — Import bereinigen:

- import { computeSubScores, computeWarnings } from '$lib/planner/variety';
+ import { computeSubScores } from '$lib/planner/variety';

Abnahmekriterien

AC-1: Warnkarte zeigt pro betroffenem Tag eine eigene Zeile (kein Erklärungstext mehr)
AC-2: Jede Zeile enthält die deutsche Wochentag-Abkürzung (Mo, Di, Mi, Do, Fr, Sa, So)
AC-3: Jede Zeile enthält den Namen des eingeplanten Rezepts
AC-4: Jede Zeile hat einen "Tauschen →" Link → /planner?week={weekStart}&swap={slotId}
AC-5: Tags mit nur einem betroffenen Tag erzeugen keine Warnkarte
AC-6: Score-Hero, Bewertungsdetails und Protein-Grid (Desktop) bleiben unverändert
AC-7: varietyScore === null → kein Warnkarten-Rendering (leere-Woche-State bleibt)
AC-8: Import von computeWarnings entfernt, TypeScript kompiliert fehlerfrei
AC-9: Tausch-Links touch-freundlich (mind. 44px Zeilenhöhe auf Mobile)

## Überblick Die `/planner/variety` Seite zeigt derzeit technische Tag-Codes in Warnkarten (`MON, WED — erwäge einen Tausch`). Der Planer muss selbst nachschlagen, welches Gericht betroffen ist, und dann manuell zum Planer navigieren. **V1 "Erweiterte Karten"** löst dies mit minimalem Aufwand: Warnkarten erhalten strukturierte Zeilen pro betroffenem Tag — mit Wochentag-Abkürzung, Rezeptname und direktem "Tauschen →" Link. Spec: `specs/frontend/c3-variety-rework-v1-spec.html` (auf master) Mockups: `specs/frontend/c3-variety-rework.html` (auf master) --- ## Scope - Kein neues Backend-Endpoint — alle Daten sind im bestehenden weekPlan-Load - Kein Layout-Umbau — Score-Hero, Bewertungsdetails und Protein-Grid bleiben unverändert - `VarietyWarningCards.svelte` ist bereits auf das neue `ActionWarning`-Format aktualisiert --- ## Betroffene Dateien | Datei | Änderung | |---|---| | `frontend/src/routes/(app)/planner/variety/+page.svelte` | `DAY_SHORT`-Konstante, `slotsByDay`-Derived, `actionWarnings`-Derived, Template-Update (2×), Import-Bereinigung | | `frontend/src/lib/planner/VarietyWarningCards.svelte` | Bereits aktualisiert — keine Änderung | | `frontend/src/lib/planner/variety.ts` | Keine — `computeWarnings` bleibt, aber Import wird entfernt | --- ## Implementierungsschritte **Schritt 1** — `DAY_SHORT`-Konstante in `+page.svelte` ergänzen: ```ts const DAY_SHORT: Record<string, string> = { MON: 'Mo', TUE: 'Di', WED: 'Mi', THU: 'Do', FRI: 'Fr', SAT: 'Sa', SUN: 'So' }; ``` **Schritt 2** — `slotsByDay`-Derived aufbauen (nach bestehenden `$derived`-Deklarationen): ```ts let slotsByDay = $derived.by(() => { const map: Record<string, { slotId: number; recipeName: string }> = {}; for (const slot of weekPlan?.slots ?? []) { if (slot.dayOfWeek && slot.recipe?.name && slot.id) { map[slot.dayOfWeek] = { slotId: slot.id, recipeName: slot.recipe.name }; } } return map; }); ``` **Schritt 3** — `actionWarnings`-Derived ersetzen `computeWarnings()`: ```ts interface WarningItem { dayShort: string; recipeName: string; slotId: number; } interface ActionWarning { title: string; items: WarningItem[]; } let actionWarnings = $derived.by((): ActionWarning[] => { const result: ActionWarning[] = []; const vs = varietyScore; if (!vs) return result; for (const repeat of vs.tagRepeats ?? []) { if ((repeat.days?.length ?? 0) < 2) continue; const items: WarningItem[] = (repeat.days ?? []) .map((day) => { const slot = slotsByDay[day]; return slot ? { dayShort: DAY_SHORT[day] ?? day, recipeName: slot.recipeName, slotId: slot.slotId } : null; }) .filter((x): x is WarningItem => x !== null); if (items.length > 0) result.push({ title: `${repeat.tagName} mehrfach diese Woche`, items }); } for (const overlap of vs.ingredientOverlaps ?? []) { if ((overlap.days?.length ?? 0) < 2) continue; const items: WarningItem[] = (overlap.days ?? []) .map((day) => { const slot = slotsByDay[day]; return slot ? { dayShort: DAY_SHORT[day] ?? day, recipeName: slot.recipeName, slotId: slot.slotId } : null; }) .filter((x): x is WarningItem => x !== null); if (items.length > 0) result.push({ title: `${overlap.ingredientName} in mehreren Gerichten`, items }); } for (const name of vs.duplicatesInPlan ?? []) { const items: WarningItem[] = Object.entries(slotsByDay) .filter(([, s]) => s.recipeName === name) .map(([day, s]) => ({ dayShort: DAY_SHORT[day] ?? day, recipeName: s.recipeName, slotId: s.slotId })); if (items.length > 0) result.push({ title: `${name} doppelt geplant`, items }); } return result; }); ``` **Schritt 4** — Template (Mobile + Desktop, je 2 Stellen): ```diff - {#if warnings.length > 0} - <VarietyWarningCards {warnings} /> + {#if actionWarnings.length > 0} + <VarietyWarningCards warnings={actionWarnings} {weekStart} /> ``` **Schritt 5** — Import bereinigen: ```diff - import { computeSubScores, computeWarnings } from '$lib/planner/variety'; + import { computeSubScores } from '$lib/planner/variety'; ``` --- ## Abnahmekriterien - [ ] AC-1: Warnkarte zeigt pro betroffenem Tag eine eigene Zeile (kein Erklärungstext mehr) - [ ] AC-2: Jede Zeile enthält die deutsche Wochentag-Abkürzung (Mo, Di, Mi, Do, Fr, Sa, So) - [ ] AC-3: Jede Zeile enthält den Namen des eingeplanten Rezepts - [ ] AC-4: Jede Zeile hat einen "Tauschen →" Link → `/planner?week={weekStart}&swap={slotId}` - [ ] AC-5: Tags mit nur einem betroffenen Tag erzeugen keine Warnkarte - [ ] AC-6: Score-Hero, Bewertungsdetails und Protein-Grid (Desktop) bleiben unverändert - [ ] AC-7: `varietyScore === null` → kein Warnkarten-Rendering (leere-Woche-State bleibt) - [ ] AC-8: Import von `computeWarnings` entfernt, TypeScript kompiliert fehlerfrei - [ ] AC-9: Tausch-Links touch-freundlich (mind. 44px Zeilenhöhe auf Mobile)

marcel commented

2026-04-09 21:04:27 +02:00

🧑‍💻 Kai — Frontend Engineer

Questions & Observations

$derived.by() usage is correct: Using $derived.by() for the two derived maps is the right Svelte 5 pattern. No concerns there.
TypeScript interfaces inline in +page.svelte: WarningItem and ActionWarning are defined directly in the page file. If VarietyWarningCards.svelte also needs to reference these types (for its props), they'll need to live in a shared types.ts or a dedicated variety.types.ts. Define them there from the start rather than inlining and moving later.
weekStart prop added to VarietyWarningCards: The template diff adds {weekStart} as a new prop. Where does weekStart come from in the page? Confirm it's already returned by the server load function. If it isn't, that's a hidden backend dependency — surface it before implementation starts.
DAY_SHORT as a module-level constant: This is a static lookup table. Declare it as a const outside the component (module scope), not inside the <script> block. It's not reactive and doesn't need to be re-evaluated per render.
Swap link construction: The swap link /planner?week={weekStart}&swap={slotId} is assembled as a template string. Ensure weekStart is formatted as a URL-safe date string (ISO format YYYY-MM-DD) — not a locale-formatted string or a Date object. Add a note in the AC or implementation steps to clarify the expected format.
Null-safe slot lookup: The slotsByDay derivation correctly filters slots where slot.dayOfWeek && slot.recipe?.name && slot.id. But confirm that slot IDs in the weekPlan response are always numeric and never 0 (falsy). A slot.id === 0 would silently drop a valid slot.

Suggestions

Move WarningItem and ActionWarning interface declarations to $lib/planner/types.ts so VarietyWarningCards.svelte can import them for its props type.
Write variety-page.test.ts red-first. Priority tests: slotsByDay builds correctly from fixture data, actionWarnings filters out single-day warnings (AC-5), actionWarnings returns [] when varietyScore === null (AC-7).

## 🧑‍💻 Kai — Frontend Engineer ### Questions & Observations - **`$derived.by()` usage is correct**: Using `$derived.by()` for the two derived maps is the right Svelte 5 pattern. No concerns there. - **TypeScript interfaces inline in `+page.svelte`**: `WarningItem` and `ActionWarning` are defined directly in the page file. If `VarietyWarningCards.svelte` also needs to reference these types (for its props), they'll need to live in a shared `types.ts` or a dedicated `variety.types.ts`. Define them there from the start rather than inlining and moving later. - **`weekStart` prop added to `VarietyWarningCards`**: The template diff adds `{weekStart}` as a new prop. Where does `weekStart` come from in the page? Confirm it's already returned by the server load function. If it isn't, that's a hidden backend dependency — surface it before implementation starts. - **`DAY_SHORT` as a module-level constant**: This is a static lookup table. Declare it as a `const` outside the component (module scope), not inside the `<script>` block. It's not reactive and doesn't need to be re-evaluated per render. - **Swap link construction**: The swap link `/planner?week={weekStart}&swap={slotId}` is assembled as a template string. Ensure `weekStart` is formatted as a URL-safe date string (ISO format `YYYY-MM-DD`) — not a locale-formatted string or a Date object. Add a note in the AC or implementation steps to clarify the expected format. - **Null-safe slot lookup**: The `slotsByDay` derivation correctly filters slots where `slot.dayOfWeek && slot.recipe?.name && slot.id`. But confirm that slot IDs in the weekPlan response are always numeric and never `0` (falsy). A `slot.id === 0` would silently drop a valid slot. ### Suggestions - Move `WarningItem` and `ActionWarning` interface declarations to `$lib/planner/types.ts` so `VarietyWarningCards.svelte` can import them for its props type. - Write `variety-page.test.ts` red-first. Priority tests: `slotsByDay` builds correctly from fixture data, `actionWarnings` filters out single-day warnings (AC-5), `actionWarnings` returns `[]` when `varietyScore === null` (AC-7).

marcel commented

2026-04-09 21:04:44 +02:00

🎨 Atlas — UI/UX Designer

Questions & Observations

"Tauschen →" link visual treatment: The spec mentions a "Tauschen →" link per row but doesn't specify the visual pattern. Is this an <a> styled as a text link, a ghost button, or an inline chevron-icon button? The design system has role="button" patterns and explicit button sizing rules (13px, weight 500, tracking 0.04em). Clarify whether this follows the button spec or the link spec — they have different visual weights.
→ arrow glyph: The spec uses a literal → Unicode character. Is this intentional (typographic arrow), or should it be the design system's chevron SVG icon? SVG icons scale cleanly and can be coloured with currentColor; the Unicode arrow is lighter to implement but less consistent with the rest of the icon vocabulary.
Row layout inside warning cards: Each row has: day abbreviation + recipe name + swap link. What's the layout? Left-aligned day abbreviation (fixed width?), then recipe name (truncating at one line?), then swap link pushed to the right? The spec body describes the content but not the layout mechanics. Define this explicitly to avoid per-implementer variation.
Touch target for swap link (AC-9): The AC requires ≥ 44px row height on mobile. A text-link-styled <a> with default padding rarely hits 44px. The row itself should set min-height: 44px with display: flex; align-items: center to guarantee this — don't rely on the link alone.
Warning card title typography: Titles like "Tofu mehrfach diese Woche" or "Nudeln in mehreren Gerichten" are dynamically generated. What's the max reasonable length? At narrow mobile widths, a long ingredient name + suffix could wrap to 3 lines. Confirm the design handles multi-line card titles gracefully.

Suggestions

Define a specific row structure in the spec (or reference an existing spec for similar list-row patterns in the app) so the implementer doesn't have to guess the layout.
The → in "Tauschen →" could be replaced by the standard chevron icon used elsewhere in the app for navigation affordance — keeps the visual language consistent without adding a new glyph.

## 🎨 Atlas — UI/UX Designer ### Questions & Observations - **"Tauschen →" link visual treatment**: The spec mentions a "Tauschen →" link per row but doesn't specify the visual pattern. Is this an `<a>` styled as a text link, a ghost button, or an inline chevron-icon button? The design system has `role="button"` patterns and explicit button sizing rules (13px, weight 500, tracking 0.04em). Clarify whether this follows the button spec or the link spec — they have different visual weights. - **`→` arrow glyph**: The spec uses a literal `→` Unicode character. Is this intentional (typographic arrow), or should it be the design system's chevron SVG icon? SVG icons scale cleanly and can be coloured with currentColor; the Unicode arrow is lighter to implement but less consistent with the rest of the icon vocabulary. - **Row layout inside warning cards**: Each row has: day abbreviation + recipe name + swap link. What's the layout? Left-aligned day abbreviation (fixed width?), then recipe name (truncating at one line?), then swap link pushed to the right? The spec body describes the content but not the layout mechanics. Define this explicitly to avoid per-implementer variation. - **Touch target for swap link (AC-9)**: The AC requires ≥ 44px row height on mobile. A text-link-styled `<a>` with default padding rarely hits 44px. The row itself should set `min-height: 44px` with `display: flex; align-items: center` to guarantee this — don't rely on the link alone. - **Warning card title typography**: Titles like "Tofu mehrfach diese Woche" or "Nudeln in mehreren Gerichten" are dynamically generated. What's the max reasonable length? At narrow mobile widths, a long ingredient name + suffix could wrap to 3 lines. Confirm the design handles multi-line card titles gracefully. ### Suggestions - Define a specific row structure in the spec (or reference an existing spec for similar list-row patterns in the app) so the implementer doesn't have to guess the layout. - The `→` in "Tauschen →" could be replaced by the standard chevron icon used elsewhere in the app for navigation affordance — keeps the visual language consistent without adding a new glyph.

marcel commented

2026-04-09 21:05:01 +02:00

🧪 QA Engineer — Test Coverage

Questions & Observations

AC-5 is the sharpest edge case: Tags with only one affected day must produce no warning card. Test this explicitly with a fixture where repeat.days.length === 1 — a common off-by-one when the filter uses < 2 vs <= 1.
AC-7 — null varietyScore: The actionWarnings derivation returns [] when vs is falsy. Test this with varietyScore === null and also varietyScore === undefined (if the load can return either).
Empty items after slot lookup: The slotsByDay filter only includes slots that have a dayOfWeek, recipe.name, and id. A warning could reference a day that has no recipe (slot empty or not yet planned). The .filter(x => x !== null) handles this, but test the case where all items for a warning group are null — items.length === 0 should not push to result. The current implementation handles this (if (items.length > 0)), but add an explicit test for it.
duplicatesInPlan logic: The duplicate recipe detection uses Object.entries(slotsByDay).filter(([, s]) => s.recipeName === name). Test with a week that has the same recipe on 3 days — verify all 3 appear as items, not just the first match.
DAY_SHORT fallback: The mapping uses DAY_SHORT[day] ?? day — if an unknown day code appears, it falls back to the raw code. Test with an unexpected day value (e.g., "HOLIDAY") to confirm graceful fallback.
Swap link URL format: Write a test that asserts the generated href is /planner?week=2026-04-07&swap=42 (exact format) — not just that a link exists. This catches weekStart format regressions early.

Suggestions

Extract the actionWarnings derivation logic into a pure function (computeActionWarnings(varietyScore, slotsByDay, weekStart)) and test it directly as a unit test — no component mount required. This is faster and more precise than testing through a rendered component.
Add data-testid="warning-swap-link" to each "Tauschen →" anchor for reliable selection in Playwright and Testing Library tests.
Write one E2E test: variety page with a seeded plan that has a repeated protein → warning card visible with correct recipe name → click "Tauschen →" → planner opens with swap drawer pre-selected.

## 🧪 QA Engineer — Test Coverage ### Questions & Observations - **AC-5 is the sharpest edge case**: Tags with only one affected day must produce *no* warning card. Test this explicitly with a fixture where `repeat.days.length === 1` — a common off-by-one when the filter uses `< 2` vs `<= 1`. - **AC-7 — null varietyScore**: The `actionWarnings` derivation returns `[]` when `vs` is falsy. Test this with `varietyScore === null` and also `varietyScore === undefined` (if the load can return either). - **Empty `items` after slot lookup**: The `slotsByDay` filter only includes slots that have a `dayOfWeek`, `recipe.name`, and `id`. A warning could reference a day that has no recipe (slot empty or not yet planned). The `.filter(x => x !== null)` handles this, but test the case where *all* items for a warning group are null — `items.length === 0` should not push to `result`. The current implementation handles this (`if (items.length > 0)`), but add an explicit test for it. - **`duplicatesInPlan` logic**: The duplicate recipe detection uses `Object.entries(slotsByDay).filter(([, s]) => s.recipeName === name)`. Test with a week that has the same recipe on 3 days — verify all 3 appear as items, not just the first match. - **`DAY_SHORT` fallback**: The mapping uses `DAY_SHORT[day] ?? day` — if an unknown day code appears, it falls back to the raw code. Test with an unexpected day value (e.g., `"HOLIDAY"`) to confirm graceful fallback. - **Swap link URL format**: Write a test that asserts the generated `href` is `/planner?week=2026-04-07&swap=42` (exact format) — not just that a link exists. This catches `weekStart` format regressions early. ### Suggestions - Extract the `actionWarnings` derivation logic into a pure function (`computeActionWarnings(varietyScore, slotsByDay, weekStart)`) and test it directly as a unit test — no component mount required. This is faster and more precise than testing through a rendered component. - Add `data-testid="warning-swap-link"` to each "Tauschen →" anchor for reliable selection in Playwright and Testing Library tests. - Write one E2E test: variety page with a seeded plan that has a repeated protein → warning card visible with correct recipe name → click "Tauschen →" → planner opens with swap drawer pre-selected.

marcel commented

2026-04-09 21:05:16 +02:00

🔒 Sable — Security Engineer

Questions & Observations

slotId in swap link query param: The "Tauschen →" link passes ?swap={slotId} as a URL parameter. The planner page must validate server-side that the slotId belongs to the requesting user's household before opening the swap drawer. URL params are user-controlled — enumerate any slotId and you can attempt to swap it. This is an existing concern that this feature makes more prominent by adding direct links.
weekStart in swap link: /planner?week={weekStart} — weekStart is a date string used for navigation. Confirm the planner page validates this as a valid ISO date and ignores malformed values, rather than using it directly in a backend query without sanitization.
Data in slotsByDay: The slotsByDay map is derived from weekPlan data loaded server-side. Confirm the server load function returns only data belonging to the authenticated user's household — no cross-household slot IDs should ever reach the client.
No new API surface: This issue introduces no new endpoints — existing weekPlan load data is reused. This is a security positive. The only security-relevant output is the swap link, which is handled by the existing planner's server-side logic.
computeWarnings import removal: The old computeWarnings function is being removed from the import. Confirm no other code path still calls it with unvalidated data — unused imports can hide previously safe validation logic that was doing more than just display work.

Suggestions

Add an explicit note in the issue or a sub-task to audit the planner's ?swap= query param handler — confirm it validates household ownership of slotId and returns 403 (not 404 or a silent no-op) for unauthorized IDs.
No new concerns from a direct feature perspective — the risk surface here is in the downstream consumer of the swap link, not in this feature itself.

## 🔒 Sable — Security Engineer ### Questions & Observations - **`slotId` in swap link query param**: The "Tauschen →" link passes `?swap={slotId}` as a URL parameter. The planner page must validate server-side that the `slotId` belongs to the requesting user's household before opening the swap drawer. URL params are user-controlled — enumerate any slotId and you can attempt to swap it. This is an existing concern that this feature makes more prominent by adding direct links. - **`weekStart` in swap link**: `/planner?week={weekStart}` — `weekStart` is a date string used for navigation. Confirm the planner page validates this as a valid ISO date and ignores malformed values, rather than using it directly in a backend query without sanitization. - **Data in `slotsByDay`**: The `slotsByDay` map is derived from `weekPlan` data loaded server-side. Confirm the server load function returns only data belonging to the authenticated user's household — no cross-household slot IDs should ever reach the client. - **No new API surface**: This issue introduces no new endpoints — existing weekPlan load data is reused. This is a security positive. The only security-relevant output is the swap link, which is handled by the existing planner's server-side logic. - **`computeWarnings` import removal**: The old `computeWarnings` function is being removed from the import. Confirm no other code path still calls it with unvalidated data — unused imports can hide previously safe validation logic that was doing more than just display work. ### Suggestions - Add an explicit note in the issue or a sub-task to audit the planner's `?swap=` query param handler — confirm it validates household ownership of `slotId` and returns 403 (not 404 or a silent no-op) for unauthorized IDs. - No new concerns from a direct feature perspective — the risk surface here is in the downstream consumer of the swap link, not in this feature itself.

marcel commented

2026-04-09 21:05:30 +02:00

⚙️ Backend Engineer

Questions & Observations

No backend changes — confirmed: The derivation runs entirely on weekPlan data already returned by the load function. This is correct for V1 scope.
weekStart availability: The swap link requires a weekStart date string. Confirm the existing weekPlan API response already includes the week start date (or that +page.server.ts derives it from the plan). If not, a minor addition to the response DTO is needed before frontend work begins.
slot.id always present: The slotsByDay derivation filters on slot.id being truthy. Confirm the existing WeekPlanResponse DTO always includes slot IDs in its slots list. If slots can be returned without IDs (e.g., virtual or unsaved slots), the filtering logic silently drops them — that should be documented as intentional behaviour.
computeWarnings deprecation: The old computeWarnings function in variety.ts is being replaced by the new actionWarnings derived state. Is computeWarnings used anywhere else in the codebase (e.g., in tests, other pages)? Confirm via a grep before deleting the import — dead code is fine to remove, but broken imports produce build failures.
Future backend suggestion endpoint: When the suggestion ranking backend work happens (noted as a separate issue), the tagRepeats and ingredientOverlaps data structures returned by the variety score API will need to be stable. The frontend is now consuming repeat.days, repeat.tagName, overlap.days, overlap.ingredientName — changes to these field names will break this feature silently.

Suggestions

Document the expected response shape for tagRepeats and ingredientOverlaps in the variety score API spec (or OpenAPI schema). The frontend is now tightly coupled to these field names — a schema change without a corresponding frontend update will cause silent rendering failures, not a build error.

## ⚙️ Backend Engineer ### Questions & Observations - **No backend changes — confirmed**: The derivation runs entirely on weekPlan data already returned by the load function. This is correct for V1 scope. - **`weekStart` availability**: The swap link requires a `weekStart` date string. Confirm the existing weekPlan API response already includes the week start date (or that `+page.server.ts` derives it from the plan). If not, a minor addition to the response DTO is needed before frontend work begins. - **`slot.id` always present**: The `slotsByDay` derivation filters on `slot.id` being truthy. Confirm the existing `WeekPlanResponse` DTO always includes slot IDs in its slots list. If slots can be returned without IDs (e.g., virtual or unsaved slots), the filtering logic silently drops them — that should be documented as intentional behaviour. - **`computeWarnings` deprecation**: The old `computeWarnings` function in `variety.ts` is being replaced by the new `actionWarnings` derived state. Is `computeWarnings` used anywhere else in the codebase (e.g., in tests, other pages)? Confirm via a grep before deleting the import — dead code is fine to remove, but broken imports produce build failures. - **Future backend suggestion endpoint**: When the suggestion ranking backend work happens (noted as a separate issue), the `tagRepeats` and `ingredientOverlaps` data structures returned by the variety score API will need to be stable. The frontend is now consuming `repeat.days`, `repeat.tagName`, `overlap.days`, `overlap.ingredientName` — changes to these field names will break this feature silently. ### Suggestions - Document the expected response shape for `tagRepeats` and `ingredientOverlaps` in the variety score API spec (or OpenAPI schema). The frontend is now tightly coupled to these field names — a schema change without a corresponding frontend update will cause silent rendering failures, not a build error.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/mealprep#51