feat(planner): wire variety-aware suggestions into RecipePicker for empty slots #46

New Issue

marcel · 2026-04-09T10:53:07+02:00

marcel commented

2026-04-09 10:53:07 +02:00

Context

RecipePicker already renders a fully-built "Empfohlen · Beste Abwechslung" section when suggestions is non-empty:

Recipe name + metadata
Green badge ↑ +N Punkte when the recipe improves variety
Yellow badge ⚠ Variationskonflikt when it doesn't

Both call sites in +page.svelte (mobile bottom sheet + desktop panel) currently hardcode suggestions={[]}, so this section never appears. The UI capability exists; the data is not wired up.

Problem

When a user opens the recipe picker for an empty slot, all recipes are shown in a flat, unsorted list with no variety signal. The user has no way to know which choices would improve the week's variety score.

What exists today

Layer	Status
`RecipePicker.svelte` — Empfohlen section	✅ Built, renders when `suggestions` prop is non-empty
`GET /v1/week-plans/{id}/variety-preview?recipeId={id}&date={date}`	✅ Exists, returns `{ simulatedScore }` for one recipe
`GET /v1/week-plans/{id}/suggestions?date={date}` (batch)	❌ Does not exist
`+page.server.ts` — suggestions fetch	❌ Not wired; passes `suggestions={[]}` always

Proposed solution

Option A — New batch backend endpoint (recommended)

Add GET /v1/week-plans/{planId}/suggestions?date={date} to the backend:

