Skip to content

feat(ios): hydrate recipe bundle-v2 catalog with legacy fallback and revision history - #151

Draft
obvious-autobuild[bot] wants to merge 13 commits into
obv/burst5-fridgeluck-spec-recipesfrom
obv/burst5-fridgeluck-rcp-loader
Draft

obvious-autobuild[bot] wants to merge 13 commits into
obv/burst5-fridgeluck-spec-recipesfrom
obv/burst5-fridgeluck-rcp-loader

Conversation

@obvious-autobuild

@obvious-autobuild obvious-autobuild Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

What changed and why

Implements the recipe-catalog persistence layer from the bundle-v2 contract (docs/spec/recipes/), on top of the migration seam from obv/burst5-fridgeluck-migration-seam:

  • RecipeCatalogMigrations fills the empty seam: recipes_catalog (one row per stable catalog_id, current revision summary, nullable legacy_recipe_id link, is_user_edited guard), recipes_catalog_ingredients vocabulary (keyed by canonical string key, fdc_id/legacy_id as evidence links, nullable per_100g), per-revision recipes_catalog_nutrition snapshots (insert-once, never rewritten), structured recipes_catalog_lines (quantity evidence: grams / count / household / unknown) and recipes_catalog_steps (timers, thermometer checks — the three thermometer columns move as all-or-nothing via CHECK). Nothing joins on integer ids; legacy recipes and cooking_history are never rewritten.
  • RecipeBundleV2Loader hydrates at launch (wired into AppDatabase.warmup after core hydration). It applies recipes-v2.json when the bundle ships it, else falls back to legacy data.json, hydrating it into catalog rows keyed rcp-NNNNNN by the legacy integer id (revision 1, quarantined rights — legacy rows predate the rights audit and carry no licence evidence, so they stay display-only — and grams-only lines; steps stay empty and legacy rows keep rendering through the existing instructions path). A v2 recipe claims its legacy successor only when the numeric rcp-NNNNNN suffix names a bundled recipe (id 1–166, source = 'bundled') with the same title — conservative, so rcp-000101 (Coconut Chickpea Curry) does not grab legacy recipe 101 (Moroccan chicken stew) while rcp-000002 does succeed the omelette and cooking history keeps resolving. Applies are idempotent: same-or-older revision is a no-op, user-edited rows are never overwritten, and per-revision nutrition history survives revision bumps byte-for-byte.
  • RecipeBundleV2Loader fallback semantics (added during CI rounds): a quarantined legacy fallback row is a stand-in until structured content lands, so a bundle recipe supersedes it even at the same revision; the fallback's legacy link survives only when the title-and-source successor rule genuinely matches, so quarantined rows never leak a wrong legacy_recipe_id into a replaced recipe.
  • RecipeRepositoryCatalog adds read paths: catalogRecipe/catalogRecipes, the legacy-id → catalog-id bridge for cooking history, and per-revision nutrition snapshots.
  • RecipeLoaderTests (new Tests/RecipeLoader/ target dir, picked up by the existing path: Tests test target): fresh install, v18 upgrade with cooking history, idempotence, revision bump, user-edit survival, loud failure naming the recipe id (malformed recipe, unpinned thermometer source, wrong bundle version), and read-path coverage. The fixture is the spec's example bundle (docs/spec/recipes/contracts/examples/bundle-v2.sample.json) embedded verbatim. The v18-upgrade and legacy-fallback tests seed the recipes rows the legacy_recipe_id foreign key requires.

