Repository navigation
Foundation C: public dump_responses, honest run-mode sentence, matrix and forced-metric template fixes - #317
Conversation
…nd forced-metric template fixes - #209: jev_cookbook.dump_responses (live.py, exported from the package) is the public, documented name for the fixture serializer; the scaffolder, the template and all 49 replay recipes' tests/test_build_fixtures.py (plus recipe 50's build and test files) import it. live._dump stays as a supported alias; a guard test keeps recipes and the scaffold off private shared names. - #233: "every number below carries that mode" becomes "every reported metric below ...", with a sentence that demo answers and per-example lines are not metrics. Markdown only, template plus 43 published notebooks. - #217 (template part): the confusion-matrix column width is the larger of the widest label and the widest count, proved by a test that runs the template cell and by mutation. - #222 gap 3 (template line): the template's evaluation section says which validation number is forced by the selection. Refs #217 Refs #222 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…he private-name guard Internal pre-review fixes: the run-mode sentence now says what offline metric lines are labelled (test: pipeline check; validation: selection step) instead of "carries that mode"; the guard also follows module aliases; the forced-metric doc says to replace, not copy, the template sentence; a stale docstring and a mislabelled matrix test case are corrected. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Opus reviewer (independent review 1 of 2): Claude Opus 5.5 ( Reviewed head Verdict in one paragraphThe Blocking findings, most severe firstB1. "per-example lines ... carry no label of their own" is false in at least 15 of the 43 published notebooks that now carry itThe sentence added to
In the template the claim happens to hold, but it does not hold in the published recipes, and the template sentence is meant to be copied into new ones. A reader of recipe 34 meets a counterexample two cells after reading the sentence. That is the same defect #233 was opened for, with the error now pointing the other way. What to change: stop claiming that such lines carry no label. One wording that is true in every notebook I scanned: "Individual answers and per-example lines are not metrics and need not carry a label; labelled or not, every line comes from the mode the header names." B2. "a
|
| injected line | result |
|---|---|
from jev_cookbook.live import _dump |
fails (caught) |
from jev_cookbook._canonical import plain_json |
passes |
import jev_cookbook._canonical |
passes |
from jev_cookbook.fixtures._scan import scan_text |
passes |
from jev_cookbook import fixtures then fixtures._scan |
passes |
jev_cookbook/_canonical.py and jev_cookbook/fixtures/_scan.py both exist. #209's last acceptance criterion is "No recipe imports an underscore-prefixed shared name afterwards, enforced by a guard". Today no recipe does this (I checked with grep), but the guard is described as enforcing the rule and it does not cover these forms.
What to change:
- Flag any
ImportorImportFromwhose dotted module path has a segment starting with_. - Build
SUBMODULESfrom package directories as well as*.pyfiles. - Add these four spellings to
test_the_detector_flags_what_it_claims_to.
Suggestions (not blocking)
- S1. "In an offline run" also covers recorded replays, where the sentence would be false. The header calls a recorded run "offline replay of recorded fixtures". The template's
offlinevariable, though, isbackend.mode in ("synthetic", "scripted"), so in a recorded replay notestline carries the pipeline-check label. Once a recipe that copied this sentence gets recorded fixtures, the sentence becomes false. "In a synthetic or scripted run" says exactly what the code does. The template'sevaluation-mdalready has the same ambiguity, and it could be fixed in the same edit. - S2. One reported metric line with no label.
recipes/47-evidence-conflict-diagnosiscellbaselinesprintsthe name-pattern baseline beats Jev here by 0.0625, ...with no{check}. The same number is labelled one line above, so it is minor, but it is a reportedtestmetric line without a label. - S3. The scaffolded
build_fixtures.pystill restates the serialization by hand.recipes/_template/build_fixtures.py:173-178writesjson.dumps(dict(sorted(...)), indent=2, ensure_ascii=False) + "\n"with a comment saying it matchesdump_responses. Now that the helper is public, the scaffold could calldump_responses(responses)directly, as recipe 50 already does. That removes the duplication the byte-identity test exists to catch. It is an interface choice made once for every remaining recipe, so it is worth deciding now rather than after more copies. - S4. Pin the new sentences. As the description notes, nothing pins the Foundation: the template tells readers every number carries the run mode, which is false for demo answer and per-example lines (32 notebooks) #233 and Foundation: four set-wide gaps from the recipe 25 review (queue attribution, threshold binding, forced-metric disclosure, baseline template line) #222 sentences. A small template test asserting both are present (and that the scaffolder's evaluation TODO keeps its forced-metric prompt) would stop a later template edit from dropping them silently.
- S5. A false comment in a cell every recipe copies. The
evaluation-validationcomment saysroutehas "exactly one review reason".routehas three, and the template's owntestoutput lists two. This is pre-existing and the description flags it, but this PR already edits the template's evaluation section, and every recipe copies that comment. - S6. The alignment test does not run the real suffix.
tests/test_template_matrix_alignment.pyruns the cell withcheck = "". A committed offline run appends the check suffix to the header and to every row, so the printed text is not quite what is tested. Running once with the real suffix (and comparing the edges of the lastn_labelstokens before it) would test what is actually printed.
What is right, verified independently
- Notebook edits are exactly as claimed. I parsed every
recipes/*/notebook.ipynbat base and head and compared every cell field and every top-level key.- 44 notebooks changed: 43 published plus the template. In each published notebook only one markdown cell's
sourcediffers (setup-md;f33940acin 05;setup-introin 20). No output, metadata, id, execution count or cell order changed. - A character-level diff shows the same replacement in all 44. The only exception is recipe 21, which wraps the new sentence across lines.
- In the template,
setup-md,evaluation-mdandevaluation-matrixchanged.evaluation-matrixchanged in source only; its outputs are unchanged. - Recipes 36, 41, 43, 44, 45, 48 and 50 are byte-identical to base.
git grep -l "carries that mode" origin/main -- 'recipes/*/notebook.ipynb'returns exactly those 44, and returns nothing at head.
- 44 notebooks changed: 43 published plus the template. In each published notebook only one markdown cell's
- Foundation: the scaffold makes all sixty recipes import the private jev_cookbook.live._dump; promote it to public API #209 migration.
- 50
tests/test_build_fixtures.pyfiles changed (49 recipes plus the template), all with the same normalized diff. G1(a) passes, so they are byte-identical to the scaffold. - 48
build_fixtures.pycomment lines changed. - Recipe 50 changes its import and two calls in
build_fixtures.py, and its import, docstring and assert intests/test_helpers.py. _dumpis the same object asdump_responses, pinned intests/test_live.py. The docstring,__all__in both modules anddocs/live.mdall document it.
- 50
- Diff modes.
git diff --raw -M origin/main...HEADshows only100644entries. The two new test files are000000 100644 A. There are no symlinks, submodules, mode changes or type changes. - Foundation: the template's confusion-matrix print block misaligns when a count is wider than the widest label #217 template part.
column = max(width, len(str(matrix.matrix.max())))sizes both the header and the counts.leader_widthcorrectly stays on label width. The test runs the cell's real source, and its mutation back tocolumn = widthis reported as misaligned. - Foundation: four set-wide gaps from the recipe 25 review (queue attribution, threshold binding, forced-metric disclosure, baseline template line) #222 gap 3.
- Under
target_accuracy=1.0,select_confidence_thresholdreturns the lowest threshold whose answered subset has accuracy at least 1.0, and raises if none does. The answered subset isnamed(queue answers only), androuteacceptsconfidence >= min_confidence. So "right among those routed automatically" onvalidationis forced, and the committed6 of 6agrees. - It is the only forced
validationline in the template; the "answers that name a queue", accuracy and "routed automatically" lines are not forced. - The
docs/recipe-template.mdinstruction to replace the sentence rather than copy it, and the scaffold TODO, are consistent with it.
- Under
- No live call, key or side effect. Nothing in the diff touches the network, a key or a real side effect.
Checks I ran (fresh Python 3.14.3 venv, pip install -e ".[dev,ml,live]" -c .github/constraints-notebooks.txt, Windows, TYPESAFE_API_KEY and JEV_COOKBOOK_LIVE unset)
ruff check .: All checks passed.ruff format --check .: 397 files already formatted.python tools/render_catalog.py --check: exit 0.python tools/check_hygiene.py: passed, 606 files.python tools/notebook_ci.py fixtures: 51 recipe folders, 0 failed.pytest: 5001 passed, 56 skipped, 50 deselected, 31 xfailed, 2 failed. The two failures aretests/test_notebook_ci_sandbox.py::test_kills_every_non_zombie_process_and_returns_once_none_remainand::test_a_process_that_will_not_die_fails_closed. Both fail only on Windows (signal.SIGKILLmissing, Foundation: two notebook-CI sandbox tests use signal.SIGKILL and fail on Windows #302), and this PR does not touch that code, so they are not findings.- Notebooks: all 50 recipes plus the template (51 of 51). For each one: copy to scratch, run
PYTHONPATH=<abs>/tools/netguard python tools/execute_notebook.py <copy>, thencheck_notebook_fresh.pyagainst the committed file, thencheck_hygiene.pyon the fresh copy. 51 of 51 OK, andgit status --porcelainis unchanged afterwards. This machine has no network namespace, so the offline guarantee here rests on netguard and the replay backend. - CI (
gh pr checks 317): 60 of 60 pass on this head. - Scripts written for this review: a parsed-JSON notebook comparison, a scan classifying every committed numeric output line by label, and the guard mutation above.
Acceptance criteria
- Public, documented helper with its own test: met.
tools/new_recipe.pygenerates the public import, and a scaffold test proves it: met (test_the_scaffolder_emits_no_private_shared_name, both modes).- Every published recipe importing
_dumpmigrated, with the enumeration recorded: met (enumeration in the PR description). - Keep-or-remove decision recorded: met (kept as an alias; reason in
live.py,docs/live.mdanddocs/recipe-template.md). - No recipe imports an underscore-prefixed shared name, enforced by a guard: partly met. No recipe does today, but the guard misses private modules (B3).
- The template sentence is replaced with a true one, and the same sentence everywhere it was copied: not met (B1, B2). The replacement was applied uniformly and markdown-only, which is correct.
#217 (template part only, by design)
- The template block aligns when a count is wider than every label, proved by a concrete case: met.
tools/new_recipe.pyand the template agree: met. The scaffold does not emit a matrix cell, and the template is the one source.- Recipe 23 convergence and the per-recipe guard: out of scope here, left open as stated.
#222 gap 3 (template line only, by design)
- Forced-metric disclosure is in the template: met, and true.
- Applying it to recipes 24, 25, 26, 27 and 29: out of scope here, left open as stated.
OPUS-REVIEW: CHANGES-NEEDED 7a5299f
|
Opus reviewer (independent review 2 of 2). Model: Claude Opus 5.5 ( As well as reading the diff, I scaffolded three throwaway recipes into a scratch folder with this head's Blocking1. The replacement #233 sentence is false in a new way, in the template and all 44 notebooks that carry it
"Instead" tells the reader that an offline The template's #233 exists because the setup sentence claimed more than the notebook shows. This PR swaps that error for a smaller one in the same sentence, across the same files, and 50 more recipes will copy it from here. Suggested wording:
This is a markdown-only change, so the outputs stay the same. Because no test pins the sentence (you note this yourself), a one-line test is worth adding. It would assert that the template's 2. The private-name guard misses private modules, and #209's last criterion is about names, not only attributes
The The fix is small:
The current docstring describes the narrower scope accurately. But the PR closes #209 on the strength of this guard, and as written it does not enforce that criterion. Suggestions (not blocking)S1. Make S2. S3. Multimodal: the scaffolded S4. The S5. S6. The template's Things I checked and found sound:
Checks run (fresh Python 3.14.3 venv,
|
…ivate-module guard, scaffold calls dump_responses - #233: the setup paragraph now only makes claims a script can check (the two label phrases and what they mark; a line without a label still comes from the header's mode). tests/test_run_mode_sentence.py pins it verbatim in the 43 published notebooks and the template and checks the claims against every committed output. - #209: the private-name guard flags private modules by every spelling, builds its module set from packages as well as files, and does not flag dunders. - The template and scaffold write responses.json with dump_responses; docs/live.md and docs/fixtures.md say what the scaffolded test checks and use the public helper; the _dump alias is described as transitional; the template's false "exactly one review reason" comment is fixed. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Opus reviewer (independent review 1 of 2, re-review): Claude Opus 5.5 ( Reviewed head
I did not read the other reviewer's comments. My previous comment is 6302632394. SummaryAll three of my blocking findings are resolved, and so are four of my six suggestions. The new run-mode paragraph is far better than the last one, and three of its four claims hold in every notebook that carries it. The fourth claim is still not true. It says the selection label "marks a number used to choose a setting on Status of my earlier findings
Blocking findingB4. "
|
…e paragraph (#233) Re-review B4 on PR 317: "marks a number used to choose a setting on validation" is false for most lines carrying the label (they report what the frozen rule does on validation), and contradicts recipe 28's own prose. The paragraph now says the label marks a number from validation, the split settings are chosen on, and never a result to report. Markdown only in all 44 notebooks; the pinned PARAGRAPH and a regression check for the old wording are updated in tests/test_run_mode_sentence.py. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Conflict in docs/recipe-template.md resolved by keeping both rows: main's side_effects.json row and this branch's updated tests/test_build_fixtures.py row. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
PR #317 review — reviewer 1 of 2 (fresh Opus)
VerdictCHANGES-NEEDED, on two findings, neither of which is in the code: the enumeration the PR body Must changeM1. The #209 enumeration in the PR body is wrong about recipe 32 — and it is the record the issue asks forPR body, "#209" bullet:
Recipe 32 is a replay recipe, it does have
Why this is a must-change rather than a nit: #209's third acceptance criterion is "Every published M2. #233 is closed while recipe 36 still tells readers every number says it is a pipeline check
That is the #233 defect, in the same cell, with the same counterexamples. The notebook prints 36
Recipe 36 fell out of the sweep because the sweep was keyed on the literal string "carries that The PR explicitly sets the bar "makes only claims a script can check", and this is a markdown-only Suggestions (not blocking)S1.
|
| command | result |
|---|---|
ruff check . |
All checks passed! (exit 0) |
ruff format --check . |
400 files already formatted (exit 0) |
python tools/notebook_ci.py fixtures |
51 recipe folders, 0 failed (exit 0) |
python tools/render_catalog.py --check |
exit 0 |
python tools/check_hygiene.py |
Hygiene check passed: 609 file(s) scanned (exit 0) |
pytest -p no:cacheprovider |
5466 passed, 54 skipped, 50 deselected, 31 xfailed in 271s, 0 failed (exit 0) |
gh pr checks 317 |
60 of 60 pass on this head (all 51 notebook jobs, both test matrices, lint, catalog, hygiene, fixtures, scope) |
Every number above matches the PR body's "How it was verified" section exactly, including the pytest
counts (5466 passed, 54 skipped, 50 deselected, 31 xfailed, 0 failed), the 400 formatted files and
the 609 hygiene files. The two Windows-only signal.SIGKILL failures the earlier reviews saw are
gone here (Linux, and #302 is on main now), so the suite is clean with no known-failure carve-out.
Notebooks (7 of the 44 changed, verified offline from a scratch copy, never in the tree). For
each: rm -rf the scratch folder, cp -R recipes/<slug> into it,
PYTHONPATH=<worktree>/tools/netguard python tools/execute_notebook.py <scratch>/<slug>, then
tools/check_notebook_fresh.py recipes/<slug>/notebook.ipynb <scratch>/<slug>/notebook.ipynb, then
tools/check_hygiene.py on the fresh copy, with a git status --porcelain before/after snapshot.
| recipe | why chosen | result |
|---|---|---|
_template |
the only notebook whose code cells changed (evaluation-matrix, evaluation-validation) with outputs unchanged |
OK (fresh + hygiene) |
01-sentiment-classification |
first published copy of the swept paragraph | OK |
06-multiple-topic-labels |
validation lines carry only the selection label (a case the paragraph deliberately does not over-claim) | OK |
21-quiz-answer-adjudication |
the one notebook where the paragraph was spliced into the middle of a wrapped cell | OK |
32-regression-test-selection |
the recipe M1 is about | OK |
47-evidence-conflict-diagnosis |
has an unlabelled metric line (#326) | OK |
49-multi-agent-task-assignment |
has unlabelled validation metric lines (#326) | OK |
7 of 7 OK, and git status --porcelain was identical before and after every run, so nothing wrote
into the checkout. check_notebook_fresh.py compares sources, metadata, execution counts, ids and
text outputs exactly (only images are compared by what they show), so the template passing it proves
the rewritten evaluation-matrix and evaluation-validation cells reproduce the committed outputs
byte for byte — which is the specific risk in changing code cells without changing outputs.
Caveat, stated as the earlier reviews did: this machine has no network namespace for the kernel, so
my offline guarantee rests on tools/netguard and the replay backend, not on a kernel-level boundary.
CI's own 51 Notebook (<recipe>) jobs, which do use the namespace and a separate unprivileged user,
are all green on this head.
I did not re-run the remaining 44 notebooks; the PR body reports 51 of 51 and CI's per-recipe jobs
agree, and the 43 published notebooks are markdown-only changes whose outputs I verified are
byte-identical to base.
Acceptance criteria
#209 (five criteria)
- A public, documented helper exists with its own test — met.
dump_responsesin
src/jev_cookbook/live.py:823, exported from both__all__s, documented in its docstring and
docs/live.md, with two dedicated tests intests/test_live.py. tools/new_recipe.pygenerates the public import, and a scaffold test proves it — met.
The emittedtests/test_build_fixtures.pyandbuild_fixtures.pyboth import
from jev_cookbook import dump_responses;test_the_scaffolder_emits_no_private_shared_name
scaffolds a real recipe in both modes and scans what it emits.- Every published recipe importing
_dumpis migrated, with the enumeration recorded — migration
met, record not met (M1). All 49 recipes with the file are migrated and byte-identical to the
scaffold; the recorded enumeration misstates recipe 32 and is not on the issue. - The keep-or-remove decision for
_dumpis recorded, and what it is for — met. Kept as a
transitional alias, with the reason and the removal condition inlive.py,docs/live.mdand
docs/recipe-template.md. - No recipe imports an underscore-prefixed shared name afterwards, enforced by a guard — met.
tests/test_no_private_shared_names.py, proven to flag the base content it had to flag and all
private-module spellings earlier rounds found it missing.
Issue point 5 (the interface question: text-returning helper vs a "write these responses" helper) is
answered implicitly by the docstring ("It returns text and writes nothing", so the test can compare
byte for byte) rather than discussed. Acceptable; it is not one of the five criteria.
#233 — not met (M2). The false sentence is gone from all 44 notebooks it was in, the
replacement is applied verbatim and uniformly, it is markdown-only, every claim in it that I could
check holds, and it is now pinned by a test. But the same class of false claim survives in recipe 36,
which the sweep's string-matched scope excluded, and the PR closes #233.
#217, template part only (as the PR scopes it) — met. The cell aligns when a count is wider
than every label, proven on a concrete case with a mutation that goes red. Recipe 23 convergence and a
per-recipe guard remain open, as the body says.
#222 gap 3, template line only (as the PR scopes it) — met and accurate. Applying it to the
five published recipes and gaps 1, 2 and 4 remain open, as the body says.
Status of each prior finding (read only after forming the above)
Prior review 1 — comment 6302632394, head 7a5299fa:
| finding | status at eff86c79 |
|---|---|
| B1 "per-example lines ... carry no label of their own" is false | resolved. The claim is gone; the paragraph now says only that a line with neither label still comes from the mode the header names. |
| B2 "labelled a selection step instead"; unlabelled validation lines in 39, 49 | resolved as worded. "instead" is gone and its absence is pinned; the paragraph makes no coverage claim, so 39/49 no longer contradict it. The labelling gaps themselves are tracked in #326. |
| B3 guard misses private modules | resolved. I re-ran all five of its spellings plus five more: every one is flagged; SUBMODULES now includes fixtures. |
| S1 "In an offline run" also covers recorded replays | resolved. Paragraph and the template's evaluation-md both say "synthetic or scripted run". |
| S2 recipe 47's unlabelled baseline line | open, disclosed and now tracked (#326 names 39, 47, 49). |
| S3 scaffold hand-restates the serializer | resolved for the template and the scaffolder. See S2 above: recipe 32 still does. |
| S4 pin the new sentences | resolved. tests/test_run_mode_sentence.py + test_the_scaffolder_evaluation_stub_keeps_the_forced_metric_prompt. |
| S5 "exactly one review reason" | resolved and pinned in tests/test_template.py. |
S6 alignment test runs check = "" |
not addressed. Immaterial (see S5 above). |
Prior review 2 — comment 6302830060, head 7a5299fa:
| finding | status at eff86c79 |
|---|---|
| Blocking 1 the "instead" sentence | resolved (same as B2). |
| Blocking 2 private-module guard | resolved (same as B3). |
S1 make dump_responses the writer in template + scaffold |
resolved. Both call it; the hand-copied expression and its comment are gone. |
S2 docs/live.md overstates what the scaffolded test checks |
resolved for the doc — it now says "what the synthetic generator would write, serialized exactly as the recorder serializes", and names the recorded-fixture limitation. Second half open: the scaffolded module docstring still says "matches jev_cookbook.live's writer" (my S3). |
| S3 multimodal scaffold cannot pass for an image-backed recipe | out of scope and recorded — added to #315 as a scope comment on 2026-10-20, and the PR body says so. |
S4 the _dump alias rationale |
resolved. It is now called a transitional alias with a removal condition, and the corrected reasoning is in the body. |
S5 docs/fixtures.md example still hand-writes JSON |
resolved, and I executed the example. |
| S6 "exactly one review reason" | resolved (same as review 1's S5). |
Prior review 3 — comment 6303724732, head 7e0a1bdd (review 1's re-review):
| finding | status at eff86c79 |
|---|---|
| B4 the selection label "marks a number used to choose a setting" is false | resolved. The paragraph now uses that review's own suggested wording verbatim ("marks a number from validation, the split settings are chosen on; it is never a result to report"), and test_the_old_false_sentence_is_gone_everywhere pins the absence of "marks a number used to choose". |
| S-a the gaps are described as tracked but nothing tracks them | resolved. #326 exists and is open. Not cited from the test (my S4). |
| S-b the recorded/live branch is vacuous; pin the code side | not addressed, and weaker than it looks — see my S1, where I show the synthetic/scripted branch is also effectively inert. |
| S-c (= S6) alignment test suffix | not addressed. Immaterial. |
All blocking findings from all three prior reviews are resolved at this head. My two must-changes are
new.
Outside my task, reported not fixed
recipes/32-regression-test-selection/build_fixtures.py:1488hand-copies the serializer (my S2).- Recipe 36's
setup-mdhonesty claim (my M2) and its exclusion from the run-mode test's coverage. recipes/16-discord-moderation-triageprints a non-numeric line carrying both labels
((evaluate_outcomes and evaluate_selective agree on all three) (a selection step, ...) (a pipeline check, ...)). Pre-existing, outside this diff, and a mild counterexample to "Two labels may follow
a number"; not worth changing on its own, but Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326's sweep could pick it up.docs/fixtures.md's five code blocks are not executed by any test
(tests/test_docs_examples.pysays so deliberately), so this PR's edit to the worked example had
no automated cover. I ran it by hand; it is correct.
OPUS-REVIEW: CHANGES-NEEDED eff86c7
Opus review 2 of 2 — PR #317 (
|
| notebook | cell | the committed line, and what it is computed over |
|---|---|---|
recipes/02-refund-intent-detection/notebook.ipynb |
queue-final |
25 items in the queue across both scored splits (a selection step, not a reported result) (a pipeline check, not a Jev result) — the loop is over val_examples + test_examples / val_answers + test_answers; the printed text itself says "across both scored splits" |
recipes/10-answer-relevance-check/notebook.ipynb |
queue |
1 item queued so far (a selection step, not a reported result) — counted over four examples, two of which (relevant_example, offtopic_example) are demo[...] |
recipes/15-sensitive-text-triage/notebook.ipynb |
queue |
preview: 2 of these four would be queued, highest noul first (a selection step, …) (a pipeline check, …) — the four are the cell's own demo_entries, and one of the two it lists as queued, DT3001, is d01-pii, split demo |
recipes/25-change-evidence-review/notebook.ipynb |
queue-preview |
preview: 1 of these four would be queued (a selection step, …) (a pipeline check, …) — the four include gap_demo and ready_demo |
recipes/26-prompt-injection-screening/notebook.ipynb |
queue-preview |
preview: 1 of these six would be queued (a selection step, …) (a pipeline check, …) — the six explicitly include flag_demo and pass_demo |
For contrast, recipes/15-sensitive-text-triage's own queue-build cell gets this right: it puts
{selection} only on the … from validation … line and leaves the … from demo and … from test
lines with {check} alone. So these five are outliers, not the house style — which is exactly why a
reader who believes the paragraph is misled on them.
I also checked the weaker direction and it holds: no selection-labelled line in any carrying
notebook is a test-only number. The gap is specifically cross-split numbers (demo+validation,
validation+test).
Why this blocks. #233 was filed because a template sentence generalized past what the notebooks
print, and a reader taking it literally "will find counterexamples within a few cells" — against
CONTRIBUTING.md section 2's honesty rule. This replacement narrows the failure but reproduces its
shape, in the same 43 published notebooks, in text the remaining recipes copy.
CONTRIBUTING.md section 2 states the implication in one direction only — "a number from the
validation split is a selection step, not a result" — and tests/test_recipe_guard.py's G1(d)
clause 1 enforces that direction. The paragraph asserts the converse, which is neither in the
contract nor true of the set.
The PR body overclaims on exactly this point. It says the old sentence is replaced "by a
paragraph that makes only claims a script can check", then lists "(a selection step, not a reported result) marks a number from validation" among those claims. tests/test_run_mode_sentence.py
checks (a) the paragraph verbatim in all 44, (b) which notebooks carry it, (c) that the set of
(a …, not a …) labels in each carrying notebook's output is a subset of the two named, and (d) the
pipeline-check ↔ synthetic/scripted correspondence. Nothing in it relates a selection-labelled
line to a split. That claim is unchecked, and false.
What to change — either:
- (a) say only what is true of the set. For example: "
(a selection step, not a reported result)marks a number that is not a result to report — a number fromvalidation, the split
settings are chosen on, or a preview of what the frozen rule would do." Then pin whatever is
written; or - (b) keep the sentence and make it checkable. Add an assertion to
tests/test_run_mode_sentence.pythat every selection-labelled output line comes from a
validation-only computation (the five cells above are findable from the cell source, which names
demo/*_demo/demo_entries/test_*), then correct those five recipes' labels — or
exempt them explicitly against Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326, so the exemption is visible rather than implied.
(b) is what the PR body already claims to have done, so it is the option that makes body and diff
agree. Either way the pinned PARAGRAPH and the 44 notebooks must move together, and the body's
"makes only claims a script can check" sentence needs correcting.
M2 — The scaffolder does not emit the paragraph, yet the new test requires every future published notebook to carry it verbatim
Where: tools/new_recipe.py:138-143 (the setup-md cell) and
tests/test_run_mode_sentence.py:44-52, 94-96.
tools/new_recipe.py emits setup-md as "The notebook runs from its own folder with the standard
library and jev_cookbook only. The header states which mode ran." plus TODOs — no paragraph;
and its scaffolded setup code cell defines check but never selection. Meanwhile
test_exactly_the_expected_notebooks_carry_the_paragraph asserts that the set of notebooks
without the paragraph equals a frozen seven-name literal.
Consequence: the next published recipe (rank 51+, or the multimodal track) turns this test red
unless its builder happens to paste the paragraph verbatim — and CONTRIBUTING.md forbids a recipe
pull request from touching tests/, so the builder cannot adjust the expectation. They get a red
check in a file they may not edit, with nothing in the scaffold output or in
docs/recipe-template.md telling them the paragraph is required text. tests/test_template.py pins
template↔scaffolder agreement for SECTIONS and for build_fixtures_test_text, but not for this
prose, so nothing catches the drift either.
#233 asked for the fix "in the template first so the remaining 29 recipes are generated
correctly". Generated recipes do not get it.
What to change (any one; the first is cheapest and this PR already edits tools/new_recipe.py):
- emit the pinned paragraph from
tools/new_recipe.py'ssetup-md(and defineselectionin the
scaffoldedsetupcell, or state indocs/recipe-template.mdthat a recipe printing a
validationnumber must add it), plus a test that the scaffoldedsetup-mdcontainsPARAGRAPH,
so the two sources cannot drift; or - derive the exemption in
test_exactly_the_expected_notebooks_carry_the_paragraphfrom a property
of the notebook (for example "prints neither label") rather than a frozen name list; or - if this is deliberately deferred, say so in the PR body and file the follow-up, rather than
leaving a frozen literal that the next recipe pull request trips over.
Proved by construction. I scaffolded recipe 51 at this head
(python tools/new_recipe.py 51 --catalog <copy> --recipes-dir /tmp/…): its setup-md is
"… The header states which mode ran." plus a TODO, with Two labels may follow absent and no
selection = anywhere in the notebook. Dropping that scaffold into a scratch copy of the repo
under recipes/ and running the new test file gives:
FAILED tests/test_run_mode_sentence.py::test_exactly_the_expected_notebooks_carry_the_paragraph
E Extra items in the left set:
E '51-stalled-loop-detection'
1 failed, 47 passed, 8 skipped
M3 — One of the four claims tests/test_run_mode_sentence.py is advertised as pinning cannot fail: stripping every label off every number leaves it green
Where: tests/test_run_mode_sentence.py:111-119.
printed_check = any(CHECK in line for line in lines)
if printed_check:
assert modes & {"synthetic", "scripted"}, …
if modes & {"synthetic", "scripted"}:
assert printed_check, "a synthetic or scripted run that never prints the pipeline-check label"CHECK is the bare substring a pipeline check, not a Jev result, and output_lines yields every
stream line — including the measured cell's fixed prose, which in every synthetic or scripted
notebook reads
Provenance: synthetic. Not measured live: a pipeline check, not a Jev result.
So printed_check is True by boilerplate, whatever any number is labelled. I mutated
recipes/47-evidence-conflict-diagnosis's committed outputs three ways in a scratch copy and ran
the file:
| output-only mutation of recipe 47 | lines changed | result |
|---|---|---|
rename (a pipeline check, not a Jev result) → (a pipeline probe, …) |
25 | red (subset assertion) — matches the body's "renaming one label … turns that case red" |
rename (a selection step, not a reported result) → (a selection stage, …) |
3 | red (subset assertion) |
remove both labels from every number (" (a pipeline check…)", " (a selection step…)") |
25 | green — 48 passed, 7 skipped |
The body presents this as "the pipeline-check label appears only in a run whose committed header
says synthetic or scripted (and every such run prints it)". The first half is real; the second
half is inert — it cannot fail on the case it exists for. (The substance is covered elsewhere:
tests/test_recipe_guard.py G1(d) clause 2 requires metric lines to carry {check} from the cell
source, and tools/check_notebook_fresh.py catches a hand-edited output on re-execution. So this is
a test-strength and body-accuracy defect, not a hole in the published set — but a new instrument
that cannot fail where it must is exactly what the brief's rule 9 asks a reviewer to catch.)
What to change: require the label on a line that is actually a labelled output —
e.g. printed_check = any(CHECK in line and re.search(r"\d", line) for line in lines) restricted to
the bracketed form f"({CHECK})", or exclude the Not measured live: provenance sentence
explicitly — then re-run the strip mutation and confirm it goes red.
SHOULD CHANGE
S1 — The #209 enumeration in the PR body is wrong about recipe 32, and "the enumeration recorded here" is one of #209's acceptance criteria
Body: "Enumeration (git grep -l _dump -- 'recipes/*' on the base): tests/test_build_fixtures.py
in 49 replay recipes (all but 32 and 36, which are scripted and have none) and the template".
Measured on origin/main:
git grep -l '_dump' origin/main -- 'recipes/*/tests/test_build_fixtures.py'→ 50 files
(49 recipes + the template). The 49 recipes are all but 36, not "all but 32 and 36":
recipes/32-regression-test-selection/tests/test_build_fixtures.pyexists, imported_dumpon
the base, and is in this diff (its import and docstring are rewritten like the other 48).git grep -l '_dump' origin/main -- 'recipes/*/build_fixtures.py'→ 49 (48 recipes + template);
the two recipes absent there are 32 and 36. So the parenthetical belongs to the
build_fixtures.pycount, not to the test-file count — and that count ("the comment line in 47 of
them", plus recipe 50's import and 2 calls) is correct.- Recipe 32 is also not scripted: its committed header prints
Mode: offline replay of synthetic fixtures, …and it hasfixtures/responses.json. 36 is the only scripted recipe
(Mode: offline, scripted backend, …, noresponses.json, notests/test_build_fixtures.py).
Fix the sentence before this closes #209, since the recorded enumeration is an acceptance criterion.
Everything else in that paragraph (49 / 47 / template / recipe 50's build_fixtures.py and
tests/test_helpers.py) I reproduced exactly.
S2 — tests/test_no_private_shared_names.py:7-8 still carries the alias framing the PR body says was corrected
live._dumpitself stays as a supported alias for code written before the rename (see its
comment inlive.py)
src/jev_cookbook/live.py:839-842 and docs/live.md:166-173 say the opposite framing — a
transitional alias kept only for code outside recipes/, to be removed once nothing outside
live.py's own tests imports it — and the PR body has a bullet headed "The alias reason,
corrected", which is a prior review's finding. Align the docstring with the decision of record;
"supported … for code written before the rename" is the wording that invites new use.
S3 — "Two labels may follow a number below" invites a stronger reading than the test backs
Plenty of other parentheticals follow numbers in these same outputs — (confidence 0.80),
(coverage 0.8421), (0 is no, 1 is yes), (1 of 4 relevant), (confidence below the threshold),
and in recipes/24-incident-priority-composition the line PRIORITY_MATRIX (data, not a formula):.
A reader can take "Two labels" as "only two parentheticals appear below". Consider "Two
disclosure labels" or "Two run-mode labels".
Relatedly, LABEL at tests/test_run_mode_sentence.py:53 is \((a [^()]*, not a [^()]*)\) —
anchored on a leading a , so a future (scripted, not a Jev result) on a numeric line would slip
past the subset assertion the body describes as covering "the only (a ..., not a ...) labels in
any carrying notebook's committed output". Widening it to \(([^()]*, not an? [^()]*)\) would also
pick up recipe 24's (data, not a formula), which would then need an explicit allowance — a useful
forcing function.
S4 — The 7 exempt notebooks now print the labels with no explanation anywhere
36, 41, 43, 44, 45, 48 and 50 print (a pipeline check, not a Jev result) (and most of them the
selection label too) but carry no paragraph defining either. This is correctly outside #233's scope
and the body does not claim otherwise, but it is worth its own issue so the published set converges
rather than splitting 43/7 permanently.
Suggestions (non-blocking)
- Markdown source wrapping. The inserted paragraph is a single ~600-character source line. In
recipes/21-quiz-answer-adjudication/notebook.ipynbit sits inside asetup-mdwhose other
source lines wrap at ~100, and the two newdocs/recipe-template.mdbullets (lines 185-186) are
single long lines among wrapped prose. Nothing enforces wrapping (E501is not in
pyproject.toml's ruffselect, and there is no markdown width check), so this is cosmetic. - New lines past the configured width.
tests/test_new_recipe.py:1057(104 chars) and
src/jev_cookbook/live.py:831(103) exceedline-length = 100. Both are inside strings, so
ruff formatleaves them andruff checkdoes not selectE501; no check fails. docs/live.mdnames the private alias in a public doc. Lines 169-173 tell the reader
jev_cookbook.live._dumpexists and is the same function. The new guard forbids recipes from
using it; advertising it on the page a builder reads works against that. Consider keeping the note
inlive.pyonly.- The test's
scriptedbranch is vacuous today.PROVENANCEmatches, (scripted) backend,
but the only scripted recipe (36) does not carry the paragraph, so no carrying notebook exercises
that alternative. Harmless, but the body's "appears only in a run whose committed header says
synthetic or scripted" is only half-exercised by the published set. - Name the tracking issue.
tests/test_run_mode_sentence.py:18says the labelling gaps are
"tracked outside this test". They now are — Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326 ("Recipes: metric lines missing their
run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37)"). Citing it in the
docstring (the file already cites Foundation: the template tells readers every number carries the run mode, which is false for demo answer and per-example lines (32 notebooks) #233 and "B4") makes the claim checkable. - The scaffolded
build_fixtures.pymodule docstring still says "matchesjev_cookbook.live's
writer exactly" (tools/new_recipe.py:740). Accurate, but now thatdump_responsesis the
public name it is the one worth printing there.
Things that check out (verified, not taken on trust)
dump_responsesis a real public helper with its own test.src/jev_cookbook/live.py:823-836;
exported fromlive.__all__and the package__all__;_dumpis the same object, pinned by
tests/test_live.py::test_dump_responses_is_public_and_the_private_name_is_the_same_object; the
serialization contract (top-level sort only, 2-space indent, no ASCII escaping, one final LF,
input not mutated) is pinned bytest_dump_responses_text_is_exactly_the_stored_form, which would
fail asort_keys=Trueorensure_ascii=Truewriter.- No recipe or template file reaches a private shared name at this head.
_dumpsurvives only
insrc/jev_cookbook/live.py,tests/test_live.py,tests/test_no_private_shared_names.py,
tests/test_hygiene.py(unrelated_dump_json/_dump_fails),docs/live.mdand
docs/recipe-template.md. - The private-name guard is a real instrument — I probed it directly, importing
private_shared_usesand feeding it 15 sources. Flagged:from jev_cookbook.live import _dump,
from jev_cookbook._canonical import plain_json,import jev_cookbook._canonical,
import jev_cookbook._canonical as c+c.x,from jev_cookbook.fixtures._scan import scan_text,
import jev_cookbook.fixtures._schema as s,from jev_cookbook import fixtures+fixtures._scan,
from jev_cookbook import _canonical,from jev_cookbook.vision import _MAX_IMAGE_BYTES,
jev_cookbook.live._dump({}),L._dump({}). Not flagged (correctly):
jev_cookbook.__version__,from jev_cookbook import dump_responses, get_backend,
from jev_cookbook.fixtures import load_inputs, responses_path,x._private; self._dump.
SUBMODULEScontains bothfixtures(package directory) and_canonical. It also scans the
scaffolder's actual output in both--mode replayand--mode scripted, not its source text. - The matrix fix is genuine and mutation-proved.
column = max(width, len(str(matrix.matrix.max())));
tests/test_template_matrix_alignment.pyexecs the template's own cell source against the
realjev_cookbook.evaluation.confusion_matrixover five label/count shapes, and
test_the_harness_reports_the_old_label_width_only_sizingshows the harness red under
column = widthand still green where the old formula was correct.leader_widthcorrectly stays
on label width. Template outputs unchanged.tools/new_recipe.pyemits no matrix block, so
Foundation: the template's confusion-matrix print block misaligns when a count is wider than the widest label #217's "template and scaffolder agree" is not broken here. - The Foundation: four set-wide gaps from the recipe 25 review (queue attribution, threshold binding, forced-metric disclosure, baseline template line) #222 gap-3 template claim is true of the template's own output. The threshold is chosen
withtarget_accuracy=1.0over queue-naming answers only, andevaluation-validationprints
right among those routed automatically: 6 of 6 (a selection step, …) (a pipeline check, …)—
forced by construction;routed automatically: 6 of 10andaccuracy of the choice: 0.80are
not, so the paragraph's "Onevalidationnumber cannot come out otherwise" is exact.
test_the_template_prints_both_labels_on_a_validation_metricand
test_the_scaffolder_evaluation_stub_keeps_the_forced_metric_promptpin both halves. - The corrected
routecomment is factually right.recipes/_template/helpers.py:70,72,74—
routehas exactly the three review reasons the new comment names ("no option fits", "not a queue
Python may use", "confidence below the threshold").tests/test_template.pypins both the new
text and the absence of the old. - The body's label-gap statement is accurate. Selection-label-only lines occur in exactly
06, 07, 08, 09, 10, 18 and 37 (the body's "06-10, 18, 37"), andrecipes/47-…does print
unlabelled aggregate lines (test examples: 16, the raw matrix rows,
the name-pattern baseline beats Jev here by 0.0625, …). Declining to claim "every metric line
carries a label" is the honest call. checkreally is mode-conditional.check = " (a pipeline check, not a Jev result)" if offline else ""withoffline = backend.mode in ("synthetic", "scripted"), so "a recorded or live run
does not print it" is true of the code, not only of today's synthetic outputs. (Nothing pins this
in a test — see the prior-review table, S-b.)- Docs consistency.
CONTRIBUTING.mdsection 2,docs/notebook-style.md's "Title convention",
docs/offline-and-live.md,docs/glossary.mdanddocs/evaluation.mdall remain consistent with
the new paragraph and need no edit.docs/recipe-template.md's two new bullets are accurate.
docs/fixtures.md's worked example now callsdump_responses(...)withnewline="\n"
(Path.write_text(newline=…)exists from 3.10, so the 3.10 floor is fine); that example is not in
tests/test_docs_examples.py's executed set, butruff format --checkcovers the fence and both
changed lines are under 100 characters. - The body's mutation claim for the label names is true. "Mutation: renaming one label in a
notebook's output turns that case red" — I renamed each label in recipe 47's committed outputs
(25 lines and 3 lines respectively) and both turned
test_the_paragraph_is_verbatim_and_its_claims_hold_in_the_committed_output[47-…]red on the
subset assertion. (The separate, weaker clause about every such run printing the label is the
one that cannot fail — M3.) - Signing. Both commits unique to this head are signed with a verified key (
%G?=G); the
earlier branch commits showU(good signature, signer key not in this machine's keyring), not
unsigned.
Checks I ran
Fresh venv in my own worktree: uv venv --python 3.14 .venv-r2 (CPython 3.14.7) +
uv pip install -e ".[dev,ml,live]" -c <constraints-notebooks.txt> with UV_LINK_MODE=symlink
(disk on /workspace was ~450-630 MB free throughout, so symlinked installs were necessary).
ruff 0.17.1 (matching pyproject.toml:24), numpy 2.5.3, matplotlib 3.11.2, scikit-learn 1.9.1.
No TYPESAFE_API_KEY, no JEV_COOKBOOK_LIVE, no network call. Every heavy job ran under
flock .claude/heavy.lock nice -n 10, with pytest -p no:cacheprovider. The venv and worktree are
removed after this review.
docs/development.md's check list, all at eff86c7 on Linux / CPython 3.14.7:
| check | result |
|---|---|
ruff check . |
All checks passed! |
ruff format --check . |
400 files already formatted |
python tools/render_catalog.py --check |
exit 0 |
python tools/check_hygiene.py |
Hygiene check passed: 609 file(s) scanned, exit 0 |
python tools/notebook_ci.py fixtures |
51 recipe folders, 0 failed, exit 0 |
pytest -q -p no:cacheprovider |
5466 passed, 54 skipped, 50 deselected, 31 xfailed, 0 failed (215 s) |
The pytest totals match the PR body exactly, and the two tests/test_notebook_ci_sandbox.py
failures the prior reviews saw on Windows do not occur here (they were the signal.SIGKILL cases
fixed by #302, which is already on origin/main at this base).
Notebooks — executed offline from a scratch copy, never in the tree (rm -rf then cp -R to
/tmp/r2/run/<slug>, PYTHONPATH=<worktree>/tools/netguard python tools/execute_notebook.py, then
tools/check_notebook_fresh.py against the committed file and tools/check_hygiene.py on the
fresh copy). 9 of 9 OK — execute: OK, matches a fresh offline run, hygiene: OK for each,
and git status --porcelain identical before and after both batches:
| notebook | execute | check_notebook_fresh.py |
check_hygiene.py |
|---|---|---|---|
recipes/_template |
OK (2 figures, identical bytes) | matches a fresh offline run | passed |
02-refund-intent-detection |
OK (3 figures) | matches | passed |
15-sensitive-text-triage |
OK | matches | passed |
25-change-evidence-review |
OK | matches | passed |
26-prompt-injection-screening |
OK | matches | passed |
28-skill-selection |
OK (1 figure) | matches | passed |
39-document-extraction-cascade |
OK (4 figures) | matches | passed |
47-evidence-conflict-diagnosis |
OK (2 figures) | matches | passed |
49-multi-agent-task-assignment |
OK (3 figures) | matches | passed |
(I picked the template, the four notebooks with the most distinctive labels — 28, 39, 47, 49 — and
the five M1 counterexamples, so every line I quote in M1 is confirmed to be what a fresh offline run
at this head actually prints, not a stale committed output. This machine has a netguard namespace;
CI re-runs under the real sandbox.)
Process note, reported against myself. My first attempt at the 15/25/26 batch wrapped
nbrun.sh — which already takes flock heavy.lock per execution — in an outer flock, so it
self-deadlocked and held the shared lock until the coordinator killed it. The earlier six-notebook
batch had no outer flock and was unaffected; 15, 25 and 26 were re-run correctly afterwards and
are the results above.
Instrument probes (my own, at this head):
tests/test_no_private_shared_names.py::private_shared_usesfed 15 sources — 11 flagged,
4 correctly not flagged (listed above).tests/test_run_mode_sentence.pyrun against four mutated scratch copies: label rename in an
output → red; selection-label rename in an output → red; paragraph reworded or dropped →
red; every label stripped off every number → green (M3).tools/new_recipe.py 51scaffolded into a scratch tree, then dropped underrecipes/:
test_exactly_the_expected_notebooks_carry_the_paragraph→ red (M2).
Static analysis I wrote and ran myself, outside the test suite:
git diff --raw -M origin/main...HEADmode/status audit (section 1).- structural base-vs-head comparison of all 44 changed notebooks (code sources, serialized outputs,
metadata, cell ids, markdown cell count) — the result in section 2. build_fixtures_test_text(recipe)byte-comparison for all 49 recipes that have the file.- a full sweep of every carrying notebook's committed stream output for
(… not a …)-shaped labels,
for selection-labelled lines, for selection-only lines, fortest-only selection-labelled lines,
and for trailing parentheticals following a number; split resolution for every M1 counterexample
against the recipe's ownfixtures/inputs.jsonl. - direct probing of
private_shared_useswith 15 sources (listed above). _dumpenumeration onorigin/mainand at the head (S1).
Acceptance criteria
#209 — promote jev_cookbook.live._dump to public API
| criterion | status |
|---|---|
| A public, documented helper exists with its own test | met — dump_responses with a full docstring, package export, docs/live.md entry, and two dedicated tests in tests/test_live.py |
tools/new_recipe.py generates the public import, and a scaffold test proves it |
met — tools/new_recipe.py:466 and :748; proved by tests/test_new_recipe.py::test_build_fixtures_writes_responses_byte_identical_to_the_recorders_writer and `tests/test_no_private_shared_names.py::test_the_scaffolder_emits_no_private_shared_name[replay |
Every published recipe importing _dump is migrated, with the enumeration recorded here |
migration met; enumeration inaccurate — all 49 recipes + template migrated (verified: no private shared name survives under recipes/), but the recorded enumeration misattributes recipe 32 (S1) |
The keep-or-remove decision for _dump is recorded, and if it stays, what it is for |
met — recorded in src/jev_cookbook/live.py:839-842, docs/live.md:166-173 and the PR body; S2 is an inconsistent restatement, not a missing decision |
| No recipe imports an underscore-prefixed shared name afterwards, enforced by a guard | met — tests/test_no_private_shared_names.py, which I probed directly on 11 positive and 4 negative spellings |
#233 — the run-mode sentence
| criterion (the issue's "Suggested handling" and its honesty premise) | status |
|---|---|
| The false sentence is replaced in the template | met — and the replacement is a genuine improvement: it drops the universal claim and is pinned verbatim |
| …and wherever it had already been copied | met — all 43 published notebooks that carried it; verified by git grep on the base |
The replacement is true (CONTRIBUTING.md section 2's honesty rule — the whole point of the issue) |
not met — the selection-label sentence is false for at least five of the 43 (M1) |
| Markdown only; invalidates no committed output, needs no re-execution | met — verified cell by cell for all 43 |
| Done "in the template first so the remaining recipes are generated correctly" | not met — tools/new_recipe.py does not emit the paragraph, and the new test will red the next recipe pull request (M2) |
| The paragraph's claims are pinned by a test (the PR's own added commitment, not the issue's) | partly met — the verbatim text, the carrier set, and the label-set subset assertion are real and fail under mutation; the "every synthetic or scripted run prints the pipeline-check label" assertion cannot fail (M3), and nothing pins the selection-label↔split claim at all (M1) |
#217 and #222 (refs only — correctly not claimed closed)
Refs #217: the template's matrix block is fixed and mutation-proved; recipe 23's convergence and
the per-recipe guard are explicitly "Not done", matching the diff; tools/new_recipe.py emits no
matrix block so the template/scaffolder criterion is not broken. Refs #222: the template's
forced-metric line and the scaffold prompt land; the five published recipes and gaps 1, 2 and 4 are
explicitly "Not done", matching the diff. Both honest.
Prior reviews — status of each finding at this head
I read these only after writing everything above:
6302632394 (review 1, head 7a5299f), 6302830060 (review 2, head 7a5299f),
6303724732 (review 1 re-review, head 7e0a1bd).
6302632394 — review 1, round 1
| finding | status at eff86c7 |
|---|---|
| B1 "per-example lines … carry no label of their own" is false in ≥15 notebooks | resolved. The claim is gone; grep finds "carry no label of their own" in 0 files. The current wording ("A line with neither label, such as an individual answer or a per-example line, still comes from the mode the header names") asserts nothing about which lines are labelled, and I confirmed individual-answer lines are unlabelled in every carrying notebook, so the illustration always has an instance. |
| B2 "labelled a selection step instead", and unlabelled validation lines in 39/49 | resolved as worded. "instead" is gone and its absence is pinned (tests/test_run_mode_sentence.py:88). "may follow" no longer contradicts 39/49. The underlying labelling gaps are now tracked by #326. |
| B3 the guard misses private modules | resolved, verified by my own probe. All five of the reviewer's spellings are flagged, SUBMODULES includes fixtures, and the __version__ false positive is gone. 12 positive and 6 negative parametrised cases now ship with the guard. |
| S1 "In an offline run" also covers recorded replays | resolved — the paragraph and the template's evaluation-md both say "synthetic or scripted run". |
| S2 recipe 47's unlabelled gap line | open, now disclosed and tracked (tests/test_run_mode_sentence.py:15-19, issue #326). |
S3 the scaffold hand-serialises responses.json |
resolved — recipes/_template/build_fixtures.py and tools/new_recipe.py both call dump_responses; docs/fixtures.md and docs/live.md match. |
| S4 pin the new sentences | resolved — tests/test_run_mode_sentence.py pins the paragraph verbatim in all 44 and pins the scaffold's forced-metric prompt and the template's "by construction" line. |
| S5 "exactly one review reason" | resolved — the comment now names the three reasons (which I verified against helpers.py), pinned in tests/test_template.py. |
S6 the alignment test runs with check = "", not the real suffix |
not addressed (tests/test_template_matrix_alignment.py:57). Low risk: the suffix is a constant appended to every line of the block, so it cannot change relative alignment. Still worth one case with the real suffix. |
6302830060 — review 2, round 1
| finding | status at eff86c7 |
|---|---|
Blocking 1 — "instead" / "(a test number)" |
resolved (same as B2). |
| Blocking 2 — guard misses private modules; dunder false positive | resolved (same as B3), both halves verified by my probe. |
S1 make dump_responses the writer in template and scaffold |
resolved. |
S2 docs/live.md overstates what the scaffolded test checks |
resolved — docs/live.md:166-173 now says the test compares the committed file with dump_responses(build_responses(ROWS)), "what the synthetic generator would write", and states that it stops holding once a recorded answer is merged. The PR body repeats that caveat. The scaffolded module docstring still says "matches jev_cookbook.live's writer exactly" (accurate; see my suggestions). |
S3 multimodal: the scaffolded tests/test_build_fixtures.py cannot pass for an image-backed recipe |
not fixed, and openly stated — the PR body routes it to #315. Unchanged by this PR. |
S4 the _dump alias rationale does not hold up |
resolved in substance — the body's "The alias reason, corrected" bullet and live.py:839-842 now call it transitional, for code outside recipes/, to be removed. But tests/test_no_private_shared_names.py:7-8 still says "supported alias for code written before the rename" — my S2. |
S5 docs/fixtures.md's example still hand-serialises |
resolved — it now calls dump_responses(responses) with newline="\n". |
| S6 "exactly one review reason" | resolved. |
6303724732 — review 1, re-review
| finding | status at eff86c7 |
|---|---|
B4 "marks a number used to choose a setting on validation" is false for most labelled lines |
resolved as raised, but its replacement introduces M1. The objected wording is gone, its absence is pinned (tests/test_run_mode_sentence.py:91), and B4's suggested sentence was adopted verbatim. The problem is B4's supporting evidence: it establishes "no selection-labelled line is a test-only number" (which I reproduced independently and confirm), and that does not establish "every selection-labelled number is a validation number". The cross-split queue and preview lines in recipes 02, 10, 15, 25 and 26 were not in that scan's predicate, and they falsify the adopted wording. So B4 is closed and M1 is new. |
| S-a the labelling gaps are called "tracked" but nothing tracked them | resolved — #326 now exists ("Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37)"). The test docstring still does not cite it; naming it would make the claim checkable. |
| S-b nothing pins the code side of "a recorded or live run does not print it" | not addressed. No test asserts that each carrying notebook defines check from backend.mode in ("synthetic", "scripted"); I verified the definition by hand in the template and spot-checked the published set. The test's recorded/live branch stays vacuous against committed outputs. |
| S-c (= S6, real suffix in the alignment test) | not addressed. |
Nothing in the three prior reviews contradicts anything I found, and none of them raised M1, M2, M3 or
S1 (the recipe-32 enumeration error).
Out of scope — reported, not fixed
docs/development.md:366says the pinned ruff isruff==0.16.10;pyproject.toml:24pins
ruff==0.17.1. Stale onorigin/main, untouched by this pull request.- Independent of how M1 is resolved,
recipes/02(queue-final),recipes/10(queue),
recipes/15(queue),recipes/25(queue-preview) andrecipes/26(queue-preview) attach
{selection}to numbers computed overdemoand/ortestexamples, whichCONTRIBUTING.md
section 2 ties to thevalidationsplit. This is a sibling of Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326 and belongs on it or on its
own issue;recipes/15's ownqueue-buildcell shows the correct labelling.
OPUS-REVIEW: CHANGES-NEEDED eff86c7
…scaffold it (#233, review fixes) - The selection-label sentence no longer says the label marks a number from `validation`: recipes 02, 10, 15, 25 and 26 attach it to queue counts and previews that span demo and validation or both scored splits. It now says the label marks a line that is not a result to report, such as a validation number or a count of what the frozen rule queues. Reworded in the template and the 43 published notebooks, byte-identical in all 44, plus the scaffolder. - Recipe 36's setup text still said every number it prints "says so"; it now carries the paragraph. Recipe 36's evaluation text, recipe 45's "every number below already being labelled" and recipe 50's "both labels on every validation line" had the same defect and are scoped to metric lines, which is what their outputs do. - tools/new_recipe.py emits the paragraph (with a TODO beside it), so a new recipe passes tests/test_run_mode_sentence.py without editing it; the test now requires the paragraph everywhere except a closed list of six legacy notebooks and fails on the retired wordings in any notebook. - The "every synthetic or scripted run prints the pipeline-check label" check now needs the label on an output line as a parenthetical, so the fixed "Not measured live" prose cannot satisfy it; a mutation test strips, renames and re-labels it and must go red. - tests/test_no_private_shared_names.py: the alias is transitional, not supported. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Clean merge. docs/recipe-template.md and docs/fixtures.md auto-merged with both sides' text kept (assets-manifest row and objective-pin routes from main; dump_responses, forced-metric and matrix-column text from this branch). Refs #317 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
PR #317 re-review — reviewer 1 of 2 (fresh Opus), round 2
VerdictCHANGES-NEEDED. Both of my round-1 blockers are properly resolved, and so are all three of the Must changeM1. The template's
|
| notebook | sentence | unlabelled numeric lines after evaluation-md |
true? |
|---|---|---|---|
_template |
"every number here, test and validation alike … and says so" |
2 (evaluation-wrong) |
no |
38-abstention-and-review-queues |
"every number drawn from it below carries both a selection-step label and the offline pipeline-check label" | 2 (evaluation-wrong) |
no |
40-hierarchical-classification |
same wording as the template | 0 | yes |
39-document-extraction-cascade |
"every number below is a pipeline check" | 0 | yes (in that section) |
Recipe 38 is a published recipe and I would accept deferring it to #326; the template I would not.
M2. tools/new_recipe.py still scaffolds the "In an offline run" wording this PR fixed in the template
tools/new_recipe.py:243-246, the evaluation-md stub every new recipe starts from:
f"{TODO_MARK}: choose every setting on `validation`, freeze it, and report once on "
"`test` with `jev_cookbook.evaluation`. In an offline run print each number with "
"`check` beside it. Say which `validation` numbers your selection forces …"
A recorded replay is also an offline run, and in it check is "" (the scaffolded setup cell emits
offline = backend.mode in ("synthetic", "scripted"), which I confirmed in the scratch scaffold).
That is precisely why the template's sentence was changed to "In a synthetic or scripted run", and
the PR body states that fix ("The template's evaluation-md ('In an offline run ...') now says
'synthetic or scripted run'") without noting that the scaffolder still says the old thing. For a
PR titled "template and scaffolder", the scaffolder should carry the same wording. One string.
Suggestions (not blocking)
- S1. Recipe 45's reworded sentence is still not quite true.
45-memory-reconciliation's
evaluation-mdnow says "each metric line below already being labelled a pipeline check". Its
evaluation-matrixcell prints the confusion matrix as a bare label list and a raw array with
no label on any line (['corroborating', …],[[4 0 0 0]…), andevaluation-test's
"test examples: 16" is unlabelled too — 12 unlabelled numeric lines afterevaluation-mdin all.
Every other recipe I looked at (template, 36, 50) labels each matrix row with{check}. The
rewrite is a clear improvement over "every number below", and Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326 is the right home for the
underlying gap — but Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326 does not list recipe 45, so please add it (its unlabelled matrix and
test examples:line) so the sentence has something behind it. - S2.
LEGACY_WITHOUT_THE_PARAGRAPH's closedness test has one hole.
test_the_legacy_list_is_closedassertsall(int(name[:2]) <= 50 for name in …). For the
three-digit rank the catalog allows (100-…),name[:2]is"10", so10 <= 50passes and a
rank-100 recipe could be exempted.int(name.split("-")[0]) <= 50closes it. - S3. Two leftover wordings, carried over from round 1 and still open (low priority).
tools/new_recipe.py:756(the scaffolded module docstring) says "the committed file matches
jev_cookbook.live's writer exactly" where every other line in that file now names
jev_cookbook.dump_responses;src/jev_cookbook/live.py:834says "what the recorder would write"
wheredocs/live.mdnow states it precisely. - S4.
build_fixtures.pyconvergence, now accurately disclosed. My round-1 S2 was wrong in
scope and the new body is right: at this head 48 of 50 recipes'build_fixtures.pystill
hand-serialize withjson.dumps(dict(sorted(...)), indent=2, ensure_ascii=False) + "\n", and only
the template and recipe 50 calldump_responses; in 47 of them the PR changed only the comment
(verified onrecipes/41-approval-scope-review/build_fixtures.py:479, wheredump_responses
appears in a comment and the import line does not mention it, and line 483 still hand-serializes).
That is a legitimate scope decision —build_fixtures.pyis not byte-pinned by G1(a) and migrating
it would mean re-verifying 48 generators — and the body now says so plainly. Worth its own issue.
My round-1 findings: status at this head
| round-1 finding | status | how I checked |
|---|---|---|
| M1 PR body's #209 enumeration misattributes recipe 32 | resolved. I re-derived the whole enumeration from git ls-tree/git grep at the base c3167129 and read the body sentence by sentence; every number is right. See the table below. |
git ls-tree -d recipes/ → 51 folders; git ls-tree -r | grep -c tests/test_build_fixtures.py → 50; the one folder without it is 36 and git ls-tree …/36-…/fixtures/ holds exactly inputs.jsonl and labels.jsonl; git grep -l _dump -- 'recipes/*' at base → 100 files, split 50 test_build_fixtures.py / 49 build_fixtures.py / 1 test_helpers.py; at head git grep -l _dump -- 'recipes/*' → 0. Recipe 32 is correctly described as a replay recipe (163 stored answers, get_backend(fixtures=…)) that has the file and was migrated. The "comment only in 47 of the 48" claim checks out on recipe 41. |
M2 recipe 36's setup-md still claimed every number says so |
resolved, and widened. 36 now carries the paragraph verbatim and is off the legacy list; 36's evaluation-md, 45's and 50's own unscoped variants were scoped in the same commit; FORBIDDEN_WORDINGS retires all four spellings. |
Paragraph verbatim in 45 notebooks, the 6 not carrying it are exactly LEGACY_WITHOUT_THE_PARAGRAPH. Each forbidden entry fires where it must: "carries that mode" → 44 notebooks at base; "marks a number from `validation`, the split settings are chosen on;" → 44 at eff86c7; "every number this notebook prints" and "every number here, `validation` and `test` alike" → recipe 36; "every number below already being labelled" → 45; "printed on every validation line" → 50; all 0 at this head. I also read 36's whole evaluation output: every metric line carries the check label, the 19 per-turn lines carry none (which its new text says), and each validation metric carries both. |
| S1 "every synthetic or scripted run prints the label" could not fail | resolved, and I proved it independently. labelled_check_lines now requires the label as a parenthetical, so the fixed "Not measured live: …" prose cannot satisfy it. |
I exec'd check_run_mode_claims against four notebooks the test's own parametrisation does not cover (02, 36, 16, 49). Committed: green. Strip (a pipeline check, not a Jev result) from every output line: red ("prints the pipeline-check label on no output line") in all four — this was the mutation that stayed green at eff86c7. Rename either label: red in all four. |
| S4 cite the issue that tracks the gaps | resolved — the docstring now says "tracked in #326". | |
| S2 recipe 32 hand-copies the serializer | superseded; see S4 above — my framing was wrong and the new body states the real picture. | |
S5 matrix test runs with check = ""; detector docstring's assignment hole |
not addressed, immaterial, fine to close. |
The other reviewer's blockers (as the coordinator described them)
| blocker | status | how I checked |
|---|---|---|
| selection-label wording false in 02/10/15/25/26 | resolved. The sentence now names no split: "marks a line that is not a result to report, such as a number from validation … or a count of the items the frozen rule queues or would queue". |
I classified every selection-labelled output line in the 45 carrying notebooks myself: 983 lines (the body's number), 0 that name test without validation, and 0 in any cell whose id contains test. The lines that drove the finding are covered word for word by the new clause — 02 [queue-build] "2 items queued so far", 02 [queue-final] "25 items in the queue across both scored splits", 10 [queue] "1 item queued so far", 10 [queue-update] "3 validation items added to the queue just now", 15 [queue] "preview: 2 of these four would be queued", 25/26/31 [queue-preview] "preview: 1 of these four would be queued". |
| scaffolder does not emit the paragraph; exemption list not frozen | resolved, proved by scaffolding. | In a scratch copy of the tree (never in the checkout) I ran tools/new_recipe.py 51 and tools/new_recipe.py 52 --mode scripted. tests/test_run_mode_sentence.py went from 54 passed, 6 skipped before to 56 passed, 6 skipped after, with no edit to the test file — the two new recipes each added a passing parametrised case and the skip set (the 6 legacy names) did not grow. The scaffolded setup-md carries the paragraph verbatim followed by the "keep this paragraph as it is; define selection …" TODO, in the same \n\n block. test_the_legacy_list_is_closed pins that the list cannot grow (one hole, S2). |
| the vacuous "every synthetic run prints the label" assertion | resolved — same as my S1 above, with my own mutation proof on four uncovered notebooks. |
Everything that changed since eff86c7, reviewed
The fix commit 91e5f0b (signed, Co-Authored-By: Claude Sonnet 5.5) touches 50 files: 47
notebooks, tests/test_run_mode_sentence.py, tests/test_no_private_shared_names.py,
tools/new_recipe.py. Nothing else.
- All 47 notebook changes are markdown-source-only. I parsed both refs and compared every cell
field and every top-level key: 46 notebooks change exactly one markdown cell'ssource, recipe 36
changes two (setup-mdandevaluation-md), and no output, metadata, id, execution count,
cell count or cell order changes anywhere. Zero problems. The 47 = the 44 that carried the old
paragraph (43 published + template) plus 36, 45 and 50. tests/test_run_mode_sentence.pyis the substance:PARAGRAPHreworded,FORBIDDEN_WORDINGS
added and checked against squashed markdown (so a wording split across source lines is caught,
which the old raw-text check would have missed),NEVER_HAD_THE_OLD_SENTENCEreplaced by a closed
LEGACY_WITHOUT_THE_PARAGRAPHwith a closedness test and a stale-exemption test, the label checks
factored intocheck_run_mode_claimswith the parenthetical requirement, and a real mutation test
(test_the_label_checks_fail_on_a_mutated_run) over_templateand recipe 47 whose strip case
first asserts the provenance prose survives, so the case cannot be vacuous.tools/new_recipe.pygainsRUN_MODE_PARAGRAPH, emits it insetup-mdwith a TODO, and the test
pinssquash(module.RUN_MODE_PARAGRAPH) == PARAGRAPHso the two cannot drift. The cross-reference
totests/test_template_scaffolder_agreement.pyis real: that file lists
selection = ' (a selection step, not a reported result)'as a template-only setup statement the
scaffold deliberately does not emit, which is why the TODO asks the builder to add it.tests/test_no_private_shared_names.py: docstring only, "supported alias" → "transitional alias
… to be removed once nothing outsidelive.py's own tests imports it". Matcheslive.py.
The merge f60324a. git show --cc prints no combined hunks at all, so every file in the
result equals one parent: a clean merge with nothing hand-edited. It brings in only origin/main's
own 23 files (#331 frames.py, #323, #305 objective-pin derivation, #330 asset manifest) — no
recipes/ path, so the sweep is untouched by it. docs/recipe-template.md and docs/fixtures.md
auto-merged and read coherently: the merge keeps main's assets/, assets-manifest.json and
side_effects.json rows alongside this branch's rewritten tests/test_build_fixtures.py row and its
two new bullets, with no duplicated or contradictory row (the apparent duplicate
tests/test_build_fixtures.py row at line 444 is the separate replay-vs-scripted table), and
docs/fixtures.md's worked example still calls dump_responses inside main's much-expanded page.
The commit message records the resolution and the co-author line.
git diff --raw -M origin/main..HEAD: 153 100644 100644 M and 3 000000 100644 A. No
symlink, submodule, mode or type change. Nothing touches the network, a key, a credential-shaped
literal or a real side effect; no absolute home path is committed.
PR body against the diff. The rewritten #209 enumeration is correct in every sentence I could
check (above). The #233 paragraph's description matches the pinned text. "983 selection-labelled
output lines in the 45 carrying notebooks" and "no line names test without validation" are both
exactly right. "Markdown only in all changed notebooks; nothing printed changes" is right. The
verification numbers match mine exactly. The one thing the body does not say is that the template
itself still carries a retired wording (M1) and that the scaffolder still carries the "In an offline
run" phrasing the body reports as fixed (M2); line 17's honest disclosure of ~25 published
evaluation-md claims is about published recipes, not about the template or the scaffolder.
Checks I ran
Fresh venv at /home/agent/r1b-venv, Python 3.14.7 via uv venv --python 3.14, deps via
uv pip install --link-mode symlink -e ".[dev,ml,live]" -c .github/constraints-notebooks.txt
(CI's install, exit 0). TYPESAFE_API_KEY and JEV_COOKBOOK_LIVE unset throughout. Every heavy job
under flock .claude/heavy.lock nice -n 10 with no nesting; pytest -p no:cacheprovider.
| command | result | body claims |
|---|---|---|
ruff check . |
All checks passed! (exit 0) | matches |
ruff format --check . |
404 files already formatted (exit 0) | matches |
python tools/render_catalog.py --check |
exit 0 | matches |
python tools/check_hygiene.py |
passed, 614 file(s) (exit 0) | matches |
python tools/check_recipe_assets.py |
passed, 461 file(s) under recipes/ (exit 0) | matches |
python tools/notebook_ci.py fixtures |
51 recipe folders, 0 failed (exit 0) | matches |
pytest |
6039 passed, 53 skipped, 50 deselected, 31 xfailed in 218 s, 0 failed (exit 0) | matches exactly |
gh pr checks 317 |
60 of 60 pass on this head | — |
Notebooks (6, offline, scratch copies, never in the tree). For each: rm -rf scratch,
cp -R recipes/<slug>, PYTHONPATH=<wt>/tools/netguard python tools/execute_notebook.py,
check_notebook_fresh.py against the committed file, check_hygiene.py on the fresh copy, with a
git status --porcelain snapshot before and after.
| recipe | why | result |
|---|---|---|
_template |
the file M1 is about; its code cells changed earlier in this PR | OK |
36-card-game-action-selection |
newly given the paragraph; evaluation-md rewritten | OK |
02-refund-intent-detection |
the queue-count lines that drove the selection-label rewrite | OK |
45-memory-reconciliation |
evaluation-md rewritten (S1) | OK |
50-model-comparison-and-calibration |
calibrate-md rewritten | OK |
47-evidence-conflict-diagnosis |
one of the two the new mutation test runs on | OK |
6 of 6 OK, git status --porcelain identical before and after. check_notebook_fresh.py
compares sources, metadata, execution counts, ids and text outputs exactly (images by content), so
the template passing it proves its unchanged code cells still reproduce the committed output.
Caveat, as in round 1: no kernel network namespace here, so my offline guarantee rests on
tools/netguard and the replay backend; CI's 51 per-recipe jobs, which do use the namespace and a
separate unprivileged user, are green on this head.
Scaffold proof (scratch copy of the tree at /home/agent/r1b-repo, removed afterwards).
tools/new_recipe.py 51 and tools/new_recipe.py 52 --mode scripted both created their folders;
tests/test_run_mode_sentence.py went 54 passed / 6 skipped → 56 passed / 6 skipped with the two
new recipes present and no edit to the test;
tests/test_new_recipe.py tests/test_template.py tests/test_no_private_shared_names.py tests/test_template_matrix_alignment.py
→ 144 passed.
Instruments run against refs where they must find something: the forbidden-wording list
(table above, each entry fires at the base or at eff86c7); private_shared_uses against the base
content of four migrated files (round 1); the label checks against three mutations on four notebooks
outside their own parametrisation.
Acceptance criteria
#209 — all five met. Public documented helper with its own tests; the scaffolder emits the
public import in both modes and a scaffold test proves it (144 passed in the scratch scaffold); every
published recipe that imported _dump is migrated (git grep → 0 at head) and the enumeration is
now recorded correctly in the body, re-derived independently above; the keep-or-remove decision is
recorded as transitional in live.py, docs/live.md, docs/recipe-template.md and the guard's
docstring; the guard enforces the no-private-name rule and demonstrably fails on the pre-migration
content.
#233 — not met, on M1 only. The setup-md sentence is gone from every notebook, the
replacement is verbatim in 45 of 51 with a closed, tested 6-name exemption, the scaffolder emits it
so new recipes inherit the true wording, the retired wordings are banned by a list I proved fires,
the label claims are now falsifiable and I proved them red under mutation, and the three published
recipes with their own unscoped variants were fixed. The template itself still carries one.
#217, template part only — met (unchanged since round 1); recipe 23 and the per-recipe guard
remain open by design.
#222 gap 3, template line only — met (unchanged); the five published recipes and gaps 1, 2, 4
remain open by design.
Outside my task, reported not fixed
- Recipe 38's
evaluation-md("every number drawn from it below carries both …") is false for the
two unlabelled lines in itsevaluation-wrongoutput — the same class as M1, in a published
recipe. Belongs on Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326. - Recipe 45's unlabelled confusion matrix and
test examples: 16line (S1) are not on Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326. - Recipe 36's
measuredcell printsN: 40 validation and 56 test turnswith only the pipeline-check
label while itsevaluation-mdsays eachvalidationmetric also carries the selection label;N
is a sample size rather than a metric, so I read the sentence as holding, but it is the kind of line
Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326's sweep should settle one way or the other. docs/fixtures.md's code blocks are still not executed by any test
(tests/test_docs_examples.pyexcludes the page deliberately), so this PR's edit to its worked
example has no automated cover; I executed it by hand in round 1 and it is correct.
OPUS-REVIEW: CHANGES-NEEDED f60324a
Opus re-review (round 2) — PR #317 (
|
print sites interpolating {selection} |
398 |
| references validation data in the same cell | 330 |
| …and is also queue/preview-shaped | 64 |
| queue/preview-shaped only | 4 |
| neither validation-referencing nor queue/preview-shaped | 0 |
| sites interpolating a value derived from the test split | 0 |
The single name that looked test-derived is val_review.test_id in recipe 32 — a regression-test
identifier on a validation review line, not the test split. So there is no selection-labelled
line that is a reported test result, and every site falls under one of the two illustrations the
paragraph names. The old wording is gone from every notebook, and
tests/test_run_mode_sentence.py's FORBIDDEN_WORDINGS pins its absence with the exact string I
objected to.
M2 (blocker) — the scaffolder never emitted the paragraph, and a frozen seven-name list redded a fresh recipe → RESOLVED
tools/new_recipe.py now holds RUN_MODE_PARAGRAPH and splices it into setup-md with a TODO
telling the builder to define and print selection. The exemption is now
LEGACY_WITHOUT_THE_PARAGRAPH — six names — and test_the_legacy_list_is_closed requires each to
still exist, to still lack the paragraph (so a stale exemption fails), and to be numbered ≤ 50.
Reproduced as the brief asked, in a scratch copy with my own venv:
scaffold 51 [] -> exit 0 created recipes/51-stalled-loop-detection/
scaffold 52 [--mode scripted] -> exit 0 created recipes/52-retrieval-pipeline-evaluation/
51-stalled-loop-detection paragraph=True selection_defined=False check_defined=True
52-retrieval-pipeline-evaluation paragraph=True selection_defined=False check_defined=True
tests/test_run_mode_sentence.py with 51 and 52 present: exit 0 — 56 passed, 6 skipped
At eff86c7 the same experiment produced
FAILED test_exactly_the_expected_notebooks_carry_the_paragraph … Extra items: '51-…'. A newly
scaffolded recipe of either mode now passes without anyone editing tests/. selection is still
not defined in the scaffolded setup cell, which is deliberate and documented: the TODO asks for
it, and tests/test_template_scaffolder_agreement.py:161 pins selection as a template-only setup
statement, so the two sources cannot silently disagree.
M3 (blocker) — "every synthetic or scripted run prints the label" could not fail → RESOLVED
The assertion now goes through labelled_check_lines, which requires the parenthesised form
f"({CHECK})", so the fixed prose Provenance: synthetic. Not measured live: a pipeline check, not a Jev result. can no longer satisfy it. test_the_label_checks_fail_on_a_mutated_run pins the
strip, the check-rename and the selection-rename for _template and recipe 47, and asserts the
provenance prose survives the strip so the case is the real one.
I ran my own strip mutation on four notebooks, not only the two pinned, each in a scratch repo
copy, confirming the prose survived each time:
| notebook | label occurrences stripped | provenance prose survives | result |
|---|---|---|---|
47-evidence-conflict-diagnosis |
25 | yes | red (2 failed) |
_template |
20 | yes | red (3 failed) |
02-refund-intent-detection |
47 | yes | red (1 failed) |
36-card-game-action-selection (scripted) |
46 | yes | red (1 failed) |
At eff86c7 the identical mutation on recipe 47 left the file green (48 passed). Recipe 36
also closes the gap I noted last round that the scripted branch of PROVENANCE was never
exercised by any carrying notebook: 36's header is Mode: offline, scripted backend, …, it now
carries the paragraph, and the mutation proves the assertion fires for a scripted run.
S1 (should-change) — the #209 enumeration misattributed recipe 32 → RESOLVED
This was also the other reviewer's finding. The rewritten body now says: 51 recipe folders, 50 have
tests/test_build_fixtures.py, the only folder without it is recipe 36 (scripted, no
responses.json), and recipe 32 is a replay recipe, has the file, and is migrated. Every part
reproduces (see §3).
S2 (should-change) — tests/test_no_private_shared_names.py still called _dump a "supported alias" → RESOLVED
Its docstring now reads "stays only as a transitional alias for code outside recipes/, to be
removed once nothing outside live.py's own tests imports it", matching live.py:839-842,
docs/live.md:166-173 and the body.
S3 (suggestion) — "Two labels" vs other trailing parentheticals; narrow LABEL regex → not taken, and I am no longer concerned
The regex is still \((a [^()]*, not a [^()]*)\). I checked the one case that newly matters:
recipe 36 now carries the paragraph and its outputs contain (not a baseline) three times — but in
every instance it qualifies a policy name (model policy, ungated (not a baseline): 6-4-0, win rate 0.6000 … (a selection step, …) (a pipeline check, …)), with the two real labels appended
after it. So no third label follows a number anywhere in the carrying set, and the paragraph is not
falsified. Left as a suggestion below.
S4 (should-change) — the exempt notebooks print the labels with no explanation → partly resolved
The exempt set shrank from seven to six: recipe 36 was given the paragraph (it was the one whose
own setup text made exactly the claim #233 removed). 41, 43, 44, 45, 48 and 50 remain, and
test_the_legacy_list_is_closed keeps the list from growing. Still worth its own issue; not a
blocker, and the body does not claim otherwise.
2. The other reviewer's findings
| finding | status |
|---|---|
| PR body's #209 enumeration misattributes recipe 32 | resolved — see S1 above and §3 |
recipe 36's setup-md still carried the old sentence |
resolved — 36's setup-md now carries the paragraph verbatim, and its evaluation-md variant ("every number this notebook prints … and says so") is replaced and pinned in FORBIDDEN_WORDINGS. Recipes 45 and 50 had their own variants and were fixed in the same sweep |
3. Everything that changed since eff86c7, verified
eff86c7 → 91e5f0b (the second fix commit)
50 paths, all 100644 100644 M — no addition, deletion, rename, mode or type change.
Non-recipe: tests/test_run_mode_sentence.py, tests/test_no_private_shared_names.py,
tools/new_recipe.py. Recipe: 47 notebook.ipynb and nothing else.
All 47 notebooks are markdown-only. I compared every one against eff86c7 on code-cell
sources, serialized outputs, notebook metadata, cell ids and order: not one code cell, output,
metadata key or cell id differs. 46 changed exactly one markdown cell (setup-md); recipe 36
changed two (setup-md and evaluation-md). 45 and 50 changed one each — their evaluation-md,
since they carry no paragraph. So the body's "Markdown only in all changed notebooks; nothing
printed changes" holds.
The three notebooks whose evaluation prose this PR rewrote make true claims. This mattered to
check, because they are new text:
- 36 — "In a scripted run each metric printed below,
validationandtestalike, is a check
that the pipeline works, and its line says so; the individual turns it lists are not metrics and
carry no label. Eachvalidationmetric also carries the selection-step label." I scanned every
committed output line: no metric line lacks the check label, and no validation metric line lacks
the selection label. - 45 — "each metric line below already being labelled a pipeline check" (was "every number
below already being labelled"). No metric line in 45 lacks the check label. - 50 — "both labels are printed on every validation metric line" (was "every validation
line"). No validation metric line in 50 lacks either label; the only unlabelled numeric lines my
scan flagged are ashow_answerblock (Score: 2.64 (confidence 0.64)) and per-example
specificity scorelines incomposition, which are individual answers and per-example lines,
not metrics.
The merge f60324a
git show --cc f60324a emits no combined-diff hunk at all, i.e. every file in the result
matches at least one parent — a genuinely clean merge with no evil-merge content. I checked the two
auto-merged docs read coherently from both sides:
docs/recipe-template.md: main's assets-manifest table row and its three objective-pin routes
(Foundation: derive objective-pin polarity and taint instead of enumerating spellings (#276) #305) are present, and the branch's three additions survive (thetests/test_build_fixtures.py
row namingdump_responses, the "A number the selection forces says so" bullet, the
"A confusion-matrix column is as wide as the widest label or the widest count" bullet).docs/fixtures.md: main's 193 added lines are present and the branch's twodump_responses
lines in the worked example survive.
Full scope at f60324a vs origin/main
159 paths: 156 M + 3 A, every mode 100644. No symlink (120000), submodule (160000),
mode change, type change, deletion or rename. Non-recipe paths are the same twelve as before;
recipe paths are 49 build_fixtures.py, 50 tests/test_build_fixtures.py, 47 notebook.ipynb,
1 tests/test_helpers.py. No stray file, no .claude/, no catalog, no workflow, no root file.
Closing keywords: Closes #209, Closes #233, Refs #217, Refs #222 — exactly the two intended
closes, nothing else anywhere in the body.
4. PR body against the diff, sentence by sentence
Everything I could measure, I measured. Reproduced exactly:
| body claim | my measurement |
|---|---|
51 recipe folders (01-50 and _template); 50 have tests/test_build_fixtures.py |
51 folders; only 36-card-game-action-selection lacks the file |
recipe 36 is the only one without it; scripted; fixtures/ holds only inputs.jsonl, labels.jsonl |
confirmed on all three counts (36's header prints Mode: offline, scripted backend, …) |
| recipe 32 is a replay recipe, has the file, and is migrated | confirmed — Mode: offline replay of synthetic fixtures, has fixtures/responses.json, its test file is in the diff |
_dump appeared in 100 recipe files on the base: 50 test files, 49 build_fixtures.py, recipe 50's tests/test_helpers.py |
50 + 49 + 1 = 100, measured on c3167129 |
at head _dump appears in no recipe file |
grep -rl _dump recipes/ → 0 |
_dump was only a comment in 47 of the 48 recipes; they still hand-serialize |
47 build_fixtures.py mention dump_responses only in a comment; 48 still contain json.dumps(dict(sorted(…))) |
only the template and recipe 50 call dump_responses |
exactly those two (the other 47 mentions are comments — a grep -l alone would mislead here, and the body is precise) |
recipe 32 hand-copies the serializer at build_fixtures.py:1488-1489 |
lines 1488-1489 of 1494 are exactly that json.dumps(...) + write_text(...) pair |
| 43 published notebooks carried the old sentence | git grep -l "carries that mode" on the base → 44 files = 43 + template |
| 983 selection-labelled output lines in the 45 carrying notebooks | 983 and 45, exactly |
296 of those lines name validation themselves |
296, exactly |
no line names test without validation |
confirmed, and strengthened: 0 of 398 print sites interpolate a test-split value |
| closed legacy list of six: 41, 43, 44, 45, 48, 50, which never had the old sentence | exactly those six lack the paragraph; git grep -L "carries that mode" on the base returns those six plus 36 |
| proved by scaffolding 51 (replay) and 52 (scripted) in a scratch copy | reproduced myself (§1, M2) |
mutation tests on _template and 47 strip or rename the labels and must raise |
reproduced, and extended to 02 and 36 (§1, M3) |
| validation lines in 06-10, 18, 37 have only the selection label; 39, 47, 49 print a metric line with no label | reproduced exactly last round and unchanged |
the template's evaluation-md now says "synthetic or scripted run" |
confirmed |
ruff check . passed; ruff format --check . 404 files |
All checks passed!; 404 files already formatted |
render_catalog --check exit 0; check_hygiene 614 files; check_recipe_assets passed; notebook_ci fixtures 51/0 |
exit 0; 614 file(s) scanned; 461 file(s) checked, passed; 51 recipe folders, 0 failed |
pytest: 6039 passed, 53 skipped, 50 deselected, 31 xfailed, 0 failed |
6039 passed, 53 skipped, 50 deselected, 31 xfailed, 0 failed (220 s) |
Honest disclosures I checked and agree with: the "Not fixed here (the markdown side of #326)"
paragraph, the #217 and #222 "Not done" paragraphs, and the two "Known gaps" bullets all match
the diff. The #326 paragraph is the right call: this PR rewrote the evaluation prose only of the
three notebooks whose wording was a variant of the retired sentence (36, 45, 50), and left the rest
to #326 rather than quietly widening the sweep.
One wording defect in the body — see S-b below.
5. Checks I ran
Fresh venv at /home/agent/venv-r2b (uv venv --python 3.14, CPython 3.14.7;
uv pip install -e ".[dev,ml,live]" -c <constraints-notebooks.txt> with UV_LINK_MODE=symlink),
ruff 0.17.1 matching pyproject.toml. No TYPESAFE_API_KEY, no JEV_COOKBOOK_LIVE, no live
call. Every heavy job under flock .claude/heavy.lock nice -n 10, taken once per job (no
nesting this time), pytest -p no:cacheprovider, long runs backgrounded with log polling.
| check | result |
|---|---|
ruff check . |
All checks passed! |
ruff format --check . |
404 files already formatted |
python tools/render_catalog.py --check |
exit 0 |
python tools/check_hygiene.py |
passed, 614 file(s) scanned, exit 0 |
python tools/check_recipe_assets.py |
passed, 461 file(s) under recipes/ checked, exit 0 |
python tools/notebook_ci.py fixtures |
51 recipe folders, 0 failed, exit 0 |
pytest -q |
6039 passed, 53 skipped, 50 deselected, 31 xfailed, 0 failed (220 s) |
Notebooks — re-executed at the MERGED head f60324a, not at 91e5f0b. This was the one real
risk left: the builder executed the notebooks at 91e5f0b and then merged main, which brought
src/jev_cookbook/evaluation.py (+199 lines), fixtures/__init__.py, vision.py and a new
frames.py into the tree the notebooks import. If any of that had changed a printed number the
committed outputs would be stale at the reviewed head, and the body's "Notebooks are markdown-only
edits … CI executes every notebook" would not have covered it. Each notebook was copied to
/home/agent/nbrun-r2b/<slug> (never executed in the tree), run with
PYTHONPATH=<worktree>/tools/netguard python tools/execute_notebook.py, then checked with
tools/check_notebook_fresh.py against the committed file and tools/check_hygiene.py on the
fresh copy.
6 of 6 OK, and git status --porcelain identical before and after:
| notebook | execute | check_notebook_fresh.py |
check_hygiene.py |
|---|---|---|---|
recipes/_template |
OK (2 figures, identical bytes) | matches a fresh offline run | passed |
36-card-game-action-selection (scripted) |
OK (3 figures) | matches | passed |
02-refund-intent-detection |
OK (3 figures) | matches | passed |
47-evidence-conflict-diagnosis |
OK (2 figures) | matches | passed |
45-memory-reconciliation |
OK (1 figure) | matches | passed |
50-model-comparison-and-calibration |
OK | matches | passed |
So main's evaluation.py/fixtures/vision.py/frames.py changes did not move any printed
number, and the committed outputs are fresh at the reviewed head — the one thing the body's
verification section did not itself establish. This covers the template, the only scripted recipe,
one M1 counterexample, the notebook my M3 mutation targeted, and both notebooks whose evaluation
prose this PR rewrote without giving them the paragraph.
Instrument probes of my own (§1): the AST classification of all 398 {selection} print sites;
the strip mutation on four notebooks; the 51/52 scaffold experiment; and a check that none of the
eight FORBIDDEN_WORDINGS appears anywhere outside a markdown cell (see S-c).
6. Acceptance criteria
#209 — promote jev_cookbook.live._dump to public API
| criterion | status |
|---|---|
| A public, documented helper exists with its own test | met — dump_responses, docs/live.md:166-173, two tests in tests/test_live.py |
tools/new_recipe.py generates the public import, and a scaffold test proves it |
met — tools/new_recipe.py:466, :748; proved by tests/test_new_recipe.py and `test_the_scaffolder_emits_no_private_shared_name[replay |
Every published recipe importing _dump is migrated, with the enumeration recorded here |
met — no _dump under recipes/ at head, and the body's enumeration now reproduces exactly (§4), including the recipe-32 correction |
The keep-or-remove decision for _dump is recorded, and if it stays, what it is for |
met — live.py:839-842, docs/live.md, the guard's docstring and the body now all say "transitional, for code outside recipes/, to be removed" |
| No recipe imports an underscore-prefixed shared name afterwards, enforced by a guard | met — tests/test_no_private_shared_names.py, which I probed on 11 positive and 4 negative spellings last round and is unchanged in behaviour |
#233 — the run-mode sentence
| criterion | status |
|---|---|
| The false sentence is replaced in the template | met |
| …and wherever it had already been copied | met — the 43 that carried it, plus 36, 45 and 50's own variants |
The replacement is true (CONTRIBUTING.md section 2 — the point of the issue) |
met — re-derived over all 398 {selection} print sites and 983 labelled lines; 0 counterexamples (M1) |
| Markdown only; invalidates no committed output, needs no re-execution | met — verified cell by cell for all 47, and confirmed by re-executing 6 at the merged head |
| Done "in the template first so the remaining recipes are generated correctly" | met — the scaffolder emits the paragraph in both modes, pinned by test_the_scaffolded_setup_cell_carries_the_paragraph (M2) |
| The paragraph's claims are pinned by a falsifiable test | met — verbatim text, carrier set, closed legacy list, retired wordings, label-set subset, and the label↔mode correspondence, each red under mutation (M3) |
#217 / #222 — refs only, correctly not closed
Unchanged and still honest: the template's matrix fix with its mutation proof, and the template's
forced-metric line plus scaffold prompt, with recipe 23, the per-recipe guard, the five published
recipes and gaps 1/2/4 all explicitly "Not done".
Suggestions (none blocking; I would merge as it stands)
- S-a.
test_the_legacy_list_is_closed's≤ 50guard has a three-digit hole.
tests/test_run_mode_sentence.pyassertsall(int(name[:2]) <= 50 for name in LEGACY_WITHOUT_THE_PARAGRAPH).CONTRIBUTING.mdsays recipe 100 carries a three-digit prefix,
andint("100-slug"[:2])is10, which passes. Use the slug's prefix up to the first-
(int(name.split("-")[0]) <= 50) so the list cannot be grown with a multimodal recipe. Low risk:
the list is a literal intests/, which a recipe pull request may not touch. - S-b. The body overstates one assertion. It says "the pipeline-check label appears only in a
run whose committed header says synthetic or scripted, and in such a run the metric lines carry
it". The assertion is existential —assert labelled_check_lines(lines), whose own message is
"prints the pipeline-check label on no output line" — and two sentences earlier the same
paragraph correctly disclaims the universal version ("It deliberately does not claim every metric
line carries a label"). Reword to "…and in such a run at least one labelled output line carries
it" so the body does not contradict itself. - S-c.
test_the_old_false_sentence_is_gone_everywherenarrowed from the raw file to markdown
cells only. Ateff86c7it readpath.read_text(); it now joins markdown sources. Harmless
today — I checked that none of the eightFORBIDDEN_WORDINGSappears in any code cell or
committed output of any notebook — but scanning the raw file is strictly stronger and costs
nothing, and a retired claim could in principle land in a code comment. - S-d. The
#326disclosure's counts are approximate and, by my patterns, low. The body says
"about 25 notebooks say inevaluation-mdthat everyvalidationnumber carries a different
label or both labels, and about 24 say everytestnumber says so". With a deliberately loose
pattern I count 29 and 34. The body hedges with "about" and routes the whole class to Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326, so
this is not a correctness problem — but Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326 should carry the measured list rather than an
estimate, since it is the issue that will have to finish the sweep. - S-e. Nothing in
docs/tells a builder the paragraph is required text. The mechanism is now
the scaffolder plus the test, which is what Foundation: the template tells readers every number carries the run mode, which is false for demo answer and per-example lines (32 notebooks) #233 asked for and is enough. A one-line row in
docs/recipe-template.md's "Choices the template makes for you" would still save the next
builder a confused moment if they ever rewritesetup-mdby hand.
Out of scope — reported, not fixed
docs/development.md:366says the pinned ruff isruff==0.16.10;pyproject.toml:24pins
ruff==0.17.1. Still stale onorigin/main, untouched here (I reported this last round too).- The markdown side of Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326 (S-d) and the six legacy notebooks that print the labels without
defining them (S4) are both real, both disclosed, and both belong to other issues.
Process note against myself
In my previous round I wrapped a notebook-run script that already took flock heavy.lock per
execution inside an outer flock, which self-deadlocked the shared lock until the coordinator
killed it. This round every heavy job takes the lock exactly once, nothing is nested, and long runs
were backgrounded with log polling. My venv (/home/agent/venv-r2b) and all scratch directories
are deleted; I removed nothing I did not create.
OPUS-REVIEW: APPROVE f60324a
…ey print The template's evaluation-md said every number there, test and validation alike, says it is a pipeline check; its own evaluation-wrong output lists two per-example confidences with no label. It now says each metric's line says so and that the listed examples are not metrics. The scaffolded evaluation-md stub said "In an offline run print each number with check beside it"; a recorded replay is also offline and prints no label, so it now says "In a synthetic or scripted run print each metric". Both wordings are pinned as retired (recipe 40, whose output is fully labelled, is a closed, stale-checked exemption). The legacy-list rank test now reads the number before the first hyphen, so a three-digit recipe cannot look legacy. Refs #233 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
PR #317 re-review — reviewer 1 of 2 (fresh Opus), round 3
VerdictAPPROVE. My single remaining blocker is resolved, and I verified the replacement line by line My round-2 blockers: statusM1 — the template's
|
| cell | lines | label |
|---|---|---|
evaluation-validation |
4 metric lines | all (a selection step…) and (a pipeline check…) |
evaluation-test |
7 metric lines | all (a pipeline check…) |
evaluation-matrix |
title, header, 4 count rows | all (a pipeline check…) |
evaluation-wrong |
heading (no number) + 2 per-example lines | no label |
So every clause holds: every metric line says so (17 of 17); the only unlabelled numeric lines in the
section are the two under Test examples the choice got wrong, and what Python did with them:,
which the sentence names (the quoted fragment is a prefix of the printed heading); and all four
validation metrics carry both labels. Unlabelled-numeric-line count in the section: 2, both
accounted for.
The retired wording is now banned: FORBIDDEN_WORDINGS gained
"every number here, \test` and `validation` alike". **It fires where it must** — I ran that entry over every notebook at both refs: at f60324ait matches_template**and**40-hierarchical-classification`; at this head it matches only recipe 40 (exempted). Running the
whole forbidden list over all 51 notebooks at this head, the only hit anywhere is recipe 40 on that
one wording, and with the exemption applied the test passes with nothing left over.
M2 — the scaffolder still said "In an offline run print each number". Resolved.
tools/new_recipe.py:241-248 now emits:
… In a synthetic or scripted run print each metric with
checkbeside it; an individual example
listed below a metric is not a metric and needs no label. …
That is true of what the scaffolded setup cell defines (offline = backend.mode in ("synthetic", "scripted"), so check is empty in a recorded replay, which is also offline). RETIRED_SCAFFOLD_WORDINGS
bans "In an offline run print each number" and "print each number with", and the new
test_the_scaffolded_evaluation_text_does_not_carry_a_retired_wording also runs the whole
FORBIDDEN_WORDINGS list against the stub and requires the positive phrase.
It fires where it must. I loaded the previous head's tools/new_recipe.py (from f60324a) and
ran this head's list against its stub: both retired wordings present and the positive phrase absent,
for --mode replay and --mode scripted; at this head, no retired wording and the positive phrase
present, in both modes.
Reviewer 2's int(name[:2]) note. Resolved.
legacy_rank / is_legacy_rank read the number before the first hyphen, test_the_legacy_list_is_closed
uses them, and test_a_three_digit_recipe_is_never_treated_as_legacy pins legacy_rank("100-…") == 100,
not is_legacy_rank("100-…"), not is_legacy_rank("51-new"), is_legacy_rank("50-…") — and, usefully,
documents the bug it replaces with assert int("100-multimodal-slug"[:2]) <= 50. That is a real
mutation record, not a restatement.
The recipe 40 judgement, checked myself
The coordinator asked me to decide this rather than accept it, so I measured it.
Is the retired sentence true in recipe 40? Its evaluation-md says "In an offline run every
number here, test and validation alike, is a check that the pipeline works, and says so." I
audited recipe 40's Evaluation section (evaluation-validation, evaluation-test,
evaluation-levels, evaluation-curve, evaluation-matrix, evaluation-queues,
evaluation-wrong, ending at measured-md): 0 unlabelled numeric lines. Recipe 40 labels even
its per-example lines (t15-wrong-topic-right-department: …, the four evaluation-wrong rows),
which is why it differs from the template. The only unlabelled lines in the section are three
non-numeric ones (a blank line and the two-line "The full-path trap:" prose, plus the
evaluation-wrong heading). So the claim is true of recipe 40's committed output, scoped by
"here" to that section. (Outside it, recipe 40 prints 9 unlabelled numeric lines in Setup / The
state / The questions / One answer up close, all before the sentence, so outside its scope.)
Is a closed, self-expiring exemption acceptable, or should recipe 40 be reworded here? The
exemption is the better choice and I would not ask for the rewording in this PR:
- the sentence is true there, so rewording buys no honesty, only conformity, and the Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326 sweep will
touch recipe 40'sevaluation-mdanyway (the body says so); - the exemption is one wording × one notebook, keyed on a wording that must itself be on the
forbidden list; test_a_retired_wording_exemption_is_still_neededmakes it genuinely self-expiring, and I proved
it falsifiable in both directions: pointing the exemption at01-sentiment-classification(which
does not say the wording) goes red with "01-sentiment-classification no longer says it; drop the
exemption", and naming a wording that is not onFORBIDDEN_WORDINGSgoes red too;- banning the wording globally is about the template, where it was false — not about the words being
inherently wrong, so an exemption for the one place it is true is the honest shape.
Two improvements I would make, neither blocking (below as S1 and S2).
Suggestions (not blocking)
- S1. Police the exemption on the claim, not on the wording.
test_a_retired_wording_exemption_is_still_needed
checks that recipe 40 still says the sentence; nothing checks that it is still true. Today
recipe 40's Evaluation section has 0 unlabelled numeric lines (I measured it); if a later edit
added one, the exemption would keep passing and the sentence would quietly become false. One
assertion closes it: for each exempted notebook, no output line between itsevaluation-mdand
the next##heading has a digit and neither label. The data the test already loads is enough. - S2.
RETIRED_BUT_TRUE_INhas no rank bound, unlikeLEGACY_WITHOUT_THE_PARAGRAPH.
test_the_legacy_list_is_closedbounds that list withis_legacy_rank, so a new recipe can never
be exempted from the paragraph. Nothing stops a future recipe 55 being added to
RETIRED_BUT_TRUE_INand passing. Applyingis_legacy_rankto its names too gives the new
exemption the same guarantee, andis_legacy_rankis right there. - S3. One loose phrase in the PR body. The new parenthetical says recipe 40 "uses the template's
retired sentence and every line it prints is labelled". What is true is narrower: every
numeric line in its Evaluation section is labelled — its Setup / state / questions / answer
sections print 9 unlabelled numeric lines, and the section itself has three unlabelled non-numeric
lines. The sentence's own scope ("here") is what makes the claim correct, so say that. - S4. Recipe 40 still says "In an offline run". A recorded replay is offline and prints no label,
which is exactly why this PR changed that phrase in the template and the scaffolder. Recipe 40 is
one of 30 published notebooks that still say it (the template and the scaffolder now say
neither), so it is squarely the class the body defers to Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326 and not a reason to touch recipe 40
here — but the exemption's comment justifies itself only on the committed output, and this is the
other half of the same sentence. Worth naming on Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326. - S5. Carried over, still open, immaterial.
tools/new_recipe.py:756("matches
jev_cookbook.live's writer") andsrc/jev_cookbook/live.py:834("what the recorder would
write") keep the looser wording thatdocs/live.mdalready states precisely;
tests/test_template_matrix_alignment.pyruns the cell withcheck = "";
tests/test_no_private_shared_names.py's docstring does not mention the plain-rebinding hole;
recipes 38 and 45'sevaluation-mdsentences and recipe 45's unlabelled confusion matrix belong on
Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326, which does not yet list them.
Everything that changed since f60324a, reviewed
The fix commit b89b689 (signed, Co-Authored-By: Claude Sonnet 5.5, Refs #233 — correctly
non-closing for a fix round) touches exactly three files: recipes/_template/notebook.ipynb,
tests/test_run_mode_sentence.py, tools/new_recipe.py.
- The only notebook change is the template's
evaluation-md, markdown source only. I parsed both
refs and compared every cell field and every top-level key: 1 notebook, 1 cell,sourceonly; no
output, metadata, id, execution count, cell count or cell order change. Zero problems. tests/test_run_mode_sentence.py(+73/−2): the new forbidden entry with a comment naming why;
RETIRED_BUT_TRUE_INand its staleness test;RETIRED_SCAFFOLD_WORDINGSand the scaffold-stub
test;legacy_rank/is_legacy_rankand the three-digit test. The exemption is applied inside
test_the_old_false_sentence_is_gone_everywherewith a per-wording lookup
(RETIRED_BUT_TRUE_IN.get(wording, ())), so it cannot leak to another wording.tools/new_recipe.py(+3/−2): the stub rewording only.RUN_MODE_PARAGRAPHis untouched, so the
paragraph the scaffolder emits still matchesPARAGRAPHbyte for byte (pinned by
test_the_scaffolded_setup_cell_carries_the_paragraph, which passed in round 2's scratch
scaffold of recipes 51 and 52).
The merge 98b79f5. git show --cc prints no combined hunks at all, so every file in the
result equals one parent — a clean merge with nothing hand-edited. It differs from parent 1 in
exactly three files (docs/backends.md, src/jev_cookbook/answers.py, tests/test_backends.py),
i.e. only origin/main's PR #332. That change is additive — DecisionResult.__contains__ and
__iter__ — and does not interact with this branch: dump_responses takes a replay-key map of
already-to_dict()-ed responses, and the scaffolded build_fixtures.py iterates
result.answers.values(), not the result itself. No recipes/ path comes in with the merge, so the
sweep is untouched.
PR body against the diff. The two edits are both corrections in the right direction and both
accurate: the retired-wordings sentence now discloses that the template's own evaluation-md and
the scaffolded stub were fixed this round and that recipe 40 sits on RETIRED_BUT_TRUE_IN with
rewording deferred to #326; and the label assertion is now described as "such a run prints the label
on at least one output line … it is an existence check, not a claim that every metric line carries
it", which replaces the round-2 wording I flagged as overclaiming and matches what the code does.
The #209 enumeration, which I re-derived from git ls-tree/git grep at the base in round 2, is
unchanged and still correct. The one loose phrase is S3.
Checks I ran
Fresh venv at /home/agent/r1c-venv, Python 3.14.7 via uv venv --python 3.14, deps via
uv pip install --link-mode symlink -e ".[dev,ml,live]" -c .github/constraints-notebooks.txt
(CI's install, exit 0). TYPESAFE_API_KEY and JEV_COOKBOOK_LIVE unset throughout. One
flock .claude/heavy.lock nice -n 10 wrapper, no nesting, output polled from a log;
pytest -p no:cacheprovider.
| command | result | body claims |
|---|---|---|
ruff check . |
All checks passed! (exit 0) | matches |
ruff format --check . |
404 files already formatted (exit 0) | matches |
python tools/render_catalog.py --check |
exit 0 | matches |
python tools/check_hygiene.py |
passed, 614 file(s) (exit 0) | matches |
python tools/check_recipe_assets.py |
passed, 461 file(s) under recipes/ (exit 0) | matches |
python tools/notebook_ci.py fixtures |
51 recipe folders, 0 failed (exit 0) | matches |
pytest |
6044 passed, 53 skipped, 50 deselected, 31 xfailed in 213 s, 0 failed (exit 0) | matches exactly |
gh pr checks 317 |
60 of 60 pass on this head | — |
Notebooks (offline, scratch copies, never in the tree). For each: rm -rf scratch,
cp -R recipes/<slug>, PYTHONPATH=<wt>/tools/netguard python tools/execute_notebook.py,
tools/check_notebook_fresh.py against the committed file, tools/check_hygiene.py on the fresh
copy, with a git status --porcelain snapshot before and after.
| recipe | why | result |
|---|---|---|
_template |
the file this round changed; the blocker's subject | OK (fresh + hygiene) |
40-hierarchical-classification |
the exempted recipe, whose output claim I had to verify | OK (fresh + hygiene) |
2 of 2 OK, and git status --porcelain was identical before and after, so nothing wrote into the
checkout. check_notebook_fresh.py compares sources, metadata, execution counts, ids and text
outputs exactly (images by content), so the template passing it confirms that this round's
markdown-only edit left its committed outputs — the 17 labelled metric lines and 2 unlabelled
per-example lines my audit rests on — reproducible byte for byte from a fresh offline run. As in the
earlier rounds, this machine gives the kernel no network namespace, so my offline guarantee rests on
tools/netguard and the replay backend; CI's 51 per-recipe jobs, which do use the namespace and a
separate unprivileged user, are green on this head.
Instruments run against a ref where they must find something (not just "the suite is green"):
| instrument | at f60324a |
at 98b79f5 |
|---|---|---|
FORBIDDEN_WORDINGS entry "every number here, \test` and `validation` alike"` |
matches _template and recipe 40 |
matches recipe 40 only (exempted) |
RETIRED_SCAFFOLD_WORDINGS + positive phrase, against the scaffolded evaluation-md |
both retired wordings present, positive phrase absent (both modes) | none present, positive phrase present (both modes) |
test_a_retired_wording_exemption_is_still_needed |
— | passes as committed; red when pointed at a notebook that does not say the wording, and red for a wording not on the forbidden list |
test_a_three_digit_recipe_is_never_treated_as_legacy |
— | passes, and records the replaced spelling's failure in its own assertion |
Earlier rounds' proofs still stand and are recorded in pr317-r1.md and pr317-r1b.md: the
private-name guard flagged the pre-migration content of four migrated files; the label checks go red
under strip/rename mutations on four notebooks outside their own parametrisation; scaffolding recipes
51 and 52 into a scratch tree took tests/test_run_mode_sentence.py from 54 to 56 passed with the
6-name skip set unchanged and no edit to the test.
Acceptance criteria
#209 — all five met (unchanged since round 2, where I re-derived the enumeration myself from
git ls-tree/git grep at the base and checked the body sentence by sentence).
#233 — met. The false setup-md sentence is gone from every notebook; the replacement is
verbatim in 45 of 51 with a closed, rank-bounded, stale-checked 6-name exemption; the scaffolder
emits it so new recipes inherit the true wording and pass the test unedited; the retired wordings —
now including the template's own and the scaffolder's — are banned by lists I proved fire at the refs
where they must; the label claims are falsifiable and I proved them red under mutation; and the
template's and the scaffolder's own evaluation sentences are now true of what they print. The one
notebook still carrying a retired wording carries it truthfully, on a tested exemption.
#217, template part only — met (unchanged); recipe 23 and the per-recipe guard remain open by
design.
#222 gap 3, template line only — met (unchanged); the five published recipes and gaps 1, 2
and 4 remain open by design.
Outside my task, reported not fixed
- Recipe 38's
evaluation-md("every number drawn from it below carries both …") is false for the two
unlabelled lines in itsevaluation-wrongoutput; recipe 45'sevaluation-md("each metric line
below already being labelled a pipeline check") is false for its entirely unlabelled confusion
matrix and itstest examples: 16line. Both are published-recipe prose in the class the body
defers to Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326, and Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326 lists neither — please add them, together with recipe 40's "In an
offline run" (S4) and recipe 36'sN:line. docs/fixtures.md's code blocks are still not executed by any test
(tests/test_docs_examples.pyexcludes the page deliberately), so this PR's edit to its worked
example has no automated cover; I executed it by hand in round 1 and it is correct.- 30 published notebooks say "In an offline run" where they mean a synthetic or scripted run. The
template and the scaffolder no longer do.
OPUS-REVIEW: APPROVE 98b79f5
Opus delta re-review (round 3) — PR #317 (
|
| changed claim | verdict |
|---|---|
| "such a run prints the label on at least one output line … it is an existence check, not a claim that every metric line carries it" | fixed — this was my suggestion (b); the body now matches the assertion (assert labelled_check_lines(lines), message "on no output line") and no longer contradicts its own disclaimer two sentences earlier |
"recipe 40 uses the template's retired sentence and every line it prints is labelled, so it is on a closed, self-expiring exemption list RETIRED_BUT_TRUE_IN and rewording it belongs with #326" |
accurate — I verified 0 unlabelled numeric lines in 40's evaluation section and that the exemption is self-expiring. Strictly, "every line it prints is labelled" is a shade broad (three pure-prose lines carry none); the claim in context is about numbers, and the numbers are all labelled |
"the template's own evaluation-md and the scaffolded evaluation stub were fixed in the last round" |
accurate — exactly the two files in the delta besides the test |
| "the list cannot grow, so a newly scaffolded recipe must carry the paragraph" | accurate, and now also true for a three-digit rank (§3) |
ruff check passed; ruff format --check 404 files |
reproduced — All checks passed!; 404 files already formatted |
render_catalog --check exit 0; check_hygiene 614 files; check_recipe_assets passed; notebook_ci fixtures 51/0 |
reproduced — exit 0; 614 file(s) scanned; 461 file(s) checked, passed; 51 recipe folders, 0 failed |
pytest: 6044 passed, 53 skipped, 50 deselected, 31 xfailed, 0 failed |
reproduced exactly — 6044 passed, 53 skipped, 50 deselected, 31 xfailed, 0 failed (214 s) |
Still accurate from the previous round and unchanged: the #209 enumeration, the 983/296/45 counts,
the closed legacy list of six, the "Not fixed here (the markdown side of #326)" disclosure, and the
#217/#222 "Not done" paragraphs.
One stale line, as last round: the verification bullet still reads "at 91e5f0b the template
and recipes 02, 15, 36, 45, 47 and 50 were executed offline". That is now two commits behind the
head. It is a body-freshness nit rather than a defect, and I closed the gap myself by executing at
the reviewed head (§6).
6. Checks I ran
Fresh venv at /home/agent/venv-r2c (uv venv --python 3.14, CPython 3.14.7,
uv pip install -e ".[dev,ml,live]" -c <constraints-notebooks.txt>, UV_LINK_MODE=symlink), ruff
0.17.1. No TYPESAFE_API_KEY, no JEV_COOKBOOK_LIVE, no live call. Every heavy job took
flock .claude/heavy.lock nice -n 10 exactly once, never nested; long runs backgrounded with
log polling.
| check | result |
|---|---|
ruff check . |
All checks passed! |
ruff format --check . |
404 files already formatted |
python tools/render_catalog.py --check |
exit 0 |
python tools/check_hygiene.py |
passed, 614 file(s) scanned, exit 0 |
python tools/check_recipe_assets.py |
passed, 461 file(s) under recipes/ checked, exit 0 |
python tools/notebook_ci.py fixtures |
51 recipe folders, 0 failed, exit 0 |
pytest -q -p no:cacheprovider |
6044 passed, 53 skipped, 50 deselected, 31 xfailed, 0 failed (214 s) |
Every number matches the PR body.
Notebooks re-executed at the reviewed head 98b79f5 (copied to /home/agent/nbrun-r2c/<slug>,
never run in the tree; PYTHONPATH=<worktree>/tools/netguard python tools/execute_notebook.py, then
tools/check_notebook_fresh.py against the committed file and tools/check_hygiene.py on the fresh
copy):
| notebook | execute | check_notebook_fresh.py |
check_hygiene.py |
|---|---|---|---|
recipes/_template |
OK (2 figures, identical bytes) | matches a fresh offline run | passed |
40-hierarchical-classification |
OK (3 figures, identical bytes) | matches a fresh offline run | passed |
2 of 2 OK, git status --porcelain identical before and after. So #332's answers.py change
moved no printed number, and both notebooks are fresh at the reviewed head — the template because
this delta edited it, recipe 40 because its exemption rests on exactly what it prints.
7. Suggestion (one line, not blocking)
RETIRED_BUT_TRUE_IN is self-expiring but, unlike LEGACY_WITHOUT_THE_PARAGRAPH, carries no rank
constraint, so nothing in the test stops a future recipe being added to it. Adding
assert all(is_legacy_rank(n) for names in RETIRED_BUT_TRUE_IN.values() for n in names) would make
it closed in the same sense as the legacy list. Low risk either way: the dict is a literal in
tests/, which a recipe pull request may not touch.
8. Out of scope — reported, not fixed
docs/development.md:366still says the pinned ruff isruff==0.16.10against
pyproject.toml:24'sruff==0.17.1. Pre-existing onorigin/main, untouched here; reported in
both my previous reviews.- The markdown side of Recipes: metric lines missing their run-mode or selection label (39, 47, 49; validation lines in 06–10, 18, 37) #326 (including recipe 40's "In an offline run" opener, §2) and the six
legacy notebooks that print the labels without defining them remain open and disclosed.
My venv (/home/agent/venv-r2c) and all scratch directories are deleted; I removed nothing I did
not create.
OPUS-REVIEW: APPROVE 98b79f5
Closes #209
Closes #233
Refs #217
Refs #222
What this adds
Foundation PR C (template and scaffolder). It changes text every later recipe copies.
#209 (public
dump_responses).jev_cookbook.dump_responses(data)(defined injev_cookbook/live.py, exported from the package, documented in its docstring anddocs/live.md) returns the exact text aresponses.jsonis stored as. It returns text and writes nothing. The recorder calls it, and so does a scaffoldedbuild_fixtures.py(the template and scaffolder no longer hand-copy the serializer). The scaffolder, the template,docs/recipe-template.mdand thedocs/fixtures.mdworked example use it._dumpis kept only as a transitional alias.tests/test_recipe_guard.pyG1(a) requires every recipe'stests/test_build_fixtures.pyto be byte-identical to the scaffold, so changing the scaffold forced the migration. Enumeration on the base (git ls-treeandgit grepat c316712): 51 recipe folders (recipes 01-50 and_template), 50 of them havetests/test_build_fixtures.py; the only folder without it is recipe 36 (scripted; itsfixtures/holds onlyinputs.jsonlandlabels.jsonl). Recipe 32 is a replay recipe, has the file, and is migrated._dumpappeared in 100 recipe files on the base: 50tests/test_build_fixtures.py, 49build_fixtures.pyand recipe 50'stests/test_helpers.py; at head it appears in none. In thebuild_fixtures.pyfiles_dumpwas only a comment in 47 of the 48 recipes (they still hand-serialize withjson.dumps(dict(sorted(...))); recipe 32 hand-copies the serializer atbuild_fixtures.py:1488-1489); only the template and recipe 50 calldump_responses. Each regenerated test file was produced bybuild_fixtures_test_text(recipe); normalised for the name, the only change is the dump name and re-wrapped docstring lines.recipes/(its own test) and is to be removed once nothing outsidelive.py's tests imports it.tests/test_no_private_shared_names.pyfails if any recipe.py, notebook code cell, or scaffolder output (replay and scripted) imports a private module (jev_cookbook._canonical,jev_cookbook.fixtures._scan, byfrom/import/as), imports an underscore name, or reaches a private attribute through a local name bound to the package, a module or a subpackage. Dunders (__version__) are not flagged. The module set comes from*.pyfiles and package directories. Each spelling is a parametrised case. Syntactic only (nogetattr/importlib).#233 (run-mode paragraph). "every number below carries that mode with it" is replaced, in the template, the scaffolder (
tools/new_recipe.py) and the 43 published notebooks that carried it, by a paragraph that makes only claims a script can check: two labels may follow a line;(a pipeline check, not a Jev result)marks a synthetic or scripted run (a recorded or live run does not print it, and its header says which mode ran);(a selection step, not a reported result)marks a line that is not a result to report, such as a number fromvalidation, the split settings are chosen on, or a count of the items the frozen rule queues or would queue; a line with neither label (an individual answer, a per-example line) still comes from the mode the header names. The wording does not say which split a labelled line is on: a script classified all 983 selection-labelled output lines in the 45 carrying notebooks (296 namevalidationthemselves, 15 are queue/preview counts, 76 report a setting the selection chose, the rest sit in validation-only cells), and no line namestestwithoutvalidation. It deliberately does not claim every metric line carries a label, because that is false of the published set (validation lines in 06-10, 18, 37 have only the selection label; 39, 47 and 49 print a metric line with no label; the rest is #326).tests/test_run_mode_sentence.pypins it: the paragraph verbatim in every notebook outside a closed legacy list of six (41, 43, 44, 45, 48, 50, which never had the old sentence; the list cannot grow, so a newly scaffolded recipe must carry the paragraph, which is proved by scaffolding recipes 51 (replay) and 52 (scripted) in a scratch copy); a list of retired wordings that no notebook may contain (recipe 36, 45 and 50 each carried an unscoped variant of the old claim and were fixed with the same sweep; the template's ownevaluation-mdand the scaffolded evaluation stub were fixed in the last round; recipe 40 uses the template's retired sentence and every line it prints is labelled, so it is on a closed, self-expiring exemption listRETIRED_BUT_TRUE_INand rewording it belongs with #326); the only(a ..., not a ...)labels in any carrying notebook's committed output are the two named; the pipeline-check label appears only in a run whose committed header says synthetic or scripted, and such a run prints the label on at least one output line (checked on parenthetical labels on output lines, so the fixed provenance prose cannot satisfy it; it is an existence check, not a claim that every metric line carries it; mutation tests on_templateand recipe 47 strip or rename the labels and must raise). The template'sevaluation-md("In an offline run ...") now says "synthetic or scripted run". Markdown only in all changed notebooks; nothing printed changes.Not fixed here (the markdown side of #326): about 25 notebooks say in
evaluation-mdthat everyvalidationnumber carries a different label or both labels, and about 24 say everytestnumber says so; both are false wherever #326's unlabelled validation lines or N/count lines exist.#217, template part only. The
evaluation-matrixcell sizes columns asmax(width, len(str(matrix.matrix.max())))for header and counts.tests/test_template_matrix_alignment.pyruns the template cell's real source against the realconfusion_matrixon counts wider than every label, and proves it can fail by mutating the cell back to label-width sizing. Template outputs are unchanged. Not done: converging recipe 23 and the guard (the issue stays open).#222 gap 3, template line only. The template's
evaluation-mdsays "right among those routed automatically" is all of them onvalidationby construction, that it describes the selection and not the rule, and tells a copying recipe to replace the sentence with what its own selection forces;docs/recipe-template.mdsays replace, not copy; the scaffold's evaluation TODO prompts for it (pinned intests/test_run_mode_sentence.py). Also in the same template cell: the comment claimingroutehas "exactly one review reason" (it has three) is corrected. Not done: applying it to the five published recipes, gaps 1, 2, 4 (the issue stays open).Recipe checklist
Foundation work; recipe checklist skipped.
How it was verified
Final head 98b79f5 (signed merge of origin/main 39ddfce into b89b689, after three fix rounds), Linux, Python 3.14 venv (
pip install -e ".[dev,ml,live]" -c .github/constraints-notebooks.txt):ruff check .: All checks passed.ruff format --check .: 404 files already formatted.pytest: 6044 passed, 53 skipped, 50 deselected, 31 xfailed, 0 failed.python tools/render_catalog.py --check: exit 0.python tools/check_hygiene.py: passed (614 files).python tools/check_recipe_assets.py: passed.python tools/notebook_ci.py fixtures: 51 recipe folders, 0 failed.tools/netguardin a scratch copy, thencheck_notebook_fresh.pyagainst the committed file andcheck_hygiene.pyon the fresh copy: all OK. CI executes every notebook.Known gaps and notes
tests/test_build_fixtures.pycannot pass for an image-backed recipe) are Foundation: the multimodal docs are unreachable and the scaffold has no image path, so the first visual recipe starts from a text-only template #315, not this PR.tests/test_build_fixtures.pycompares the committed file with the synthetic generator's output, so it stops holding once a recipe merges a recorded answer (pre-existing; noted indocs/live.md).🤖 Generated with Claude Code