Skip to content

fix: include menu freehand ingredients in web shopping list#324

Merged
dubadub merged 1 commit intomainfrom
fix/menu-freehand-ingredients-in-shopping-list
Apr 13, 2026
Merged

fix: include menu freehand ingredients in web shopping list#324
dubadub merged 1 commit intomainfrom
fix/menu-freehand-ingredients-in-shopping-list

Conversation

@dubadub
Copy link
Copy Markdown
Member

@dubadub dubadub commented Apr 13, 2026

Summary

  • Web shopping list was missing freehand ingredients from .menu files (e.g. @coffee{1%cup} directly in a menu), causing it to diverge from cook shopping-list foo.menu
  • When expanding a plan/menu entry in templates/shopping_list.html, also emit a request for the menu path itself with included_references: []extract_ingredients then adds the menu's non-reference ingredients and skips every recipe reference (already covered by the per-recipe requests)

Test plan

  • Verified CLI cook shopping-list "2 Day Plan.menu" already produces freehand items (maple syrup, coffee, bread, soy sauce, oats, sugar, orange juice, green salad)
  • Reproduced missing items in web UI via /api/shopping_list with old payload shape
  • Confirmed with the fix the same payload now yields those items (diff: +bread +coffee +green salad +maple syrup +oats +orange juice +soy sauce +sugar)

When a menu was added to the shopping list, only its nested recipe
references were sent to /api/shopping_list — freehand ingredients
written directly in the .menu file (e.g. `@coffee{1%cup}`) were
silently dropped, causing the web UI's output to differ from the
CLI's `cook shopping-list foo.menu`.

The frontend now also emits a request for the menu path itself
with an empty `included_references` list. extract_ingredients
adds the menu's non-reference ingredients via add_recipe and
skips every recipe reference (handled by the per-recipe requests).
@claude
Copy link
Copy Markdown

claude bot commented Apr 13, 2026

Code Review

Summary: Clean, focused fix for a real discrepancy between the CLI and web UI shopping list. The root cause diagnosis is correct and the implementation is well-reasoned.

What the fix does

When a .menu file is expanded in the frontend, the fix appends an extra request for the menu path itself with included_references: []. On the backend, extract_ingredients treats Some([]) as "include no references" — so it adds the menu's own direct ingredients while skipping every recipe reference (which is already covered by the per-recipe requests in the same payload). This is a neat use of an existing convention.

Positives

  • Minimal diff — 12 lines changed, no unnecessary refactoring.
  • Correct semantics — passing included_references: [] (empty Some) is exactly the right signal to add freehand ingredients only, consistent with how the API already distinguishes None (all refs) vs Some([...]) (listed refs only).
  • Scale is preserveditem.scale is correctly forwarded for the menu request, so freehand ingredients are scaled consistently with the recipe entries.
  • Test plan is solid — reproducing both the CLI baseline and the API payload diff gives good confidence.

One gap to be aware of (pre-existing, not introduced here)

aggregate_current_ingredient_names in shopping_list.rs:372 iterates item.recipes but does not emit a call for the menu path itself — meaning freehand menu ingredients are still absent from the set used by the "compact checked items" stale-check logic. A user who checks off coffee from a menu will find that checked state survives a compact even after the ingredient is logically removed (or vice-versa). This was broken before this PR and is out of scope here, but worth a follow-up issue so it doesn't get lost.

Minor nit (no change required)

The implicit convention of included_references: [] meaning "freehand only" is undocumented at the call site. A one-line comment above the menuFreehand object explaining why the empty array achieves this would help future readers who haven't traced through extract_ingredients. The existing comment is good but stops just short of explaining the mechanism.

// included_references: [] → backend skips all recipe references,
// adding only the menu's own freehand ingredients.
const menuFreehand = {
    recipe: item.path,
    scale: item.scale,
    included_references: []
};

Verdict

Approve. The fix is correct, well-scoped, and matches the documented test results. The gap in aggregate_current_ingredient_names is worth tracking but doesn't block this change.

@dubadub dubadub merged commit 069caee into main Apr 13, 2026
6 checks passed
@dubadub dubadub deleted the fix/menu-freehand-ingredients-in-shopping-list branch April 13, 2026 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant