[obvious-autobuild]

What changed and why

Machine-checked contract layer for the recipe system, under docs/spec/recipes/ — the data contracts the in-review recipe drafts (#113, #144, #62, #114, #127, #126) currently assume but nothing pins:

• SPEC.md — the design doc: identity rules, bundle semantics, taste-event boundary, decisions and alternatives, and the load-bearing assumption (canonical identity removes the recipe↔catalog id-mismatch class; what evidence would break that premise).
• contracts/recipe-bundle-v2.schema.json — strict JSON Schema for the shipped, versioned bundle: stable rcp-NNNNNN catalog ids + revision (existing recipe ids and cooking-history references survive migration), four quantity kinds (grams / count / household / unknown — "to taste" never gets a guessed gram value), steps referencing ingredients by index with optional timers and FSIS-pinned thermometer checks, diet claims that are boolean | null (null = unknown, treated as fail by allergen checks), per-serving nutrition with a required coverage band (computed / partial / estimated) and a free-text basis naming which lines carry what, and rights provenance (original | quarantined — fail-closed; licensed/attribution_only are future statuses once a licence pipeline exists).
• contracts/ingredient-identity.schema.json + generated map — maps the 50 legacy data.json ingredient ids onto the canonical vocabulary against the bundled 800-row USDA catalog: 13 resolved (unique display-name/alias match), 10 proposed (unique prefix/stem candidate whose qualifiers don't change the food — pipeline confirms against full FDC before shipping), 27 unresolved (multi-variant foods like milk/cheese/mushroom, and misdirection traps: the only catalog hit for egg is salted-and-frozen, for banana it's Banana Pepper, for tofu it's Tofu Yogurt — all recorded as candidates, none auto-resolved). Nothing is invented; unresolved entries are resolved against full FoodData Central (CC0) with pinned verification_source, mirroring the catalog's own verification columns.
• contracts/taste-events-v1.schema.json — the event envelope (six types, bound to catalog_id + revision, opaque household/member ids, closed-set dismiss reasons, structured substitutions) that the recipe system emits to on-device taste learning. The profile and residual-ranker semantics stay owned by PR docs(spec): taste-residual-v1 taste profile and learning spec with machine-checked contracts #146; this fixes only the event vocabulary so bundle revisions can't drift from what the learner replays.
• contracts/examples/ — a 6-recipe / 20-ingredient sample bundle covering every quantity kind, two thermometer checks (egg dishes, ground beef), all three nutrition coverage levels, a revision bump, optional/staple lines; the 50-entry identity map; six taste events covering all six types.
• contracts/check.py + requirements.txt — the machine check.

How I checked it (commands and results)

• python3 docs/spec/recipes/contracts/check.py → all 7 checks pass (3 schemas valid JSON Schema 2020-12; 3 examples validate; cross-checks: bundle keys/ids unique, ingredient keys referenced, uses indexes in range, computed coverage means every gram-weighed line has a per-100g row, identity map covers legacy ids 1–50 exactly with names matching data.json, every non-null fdc_id exists in the bundled catalog, events reference only bundle recipes with revisions within the shipped range and exercise the rev-2 drift scenario, SPEC rights statuses match the schema enum).
• Adversarial tamper tests, committed as contracts/tamper-tests.py: 15 named mutations, 15/15 caught at the current head — uses index out of range; bundle fdc_id not in catalog; identity fdc_id not in catalog; identity map missing an entry; rating out of range; event referencing an unknown recipe; recipe_rated with null or missing rating; rating on a non-rated event; suggestion_dismissed with null or missing dismiss_reason; computed coverage with a null per-100g weighed line; event revision beyond shipped; rights.status outside the enum; plus a failure-message assertion (a catalog miss names the failing entry's own legacy id). Each mutation makes the checker exit nonzero.
• Identity map regenerated programmatically from the two evidence passes against the live sqlite (exact-name/alias, then prefix/stem with an alternate-food blacklist), not hand-authored.
• Self-review fixes before commit: the omelette's nutrition coverage was labeled partial while its basis said all values were estimates — corrected to estimated; generator bugs found and fixed along the way (non-lowercased normalization, alias table's ingredient_id being the catalog row id rather than the FDC id).

What I couldn't check

• Nothing here touches Swift, so no Xcode build applies; CI (macOS) runs on this PR as usual.
• The "proposed" identity mappings are candidates only — they need confirmation against full FDC (form/brand qualifiers like chickpea-dry, avocado-Hass, apple-Golden-Delicious) before any app code consumes them. That import step is explicitly out of scope here.
• The 27 unresolved identities have no resolution in this PR by design.

Assumptions

• Base branch is alpha (where the other recipe drafts are reviewed), not main.
• The canonical ingredient key (stable slug) is the join key going forward; integer ids (legacy 1–50, FDC) are evidence links only.
• Sodium is milligrams in bundle v2; the bundled catalog stores grams and the compiler converts.
• Taste events stay on-device; PR docs(spec): taste-residual-v1 taste profile and learning spec with machine-checked contracts #146 remains the owner of profile semantics.
• Sample-bundle ids like rcp-000002/revision 2 are illustrative fixtures, not a migration commitment for specific rows.

🚢 Draft — ready for review of the contract shapes; will mark ready once the identity-resolution rules get a look from someone who knows the FDC import plan.

Review round 1 (2026-10-15) — changes needed → fixed at e0eedd7

Independent review (pull/148#issuecomment-6199442480) ranked six findings; all six are addressed, none disputed. Failing-first evidence: the tamper suite run against the reviewed head b4a19d7 caught 8/15 — every new invariant case slipped through — and 15/15 at e0eedd7; check.py went from 6 checks that pinned none of the flagged invariants to 7 that pin them.

• 1 High — schema/SPEC rights disagreement (fixed on the SPEC side). The schema's original | quarantined two-state enum is the deliberate fail-closed contract (its own description: "Unknown rights means quarantine"), and it is what the fix(scraper): repair ingredient identity, gram estimates, and diet tags from measured import failures #43 pipeline actually produces — unproven rows ship quarantined. SPEC §2 and this body now say original | quarantined with licensed/attribution_only as future statuses; a new check.py assertion (SPEC rights statuses match the bundle schema enum) fails on the old SPEC text and passes now, so this drift class is machine-caught from here on.
• 2 Medium — event payload invariants (fixed). taste-events-v1.schema.json gained allOf/if/then branches requiring rating as integer 1–5 on recipe_rated, a non-null enum dismiss_reason on suggestion_dismissed, and null on every other type — both directions pinned. Five new tamper cases (null/missing rating, rating on a non-rated event, null/missing dismiss_reason) were all uncaught at b4a19d7 and are caught now.
• 3 Medium — computed coverage violated by its own sample (fixed, plus one adjacent find). ginger's per_100g is now populated from its catalog row (FDC 169231, "Ginger Root (Raw)": 80 kcal, 1.82 P / 17.77 C / 0.75 F, 2.0 fiber, 0.013 g sodium → 13 mg, queried live from usda_ingredient_catalog.sqlite), keeping rcp-000301 computed honestly. Adjacent, same error class: rcp-000301's per-serving row did not match strict sums from its own per-100g rows (claimed 475 kcal / 34 g fat / 1720 mg sodium; strict sums are 354 / 22 / 982) — recomputed with the basis naming the method. The new coverage-vs-weighed-lines cross-check (a computed recipe may not carry a gram-weighed line whose ingredient has no per-100g row) was uncaught at b4a19d7 and is caught now.
• 4 Low — event revision drift (fixed). check.py now bounds each event's recipe_revision to the referenced recipe's shipped revisions and requires the sample to exercise the rev-2 scenario: the cook_abandoned event is bound to rcp-000002 revision 2. The "revision 9 on a rev-1 recipe" mutation was uncaught at b4a19d7 and is caught now.
• 5 Low — checker hygiene (fixed). The ... or True no-op assertion is gone; the unused meta-schema validator is gone; the catalog-miss message now uses e["legacy_id"] instead of the loop-leaked lid (a tamper case asserts the message names the mutated entry's own legacy id).
• 6 Low — tamper count (fixed). The suite is committed (contracts/tamper-tests.py, 15 named cases), so the count is checkable instead of narrated.

Generator provenance (review's "couldn't verify" note). The identity map's generator lived in scratch and is not in the repo. A from-scratch reconstruction does not reproduce the committed map byte-identically — it diverges on rule details (11/13/26 vs the committed 13/10/27 split, alias-match phrasing, candidate collection). Rather than overwrite a review-verified artifact with an approximation, the map stands as committed, bounded by check.py's semantic validation (names match data.json, every non-null fdc_id exists in the bundled catalog, 50 entries, statuses complete) — all of which the independent review reproduced against the live catalog. Follow-up: rewrite the generator deliberately from SPEC §1 and regenerate the map as its own reviewed change.

Commands run this round

• python3 docs/spec/recipes/contracts/tamper-tests.py at b4a19d7 → 8/15 caught (the 7 new invariant cases MISSED)
• python3 docs/spec/recipes/contracts/check.py at e0eedd7 → 7/7 checks pass
• python3 docs/spec/recipes/contracts/tamper-tests.py at e0eedd7 → 15/15 caught
• git diff b4a19d7..e0eedd7 → 6 files, all under docs/spec/recipes/

[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: dd78359f-fd6e-47b4-ac83-3875ce7d898a

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 contract layer is solid and honestly evidenced — identity counts, cited mappings, and traps all reproduce against the live catalog — but the shipped schema contradicts its own spec on rights, and the reference sample mislabels one recipe's nutrition coverage in the exact error class the self-review says was already fixed.

Findings (by severity)

1. High — schema and SPEC disagree on the rights enum. recipe-bundle-v2.schema.json:241 enforces status: ["original", "quarantined"], but SPEC.md:81–82 (and the PR body) state the schema enforces original | licensed | attribution_only. Reproduced: setting rights.status to "licensed" or "attribution_only" on the sample fails validation (checker exit 1) — the shipped contract cannot express two of the three states the SPEC says it pins, including attribution_only, whose display-only/excluded-from-sharing behavior SPEC §2 describes. Smallest fix: pick one direction — extend the enum to match the SPEC (keeping the fail-closed consumer rule), or amend SPEC.md:81–82 and the body to say bundles ship original only and everything else is quarantined until provenance lands. The schema's own description ("Unknown rights means quarantine") suggests the simplification is deliberate; if so, the SPEC is the side that's wrong.

2. Medium — recipe_rated / suggestion_dismissed payload invariants aren't pinned. taste-events-v1.schema.json:29 and :30–33 describe rating / dismiss_reason as "Required for recipe_rated / suggestion_dismissed, null otherwise", but both are typed [x, "null"] with no conditional required, and check.py doesn't cross-check either. Reproduced: a recipe_rated event with rating: null passes the checker; a cook_started event carrying rating: 4 passes; a suggestion_dismissed with dismiss_reason: null passes. For a PR whose stated purpose is pinning the event vocabulary "so bundle revisions can't drift from what the learner replays", the two per-type payloads the body names are exactly what's unenforced. Smallest fix: two if/then branches under allOf (draft 2020-12 supports it) requiring rating as integer 1–5 when type == "recipe_rated", and a non-null dismiss_reason when type == "suggestion_dismissed".

3. Medium — the sample violates its own coverage: "computed" definition. bundle-v2.sample.json:357 (rcp-000301) sets coverage: "computed" (:408) with basis (:409) claiming "every weighed line has a catalog per-100g row" — but its ginger line is kind: "grams", grams: 25 and ginger's per_100g is null, violating the definition in SPEC §2 and the schema description ("computed: every weighed line has per-100g values"). Same error class as the omelette fix the body's self-review already made. Reproduced: flipping a coverage label passes the checker — nothing pins it. Smallest fix: populate ginger's per_100g from its catalog row (FDC 169231 carries values: 80 kcal, 0.013 g sodium per 100 g) so the label stays honest, or relabel rcp-000301 partial — and add a coverage-vs-weighed-lines cross-check to check.py so the class stays caught.

4. Low — event revisions aren't cross-checked. check_events_example validates recipe_catalog_id against the bundle but ignores recipe_revision: an event claiming revision 9 for a rev-1 recipe passes. Binding events to revisions is the stated drift guard; one assert comparing against the bundle's revision closes it. The sample also never references the rev-2 recipe (rcp-000002), so the drift scenario isn't exercised.

5. Low — checker hygiene. check.py:93 is an ... or True no-op assertion ("unused text heuristic (informational)") that can never fail and reads like a real check; check.py:119 formats its failure message with lid leaked from the earlier loop, so a catalog miss reports the wrong legacy id (use e["legacy_id"]); check.py:54–56 builds a validator that is never used. These sit in the tool this spec designates as the machine gate, so they're worth cleaning.

6. Low — body count nit. "Adversarial tamper tests, 7/7 caught" lists six mutations. I reproduced all six as caught; name the seventh or fix the count.

What I verified (commands and results)

• gh pr view 148 --comments and gh pr diff 148: full body, all comments (only a CodeRabbit skip notice — no human reviews yet), and the complete diff: 9 files, all new under docs/spec/recipes/, no Swift or app code — matches the claimed scope.
• Checked out head b4a19d7 (based on current alpha tip 0309dd1). Followed AGENTS.md and read .obvious/skills/pr-writing and pstack-technical-writing; no CLAUDE.md or CONTRIBUTING exists.
• python3 docs/spec/recipes/contracts/check.py (jsonschema 4.26.0): all 6 checks pass, exit 0 — matches the body's claim.
• Evidence base is real: apps/ios/Resources/usda_ingredient_catalog.sqlite has 800 ingredient_catalog rows and exactly 3,998 aliases; data.json has 50 ingredients; both files are identical between origin/alpha and the PR head (empty git diff), so the cross-checks run against the same evidence on both branches.
• Identity claims verified against the live sqlite: the status split is exactly 13 resolved / 10 proposed / 27 unresolved; cited mappings check out (olive_oil→171413 "Olive Oil", soy_sauce→174278 "Soy Sauce (Tamari)", lime→168155 "Limes (Raw)", cumin→170923 "Cumin Seed", chickpea→2644282 "Chickpeas (Dry)", apple→168202 "Apples (Golden Delicious, Raw)"); the three misdirection traps are real (only egg row is "Egg (Whole, Raw, Frozen, Salted)" 170893, only banana hit is "Banana Pepper (Raw)" 169394, tofu→"Tofu Yogurt" 167722) and all three are held unresolved with candidates recorded.
• Reproduced all six listed tamper mutations; each exited 1 (caught), matching the body.
• Sample completeness: all four quantity kinds present; two FSIS-pinned thermometer checks; three coverage levels; the revision bump (rcp-000002 rev 2) with an honest estimated basis; required:false and assumed_staple lines; cook_completed with structured cilantro→lemon substitution and scaled_servings; gluten_free: null fail-closed examples; the sodium figure in rcp-000301's basis matches the catalog (5.586 g/100 g → 5586 mg).
• CI on the PR head: build-and-test pass (7m40s) and dead-code-advisory pass (run 38722143586).

What I couldn't verify

• The identity map's generator is not in the PR, so "regenerated programmatically … not hand-authored" can't be reproduced from the repo. check.py re-validates the committed artifact against the live catalog (names, ids, counts all match), which bounds the risk; committing the generator script (or a pointer to it) would make the map's provenance auditable.
• The "proposed" mappings' food semantics (chickpea dry form, apple Golden Delicious qualifier) need full-FDC confirmation — out of scope by the PR's own statement.
• The Swift-side id-mismatch history traced in the load-bearing assumption (BundledDataLoader.swift etc.) — I did not re-derive it; out of scope for this contract review.
• No Xcode here, so I did not build the iOS app locally; macOS CI green is the build evidence, and nothing in this diff touches Swift.

Draft state is right — the contract shapes are worth reviewing now. Findings 1–3 are small, mechanical fixes; none of them questions the identity-resolution rules this review was waiting on.

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