How I checked it

  • Hosted macOS CI is green at 72e5d96 (run 38731987703, build-and-test): the full test suite compiles and passes on the Apple toolchain, including all RecipeLoaderTests. Earlier runs on this PR caught and fixed, in order: an invalid where let loop clause (2da715a), a Codable synthesis failure on the ingredients coding key (743cd75), repository access and API-label issues (d41c59c), test-helper typing issues (12d7d95, d2a73c3), unseeded legacy foreign-key rows plus a missing migration-id assertion (01ba691), lost seed calls from a same-file edit race plus quarantined-fallback supersession semantics (2ccba13), and the RecipeCatalogRightsStatus type reference (72e5d96).
  • Fixture expectations verified against the sample with Python: 6 recipes, 20 ingredients, 23 steps total; the omelette is at revision 2 with 3 lines / 4 steps, step timer 60 s at position 1 and FSIS-pinned thermometer at position 3; curry line/step positions, assumed staples, uses indices, nutrition coverage values, and rights status all match the assertions in the tests. Each negative-test string mutation ("revision": 2, "servings": 1, "uses": [0, 2], usda-fsis: ground beef 160 F, "bundle_version": 2) was confirmed unique in the fixture so the mutations only touch the intended recipe.
  • Fixture round-trips as JSON after embedding in the Swift raw string; no """ or "# sequences in it.
  • API usage cross-checked against the repo by reading the sources the new code touches: DatabaseMigrations.migrate(_:upTo:) and the v18_cooking_history_swaps id, bundled_recipe_state (v8), recipe_ingredients/cooking_history column orders for the seeded rows, RecipeTags bit positions, BundledData/IngredientArray/RecipeArray field names and sodium-in-grams semantics (×1000 → mg), RecipeRepository/NutritionService/HealthScoringService/PersonalizationService initializers, and Equatable conformance on every model type used in XCTAssertEqual.

What I could NOT check

  • No local compile. There is no Swift toolchain in this environment (no swiftc, and installing one risks the 6.7 GB free disk), so nothing was built or run locally — but the repository's hosted macOS CI at 72e5d96 compiles the code and runs the full test suite green (run 38731987703), which supersedes the earlier "uncompiled" caveat for type-level and test correctness.
  • No device or Xcode run. CI covers the simulator/test target only; nothing was exercised in the app UI, on hardware, or against a real recipes-v2.json bundle shipped through an Xcode target.
  • The JSON paths in bundleV2URL(in:) (RecipeCatalog/ subdirectory) are untested until an Xcode target actually ships the resource; until then the legacy fallback path is what runs.
  • GRDB 7 API details are exercised only through the passing CI suite against the pinned GRDB version, not against other GRDB releases.

Assumptions

  • The PR targets obv/burst5-fridgeluck-spec-recipes (the branch this work branches from), not main, matching the spec-series flow.
  • Legacy rows are quarantined (rights_status = 'quarantined') rather than guessed at: they predate the rights audit, so they are display-only and excluded from sharing until provenance is established.
  • A quarantined fallback row is treated as supersedeable at the same revision by bundle content; its legacy link is dropped unless the successor title rule matches.
  • The bundled_recipe_state marker key recipes_catalog_v2_marker records bundle identity (size+mtime); it is informational (re-apply decisions are revision-driven), kept for parity with the existing v7/v8 marker pattern.
  • Launch failure on a malformed bundle throws (fail-closed per spec) rather than silently skipping the recipe; the run.sh-style lease/disk tooling used by other repos' runners was not replicated here.
  • Draft PR — not for merge without human review.

SamGu-NRX and others added 4 commits October 15, 2026 10:28
Register four subsystem migration files (RecipeCatalog, Taste, CookSession,
Community) after the core v18 migration so parallel branches can add local
tables without editing Migrations.swift. Registration order is fixed because
shipped databases record applied identifiers and subsystem migrations may
reference core tables and earlier subsystems. Structural seam tests pin the
contract: full migrate, upTo v18 then finish, identifier classification, and
block ordering.
applied[.. < firstSubsystem] parses as a comparison, not a half-open range,
which failed the CI build. prefix(_:) and suffix(from:) express the same
slices without operator spacing pitfalls.

Co-authored-by: Obvious Autobuild <agents@obvious.bot>
…revision history

Adds the recipe-catalog persistence layer: subsystem migrations behind the
recipes_ identifier seam, a bundle-v2 loader with semantic validation, and
read paths on RecipeRepository.

- RecipeCatalogMigrations: recipes_catalog (+ nutrition snapshots, structured
  lines, structured steps), recipes_catalog_ingredients vocabulary; thermometer
  columns move as all-or-nothing; nutrition history is insert-once per revision.
- RecipeBundleV2Loader: applies recipes-v2.json when the bundle ships it, else
  hydrates legacy data.json into catalog rows keyed rcp-NNNNNN by legacy id
  (revision 1, quarantined rights, grams-only lines). v2 recipes claim their
  legacy successor only when the numeric suffix names a bundled recipe with a
  matching title, so cooking history keeps resolving without false links.
  Idempotent on re-apply; user-edited rows never overwritten; historical
  nutrition preserved byte-for-byte across revision bumps.
- RecipeRepositoryCatalog: catalogRecipe/catalogRecipes reads, legacy-id
  bridge, and per-revision nutrition snapshots.
- AppDatabase warmup applies the catalog after core hydration.
- RecipeLoaderTests: fresh install, v18 upgrade with cooking history,
  idempotence, revision bump, user-edit survival, loud failure naming the
  recipe id (malformed recipe, unpinned thermometer source, wrong bundle
  version), and read-path coverage against the spec example bundle embedded
  verbatim.

Co-authored-by: obvious-autobuild[bot] <262744130+obvious-autobuild[bot]@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

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: 58667e6d-6d92-40b8-9d4f-c95dc46202a8

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

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

SamGu-NRX and others added 6 commits October 15, 2026 11:06
…alidation

Co-authored-by: obvious-autobuild[bot] <262744130+obvious-autobuild[bot]@users.noreply.github.com>
…e synthesizes

Co-authored-by: obvious-autobuild[bot] <262744130+obvious-autobuild[bot]@users.noreply.github.com>
- Open RecipeRepository.db to internal so repository extensions in separate
  files can read the queue.
- Add the missing raw: argument label and try on the throwing vocabulary
  lookup in the legacy hydration path.

Co-authored-by: obvious-autobuild[bot] <262744130+obvious-autobuild[bot]@users.noreply.github.com>
Co-authored-by: obvious-autobuild[bot] <262744130+obvious-autobuild[bot]@users.noreply.github.com>
…elpers

Co-authored-by: obvious-autobuild[bot] <262744130+obvious-autobuild[bot]@users.noreply.github.com>
Co-authored-by: obvious-autobuild[bot] <262744130+obvious-autobuild[bot]@users.noreply.github.com>
SamGu-NRX and others added 3 commits October 15, 2026 11:44
…w migration

- The legacy loader stamps legacy_recipe_id, which FK-points at recipes(id);
  tests that apply the legacy JSON now seed matching recipes rows first,
  mirroring the production upgrade state.
- The migration-order test now expects recipes_catalog_v1 as the fourth
  migration.

Co-authored-by: obvious-autobuild[bot] <262744130+obvious-autobuild[bot]@users.noreply.github.com>
- A quarantined fallback row (rights_status = quarantined with a legacy
  link) is a stand-in until structured content lands, so a bundle recipe
  now supersedes it even at the same revision. The legacy link survives
  only when the title-and-source rule genuinely matches; preserving it
  blindly misattributed cooking history for rcp-000101.
- Capture the quarantined revision between the legacy and bundle applies
  in the v18 upgrade test, and seed the legacy recipes rows the FK
  requires in the two fresh-database legacy tests (an earlier same-file
  edit race dropped these calls from the pushed commit).

Co-authored-by: obvious-autobuild[bot] <262744130+obvious-autobuild[bot]@users.noreply.github.com>
RecipeCatalogRightsStatus is a top-level type in RecipeBundleV2.swift,
not nested under RecipeCatalogRights.

Co-authored-by: obvious-autobuild[bot] <262744130+obvious-autobuild[bot]@users.noreply.github.com>
@obvious-autobuild

Copy link
Copy Markdown
Author

Independent review (Obvious, Autobuild + Max)

Verdict: ready. No blocking defect found. This draft was reviewed independently of its authorship; I executed the decode/validate layer locally (Swift 6.4 on Linux, byte-identical PR source), traced every loader/repository claim in the diff against repo sources and production data.json, and confirmed hosted macOS CI green at the head SHA. Findings below are low/informational — worth fixing, none blocks this PR.

Findings (ranked by severity)

  1. Low — positional migration assertion breaks again at the next subsystem migration. apps/ios/Tests/MigrationUpgradeTests.swift:19 asserts applied.suffix(4) equals [v16, v17, v18, recipes_catalog_v1]. The first real migration added to TasteMigrations/CookSessionMigrations/CommunityMigrations re-breaks it — this PR itself had to bump the same assertion from suffix(3) to suffix(4) to stay green. Smallest fix: assert containment (v18_cooking_history_swaps is followed by recipes_catalog_v1) and leave ordering to MigrationSeamTests.testCoreMigrationsRunBeforeSubsystemMigrationsInFixedOrder, which already pins subsystem block order structurally.

  2. Low (informational) — two schema constraints are not mirrored app-side. recipe-bundle-v2.schema.json requires meal_slots minItems: 1 and cuisines minItems: 6; RecipeBundleV2.validate() (apps/ios/FeatureLogic/Recipe/RecipeBundleV2.swift:560–623) checks neither. Executed evidence: a bundle with an empty meal_slots array decodes and validates in my harness. Bounded consequence — the shipped bundle is schema-checked at source (check.py passes), so this only weakens defense-in-depth: a decoded-but-invalid bundle would hydrate with empty slots and silently vanish from meal-slot-filtered browse. Smallest fix: one guard !recipe.mealSlots.isEmpty in validate() (plus a cuisines-count check).

  3. Low (informational, shared with the spec) — thermometer pin is prefix-only. RecipeBundleV2.validate() at RecipeBundleV2.swift:609 accepts any source with prefix usda-fsis; "usda-fsis-not-really: a blog" decodes and validates (executed). This exactly mirrors the schema's pattern: "^usda-fsis", so it is a spec weakness, not a PR bug. If the pin is meant to be load-bearing for food safety, tighten both spec and code to a stricter form (e.g. ^usda-fsis: allowlist) in a spec revision — not this PR.

  4. Low — legacy line hydration is fail-open per line. RecipeBundleV2Loader.writeLegacyLine (apps/ios/Platform/Persistence/Bundle/RecipeBundleV2Loader.swift:398) silently drops a line whose ingredient id has no vocabulary row (bundledIngredientName returns nil, loader line 413). It is the one deliberate exception to this PR's otherwise fail-closed posture and is commented as corrupt-bundle tolerance; production data.json is self-consistent today (verified below), so I would not change the behavior — but the drop is silent. Smallest fix: log or accumulate a skipped-lines count for the DEBUG warmup diagnostics already printed in AppDatabase.warmBundledContentIfNeeded.

What I verified (commands and results)

  • CI: gh pr view 151 --repo SamGu-NRX/FridgeLuck --json … and gh api repos/SamGu-NRX/FridgeLuck/commits/72e5d96a444cf2df1294f492f32876a7e30f39a8/check-runs → run 38731987703 "iOS CI" success at exactly the head SHA; build-and-test and dead-code-advisory both green. The body's CI claim is accurate, and its commit narrative (2da715a → 72e5d96) matches git log origin/obv/burst5-fridgeluck-spec-recipes..72e5d96.
  • Fixture = spec sample: extracted the embedded raw string from RecipeLoaderTests.swift (lines 444–974) and compared with docs/spec/recipes/contracts/examples/bundle-v2.sample.json in Python — parsed JSON equal: True; 6 recipes / 20 ingredients / 23 steps; omelette rev 2 with 3 lines / 4 steps, timer 60 s at step 1, FSIS-pinned thermometer at step 3. The repo's own python3 check.py (docs/spec/recipes/contracts) passes: "all checks passed (50 identity entries, 6 recipes, 6 events)".
  • Executable decode-layer harness (Swift 6.4, Linux; apps/ios/FeatureLogic/Recipe/RecipeBundleV2.swift copied byte-identical, sha256 1ad072ae…): 23/23 checks passed — pristine fixture decodes; the body's four negative mutations each fail closed naming the right recipe (uses [0,99] → rcp-000002, removed servings → rcp-000002, unpinned thermometer → rcp-000301, bundle_version: 3 → unsupportedBundleVersion(3)); revision bump 2→3 is decode-clean, so the loader-level revision-bump test really exercises supersession; duplicate ingredient key / duplicate catalog_id / revision 0 / unknown line key / malformed rcp-12 catalog_id all rejected; hand-rolled quantity-enum Codable round-trips and unknown-kind rejection pass.
  • Revert check (negative control): recompiled the same file with only the hasPrefix("usda-fsis") guard deleted — the unpinned-thermometer mutation then decodes successfully ("NEGCTL: DECODES WITHOUT THE GUARD"), i.e. testUnpinnedThermometerSourceFailsClosed is load-bearing on that exact check rather than on incidental decode ordering.
  • Loader semantics (diff vs repo sources): user-edit guard checked before the quarantined-fallback clause (RecipeBundleV2Loader.swift:105–118); quarantined fallback superseded even at equal revision, and the upgrade test pins this via rcp-000101 — without that clause the fallback row would keep legacy_recipe_id = 101 and the test's XCTAssertNil would fail; same-or-older revision is otherwise a no-op; nutrition history is INSERT OR IGNORE keyed (catalog_id, revision); each apply is one dbQueue.write transaction, and the malformed-bundle test asserts nothing was written.
  • Production successor rule: apps/ios/Resources/data.json has exactly 166 recipes, ids 1–166; recipe 2 is titled Simple Cheese Omelette (so rcp-000002 links in production and cooking history keeps resolving) and recipe 101 is Moroccan chicken stew (so rcp-000101 "Coconut Chickpea Curry" correctly stays unlinked). Both body examples verified against real data.
  • API/schema cross-references: test seed rows match recipes / recipe_ingredients / cooking_history column orders; legacy sodium is grams (egg = 0.124), so ×1000 → mg is correct; RecipeTags bit names used by the legacy mapping all exist; bundled_recipe_state (v8) matches setMarker's upsert; foreignKeysEnabled = true (AppDatabase.swift:19) so the legacy_recipe_id FK is real; FridgeLuckTests sources are apps/ios/Tests (project.yml:34–39), so the new Tests/RecipeLoader/ dir is picked up; no recipes-v2.json ships in Resources/ today, so the legacy fallback is the live path (body discloses this); subsystem migrations register after all core vN_ migrations and are pinned by the new seam tests.

What I could not verify

  • The GRDB-backed tests (loader, repository, migrations) did not run locally: the apps/ios package does not compile on Linux (@testable import FridgeLuck, pre-existing). Their evidence is the hosted macOS CI run above, which I confirmed green at the head SHA but did not execute myself.
  • No Xcode/device run: bundleV2URL(in:) resource lookup is unexercised until an Xcode target actually ships recipes-v2.json; the subdirectory: "RecipeCatalog" path is untested (the body discloses both).
  • I did not re-run the Periphery dead-code advisory (green in CI) and ran check.py only for the contract samples, not for a rebuilt production bundle.

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.

2 participants