[obvious-autobuild]

What changed and why

Cooking a catalog recipe previously logged the meal with a legacy integer recipe id at revision 1: no link to the catalog recipe, no revision, and a recipe_cooked taste event that named the wrong identity. A later revision bump did not rewrite the stored recipes_catalog_nutrition rows (they are insert-once) — but with no link from a meal to the revision it was cooked from, nothing could resolve a past meal's nutrition back to the snapshot it was actually cooked against: reads reported against whatever was currently installed, and surfaces computing from legacy ingredient rows (the journal) followed the installed data instead.

Now the finish path logs the meal against the exact revision that was cooked:

• recipes_cooking_catalog_refs_v1 (new migration in RecipeCatalogMigrations, after the existing recipes_ migrations): one row per cooking_history entry — history_id (PK, cascade), catalog_id (restrict), revision (check ≥ 1) — plus a (catalog_id, revision) index.
• PersonalizationService.recordCooking writes the link inside the same transaction as the history row and names the catalog ref (catalog_id, revision) in the recipe_cooked event (and its swap events) instead of the integer id. A half-specified identity (id without revision or the reverse) throws CookingLogError.catalogIdentityIncomplete before any write — fail closed.
• MealLogService.logMeal carries catalogId/catalogRevision through both overloads into recordCooking.
• RecipeRepository.catalogNutrition(forHistoryID:) resolves the meal's per-serving nutrition by joining the link to recipes_catalog_nutrition at that revision — the frozen snapshot, never the currently installed revision.
• CookModeViewModel takes an optional CatalogCookContent (catalog id, revision, structured CookSteps from RecipeRepository.catalogCookPacket): the session ref becomes (catalog, catalog_id, revision), steps come from the packet on both start and resume instead of parsing instructions, and the finish path routes to a new Dependencies.logCatalogMeal (nil logger → nothing written rather than a half-linked row). Dependencies.ephemeral().substituteIsAllergen now reports unknown (nil → blocks) instead of false (clears) — the guide's production wiring already returned nil; the ephemeral default contradicted the fail-closed contract.
• CookTimerStore.create records the timer's full duration (totalSeconds) like adjust already did, so a timer restored after relaunch renders its drain ring instead of falling back to deadline-only.

Independent review findings, fixed

The 2026-10-16 independent review (changes-needed verdict) drove commits 5c8ac46, 770ab5e, and d6252a7:

1. The production guide only resolved catalog content through a legacy integer id. CookingGuideView.catalogContent required scoredRecipe.recipe.id, but the recommend pool builds catalog-native results as Recipe(id: nil, …) — every catalog-native recipe would have cooked the legacy path with a synthetic integer ref, a defect no test could see because tests built the view model directly. The gate is now extracted into CookingGuideView.makeCatalogContent(for:in:) — a bridged legacy recipe resolves through recipes_catalog.legacy_recipe_id, a catalog-native result through its own match's catalog ref, and unsaved drafts (no id, no match) stay on the parsed-instructions path. Covered by two new tests (catalog-native without a legacy id, and the bridged path).
2. The frozen-nutrition accessor had no journal consumer. UserDataRepository.computeConsumedMacros computed macros from legacy ingredient rows, which a bundle apply never writes for catalog recipes — the journal reported 0 for every catalog meal. It now prefers the frozen per-serving snapshot at the linked revision, scaled by servings consumed × portion multiplier (same join as the accessor, cross-referenced in both docstrings), and keeps the legacy ingredient math as the fallback for cooks with no catalog link. Covered by a test that cooks a real catalog meal end to end and asserts the journal reports snapshot × servings.
3. The PR body overstated revision-bump effects on historical nutrition. Corrected above: revision bumps never rewrote the insert-once snapshot rows; the defect was the missing link that made past meals unresolvable to their cooked revision.

All three fixes are fail-first: commit 5c8ac46 pushed the tests first and hosted CI went red exactly where predicted — the catalog-native gate test got nil content, and the journal test read 0.0 kcal against an expected 840.0 (run 38835855949). The fix commits turn them green.

How I checked it