Takes the plan ID + target date
Internally calls the existing variety-preview logic for all household recipes
Returns top N (e.g. 5) recipes sorted by simulatedScore descending, with the score included
Filters out recipes already planned for the target date (though this isn't currently possible for an empty slot)

Then in +page.server.ts:

Load suggestions lazily via a SvelteKit load dependency on selectedDay, or
Load them eagerly alongside the week plan (costs one extra request per page load but simpler)

In +page.svelte:

Pass real suggestions to both RecipePicker instances

Option B — N parallel frontend-server calls (simpler, no backend change)

In +page.server.ts, call variety-preview for every recipe in parallel and rank client-side. Feasible for small recipe libraries; may be slow at scale (100+ recipes = 100 requests).

Option A is preferred — one request instead of N, and the ranking logic lives server-side where it belongs.

Acceptance criteria

Opening RecipePicker for an empty slot shows a non-empty Empfohlen · Beste Abwechslung section (top 5 recipes by simulated variety score)
Each suggestion shows the correct ↑ +N Punkte or ⚠ Variationskonflikt badge
The existing Alle Rezepte list and search still work below the suggestions
Suggestions are specific to the target date (ingredient overlap is day-aware)
The swap flow (SwapSuggestionList) is not affected — it uses easiest-first sorting intentionally
Backend endpoint is covered by Spring Boot tests
RecipePicker suggestions rendering already has tests (via existing SuggestionCard.test.ts + RecipePicker.test.ts); add server load tests for the new fetch

Out of scope

Changing the swap flow sort order (that remains easiest-first per J4 spec)
Personalisation beyond ingredient overlap (e.g. user preference weighting)
Pagination of suggestions

## Context `RecipePicker` already renders a fully-built **"Empfohlen · Beste Abwechslung"** section when `suggestions` is non-empty: - Recipe name + metadata - Green badge `↑ +N Punkte` when the recipe improves variety - Yellow badge `⚠ Variationskonflikt` when it doesn't Both call sites in `+page.svelte` (mobile bottom sheet + desktop panel) currently hardcode `suggestions={[]}`, so this section **never appears**. The UI capability exists; the data is not wired up. ## Problem When a user opens the recipe picker for an empty slot, all recipes are shown in a flat, unsorted list with no variety signal. The user has no way to know which choices would improve the week's variety score. ## What exists today | Layer | Status | |---|---| | `RecipePicker.svelte` — Empfohlen section | ✅ Built, renders when `suggestions` prop is non-empty | | `GET /v1/week-plans/{id}/variety-preview?recipeId={id}&date={date}` | ✅ Exists, returns `{ simulatedScore }` for one recipe | | `GET /v1/week-plans/{id}/suggestions?date={date}` (batch) | ❌ Does not exist | | `+page.server.ts` — suggestions fetch | ❌ Not wired; passes `suggestions={[]}` always | ## Proposed solution ### Option A — New batch backend endpoint (recommended) Add `GET /v1/week-plans/{planId}/suggestions?date={date}` to the backend: - Takes the plan ID + target date - Internally calls the existing variety-preview logic for all household recipes - Returns top N (e.g. 5) recipes sorted by `simulatedScore` descending, with the score included - Filters out recipes already planned for the target date (though this isn't currently possible for an empty slot) Then in `+page.server.ts`: - Load suggestions lazily via a SvelteKit `load` dependency on `selectedDay`, or - Load them eagerly alongside the week plan (costs one extra request per page load but simpler) In `+page.svelte`: - Pass real `suggestions` to both `RecipePicker` instances ### Option B — N parallel frontend-server calls (simpler, no backend change) In `+page.server.ts`, call `variety-preview` for every recipe in parallel and rank client-side. Feasible for small recipe libraries; may be slow at scale (100+ recipes = 100 requests). **Option A is preferred** — one request instead of N, and the ranking logic lives server-side where it belongs. ## Acceptance criteria - [ ] Opening `RecipePicker` for an empty slot shows a non-empty **Empfohlen · Beste Abwechslung** section (top 5 recipes by simulated variety score) - [ ] Each suggestion shows the correct `↑ +N Punkte` or `⚠ Variationskonflikt` badge - [ ] The existing **Alle Rezepte** list and search still work below the suggestions - [ ] Suggestions are specific to the target date (ingredient overlap is day-aware) - [ ] The swap flow (`SwapSuggestionList`) is **not** affected — it uses easiest-first sorting intentionally - [ ] Backend endpoint is covered by Spring Boot tests - [ ] `RecipePicker` suggestions rendering already has tests (via existing `SuggestionCard.test.ts` + `RecipePicker.test.ts`); add server load tests for the new fetch ## Out of scope - Changing the swap flow sort order (that remains easiest-first per J4 spec) - Personalisation beyond ingredient overlap (e.g. user preference weighting) - Pagination of suggestions

marcel commented

2026-04-09 10:53:32 +02:00

👨‍💻 Felix Brandt — Review

Verdict: ⚠️ Approved with concerns

Good issue — problem, existing state, and acceptance criteria are all clear. A few things to nail down before implementation starts.

Blockers

1. Lazy vs. eager loading is unresolved — it matters for architecture

The issue lists both options without choosing:

"Load suggestions lazily via a SvelteKit load dependency on selectedDay, or load them eagerly alongside the week plan"

These lead to very different implementations:

Eager (load on page load, for the current week): suggestions are computed once per page load for a fixed date. If the user switches days, the suggestions are stale. Only makes sense if we show suggestions for today's slot by default.
Lazy (load when the picker opens for a specific day): requires either a separate +server.ts endpoint the client fetches from, or an invalidation-based pattern. More correct UX, more complex to implement.

Decision needed: Which approach? My recommendation is lazy — fetch suggestions when pickerOpen becomes true (mobile) or panelState transitions to recipe-picker (desktop), using a $derived-triggered fetch. This keeps the initial page load fast and suggestions always current for the selected day.

2. Top-N is hardcoded to 5 — is that a product decision or a backend default?

The issue says "top N (e.g. 5)". If it's 5 for now, say so explicitly. If it should be configurable, say who configures it. Ambiguity here leads to inconsistency between backend and frontend implementations.

Suggestions

The SuggestionCard.svelte component is mentioned but appears orphaned

The issue references SuggestionCard.test.ts as providing existing test coverage, but after the C2 route was removed (commit 4333dc0), SuggestionCard.svelte is no longer used anywhere. RecipePicker.svelte renders suggestion rows inline — it does not use SuggestionCard. The existing test coverage for suggestions therefore lives in RecipePicker.test.ts, not SuggestionCard.test.ts. Worth correcting so the implementer doesn't go looking for coverage that doesn't apply.

The acceptance criterion "specific to the target date" needs clarification

Suggestions are specific to the target date (ingredient overlap is day-aware)

The existing variety-preview endpoint already takes a date param, so day-awareness is handled backend-side. This criterion is satisfied automatically if the batch endpoint accepts date. No special frontend work needed — but worth confirming the backend actually uses the date to compute overlap context (i.e. it excludes the slot being filled from the current-week context).

Overall

Solid spec. Fix the lazy/eager decision and the top-N number before handing this to an implementer and it's ready to go.

## 👨‍💻 Felix Brandt — Review **Verdict: ⚠️ Approved with concerns** Good issue — problem, existing state, and acceptance criteria are all clear. A few things to nail down before implementation starts. --- ### Blockers **1. Lazy vs. eager loading is unresolved — it matters for architecture** The issue lists both options without choosing: > "Load suggestions lazily via a SvelteKit `load` dependency on `selectedDay`, or load them eagerly alongside the week plan" These lead to very different implementations: - **Eager** (load on page load, for the current week): suggestions are computed once per page load for a fixed date. If the user switches days, the suggestions are stale. Only makes sense if we show suggestions for today's slot by default. - **Lazy** (load when the picker opens for a specific day): requires either a separate `+server.ts` endpoint the client fetches from, or an invalidation-based pattern. More correct UX, more complex to implement. **Decision needed:** Which approach? My recommendation is lazy — fetch suggestions when `pickerOpen` becomes true (mobile) or `panelState` transitions to `recipe-picker` (desktop), using a `$derived`-triggered `fetch`. This keeps the initial page load fast and suggestions always current for the selected day. **2. Top-N is hardcoded to 5 — is that a product decision or a backend default?** The issue says "top N (e.g. 5)". If it's 5 for now, say so explicitly. If it should be configurable, say who configures it. Ambiguity here leads to inconsistency between backend and frontend implementations. --- ### Suggestions **The `SuggestionCard.svelte` component is mentioned but appears orphaned** The issue references `SuggestionCard.test.ts` as providing existing test coverage, but after the C2 route was removed (commit `4333dc0`), `SuggestionCard.svelte` is no longer used anywhere. `RecipePicker.svelte` renders suggestion rows inline — it does not use `SuggestionCard`. The existing test coverage for suggestions therefore lives in `RecipePicker.test.ts`, not `SuggestionCard.test.ts`. Worth correcting so the implementer doesn't go looking for coverage that doesn't apply. **The acceptance criterion "specific to the target date" needs clarification** > Suggestions are specific to the target date (ingredient overlap is day-aware) The existing `variety-preview` endpoint already takes a `date` param, so day-awareness is handled backend-side. This criterion is satisfied automatically if the batch endpoint accepts `date`. No special frontend work needed — but worth confirming the backend actually uses the date to compute overlap context (i.e. it excludes the slot being filled from the current-week context). --- ### Overall Solid spec. Fix the lazy/eager decision and the top-N number before handing this to an implementer and it's ready to go.

marcel commented

2026-04-09 10:56:43 +02:00

👨‍💻 Felix Brandt — Senior Fullstack Developer

I already flagged the two blockers above (lazy/eager decision + top-N ambiguity). Not repeating those — they stand.

One thing I didn't cover:

Questions & Observations

Error handling path is unspecified. What happens if the new /suggestions endpoint returns a 5xx, or times out? The issue assumes a happy path. My expectation: treat a failed suggestions fetch as suggestions=[] and still show the picker with the full "Alle Rezepte" list. The user should never see a broken picker because the recommendations couldn't be loaded. This should be an explicit acceptance criterion.
SuggestionCard.svelte is orphaned — the issue references SuggestionCard.test.ts as existing coverage, but the component is unused since commit 4333dc0. Either delete it as part of this issue or explicitly keep it out of scope, but don't cite it as active test coverage.

Suggestions

Add a criterion: "If the suggestions endpoint fails, the picker degrades gracefully to suggestions=[]"
Decide whether to clean up SuggestionCard.svelte here or open a separate chore issue

## 👨‍💻 Felix Brandt — Senior Fullstack Developer I already flagged the two blockers above (lazy/eager decision + top-N ambiguity). Not repeating those — they stand. One thing I didn't cover: ### Questions & Observations - **Error handling path is unspecified.** What happens if the new `/suggestions` endpoint returns a 5xx, or times out? The issue assumes a happy path. My expectation: treat a failed suggestions fetch as `suggestions=[]` and still show the picker with the full "Alle Rezepte" list. The user should never see a broken picker because the recommendations couldn't be loaded. This should be an explicit acceptance criterion. - **`SuggestionCard.svelte` is orphaned** — the issue references `SuggestionCard.test.ts` as existing coverage, but the component is unused since commit `4333dc0`. Either delete it as part of this issue or explicitly keep it out of scope, but don't cite it as active test coverage. ### Suggestions - Add a criterion: "If the suggestions endpoint fails, the picker degrades gracefully to `suggestions=[]`" - Decide whether to clean up `SuggestionCard.svelte` here or open a separate chore issue

marcel commented

2026-04-09 10:56:56 +02:00

🏛️ Architect

Questions & Observations

Where does the new endpoint belong in the backend layer structure?
The existing variety-preview lives on WeekPlanController. The new /suggestions endpoint should live there too. But internally it will need to call the variety-preview logic for every recipe — make sure this goes through the service layer (PlanningService) and not by calling the controller method directly. The service method getVarietyPreview should be extracted/reused, not duplicated.

Lazy loading requires a new API surface on the frontend side.
The existing +page.server.ts load function runs once per navigation. To load suggestions lazily (when the picker opens for a specific day), you need either:

A SvelteKit +server.ts route (GET /planner/suggestions?planId=&date=) that the client fetches from with a regular fetch() call — this is clean and testable
Or a $effect that fires a fetch() directly to the backend — bypasses SvelteKit's server layer and loses type safety

Option 1 is architecturally cleaner: the frontend server proxies the backend, keeps auth headers, and stays within the SvelteKit layer boundary.

Graceful degradation must be explicit in the architecture.
The suggestions fetch is optional enrichment. The component tree must be designed so that suggestions=[] is always a valid state, and the fetch failure doesn't block rendering the picker. This is already true of RecipePicker (it renders fine with no suggestions), but the fetch logic needs to explicitly catch errors and default to [].

The slotMap + selectedDay → pickerDate → suggestions chain.
On the desktop panel, pickerDate comes from panelState.date. On mobile, it's selectedDay. These are different state sources — ensure the lazy fetch is triggered by the right variable in each context, not a shared one that could cause stale data when switching between mobile and desktop.

Suggestions

Introduce a GET /planner/suggestions SvelteKit server route as the frontend API surface
Keep suggestions fetch isolated: one async function, one error boundary, defaulting to []
Confirm the backend service layer reuses the existing variety-preview scoring logic without duplication

## 🏛️ Architect ### Questions & Observations **Where does the new endpoint belong in the backend layer structure?** The existing `variety-preview` lives on `WeekPlanController`. The new `/suggestions` endpoint should live there too. But internally it will need to call the variety-preview logic for every recipe — make sure this goes through the service layer (`PlanningService`) and not by calling the controller method directly. The service method `getVarietyPreview` should be extracted/reused, not duplicated. **Lazy loading requires a new API surface on the frontend side.** The existing `+page.server.ts` `load` function runs once per navigation. To load suggestions lazily (when the picker opens for a specific day), you need either: 1. A SvelteKit `+server.ts` route (`GET /planner/suggestions?planId=&date=`) that the client fetches from with a regular `fetch()` call — this is clean and testable 2. Or a `$effect` that fires a `fetch()` directly to the backend — bypasses SvelteKit's server layer and loses type safety Option 1 is architecturally cleaner: the frontend server proxies the backend, keeps auth headers, and stays within the SvelteKit layer boundary. **Graceful degradation must be explicit in the architecture.** The suggestions fetch is optional enrichment. The component tree must be designed so that `suggestions=[]` is always a valid state, and the fetch failure doesn't block rendering the picker. This is already true of `RecipePicker` (it renders fine with no suggestions), but the fetch logic needs to explicitly catch errors and default to `[]`. **The `slotMap` + `selectedDay` → `pickerDate` → `suggestions` chain.** On the desktop panel, `pickerDate` comes from `panelState.date`. On mobile, it's `selectedDay`. These are different state sources — ensure the lazy fetch is triggered by the right variable in each context, not a shared one that could cause stale data when switching between mobile and desktop. ### Suggestions - Introduce a `GET /planner/suggestions` SvelteKit server route as the frontend API surface - Keep suggestions fetch isolated: one async function, one error boundary, defaulting to `[]` - Confirm the backend service layer reuses the existing variety-preview scoring logic without duplication

marcel commented

2026-04-09 10:57:14 +02:00

🧪 Tester

Questions & Observations

Edge cases not covered by the acceptance criteria:

Fewer than 5 recipes in the library — if the household only has 3 recipes, the endpoint should return 3, not error. Does the backend handle N > library size gracefully?
All recipes already planned this week — every recipe has a ⚠ Variationskonflikt badge. Should they still appear in Empfohlen? If yes, the user sees 5 warnings with no green option. Is that intentional? The excludeRecipeId filter (from J4) already removes the slot's current recipe — but the rest could all be conflicts.
First slot of the week (no meals planned yet) — the variety-preview has no context. Does the backend return a meaningful simulatedScore (probably the score of adding just that one recipe), or does it error?
Week plan doesn't exist yet — the planner shows a "Wochenplan erstellen" state. Can the picker even be opened? If not, the suggestions fetch can't be triggered. This should be a stated precondition.
Day switches while picker is open — on desktop, if the user switches the selected day while the suggestions panel is open, do the suggestions update for the new day? Or do they show stale data?

Test strategy questions:

The issue says "add server load tests for the new fetch" — if we use a +server.ts route (as the architect recommends), the test target is the route handler, not the load function. Which is it?
The lazy fetch will involve a $effect or $derived that calls fetch() — this is harder to unit-test than a server load. What's the plan for testing the trigger logic?
Is there an integration/e2e test for the full flow (open picker → see suggestions → pick one → verify slot updated)?

Suggestions

Add edge-case acceptance criteria for empty library, all-conflict library, and first-slot-of-week
Specify clearly what "covered by Spring Boot tests" means: happy path only, or also the edge cases above?
Consider an e2e test in Playwright that opens the picker and asserts the Empfohlen section is visible

## 🧪 Tester ### Questions & Observations **Edge cases not covered by the acceptance criteria:** - **Fewer than 5 recipes in the library** — if the household only has 3 recipes, the endpoint should return 3, not error. Does the backend handle `N > library size` gracefully? - **All recipes already planned this week** — every recipe has a `⚠ Variationskonflikt` badge. Should they still appear in Empfohlen? If yes, the user sees 5 warnings with no green option. Is that intentional? The `excludeRecipeId` filter (from J4) already removes the slot's current recipe — but the rest could all be conflicts. - **First slot of the week (no meals planned yet)** — the variety-preview has no context. Does the backend return a meaningful `simulatedScore` (probably the score of adding just that one recipe), or does it error? - **Week plan doesn't exist yet** — the planner shows a "Wochenplan erstellen" state. Can the picker even be opened? If not, the suggestions fetch can't be triggered. This should be a stated precondition. - **Day switches while picker is open** — on desktop, if the user switches the selected day while the suggestions panel is open, do the suggestions update for the new day? Or do they show stale data? **Test strategy questions:** - The issue says "add server load tests for the new fetch" — if we use a `+server.ts` route (as the architect recommends), the test target is the route handler, not the `load` function. Which is it? - The lazy fetch will involve a `$effect` or `$derived` that calls `fetch()` — this is harder to unit-test than a server `load`. What's the plan for testing the trigger logic? - Is there an integration/e2e test for the full flow (open picker → see suggestions → pick one → verify slot updated)? ### Suggestions - Add edge-case acceptance criteria for empty library, all-conflict library, and first-slot-of-week - Specify clearly what "covered by Spring Boot tests" means: happy path only, or also the edge cases above? - Consider an e2e test in Playwright that opens the picker and asserts the Empfohlen section is visible

marcel commented

2026-04-09 10:57:27 +02:00

🔒 Security Expert

Questions & Observations

Authorization scope on the new batch endpoint

The existing variety-preview endpoint is guarded with @RequiresHouseholdRole("member"). The new /suggestions batch endpoint must carry the same guard — and critically, it must verify that the planId in the path belongs to the requesting user's household. A user from household A should not be able to call /v1/week-plans/{householdB-planId}/suggestions and get recipe data for another household.

This check almost certainly exists in the service layer already (the old getVarietyPreview resolves the householdId from the principal and validates plan ownership). Confirm this validation is inherited by the new endpoint — don't assume it comes for free.

Recipe data in the response

The suggestions response will return recipe names and metadata for the top N recipes. These are household-owned recipes — confirm the backend scopes the recipe library query to the caller's household before scoring and returning. If recipe IDs are globally unique UUIDs and the library query is household-scoped, this is fine. Worth a quick audit.

The SvelteKit +server.ts proxy route (if adopted)

If the frontend introduces a GET /planner/suggestions server route that proxies to the backend, it must forward the session cookie / auth token to the backend. SvelteKit's event.fetch does this automatically — but only if the backend URL is fetched via the SvelteKit fetch wrapper, not native fetch. Verify the existing apiClient pattern is used here (it is in +page.server.ts).

No new input validation concerns — planId is a UUID (validated by Spring path variable binding), date is a LocalDate (type-safe). No free-form user input in the request.

Suggestions

Add to acceptance criteria: "Requests for a plan not belonging to the caller's household return 403"
Confirm recipe library query in the batch endpoint is scoped to the household, not global

## 🔒 Security Expert ### Questions & Observations **Authorization scope on the new batch endpoint** The existing `variety-preview` endpoint is guarded with `@RequiresHouseholdRole("member")`. The new `/suggestions` batch endpoint must carry the same guard — and critically, it must verify that the `planId` in the path belongs to the requesting user's household. A user from household A should not be able to call `/v1/week-plans/{householdB-planId}/suggestions` and get recipe data for another household. This check almost certainly exists in the service layer already (the old `getVarietyPreview` resolves the householdId from the principal and validates plan ownership). Confirm this validation is inherited by the new endpoint — don't assume it comes for free. **Recipe data in the response** The suggestions response will return recipe names and metadata for the top N recipes. These are household-owned recipes — confirm the backend scopes the recipe library query to the caller's household before scoring and returning. If recipe IDs are globally unique UUIDs and the library query is household-scoped, this is fine. Worth a quick audit. **The SvelteKit `+server.ts` proxy route (if adopted)** If the frontend introduces a `GET /planner/suggestions` server route that proxies to the backend, it must forward the session cookie / auth token to the backend. SvelteKit's `event.fetch` does this automatically — but only if the backend URL is fetched via the SvelteKit `fetch` wrapper, not native `fetch`. Verify the existing `apiClient` pattern is used here (it is in `+page.server.ts`). **No new input validation concerns** — `planId` is a UUID (validated by Spring path variable binding), `date` is a `LocalDate` (type-safe). No free-form user input in the request. ### Suggestions - Add to acceptance criteria: "Requests for a plan not belonging to the caller's household return 403" - Confirm recipe library query in the batch endpoint is scoped to the household, not global

marcel commented

2026-04-09 10:57:54 +02:00

🎨 UI / UX Expert

Questions & Observations

What does the picker look like while suggestions are loading?

The issue describes lazy loading but says nothing about the loading state. If suggestions take 300–800ms to arrive, the picker opens showing only "Alle Rezepte" and then the "Empfohlen" section suddenly appears above it — this is a jarring layout shift. Options:

Show a skeleton row or spinner in the Empfohlen area while loading
Show "Empfohlen" with a subtle loading indicator, then populate in place
Load suggestions before opening the picker (eager per-open, not per-page) so the section is ready when the sheet slides in

The third option is best UX: trigger the suggestions fetch the moment the user taps the empty slot (before the animation completes), and delay showing the picker by 200ms if needed. This hides latency behind the open animation.

What if there are zero suggestions?

The Empfohlen section is currently hidden when suggestions=[]. If the batch endpoint returns an empty list (e.g., all recipes conflict), the section disappears entirely and the user sees only "Alle Rezepte" — which is the current broken state, just correctly hidden. That's fine, but worth stating explicitly: the section should not show at all if empty, not show with "Keine Empfehlungen" messaging.

Badge copy — "⚠ Variationskonflikt" for every recipe in a full week

If all 5 suggestions carry the yellow warning badge, the Empfohlen section becomes a list of warnings. This could feel discouraging. Consider suppressing the Empfohlen section entirely if all top-5 scores are neutral/negative — or at minimum reorder so any positive suggestions come first (the backend presumably already does this by simulatedScore DESC).

The "Alle Rezepte" list after suggestions

Currently the full recipe list is unsorted (API order). After the Empfohlen section is visible, the same recipes will appear again in "Alle Rezepte". A user might wonder why a recipe recommended above also appears in the full list. Adding a subtle "Bereits empfohlen" label or filtering them out of "Alle Rezepte" would reduce confusion — though this is polish, not a blocker.

Suggestions

Add a loading state spec to the issue (skeleton or delayed open)
Clarify the empty-suggestions behaviour explicitly
Consider filtering suggested recipes from "Alle Rezepte" to avoid duplication

## 🎨 UI / UX Expert ### Questions & Observations **What does the picker look like while suggestions are loading?** The issue describes lazy loading but says nothing about the loading state. If suggestions take 300–800ms to arrive, the picker opens showing only "Alle Rezepte" and then the "Empfohlen" section suddenly appears above it — this is a jarring layout shift. Options: - Show a skeleton row or spinner in the Empfohlen area while loading - Show "Empfohlen" with a subtle loading indicator, then populate in place - Load suggestions before opening the picker (eager per-open, not per-page) so the section is ready when the sheet slides in The third option is best UX: trigger the suggestions fetch the moment the user taps the empty slot (before the animation completes), and delay showing the picker by 200ms if needed. This hides latency behind the open animation. **What if there are zero suggestions?** The Empfohlen section is currently hidden when `suggestions=[]`. If the batch endpoint returns an empty list (e.g., all recipes conflict), the section disappears entirely and the user sees only "Alle Rezepte" — which is the current broken state, just correctly hidden. That's fine, but worth stating explicitly: the section should not show at all if empty, not show with "Keine Empfehlungen" messaging. **Badge copy — "⚠ Variationskonflikt" for every recipe in a full week** If all 5 suggestions carry the yellow warning badge, the Empfohlen section becomes a list of warnings. This could feel discouraging. Consider suppressing the Empfohlen section entirely if all top-5 scores are neutral/negative — or at minimum reorder so any positive suggestions come first (the backend presumably already does this by `simulatedScore DESC`). **The "Alle Rezepte" list after suggestions** Currently the full recipe list is unsorted (API order). After the Empfohlen section is visible, the same recipes will appear again in "Alle Rezepte". A user might wonder why a recipe recommended above also appears in the full list. Adding a subtle "Bereits empfohlen" label or filtering them out of "Alle Rezepte" would reduce confusion — though this is polish, not a blocker. ### Suggestions - Add a loading state spec to the issue (skeleton or delayed open) - Clarify the empty-suggestions behaviour explicitly - Consider filtering suggested recipes from "Alle Rezepte" to avoid duplication

marcel commented

2026-04-09 10:58:01 +02:00

⚙️ DevOps

Questions & Observations

Performance budget for the new endpoint

The batch endpoint will call the variety-preview scoring logic for every recipe in the household library on each request. What's acceptable latency? The existing /variety-score endpoint presumably completes quickly (it scores the current plan), but scoring all N recipes is O(N × overlap-computation). With 50 recipes this may be fine; with 500 it may not. No latency target is mentioned in the issue.

Suggested addition: define a p95 latency target (e.g. < 500ms for up to 100 recipes) and add a note to the Spring Boot test about verifying the query count doesn't grow with recipe library size (N+1 query risk).

Will this endpoint be called on every slot click?

If suggestions are fetched lazily every time the picker opens, a user who opens and closes the picker five times in one session fires five backend requests. For a household that plans together, this could happen concurrently. Not a scalability concern for a household app, but worth noting — if the response is stable between slot mutations, a short client-side cache (e.g. memoised by planId + date, invalidated on invalidateAll) would eliminate redundant calls.

No new infrastructure needed

No new environment variables
No new database tables or migrations
No changes to the API spec beyond adding one endpoint — confirm the OpenAPI spec (openapi.yaml or equivalent) is updated if one exists
The existing CI pipeline covers Spring Boot tests; no pipeline changes needed

Observability

No structured logging or metrics are mentioned. For a household app this is fine. If latency becomes an issue later, the endpoint is a natural place to add a @Timed annotation.

Suggestions

Add a note about N+1 query risk in the backend implementation task
Consider memoising the suggestions response client-side for the duration of one page session (keyed by planId + date)

## ⚙️ DevOps ### Questions & Observations **Performance budget for the new endpoint** The batch endpoint will call the variety-preview scoring logic for every recipe in the household library on each request. What's acceptable latency? The existing `/variety-score` endpoint presumably completes quickly (it scores the current plan), but scoring all N recipes is O(N × overlap-computation). With 50 recipes this may be fine; with 500 it may not. No latency target is mentioned in the issue. Suggested addition: define a p95 latency target (e.g. < 500ms for up to 100 recipes) and add a note to the Spring Boot test about verifying the query count doesn't grow with recipe library size (N+1 query risk). **Will this endpoint be called on every slot click?** If suggestions are fetched lazily every time the picker opens, a user who opens and closes the picker five times in one session fires five backend requests. For a household that plans together, this could happen concurrently. Not a scalability concern for a household app, but worth noting — if the response is stable between slot mutations, a short client-side cache (e.g. memoised by `planId + date`, invalidated on `invalidateAll`) would eliminate redundant calls. **No new infrastructure needed** - No new environment variables - No new database tables or migrations - No changes to the API spec beyond adding one endpoint — confirm the OpenAPI spec (`openapi.yaml` or equivalent) is updated if one exists - The existing CI pipeline covers Spring Boot tests; no pipeline changes needed **Observability** No structured logging or metrics are mentioned. For a household app this is fine. If latency becomes an issue later, the endpoint is a natural place to add a `@Timed` annotation. ### Suggestions - Add a note about N+1 query risk in the backend implementation task - Consider memoising the suggestions response client-side for the duration of one page session (keyed by `planId + date`)

marcel commented

2026-04-09 11:10:57 +02:00

⚙️ Senior Backend Engineer — Spring Boot / PostgreSQL Specialist

Backend-focused discussion — clarifying DTO shape, query strategy, top-N, filter semantics, and test depth.

Resolved

Response DTO shape — SuggestionItem gets two new fields: scoreDelta (double) and hasConflict (boolean). Backend computes both: current score is fetched once from existing slots, scoreDelta = simulatedScore − currentScore, hasConflict = scoreDelta <= 0. Recipe metadata fields (id, name, effort, cookTimeMin) are already present via SlotRecipe. The currentVarietyScore prop on RecipePicker can be removed — it's no longer needed on the frontend.
N+1 query risk — none — PlanningService.getSuggestions() already loads all data once (plan, household recipes, cooking history) and scores in-memory. No per-recipe DB queries. The batch scoring is already safe.
Top-N — Already implemented as an optional topN query param defaulting to 5. No changes needed here.
Filter clause ("not currently possible for an empty slot") — The usedRecipeIds filter is week-level by design: any recipe already planned anywhere in the week is excluded from suggestions, regardless of day. This is intentional.
Test depth — Service unit tests covering scoreDelta and hasConflict computation (current score, delta, conflict flag). The existing controller test (getSuggestionsShouldReturn200) will need to be updated to match the new SuggestionItem shape.

Key finding

The batch endpoint already exists. GET /v1/week-plans/{id}/suggestions is fully implemented in WeekPlanController and PlanningService. The issue describes it as ❌, but it's live. The actual backend work is narrow: add scoreDelta and hasConflict to SuggestionResponse.SuggestionItem, update the service to compute them, fix one controller test. The frontend work is: call the endpoint, update the Suggestion type, drop the currentVarietyScore prop.

Overall: smaller than it looks. Most of the backend scaffolding is already in place — this is a DTO update and a wiring task, not a new feature build.

## ⚙️ Senior Backend Engineer — Spring Boot / PostgreSQL Specialist Backend-focused discussion — clarifying DTO shape, query strategy, top-N, filter semantics, and test depth. --- ### Resolved - **Response DTO shape** — `SuggestionItem` gets two new fields: `scoreDelta` (double) and `hasConflict` (boolean). Backend computes both: current score is fetched once from existing slots, `scoreDelta = simulatedScore − currentScore`, `hasConflict = scoreDelta <= 0`. Recipe metadata fields (`id`, `name`, `effort`, `cookTimeMin`) are already present via `SlotRecipe`. The `currentVarietyScore` prop on `RecipePicker` can be removed — it's no longer needed on the frontend. - **N+1 query risk — none** — `PlanningService.getSuggestions()` already loads all data once (plan, household recipes, cooking history) and scores in-memory. No per-recipe DB queries. The batch scoring is already safe. - **Top-N** — Already implemented as an optional `topN` query param defaulting to 5. No changes needed here. - **Filter clause ("not currently possible for an empty slot")** — The `usedRecipeIds` filter is week-level by design: any recipe already planned anywhere in the week is excluded from suggestions, regardless of day. This is intentional. - **Test depth** — Service unit tests covering `scoreDelta` and `hasConflict` computation (current score, delta, conflict flag). The existing controller test (`getSuggestionsShouldReturn200`) will need to be updated to match the new `SuggestionItem` shape. --- ### Key finding **The batch endpoint already exists.** `GET /v1/week-plans/{id}/suggestions` is fully implemented in `WeekPlanController` and `PlanningService`. The issue describes it as ❌, but it's live. The actual backend work is narrow: add `scoreDelta` and `hasConflict` to `SuggestionResponse.SuggestionItem`, update the service to compute them, fix one controller test. The frontend work is: call the endpoint, update the `Suggestion` type, drop the `currentVarietyScore` prop. --- Overall: smaller than it looks. Most of the backend scaffolding is already in place — this is a DTO update and a wiring task, not a new feature build.

marcel commented

2026-04-09 11:18:05 +02:00

🎨 Kai — Frontend Engineer

Frontend-focused discussion — settling the fetch strategy, Svelte 5 trigger pattern, loading state, dead code, and prop cleanup.

Resolved

Lazy fetch via +server.ts — Suggestions are fetched lazily when the picker opens, not on page load. A new GET /planner/suggestions?planId=&date= SvelteKit +server.ts route proxies to the backend using event.fetch (auth handled automatically). No direct backend calls from the client.
Fetch trigger pattern — A single $derived pickerDate unifies both contexts:
```
const pickerDate = $derived(
    pickerOpen ? selectedDay
    : panelState.kind === 'recipe-picker' ? panelState.date
    : null
);
```
One $effect watches pickerDate — fetches when it becomes non-null, clears suggestions when it goes back to null. Errors caught and degraded to suggestions = [].
Loading state — in scope — Add isLoadingSuggestions boolean state. Show a skeleton/spinner in the Empfohlen area while the fetch is in flight to prevent layout shift when the section appears.
SuggestionCard.svelte — delete it — The component is dead code (unused since 4333dc0). Delete it and its test file as part of this issue. Update the acceptance criteria: replace the SuggestionCard.test.ts reference with RecipePicker.test.ts.
currentVarietyScore prop removal — With scoreDelta pre-computed by the backend, RecipePicker no longer needs currentVarietyScore. Remove the prop from the component and both call sites in +page.svelte (lines 285 and 563). The Suggestion interface changes from { recipe, simulatedScore } to { recipeId, name, effort?, cookTimeMin?, scoreDelta, hasConflict }. varietyScore itself stays — it's still used by VarietyScoreCard.

Overall: the frontend work is well-scoped — one new +server.ts route, one $derived + one $effect in +page.svelte, a loading state, prop cleanup, and dead code removal. Nothing architecturally surprising.

## 🎨 Kai — Frontend Engineer Frontend-focused discussion — settling the fetch strategy, Svelte 5 trigger pattern, loading state, dead code, and prop cleanup. --- ### Resolved - **Lazy fetch via `+server.ts`** — Suggestions are fetched lazily when the picker opens, not on page load. A new `GET /planner/suggestions?planId=&date=` SvelteKit `+server.ts` route proxies to the backend using `event.fetch` (auth handled automatically). No direct backend calls from the client. - **Fetch trigger pattern** — A single `$derived pickerDate` unifies both contexts: ```ts const pickerDate = $derived( pickerOpen ? selectedDay : panelState.kind === 'recipe-picker' ? panelState.date : null ); ``` One `$effect` watches `pickerDate` — fetches when it becomes non-null, clears `suggestions` when it goes back to null. Errors caught and degraded to `suggestions = []`. - **Loading state — in scope** — Add `isLoadingSuggestions` boolean state. Show a skeleton/spinner in the Empfohlen area while the fetch is in flight to prevent layout shift when the section appears. - **`SuggestionCard.svelte` — delete it** — The component is dead code (unused since `4333dc0`). Delete it and its test file as part of this issue. Update the acceptance criteria: replace the `SuggestionCard.test.ts` reference with `RecipePicker.test.ts`. - **`currentVarietyScore` prop removal** — With `scoreDelta` pre-computed by the backend, `RecipePicker` no longer needs `currentVarietyScore`. Remove the prop from the component and both call sites in `+page.svelte` (lines 285 and 563). The `Suggestion` interface changes from `{ recipe, simulatedScore }` to `{ recipeId, name, effort?, cookTimeMin?, scoreDelta, hasConflict }`. `varietyScore` itself stays — it's still used by `VarietyScoreCard`. --- Overall: the frontend work is well-scoped — one new `+server.ts` route, one `$derived` + one `$effect` in `+page.svelte`, a loading state, prop cleanup, and dead code removal. Nothing architecturally surprising.

marcel commented

2026-04-09 11:48:12 +02:00

Implemented ✅

All tasks completed on branch feat/issue-46-wire-variety-aware-suggestions.

What was built

Backend

SuggestionItem DTO: replaced simulatedScore: double with scoreDelta: double + hasConflict: boolean
PlanningService.getSuggestions(): computes currentScore once from existing slots, then derives scoreDelta = simulatedScore - currentScore and hasConflict = scoreDelta <= 0 per candidate; results sorted by scoreDelta desc

Frontend

Deleted dead code: SuggestionCard.svelte + SuggestionCard.test.ts
schema.d.ts + openapi.json: SuggestionItem updated to { scoreDelta, hasConflict }, removed simulatedScore
RecipePicker.svelte: badge logic now uses hasConflict (green = variety gain, yellow = conflict); added isLoading prop with pulse skeleton; dropped currentVarietyScore prop
+server.ts (GET /planner): proxy route sorts by scoreDelta desc, degrades gracefully on all error paths
+page.svelte: $derived activePickerDate unifies mobile + desktop picker triggers; $effect lazy-fetches /planner?planId&date when picker opens, wires suggestions + isLoading into both RecipePicker instances

Commits

feat(planner): remove SuggestionCard — dead code replaced by RecipePicker
feat(planner): replace simulatedScore with scoreDelta and hasConflict in SuggestionItem
feat(planner): compute scoreDelta and hasConflict in PlanningService.getSuggestions
feat(planner): update SuggestionItem schema — scoreDelta + hasConflict
feat(planner): wire scoreDelta/hasConflict badges and isLoading into RecipePicker
feat(planner): add GET /planner proxy route for variety suggestions
feat(planner): lazy-fetch variety suggestions in RecipePicker for empty slots

Test results

Backend: 294 tests, 0 failures
Frontend: 599 tests passing (2 pre-existing failures in recipes/page.test.ts unrelated to this issue)
npm run check: 0 errors in production code (2 pre-existing errors in test-setup.ts)

## Implemented ✅ All tasks completed on branch `feat/issue-46-wire-variety-aware-suggestions`. ### What was built **Backend** - `SuggestionItem` DTO: replaced `simulatedScore: double` with `scoreDelta: double` + `hasConflict: boolean` - `PlanningService.getSuggestions()`: computes `currentScore` once from existing slots, then derives `scoreDelta = simulatedScore - currentScore` and `hasConflict = scoreDelta <= 0` per candidate; results sorted by `scoreDelta` desc **Frontend** - Deleted dead code: `SuggestionCard.svelte` + `SuggestionCard.test.ts` - `schema.d.ts` + `openapi.json`: `SuggestionItem` updated to `{ scoreDelta, hasConflict }`, removed `simulatedScore` - `RecipePicker.svelte`: badge logic now uses `hasConflict` (green = variety gain, yellow = conflict); added `isLoading` prop with pulse skeleton; dropped `currentVarietyScore` prop - `+server.ts` (`GET /planner`): proxy route sorts by `scoreDelta` desc, degrades gracefully on all error paths - `+page.svelte`: `$derived activePickerDate` unifies mobile + desktop picker triggers; `$effect` lazy-fetches `/planner?planId&date` when picker opens, wires `suggestions` + `isLoading` into both `RecipePicker` instances ### Commits - `feat(planner): remove SuggestionCard — dead code replaced by RecipePicker` - `feat(planner): replace simulatedScore with scoreDelta and hasConflict in SuggestionItem` - `feat(planner): compute scoreDelta and hasConflict in PlanningService.getSuggestions` - `feat(planner): update SuggestionItem schema — scoreDelta + hasConflict` - `feat(planner): wire scoreDelta/hasConflict badges and isLoading into RecipePicker` - `feat(planner): add GET /planner proxy route for variety suggestions` - `feat(planner): lazy-fetch variety suggestions in RecipePicker for empty slots` ### Test results - Backend: 294 tests, 0 failures - Frontend: 599 tests passing (2 pre-existing failures in `recipes/page.test.ts` unrelated to this issue) - `npm run check`: 0 errors in production code (2 pre-existing errors in `test-setup.ts`)

marcel referenced a pull request that will close this issue

2026-04-09 11:55:34 +02:00

feat(planner): wire variety-aware suggestions into RecipePicker for empty slots #47

marcel closed this issue

2026-04-09 16:33:13 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/mealprep#46