Repository navigation
feat(ios): weekly horizon for the LeChef plan — one allocator, repeats priced, leftovers carried - #178
Conversation
SPEC.md §14 on #52 (adapt): the same allocator core across five-to-seven slots — repeats become rankable but penalized, the same main ingredient two days in a row is penalized, and a dinner's surplus servings can cover a later leftovers slot. A nil policy keeps the three-dinner behavior byte-for-byte. Co-authored-by: Sam Gu <sgu07966@gmail.com>
Persist the weekly horizon's leftovers marker (cook_plan_leftover_carry_v1) through save/load, read the household's dietary restrictions to screen reviewed substitutions, and let loadSnapshot merge bundle-v2 catalog recipes alongside legacy ones through the ingredient-identity bridge: unresolved keys stay as negative-sentinel unknown-quantity lines, assumed staples join the staples set, and a catalog successor yields to its still-usable legacy recipe. Co-authored-by: Sam Gu <sgu07966@gmail.com>
Five-to-seven dinners run the weekly policy with the shipped repeat and consecutive-main penalties; three-to-four keep the strict no-repeat default. Co-authored-by: Sam Gu <sgu07966@gmail.com>
Allocator coverage for repeats, consecutive mains, leftover carry, the short-line cap in weekly mode, and allergen exclusion; persistence round-trips through the cook-plan tables; adapter coverage for catalog merging, staples, sentinel lines, and successor yielding. Co-authored-by: Sam Gu <sgu07966@gmail.com>
The base branch advanced past 9f5a701 while its build-and-test remains red (CookThermometer unresolved in RecipeCatalogRecommendTests.swift, owned by the catalog-recommend thread); this work builds on the latest head per the task's fallback. Co-authored-by: Sam Gu <sgu07966@gmail.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The shared lineage's TasteEventStoreTests.swift declared the helper '-> TasteEvent' while constructing 'TasteEventRecord', so the iOS test target stopped compiling. Earlier base-branch CI runs aborted at the CookThermometer error before type-checking this file, masking it; with that import fixed, the seven type errors surface in this PR's merge preview. One-line fix cherry-picked from the trunk integration's 9de2985 (CI-green there, run 38805706914). (cherry picked from commit 9de2985) Co-authored-by: Sam Gu <sgu07966@gmail.com>
|
Note Review skipped Codemetrics is installed on this repository, but it hasn't been added to your team yet. To enable AI code reviews for this repo, go to your dashboard and add FridgeLuck to your team's repositories. |
 [High impact] Adds weekly meal-planning logic with database schema change. Do not merge until swaps respect blocked allergens and available stock, and leftovers name portions that actually exist.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for swap in substitutionService.substitutions( | ||
| for: ingredientID, dietaryRestrictions: dietaryRestrictions) { | ||
| guard swap.ratio > 0 else { continue } | ||
| substitutions.append( | ||
| PlanSubstitution( | ||
| originalID: swap.originalId, substituteID: swap.substituteId, | ||
| substituteGramsPerOriginalGram: swap.ratio, note: swap.note, reviewed: true)) |
There was a problem hiding this comment.
The new substitution list screens dietary restrictions but not blocked ingredient IDs. With allergen_ingredient_ids = [23], no dietary restrictions, some chicken stock, and enough tofu stock, the shipped chicken→tofu swap remains available. restrictionClean checks only the original recipe, and coveringSubstitution never checks the blocked IDs. The plan can therefore mark the dinner covered using tofu that the household explicitly blocked.
Filter substitutes against every eater's blocked ingredients before allocation.
Knowledge Base Used: Recipe recommendation and cooking
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/ios/Platform/Persistence/KitchenPlan/PlanDataAdapter.swift
Line: 130-136
Comment:
**Swaps bypass blocked allergens**
The new substitution list screens dietary restrictions but not blocked ingredient IDs. With `allergen_ingredient_ids = [23]`, no dietary restrictions, some chicken stock, and enough tofu stock, the shipped chicken→tofu swap remains available. `restrictionClean` checks only the original recipe, and `coveringSubstitution` never checks the blocked IDs. The plan can therefore mark the dinner covered using tofu that the household explicitly blocked.
Filter substitutes against every eater's blocked ingredients before allocation.
**Knowledge Base Used:** [Recipe recommendation and cooking](https://app.greptile.com/s-gu/-/custom-context/knowledge-base/samgu-nrx/fridgeluck/-/docs/recipe-recommendation-and-cooking.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| substitutions.append( | ||
| PlanSubstitution( | ||
| originalID: swap.originalId, substituteID: swap.substituteId, | ||
| substituteGramsPerOriginalGram: swap.ratio, note: swap.note, reviewed: true)) |
There was a problem hiding this comment.
Supplying swaps here exposes double-counted stock within a dinner. allocateDinner reads the same unchanged pool for every line and swap. For a bill needing 100 g chicken and 100 g tofu, with 50 g chicken and 100 g tofu available, the chicken line can claim 50 g tofu as a swap. The tofu line can then claim all 100 g again. Both lines become sufficient, and the shopping list misses 50 g.
Reserve stock in a temporary pool while trying each candidate, then commit only the chosen candidate's use.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/ios/Platform/Persistence/KitchenPlan/PlanDataAdapter.swift
Line: 133-136
Comment:
**Swaps double-count pantry stock**
Supplying swaps here exposes double-counted stock within a dinner. `allocateDinner` reads the same unchanged pool for every line and swap. For a bill needing 100 g chicken and 100 g tofu, with 50 g chicken and 100 g tofu available, the chicken line can claim 50 g tofu as a swap. The tofu line can then claim all 100 g again. Both lines become sufficient, and the shopping list misses 50 g.
Reserve stock in a temporary pool while trying each candidate, then commit only the chosen candidate's use.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if let eaters = request.weekly?.eatersPerDinner, eaters >= 1, | ||
| pendingLeftoverServings >= eaters, let carried = lastCooked { | ||
| pendingLeftoverServings -= eaters | ||
| let dinnerID = "\(request.planID)-d\(position)" | ||
| dinners.append( | ||
| PlannedDinner( | ||
| position: position, dinnerID: dinnerID, recipe: carried.ref, | ||
| recipeDatabaseID: carried.databaseID, title: carried.title, servings: eaters, | ||
| timeMinutes: carried.timeMinutes, lines: [], leftoverOfDinnerID: carried.dinnerID)) |
There was a problem hiding this comment.
pendingLeftoverServings pools portions from different dishes, but the leftovers dinner lists only lastCooked. Cook two different dishes for three servings each with two eaters: each leaves one portion, yet slot three promises two portions of the second dish. Its leftoverOfDinnerID points only to that dinner, which has just one portion left.
Track surplus per dinner and require the referenced dinner to cover the slot, or list every dish contributing to a mixed leftovers dinner.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/ios/FeatureLogic/CookMode/Plan/PlanAllocating.swift
Line: 198-206
Comment:
**Leftovers promise the wrong dish**
`pendingLeftoverServings` pools portions from different dishes, but the leftovers dinner lists only `lastCooked`. Cook two different dishes for three servings each with two eaters: each leaves one portion, yet slot three promises two portions of the second dish. Its `leftoverOfDinnerID` points only to that dinner, which has just one portion left.
Track surplus per dinner and require the referenced dinner to cover the slot, or list every dish contributing to a mixed leftovers dinner.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| databaseID: Self.catalogDatabaseID(recipe.catalogID), | ||
| ref: RecipeRef(source: .catalog, id: recipe.catalogID, revision: recipe.revision), |
There was a problem hiding this comment.
Catalog dinners now use IDs such as rcp-000201 and numeric identity -201, but load(planID:) still reconstructs their identity with Int64(recipe.id) ?? 0. Every such dinner reloads with recipeDatabaseID = 0. Loading is currently used only by tests, but future restore callers would lose the ID needed to exclude a dinner during replacement or regeneration.
Preserve the catalog identity when loading, and add a round-trip test using a real catalog ID. The new persistence test uses numeric strings and misses this case.
Knowledge Base Used: Mobile domain model and persistence
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/ios/Platform/Persistence/KitchenPlan/PlanDataAdapter.swift
Line: 227-228
Comment:
**Saved catalog IDs become zero**
Catalog dinners now use IDs such as `rcp-000201` and numeric identity `-201`, but `load(planID:)` still reconstructs their identity with `Int64(recipe.id) ?? 0`. Every such dinner reloads with `recipeDatabaseID = 0`. Loading is currently used only by tests, but future restore callers would lose the ID needed to exclude a dinner during replacement or regeneration.
Preserve the catalog identity when loading, and add a round-trip test using a real catalog ID. The new persistence test uses numeric strings and misses this case.
**Knowledge Base Used:** [Mobile domain model and persistence](https://app.greptile.com/s-gu/-/custom-context/knowledge-base/samgu-nrx/fridgeluck/-/docs/mobile-domain-and-persistence.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| let id: Int64 = row["ingredient_id"] | ||
| return stapleTiers.contains(tier) ? id : nil | ||
| }) | ||
| let allStaples = staples.union(catalog.stapleIngredientIDs) |
There was a problem hiding this comment.
The catalog staple union overrides an explicit only_if_confirmed household choice. If a bridged ingredient has that household tier but any catalog line marks it assumed_staple, it is added back to allStaples. With no stock, the allocator then marks it assumed and strikes through its shopping entry instead of treating it as missing.
Apply catalog defaults only when the household has not explicitly chosen a tier.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/ios/Platform/Persistence/KitchenPlan/PlanDataAdapter.swift
Line: 113
Comment:
**Catalog overrides household staple choices**
The catalog staple union overrides an explicit `only_if_confirmed` household choice. If a bridged ingredient has that household tier but any catalog line marks it `assumed_staple`, it is added back to `allStaples`. With no stock, the allocator then marks it assumed and strikes through its shopping entry instead of treating it as missing.
Apply catalog defaults only when the household has not explicitly chosen a tier.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Independent review — executed allocator probes + harness runVerdict: changes needed. Three defects reproduced by execution in a scratch Linux harness, plus two lower-urgency findings. Context: all 37 repository tests pass in the same harness (0 skipped) — none of them covers these paths. 1. HIGH — a substitution can bypass a blocked allergen (executed repro)
Tofu lands on the plate for an allergic household. Suggested fix: thread blocked ingredient IDs into the allocator (or filter 2. HIGH — substitute stock double-counted within one dinner (executed repro)
The swap consumed 50 g tofu for the chicken line; the tofu line then counted the full 100 g pool again. Suggested fix: consume as you allocate (sequential pool mutation, the same pattern the lots path already uses) so later lines see what earlier lines took. 3. MEDIUM-HIGH — a leftovers night can promise another dish's surplus (executed repro)Surplus servings are pooled across cooked dinners, but a leftovers slot references only Suggested fix: keep surplus per dinner and draw a leftovers promise from the referenced dinner's own surplus, falling back to cooking when it can't cover the slot. 4. LOW-MEDIUM — catalog identity doesn't survive plan persistence (code-read, callers test-only today)
5. LOW — catalog
|
… staples Review round on PR #178 (five findings), each pinned by a failing-first regression test in LeChefPlanWeeklyTests: - Blocked substitutes are never offered or applied: the snapshot drops swaps whose substitute is a household allergen, and the allocator refuses any swap whose substitute an eater blocks (previously only recipe lines were screened). - Stock is claimed within a dinner: a line's swap sees the pool minus what earlier lines of the same dinner already claimed, so the substitute line reports its real remainder instead of hiding it. - Leftover surplus is tracked per cooked dinner: a leftovers night re-serves one dish and must be covered by that dish's own surplus, never by portions pooled across different dishes. - Saved catalog dinners reload their numeric identity (the negative digit tail of the rcp- id) instead of 0, keeping replacement and exclusion working after a plan reload. - Catalog assumed_staple defaults yield to an explicit household pantry tier: an ingredient marked only_if_confirmed is never re-assumed. Verified: Linux scratch harness 48/48 (the six new/updated tests fail before the fixes and pass after). Hosted macOS CI runs the app-target suite. Co-authored-by: Sam Gu <sgu07966@gmail.com>
|
All five findings from the independent review are accepted and fixed at
Each pinned test failed before its fix (11 failing assertions reproducing the findings) and the full Linux harness passes 48/48 after; hosted macOS CI is green on this head (run 38836748879). One drafted test for finding 3 was removed instead of landed — it described a state the corrected model cannot reach; reasoning is in the PR body. |
2 participants
feat(ios): weekly horizon for the LeChef plan — one allocator, repeats priced, leftovers carried
Human author: Sam Gu (sgu07966@gmail.com)
Stacked on #172 (base
obv/burst6-fridgeluck-catalog-recommend, merged at832c5921).What changed and why
SPEC.md §14 on #52 (marked "Adapt"): five-to-seven-dinner plans run the same allocator core across the whole week instead of a second planner.
PlanAllocatinggainsPlanWeeklyPolicy(eaters-per-dinner, repeat penalty, consecutive-main-ingredient penalty).nil— the default onPlanRequest— keeps three-dinner behavior byte-for-byte: repeats excluded outright, no surplus carry. With a policy, repeats become rankable but penalized (+1000g coverage-equivalent), the same main two days in a row is penalized (+500g), and a dinner cooked above eaters-per-dinner leaves surplus that can cover a later leftovers slot (no new reservations; the slot re-lists the earlier dinner's dish).cook_plan_leftover_carry_v1addscook_plan_dinner.leftover_of_dinner_id; save writes it, load selects and restores it, andPlannedDinnercarries it (nilfor fresh dinners). A reload of a saved week distinguishes a leftovers night from a planned one.PlanDataAdapter.loadSnapshotmerges bundle-v2 catalog recipes alongside legacy ones through the ingredient-identity bridge (PR feat(ios): hydrate recipe bundle-v2 catalog with legacy fallback and revision history #151 convention: one key → smallest bridged app-ingredient id). Unresolved keys stay as stable negative-sentinel lines with unknown quantity (quantity_unknownattention, never a purchase); an unresolved staple line is dropped (households have it); a resolved line the bundle flagsassumed_staplejoins the staples set; and a catalog successor yields to its still-usable legacy recipe. Allergen filtering stays where it was — candidates remain a raw inventory, the blocked ids rideeaters, and allocation enforces the hard filter.PlanSheetViewModel: five-to-seven dinners select the weekly policy with the shipped penalties; three-to-four keep the strict default.How I checked it
LeChefPlanWeeklyTests.swift(16 tests): weekly repeats and consecutive-main penalties, leftover carry, short-line cap in weekly mode, allergen exclusion through allocation, plan round-trip through the cook-plan tables, catalog merging/staples/sentinel/successor behavior.substitutions-v1.json): 44 tests, 0 failures at merged head30436d26(swift test -Xswiftc -DSQLITE_DISABLE_SNAPSHOT).git statusclean before push.121244d5(run 38818234422).What I could not check
build-and-testanddead-code-advisory) was not run locally. CI on this PR covers it.What I assumed
leftoverOfDinnerIDpoints at the covering dinner, and the snapshot test pins the drop of an unresolved staple line rather than a sentinel for it.eaters(existing convention), so the adapter test asserts successor-return plus allocation-time exclusion, not snapshot filtering.Review round (independent review — five findings, all accepted and fixed)
Fixed at head
1ca4ec2a; each finding is pinned by a regression test that fails before its fix and passes after (verified on the Linux harness, not just asserted).PlanDataAdapter.loadSnapshotdrops any swap whose substitute is a household allergen;PlanAllocating.coveringSubstitutionadditionally refuses substitutes an eater blocks, so allocator-level callers are covered too.testSnapshotFiltersBlockedSubstitutes(pre-fix: the blocked swap was offered) andtestSwapCandidateRespectsBlockedIngredients(pre-fix: linesufficientwith the blocked swap applied)allocateDinnerkeeps aclaimedledger: every line (original or substitute consumption) reads the pool minus what earlier lines of the same dinner claimed; swap eligibility checks the claimed-adjusted substitute stock too.testSwapClaimReservesStockWithinADinner(pre-fix: tofu linesufficient, shopping list empty while 50 g was actually missing)surplusByDinner); a leftovers night fires only when the referenced dinner's own surplus covers a full round of eaters, and it re-serves that dish.testLeftoverCarryNeedsAFullRoundOfEaters— rewritten from pooled to per-dinner semantics (pre-fix: a pooled leftovers night was promised); the existing positive leftovers tests still pass unchangedrecipeDatabaseID == 0(load parsedRecipeRef.idas a legacy integer only), so replacement and exclusion lost their identity after a reload.loadreconstructs both conventions: positive integer ids pass through,rcp-…refs take the same negative digit-tail identity the snapshot assigns.testSavedCatalogDinnersReloadTheirDatabaseID(pre-fix: reloaded 0, expected -201)assumed_stapledefaults unioned over an explicit householdonly_if_confirmedtier, silently re-assuming an ingredient the household downgraded.testHouseholdStapleChoiceSurvivesTheCatalog(pre-fix: egg re-assumed as staple)One test was removed rather than fixed: the drafted
testLeftoverNightIsCoveredByTheDishItReServesdescribed a state the corrected per-dinner model cannot reach (a leftovers slot always fires while any dinner holds a full round, so "most recent cannot cover, an earlier one can" is unreachable); per-dinner coverage is already pinned by finding 3's test plus the two existing positive leftovers tests.Verification for this round: failing-first run on the Linux harness — the six review-round tests failed 11 assertions reproducing the findings exactly; after the fixes, the full harness passes 48 tests, 0 failures (
swift test -Xswiftc -DSQLITE_DISABLE_SNAPSHOTat1ca4ec2a). Hosted macOS CI on this head: build-and-test and dead-code-advisory passed (run 38836748879 at1ca4ec2a). One review-fix commit, PR state otherwise unchanged (not merged, workflows untouched).Inherited CI note
The base lineage carries two pre-existing test-target breakages, both in other threads' files and both masked behind each other in CI (each aborted compilation before the next file type-checked):
RecipeCatalogRecommendTests.swift:179 cannot find 'CookThermometer' in scope— fixed on the base at38263783(explicitFLFeatureLogicimport).TasteEventStoreTests.swift:50— thecookedEventhelper declared-> TasteEventwhile constructingTasteEventRecord(seven type errors;store.appendtakes the record type). Present verbatim at this branch's base point9f5a7017(verified withgit show). Fixed here by cherry-picking the trunk integration's one-line fix9de29858(CI-green at trunk, run 38805706914) as121244d5. If the base or trunk lands its own fix, this merge resolves to identical content.Every earlier
build-and-testfailure visible from this PR was inherited, not introduced by this delta. Resolved: the rerun at121244d5passed (run 38818234422) — nothing else surfaced.🔗 Obvious Project · 🧵 Obvious Thread · 🔀 Obvious PR Artifact