• Hosted macOS CI is green at head d6252a7 (run 38838347813): build-and-test pass 4m55s, dead-code-advisory pass 3m03s. The fail-first run at 5c8ac46 was red exactly as designed (run 38835855949: only the two new gate/journal tests failed, for the predicted reasons); the first fix attempt at 770ab5e failed fast on a missing FLFeatureLogic import (run 38837004821), fixed at d6252a7. The earlier head 4fd46bc was green (run 38823477054: build-and-test pass 7m31s, dead-code-advisory pass 4m57s). No toolchain on the working sandbox (Linux; no Xcode), so nothing was compiled or run locally — CI on this PR's head is the compiler and test verifier.
• New CatalogCookFlowTests (app-hosted, run by CI): the full cook flow through the real view model over a hydrated bundle-v2 fixture — suggested 90 s timer from the catalog step, persisted cook_timer row with its full duration restoring after a simulated relaunch, thermometer step surfacing as a safety check (74 °C + pinned source), one history row linked to revision 1, logged nutrition equal to the revision-1 snapshot, and the cooked event carrying (rcp-900001, 1, catalog) with rating; a revision-2 loader bump leaving the old log's nutrition byte-equal; unknown substitute blocking; half-specified catalog identity refused; legacy recipes keeping the parsed-instructions path and integer-id ref. Review additions: the guide's gate resolving for a catalog-native result without a legacy id, the gate resolving through the legacy bridge, and the journal reporting the frozen snapshot for a catalog meal.
• Migration and read-path SQL mirror the existing recipes_catalog_nutrition join patterns in RecipeRepositoryCatalog; the new identifier is recipes_-prefixed and registered inside the contiguous RecipeCatalog subsystem block (MigrationSeamTests pass on CI).

Commit trail

Eleven commits; the failure history is documented here so reviewers can skip to the interesting parts:

• 98beb69 feature commit (migration, link, catalog nutrition read path, catalog-aware guide/celebration wiring, ephemeral allergen fix, tests) — CI lost parts of the intended wiring; 3cfdf7a + 23705f8 re-landed it.
• 95ec322 test fixes: @MainActor isolation and a pinned logMeal overload.
• 9a3fc2e aligns the inherited TasteEventStoreTests helper with the trunk cookedEvent fix (base branch had not picked it up yet).
• 39a347c fixture: the catalog recipe needed two ingredient lines to meet bundle-v2 validation.
• 1cdce5d the substantive fix the tests drove out: startOrResume was ignoring the catalog content — session ref from the legacy integer id, steps parsed from legacy instructions on both start and resume. A catalog cook now runs the packet's structured steps and carries (catalogId, revision) in the session ref.
• 4fd46bc two more the tests drove out: recordCooking built the taste ref from the integer id despite its own docstring, and timer create dropped the duration. Includes the regenerated taste-store-check mirror + pin per Scripts/refresh.sh.
• 5c8ac46 fail-first review tests (red by design — run 38835855949).
• 770ab5e the review fixes: catalog-native gate resolution in the guide, the journal's frozen-snapshot consumer, and FK-safe insert order in the bridge test.
• d6252a7 the missing FLFeatureLogic import the 770ab5e compile exposed.

What I couldn't check

• Device behavior: notification scheduling for restored timers (uses NoopTimerScheduler in tests) and the drawer/guide UI over catalog packets. No Xcode/simulator here; iOS UI was not visually exercised.
• Xcode build of the app target beyond what hosted CI covers.
• The celebration's swap-macro path still gates on recipe.id — noted by the review as adjacent, not changed here.

Assumptions

• "Logged nutrition equals the revision snapshot" means the meal reads nutrition through the link at its recorded revision — recipes_catalog_nutrition rows are already insert-once, so the link is the freeze; values are not duplicated into the history row.
• The journal's snapshot path intentionally reports the linked revision's own per-serving nutrition; swap deltas are not folded in (unknown substitutes already block the log fail-closed, so only known-ratio swaps can be logged against a snapshot).
• The catalog packet's structured steps are what a catalog cook follows; legacy recipes keep today's parse path unchanged (their refs and events are byte-identical to before).
• logCatalogMeal being nil on a catalog session is a wiring error: the log is refused (no row) rather than written without its revision link.

Human author: Sam Gu (sgu07966@gmail.com)

🔗 Obvious Project · 🧵 Obvious Thread · 🔀 Obvious PR Artifact

[coderabbitai]

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 461d1cf1-b253-418b-afae-f1664e0eb349

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[obvious-autobuild]

Independent review (Obvious, Autobuild + Max)

Verdict: changes needed — the service-layer work is correct, transactional, and genuinely tested, but the catalog cook flow only engages through the legacy-id bridge, and the frozen-nutrition read path has no production consumer. Both gaps sit in the glue no test exercises.

Findings

1. [Medium] The end-to-end catalog cook never engages for catalog-native recipes — for most of the shipped catalog, the exact defect the body says is fixed still happens.

CookingGuideView.catalogContent (apps/ios/Feature/Recipe/CookingGuideView.swift:59-62) resolves the packet only through a legacy integer id: guard let legacyId = scoredRecipe.recipe.id, let catalogId = try? deps.recipeRepository.catalogRecipeID(forLegacyRecipeID: legacyId). But the guide's only production caller (RecipeResultsView.swift:192, fed from the recommendation engine) builds catalog results with no integer id — RecipeRepositoryCatalogRecommend.swift:141-142 constructs Recipe(id: nil, …) — and a catalog recipe without a resolved legacy successor has no recipes_catalog.legacy_recipe_id to bridge through. For those recipes catalogContent is nil, so CookModeViewModel.swift:151 takes the legacy ref String(recipe.id ?? -1) → "-1" at revision 1, steps are parsed from the recommend path's empty instructions, and the meal logs with no catalog link and a taste event naming a synthetic integer id — precisely the pre-PR behavior the body describes as the bug. Only recipes bridged at hydration (124 successor rows) get the catalog path; the other ~639 shipped catalog recipes silently don't. The new tests build CookModeViewModel directly with a packet, so CI never sees this gate.

