Repository navigation
feat(evaluation): re-gate taste residual on the app recommendation pool - #184
obvious-autobuild[bot] wants to merge 2 commits into
Conversation
Re-run the learned-ranking gate over the pool RecipeRepositoryCatalogRecommend actually ranks instead of the wide-pantry synthetic setup: app mode draws 15-40 non-staple pantry keys plus assumed staples and ranks the exact tier plus the at-most-two-missing near-match tier (legacy mode retains the committed gate's 160-260-key, exact-only setup for reproducibility). - simulator: pool modes with product-shaped pantries and candidate pools; assumed-staple lines never block feasibility and count like optional matches (+2.5) when present, mirroring the app's sharedRankingScore - two lab-only ranking variants: churn_bounded (reorders confined to consecutive rule-score runs within a tuned band) and shrunk (exposure-count shrinkage n/(n+k)); fixture variant pinning and CLI flags for both - tuning-and-gate command: tune on seeds 7/11, evaluate on all five seeds (7/11/13/17/19), pre-registered pass rule (held-out delta CI lower bound > 0 and top-10 churn <= 5% on each held-out seed) - verdict: no arm passes - residual delta +0.0004..+0.0012 NDCG@10 with CI spanning zero on every held-out seed; churn 2.5-4% everywhere; the band never binds. TasteResidualSettings.builtInDefault stays false; no Swift ported. Report: evaluation/taste-lab/results/app-pool-gate.md - fixtures regenerated in app mode; 17 lab tests pass; contracts check green 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 |
|
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. |
 [Medium impact] Adds ranking variants to the recipe recommendation evaluation lab. Correct the app-mode pantry and regenerate the gate evidence before merging.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if catalog.is_v2: | ||
| lo, hi = (15, 40) if self.pool_mode == "app" else (160, 260) |
There was a problem hiding this comment.
app mode does not add the assumed staples promised by the gate setup. pantry_for_day gets its stable set from Catalog.staple_ids(), which counts only blocking required ingredients. The v2 loader excludes assumed staples from that list, so those keys compete for the random 15–40-key draw instead of being added first.
This makes the new +2.5 staple bonuses depend on that draw, changing the rule order and measured churn. Build the app-mode staple set from the catalog's assumed-staple lines, leave legacy unchanged, and regenerate the results and fixtures.
Prompt To Fix With AI
This is a comment left during a code review.
Path: evaluation/taste-lab/taste_lab/sim.py
Line: 114-115
Comment:
**Assumed staples are not guaranteed**
`app` mode does not add the assumed staples promised by the gate setup. `pantry_for_day` gets its stable set from `Catalog.staple_ids()`, which counts only blocking required ingredients. The v2 loader excludes assumed staples from that list, so those keys compete for the random 15–40-key draw instead of being added first.
This makes the new +2.5 staple bonuses depend on that draw, changing the rule order and measured churn. Build the app-mode staple set from the catalog's assumed-staple lines, leave `legacy` unchanged, and regenerate the results and fixtures.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| if run and (run[-1].rule_score - entry.rule_score) > band: | ||
| ordered.extend(sorted(run, key=lambda x: (-x.total_score, x.recipe_id))) |
There was a problem hiding this comment.
_churn_bounded_reorder checks each gap against the previous entry, so a chain can defeat the promised bound. With rule scores 60, 58.5, and 57 and band=2, all three entries enter one run. A learned contribution above 3 can then move the last entry ahead of the first, despite their rule gap exceeding the band.
The new test forbids that inversion, but its sampled history does not exercise it. Bound each run's full score spread and add a three-entry chain test so future experiments do not report these reorders as bounded.
Prompt To Fix With AI
This is a comment left during a code review.
Path: evaluation/taste-lab/taste_lab/engine.py
Line: 448-449
Comment:
**Chained scores escape the band**
`_churn_bounded_reorder` checks each gap against the previous entry, so a chain can defeat the promised bound. With rule scores 60, 58.5, and 57 and `band=2`, all three entries enter one run. A learned contribution above 3 can then move the last entry ahead of the first, despite their rule gap exceeding the band.
The new test forbids that inversion, but its sampled history does not exercise it. Bound each run's full score spread and add a three-entry chain test so future experiments do not report these reorders as bounded.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.…etup Re-runs the five-seed gate (seeds 7/11/13/17/19, 40 households x 30 days) and regenerates the golden fixtures after guaranteeing assumed staples in app-mode pantries and bounding churn_bounded runs by full rule-score spread. app-pool-gate.md gains a paired table of #184's numbers beside the re-gate's, per arm and seed. Verdict is unchanged: no arm passes the pre-registered rule (held-out paired delta-vs-rule CI lower bound > 0 and top-10 churn <= 5%), so TasteResidualSettings.builtInDefault stays false and nothing is ported. The band fix makes churn_bounded bind for the first time (chosen band moves 1.0 -> 6.0); shrunk clears seed 19 for the first time but still fails seeds 13 and 17. Co-authored-by: obvious-autobuild[bot] <262744130+obvious-autobuild[bot]@users.noreply.github.com>
… into obv/burst9-fridgeluck-taste-gate
What changed and why
The committed taste gate ran over a synthetic setup: pantries drawing 160–260 of 299 non-staple keys, ranked over the exact-feasible tier only. The pool the app actually ranks is different —
RecipeRepositoryCatalogRecommendranks the exact tier plus recipes missing at most two blocking required lines over a product-shaped pantry. This burst re-runs the gate on that pool before porting anything to Swift.appmode (pantry = 15–40 non-staple keys + assumed staples; candidate pool = exact-feasible ∪ {≤ 2 missing blocking lines}; assumed-staple lines never block feasibility and count like optional matches, +2.5, when present — the app'ssharedRankingScorebehavior) andlegacymode (the committed gate's setup, retained for reproducibility). App-mode pool sizes land at 75–181 recipes (median 108), none skipped.churn_boundedconfines learned reorders to runs of consecutive pool entries whose rule scores lie within a tuned band;shrunkscales contributions by n/(n+k) on exposure count. Both are fixture-pinnable and CLI-selectable. No Swift changed.python3 -m taste_lab gatetunes each variant on seeds 7/11 only (grid: band ∈ {1,2,3,6}, k ∈ {2,4,8,16}), then evaluates on seeds 7/11/13/17/19 with the pre-registered pass rule: on each held-out seed (13/17/19), paired delta-vs-rule CI lower bound > 0 and top-10 churn ≤ 5%. ~45 s per run.golden-hh00/hh01/hh23, seeds 7/11/13) so replay covers the event model the gate measured.TasteResidualSettings.builtInDefault = false). No arm passes; per the pre-registered rule, nothing was ported. Full report:evaluation/taste-lab/results/app-pool-gate.md.Gate results
Every arm fails (CI lower bound ≤ 0 on held-out seeds); churn passes everywhere at 2.5–4% vs the 5% limit. Notable findings:
How I checked it
python3 -m pytest tests/— 17 passed (was 10; added pool-mode, pantry-shape, band-binding, shrunk-scaling, variant-validation, and fixture-pinning tests).docs/spec/taste/contracts/check.py— all examples valid and self-consistent.python3 -m taste_lab validate-fixtures --catalog apps/ios/Resources/RecipeCatalog/recipes-v2.json --fixtures evaluation/taste-lab/fixtures— all fixtures replay exactly.python3 -m taste_lab gate --catalog apps/ios/Resources/RecipeCatalog/recipes-v2.json --households 40 --days 30 --out-prefix results/app-pool— producedresults/app-pool-seed{7,11,13,17,19}.json+app-pool-gate.jsonand the fail verdicts above; reports reproducible end-to-end.What I couldn't check
Assumptions
Human author
Human author: Sam Gu (sgu07966@gmail.com)
🔗 Obvious Project · 🧵 Obvious Thread · 🔀 Obvious PR Artifact
CI status note (at 9096d45)
The first
iOS CIrun (38842276172) failed inbuild-and-testat a Swift compile error this PR does not touch:apps/ios/Feature/CookMode/CookModeViewModel.swift:540:27—CookReceiptLog(store:)called without thesessionID/sessionRevisionits initializer requires. That mismatch exists verbatim at this branch's base commit (3e152dd), at this PR's head, and at the base branch tipe398c475(CookMode/has zero commits since the branch point —git log 3e152dd6..e398c475 -- apps/ios/Feature/CookMode/is empty). This PR's diff contains zero files underapps/ios/. The base branch's own PR runs were red before and after this push (runs 38839878965 / 38840455779 / 38840752062 / 38841007670, GoalPath test compile errors at 02:20 and runtime test failures — including the known GRDB non-reentrant fatal — at 02:33). A re-run of the failed job (02:53) reproduces the identical error, so the compile failure is deterministic on this head. One unexplained discrepancy is left on record: the base branch's own run 38841007670, on app-target content byte-identical to this head (the trees differ only inapps/ios/Tests/GoalPath/GoalPathTests.swift), executed 33 test-suite runs with no missing-arguments error - an outcome that could not be reconciled with the source content from the available logs. The failed job belongs to the goal-path burst's in-flight repair; fixing Swift here would collide with that work and is out of scope for a Python-only re-gate.Update: base CI repair merged forward (2cbd7cc)
The goal-path burst repaired the inherited break on
obv/burst8-fridgeluck-goal-pathand its own iOS CI went green ate87ee7b2(run 38851718150,build-and-testsuccess). The call site now readsCookReceiptLog(store: sessionID: sessionRevision:)and the repair is confirmed in the merged tree. The green base tip was merged forward into this branch as merge commit2cbd7ccf(zero conflicts - this PR's diff is evaluation/taste-lab only); the taste-lab suite was re-run post-merge (17 passed in 1.02s). CI run 38854123101 on2cbd7ccfcompleted success in 12m07s, so the PR's iOS CI now judges this PR's own content rather than the inherited break: build-and-test and dead-code-advisory are green at the current head. The earlier red runs on9096d456(compile error atCookModeViewModel.swift:540, reproduced deterministically) are retained below as the historical record.