Repository navigation
Conversation
ScoredRecipe now carries catalog provenance and a pooled identity that is unique across both pools. findMakeable and findNearMatch search the catalog pool beside the legacy pool and merge one ranked list: catalog candidates must resolve every required line through the identity bridge, pass allergen and diet-claim gates, and gain use-soon urgency from the existing expiry query; legacy-successor recipes are excluded per candidate so a catalog row and its legacy recipe never double-rank. Shared ranking keeps ordering identical across pools. Co-authored-by: Sam Gu <sgu07966@gmail.com>
The bundled catalog names ingredients by stable keys (fdc_id or legacy id) while the app pantry stores ingredient row ids. Add the recipes_catalog_ingredient_bridge table and RecipeCatalogIngredientBridge, which resolves each catalog key to app ingredient ids via legacy_id or the bundled USDA name/alias mapping, leaving unresolved keys unmapped rather than guessing. Rebuild the bridge inside the same transaction as the vocabulary upsert on both the v2 and legacy hydration paths so key resolution can never drift from the vocabulary. The migration registers after the catalog vocabulary migration so its parent tables exist first. Co-authored-by: Sam Gu <sgu07966@gmail.com>
Add a catalog cook-packet API that maps catalog lines to CookQuantity and structured steps (timers and thermometer checks) to CookStep, and wire the preview drawer to load bridged catalog ingredients with quantity display helpers instead of the legacy recipe path. Co-authored-by: Sam Gu <sgu07966@gmail.com>
Results-grid and transition identities move to pooled ScoredRecipe identity, and the reverse-scan selection key follows it so catalog recipes with no legacy integer id stay distinct instead of colliding. Co-authored-by: Sam Gu <sgu07966@gmail.com>
Twelve cases covering merged pools and identity, unresolved bridge keys, allergen and diet gates, successor deduplication, use-soon scoring and reasons, cook packets, and identity-bridge rebuilding. Co-authored-by: Sam Gu <sgu07966@gmail.com>
Co-authored-by: Sam Gu <sgu07966@gmail.com>
|
@codex review |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f5a7017fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let recipe = Recipe( | ||
| id: nil, | ||
| title: title, | ||
| timeMinutes: minutes, | ||
| servings: servings, | ||
| instructions: "", | ||
| tags: 0, | ||
| source: .bundled, |
There was a problem hiding this comment.
Catalog matches are materialized as a Recipe with id: nil and an empty instructions string, but the start-cooking flow still constructs CookingGuideView from that Recipe. As a result, selecting any catalog recommendation opens a guide with no ingredient rows or steps (CookingGuideView.loadIngredients returns immediately for a nil id and CookingGuideSteps parses the empty instructions), so the newly recommended catalog recipes cannot actually be cooked.
Useful? React with 👍 / 👎.
| switch profile.selectedDietID { | ||
| case "vegetarian" where claims?.vegetarian != true: | ||
| continue | ||
| case "vegan" where claims?.vegan != true: | ||
| continue | ||
| default: | ||
| break | ||
| } |
There was a problem hiding this comment.
The catalog diet gate only rejects recipes for vegetarian and vegan; HealthProfile also supports keto and pescatarian, but those cases fall through to default and every catalog recipe is admitted regardless of its claims. The legacy query applies the corresponding tag mask, so users with either restriction can receive catalog recipes that violate their selected diet.
Useful? React with 👍 / 👎.
| switch profile.selectedDietID { | ||
| case "vegetarian" where claims?.vegetarian != true: | ||
| continue | ||
| case "vegan" where claims?.vegan != true: | ||
| continue | ||
| default: | ||
| break | ||
| } |
There was a problem hiding this comment.
The catalog pool bypasses HealthProfile.requiredRecipeTagMask and only checks vegetarian/vegan claims. For keto and pescatarian, selectedDietID falls through this switch, so catalog recipes with no compatible claim are still recommended even though the legacy pool excludes them; this can surface recipes that violate the user's active dietary restriction.
Useful? React with 👍 / 👎.
| let recipe = Recipe( | ||
| id: nil, | ||
| title: title, | ||
| timeMinutes: minutes, | ||
| servings: servings, | ||
| instructions: "", | ||
| tags: 0, | ||
| source: .bundled, |
There was a problem hiding this comment.
When a catalog result is sent to Le Chef, the results view only loads ingredients through ingredientsForRecipe when scored.recipe.id is non-nil. Catalog results deliberately have no legacy integer id, so LiveAssistantRecipeContext receives an empty ingredient list and the synthesized recipe has empty instructions as well, leaving the assistant without the structured catalog content that was just matched.
Useful? React with 👍 / 👎.
The drawer's quantity helpers reference CookQuantity from the FLFeatureLogic package; sibling feature files already import it, and macOS CI flagged the two missing-symbol errors this import fixes. The Linux portable check cannot compile SwiftUI targets, so this surface is CI-verified only. Co-authored-by: Sam Gu <sgu07966@gmail.com>
The candidate id, its selection state, and the dedupe set move to the pooled ScoredRecipeID, which already covers the old title-hash fallback via .unsaved. The screen's chosen-recipe key now branches on how the choice was made: the legacy row id for manual picks, the pooled identity for candidate picks, so catalog entries stay distinct. Co-authored-by: Sam Gu <sgu07966@gmail.com>
The test target compiles against the FLFeatureLogic framework target, so its public types need the explicit import; the Linux harness builds the same sources as one module and could not surface this. Co-authored-by: Sam Gu <sgu07966@gmail.com>
|
Heads-up from the weekly-plan PR (#178): the It is masked in every run so far because compilation aborted at the The trunk integration already carries the one-line fix: |
The app and catalog targets compile at 95ec322; CI now fails in the inherited TasteEventStore tests — the cookedEvent helper is declared -> TasteEvent while constructing a TasteEventRecord, which only compiles in PR #172's merge preview because the trunk branch supplies its fixed copy (9de2985). The catalog-recommend base carries the pre-fix file, so PR #174's merge preview fails. Apply the same one-line return-type correction the trunk made, so the branch matches the trunk-fixed version and future base merges resolve cleanly. Co-authored-by: Sam Gu <sgu07966@gmail.com>
|
Independent review (Obvious, Autobuild + Max) Verdict: changes neededThe engine work is solid and the test claims check out (12/12 reproduced independently on Linux against the production sources, both key behaviors fails-first-verified). What blocks readiness is that the new recommendations dead-end at the two actions that make them useful — cooking and Le Chef — and the keto diet gate fails open on the catalog pool. Findings1. High — catalog recommendations can't actually be cooked, and Le Chef sees an empty plate.
2. High — keto gate fails open on the catalog pool. 3. Low, pre-existing — Linux-portable runs on this lineage trip over What I verified (commands + results)
Couldn't verify
Codex's four inline findings: #1 (cook dead-end) and #4 (Le Chef empty context) confirmed; #2/#3 confirmed for keto, not for pescatarian (no legacy gate exists to be asymmetric with). 🔗 Obvious Project · 🧵 Obvious Thread · 🔀 Obvious PR Artifact |
The claims schema has no low-carb field, so no catalog recipe can prove keto compliance, while the legacy pool enforces the .lowCarb tag mask — without this arm a keto user loses every legacy result yet keeps every catalog result. Fails-first: testKetoDietExcludesCatalogPoolFailingClosed reproduces the ungated catalog entry before the gate arm lands. Co-authored-by: Sam Gu <sgu07966@gmail.com>
Catalog entries materialize as nil-id legacy stand-ins, so Start Cooking rendered an empty guide, loadIngredients returned nothing, and Le Chef received zero ingredient context. Add catalogCookPacket(for:) and cookPacketIngredients(_:) on the repository — the single structured path — and route the preview drawer, the cooking guide, and the assistant context through it. Fails-first: the packet path had no repository API before this commit and testCatalogRecommendationYieldsCookPacketAndBridgedIngredients pins non-empty steps and bridged ingredient lines for a catalog result. Co-authored-by: Sam Gu <sgu07966@gmail.com>
TasteEventStoreTests declared cookedEvent -> TasteEvent while constructing a TasteEventRecord, which Linux Swift 6.4 rejects with 16 conversion errors (reproduced in the portable harness; fails-first). Ports trunk commit 9de2985 so the Linux portable checks compile on this lineage too. macOS CI compiles both forms; semantics are unchanged. Co-authored-by: Sam Gu <sgu07966@gmail.com>
2 participants
fleet-packet: burst6-fridgeluck-catalog-recommend
Human author: Sam Gu (sgu07966@gmail.com)
What changed and why
The shipped bundle-v2 catalog (763 recipes) was hydrated into
recipes_catalogbut invisibleto recommendations:
findMakeable/findNearMatchonly searched the legacy pool, and thepreview drawer loaded catalog recipes through the legacy recipe path, which does not exist for
catalog rows. This makes the catalog a first-class recommendation pool:
RecipeCatalogIngredientBridge+recipes_catalog_ingredient_bridgemigration): resolves every catalog ingredient key to app ingredient row ids via
legacy_idor the bundled USDA name/alias mapping; keys with no safe resolution stay unmapped. The
bridge is rebuilt inside the same transaction as the vocabulary upsert on both the v2 and
legacy hydration paths, so resolution can never drift from the vocabulary. The migration
registers after the catalog vocabulary migration (its FK parent table must exist first).
RecipeRepositoryCatalogRecommend.swift,RecipeScoring.swift,RecipeRepository.swift): catalog candidates must resolve every required line through thebridge, pass allergen and diet-claim gates, and gain use-soon urgency from the existing
three-day expiry query. Results share the legacy ranker;
ScoredRecipenow carries catalogprovenance and a pooled
ScoredRecipeIDidentity unique across pools, and legacy-successorcatalog rows are excluded per candidate so a legacy recipe and its catalog successor never
double-rank (legacy wins; its integer id anchors history).
RecipeRepositoryCatalog.swift,RecipePreviewDrawer.swift): acatalog cook-packet API maps catalog lines to
CookQuantityand structured steps (timers,thermometer checks) to
CookStep; the drawer loads bridged catalog ingredients. Resultsgrid, transitions, cook sessions, and the reverse-scan selection key moved to pooled
identity so catalog recipes with no legacy integer id stay distinct.
apps/ios/Tests/RecipeCatalogRecommendTests.swift): 12 cases covering mergedpools and identity, unresolved keys, allergen/diet gates, successor dedup, use-soon scoring
and reasons, cook packets, and bridge rebuilding.
How I checked it
All commands run on Linux (Swift 6.4 toolchain) against production sources:
swiftc -parseover every changed Swift file — no diagnostics.GRDB:
swift test— all tests pass, including the 12 catalog cases (12/12).AppDatabasebootstraps it (loadInto→ensureBundledRecipesHydrated→ensureUSDACatalogHydrated→ensureCatalogHydrated) with the shipped resources:legacy=166 catalog=763 lines=5010, bridge resolves 124 rows via the real Bundle.main USDApath, zero dangling
legacy_recipe_idlinks under enforced foreign keys.findMakeable merged=20 (catalog=20)in 0.07s; full pantry →merged=20 (catalog=12, legacy=8), proving both poolscoexist in one ranked list; identities unique in every run; near-match tier runs over the
merged pool (20); cook packet retrieval for
rcp-000269rev1 returns 8 lines, 3 steps.swift-format lintclean on all 12 changed files; base branch CI green (run 38805706914).What I could not check
target have only been verified by
swiftc -parsehere; hosted macOS CI runs on this push.but is not part of the repo test suite.
rcp-000001–rcp-000166for legacy and starts atrcp-000167, so it ships zero successors by design;that path is covered by unit tests only.
Assumptions
legacy_recipe_idfields and 232 of 329 ingredient keys arefdc-only, so bridge coverage comes from USDA names/aliases (124 rows); 116 of the bundle's
219 distinct FDC ids are absent from the bundled USDA catalog and stay unresolved rather
than guessed — those lines gate their recipes out of exact matches until the data pipeline
ships legacy ids or a broader USDA catalog.
AppDatabasebootstrap sequence used in the scale run;ensureBundledRecipesHydratedskipped its catalog-mapping stage via marker logic in thisenvironment, matching what the same sequence does on a fresh install.
obv/burst6-fridgeluck-trunk(PR feat(ios): one integration branch where every overhaul slice builds together #170's integration branch) per the burstintegration flow; this branch was cut from
90386c6on that trunk.🔗 Obvious Project · 🧵 Obvious Thread · 🔀 Obvious PR Artifact
Opened by the maintainer from the Obvious thread's own description, so every review bot reviews it (CodeRabbit skips PRs authored by bots). The work and its commits are by Obvious.
Co-authored-by: obvious-autobuild[bot] <262744130+obvious-autobuild[bot]@users.noreply.github.com>
Review round (2026-10-16) — changes-requested review addressed
Independent review (comment 6205373538) verdict "changes needed"; all three ranked findings fixed at head
5a33152, no findings disputed.Finding 1 (High) — catalog recommendations dead-ended at cooking and Le Chef. Fixed:
RecipeRepositoryCatalog.swiftgainscatalogCookPacket(for: ScoredRecipe)andcookPacketIngredients(_:)— the single structured path (steps + bridged ingredient lines in the legacy(Ingredient, RecipeIngredient)shape).CookingGuideViewnow builds its guide steps and ingredient list from the packet (async load, same pattern as before),RecipeResultsView.storeRecipeForAssistantpasses the bridged lines toLiveAssistantRecipeContext, andRecipePreviewDraweruses the same repository helper instead of its private mapping. Fails-first: pre-fix, the repository had no path at all —no member 'cookPacketIngredients', nocatalogCookPacket(for:)(compile error in the portable harness); new testtestCatalogRecommendationYieldsCookPacketAndBridgedIngredientspins non-empty steps and bridged lines for a catalog result and passes after the fix (harness 14/14).Finding 2 (High) — keto gate failed open on the catalog pool. Fixed: the gate switch now has a
ketoarm that excludes the pool — the claims schema has no low-carb field, so no catalog recipe can prove compliance while the legacy pool enforces.lowCarb. Fails-first:testKetoDietExcludesCatalogPoolFailingClosedreproduced the ungated catalog entry (XCTAssertFalse failed) before the arm landed; green after. The review's pescatarian note (no legacy gate, no asymmetry) matches the code, so pescatarian is untouched.Finding 3 (Low, pre-existing) —
TasteEventStoreTestsrejected by Linux Swift 6.4. Ported trunk fix9de29858(cookedEventreturnsTasteEventRecord). Fails-first reproduced locally: reverting just that line in the portable harness gives 16 conversion errors / build exit 1; with the fix, build exit 0. Not a semantic change (the function returns what it constructs).Commands and results: portable harness (Linux x86_64 Swift 6.4, production persistence sources, GRDB): red runs as above, then
swift test --filter RecipeCatalogRecommendTests→Executed 14 tests, with 0 failures(12 prior + 2 new);swift format lint --strictclean on the six touched files (TasteEventStoreTests.swiftcarries pre-existingLineLengthfindings on lines 82+ that this PR does not introduce and does not touch). Hosted macOS/iOS CI on the new head: see the checks below — started on push of5a33152.Not verified: no Xcode/device — the SwiftUI cooking flow (Start Cooking on a catalog entry, Le Chef handoff rendering) is compile-checked by hosted macOS CI and call-chain-reviewed, not runtime-verified; Linux portable checks cover the repository layer, which is where all logic in this round lives.