Smallest fix: when recipe.id is nil, resolve from the pool's own match — scoredRecipe.catalog already carries RecipeRef.catalog(id:revision:) (CatalogRecipeMatch) — and fetch catalogCookPacket(catalogId:) from that ref. Extract catalogContent into a testable helper and cover both the id-nil and bridged paths.

2. [Low] The frozen-nutrition read path has no production consumer.

catalogNutrition(forHistoryID:) (apps/ios/Platform/Persistence/Repository/RecipeRepositoryCatalog.swift:76) is called only from its own tests. What users actually see still computes live from legacy rows: journal macros via computeConsumedMacros (UserDataRepository.swift:292, :453), daily totals via the same join, celebration/sync macros via NutritionService.macros(for: recipeId). So even in the working successor path, the journal reports the legacy recipe's macros, not the revision snapshot the meal was logged against. No regression — but "logged with frozen nutrition" is, today, true only through an accessor nothing calls.

Smallest fix: prefer catalogNutrition(forHistoryID:) in the journal when a link row exists (legacy computation as fallback), or scope the title/claim to "link + accessor landed; display wiring follows".

3. [Low] The body's drift sentence overstates the pre-PR behavior.

"Any later revision bump silently rewrote what every past meal reported as its nutrition": journal and daily-total macros come from legacy recipes/recipe_ingredients rows, which bundle apply never rewrites (RecipeBundleV2Loader writes only recipes_catalog* tables). What actually drifted was the identity-resolution layer — taste events named String(recipeId) with source catalog, and feature extraction resolved that through the bridge to the currently installed revision — which this PR does fix. Suggest tightening the sentence so the claim names the taste/identity layer, matching the mechanism.

Things I checked and consider sound (not findings): the fail-closed half-identity guard; the link written in the same transaction as the history row; restrict/cascade FK choices; recipes_cooking_catalog_refs_v1 registered inside the RecipeCatalog subsystem block with the recipes_ prefix (MigrationSeamTests enforce the ordering); the ephemeral substituteIsAllergen nil default (matches the production wiring's fail-closed contract); CookTimerStore.create now carrying totalSeconds like adjust does; legacy events staying byte-identical (catalogId ?? String(recipeId), revision ?? 1); the guide intentionally not wiring logCatalogMeal (the celebration view logs with the catalog params; the VM's nil-logger refusal is the safety seam).

What I verified (commands and results)

• Linux portable suite at PR head 4fd46bc1: cd apps/ios/Tools/taste-store-check && swift test → Executed 49 tests, 0 failures (this compiles the changed PersonalizationService).
• Mirror integrity: diff of app vs mirror PersonalizationService.swift → identical; sha256sum matches the SOURCES.txt pin (b50440f6…); Scripts/refresh.sh → "taste-store-check mirror is in sync" and regenerated TasteStoreLinuxTests in sync; git status clean afterwards.
• Failing-first (temporary local probe, deleted after): a probe asserting (a) a half-specified identity throws CookingLogError.catalogIdentityIncomplete with zero rows written and (b) a catalog cook writes the (catalog_id, revision) link and names it in the cooked event — passed 2/2 at the head; with the guard, the link INSERT, and the catalog ref reverted locally (API shape kept), it failed exactly those assertions: no throw, one history row written, no link row. Restored via git checkout + refresh.sh → 49/49 again, tree clean.
• macOS CI: gh run view 38823477054 --json headSha → 4fd46bc10d05a45d47dd65c5d601f17b24b4cf3a, equal to the PR head; build-and-test success 23:25:35→23:33:06Z (7m31s), dead-code-advisory success 23:33:13→23:38:10Z (4m57s) — both match the body's numbers. CI log shows Test Suite 'CatalogCookFlowTests' … Executed 6 tests, with 0 failures — the new suite genuinely ran on macOS.
• Commit trail: 8 commits on the branch, matching the body's list.

What I couldn't verify

• Anything needing Xcode (no Xcode on this Linux sandbox): the app-target build is hosted-CI-only (green), and the guide's packet resolution, celebration wiring, drawer UI over catalog packets, and restored-timer notifications (NoopTimerScheduler in tests) are compile-checked but not exercised by any test. Finding 1 lives exactly in that untested glue.
• On-device rendering of the restored timer's drain ring and notification scheduling.