Skip to content

Foundation C: public dump_responses, honest run-mode sentence, matrix and forced-metric template fixes - #317

Merged
CompleteDotTech merged 11 commits into
mainfrom
foundation/template-scaffolder
Oct 21, 2026
Merged

CompleteDotTech merged 11 commits into
mainfrom
foundation/template-scaffolder

Conversation

@CompleteDotTech

@CompleteDotTech CompleteDotTech commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

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 in jev_cookbook/live.py, exported from the package, documented in its docstring and docs/live.md) returns the exact text a responses.json is stored as. It returns text and writes nothing. The recorder calls it, and so does a scaffolded build_fixtures.py (the template and scaffolder no longer hand-copy the serializer). The scaffolder, the template, docs/recipe-template.md and the docs/fixtures.md worked example use it.

  • Migration decision: published recipes' test files are migrated, and _dump is kept only as a transitional alias. tests/test_recipe_guard.py G1(a) requires every recipe's tests/test_build_fixtures.py to be byte-identical to the scaffold, so changing the scaffold forced the migration. Enumeration on the base (git ls-tree and git grep at c316712): 51 recipe folders (recipes 01-50 and _template), 50 of them have tests/test_build_fixtures.py; the only folder without it is recipe 36 (scripted; its fixtures/ holds only inputs.jsonl and labels.jsonl). Recipe 32 is a replay recipe, has the file, and is migrated. _dump appeared in 100 recipe files on the base: 50 tests/test_build_fixtures.py, 49 build_fixtures.py and recipe 50's tests/test_helpers.py; at head it appears in none. In the build_fixtures.py files _dump was only a comment in 47 of the 48 recipes (they still hand-serialize with json.dumps(dict(sorted(...))); recipe 32 hand-copies the serializer at build_fixtures.py:1488-1489); only the template and recipe 50 call dump_responses. Each regenerated test file was produced by build_fixtures_test_text(recipe); normalised for the name, the only change is the dump name and re-wrapped docstring lines.
  • The alias reason, corrected: no recipe can use it (the guard fails one that does, and a pre-rename branch runs against its own old base until rebased, then fails the guard). It is kept only for code outside recipes/ (its own test) and is to be removed once nothing outside live.py's tests imports it.
  • Guard: tests/test_no_private_shared_names.py fails if any recipe .py, notebook code cell, or scaffolder output (replay and scripted) imports a private module (jev_cookbook._canonical, jev_cookbook.fixtures._scan, by from/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 *.py files and package directories. Each spelling is a parametrised case. Syntactic only (no getattr/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 from validation, 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 name validation themselves, 15 are queue/preview counts, 76 report a setting the selection chose, the rest sit in validation-only cells), and no line names test without validation. 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.py pins 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 own evaluation-md and 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 list RETIRED_BUT_TRUE_IN and 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 _template and recipe 47 strip or rename the labels and must raise). The template's evaluation-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-md that every validation number carries a different label or both labels, and about 24 say every test number says so; both are false wherever #326's unlabelled validation lines or N/count lines exist.

#217, template part only. The evaluation-matrix cell sizes columns as max(width, len(str(matrix.matrix.max()))) for header and counts. tests/test_template_matrix_alignment.py runs the template cell's real source against the real confusion_matrix on 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-md says "right among those routed automatically" is all of them on validation by 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.md says replace, not copy; the scaffold's evaluation TODO prompts for it (pinned in tests/test_run_mode_sentence.py). Also in the same template cell: the comment claiming route has "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.
  • Notebooks are markdown-only edits; at 91e5f0b the template and recipes 02, 15, 36, 45, 47 and 50 were executed offline under tools/netguard in a scratch copy, then check_notebook_fresh.py against the committed file and check_hygiene.py on the fresh copy: all OK. CI executes every notebook.

Known gaps and notes

🤖 Generated with Claude Code

CompleteDotTech and others added 3 commits October 20, 2026 17:16
…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>
@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

Opus reviewer (independent review 1 of 2): Claude Opus 5.5 (claude-opus-5-5)

Reviewed head 7a5299fa3b9cba784dd66f9e7b73e3163090b925 against base 9a7f152202f13a1b1e4fc004d978390fc7c3232f (current origin/main; the merge base is the same commit). I read #209, #233, #217 and #222 in full with their comments, plus CONTRIBUTING.md and docs/development.md. The repository has no AGENTS.md. I did not read the other review.

Verdict in one paragraph

The dump_responses work for #209 is correct and the migration is exactly what the description says. The template matrix fix for #217 is correct and tested by mutation. The template line for #222 gap 3 is true. Every touched notebook still runs offline and matches a fresh run. The problem is the deliverable for #233. Its whole purpose is to replace a false sentence about labels with a true one, and the replacement is still false. Two of its three claims fail in published notebooks, and the word "instead" contradicts the template's own output and its own evaluation prose. There is also a cheap gap in the new private-name guard. I would not want the remaining recipes to copy this sentence as it stands.

Blocking findings, most severe first

B1. "per-example lines ... carry no label of their own" is false in at least 15 of the 43 published notebooks that now carry it

The sentence added to setup-md (in recipe 05 the cell is f33940ac, in recipe 20 it is setup-intro) says: "The individual answers and per-example lines shown along the way are not metrics and carry no label of their own; they come from the mode the header names." I scanned the committed outputs of every notebook that carries the sentence. Per-example lines that do carry the pipeline-check label (and sometimes the selection-step label as well) appear in recipes 14, 15, 17, 24, 28, 31, 32, 33, 34, 35, 39, 40, 42, 47 and 49. Some examples, quoted from committed outputs:

  • recipes/34-evidence-relationship-labels, cell python-threshold: v01: works_for -> accepted (composed from its evidence) (a pipeline check, not a Jev result)
  • recipes/14-source-span-selection, cell evaluation-code-test: t07 (D17): chose s2, sent to review (confidence below the threshold); the raw choice was right (a pipeline check, not a Jev result)
  • recipes/42-outbound-disclosure-screening, cell python-rule: pricing: noul 0.92 -> violates (a pipeline check, not a Jev result)
  • recipes/49-multi-agent-task-assignment, cell headline-code: validation s1-api-schema: assigned -> nova, gold-best nova (a pipeline check, not a Jev result)
  • recipes/24-incident-priority-composition, cell python-threshold: v18-wrong: major/low -> withheld (review: confidence below the threshold) (a pipeline check, not a Jev result)

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 validation number is labelled a selection step instead" contradicts the template and three recipes' own prose, and is false in recipes 39 and 49

There are two separate problems with this clause.

  1. "instead". Read naturally, it says a validation number carries the selection-step label in place of the pipeline-check label. In the template's own committed output every validation metric line carries both labels, for example right among those routed automatically: 6 of 6 (a selection step, not a reported result) (a pipeline check, not a Jev result). The template's evaluation-md says so too: "every number here, test and validation alike, is a check ... every validation number also carries ...". Recipe 16's evaluation prose says "which is why it carries both labels", and recipe 39's says "a validation number is additionally a selection step". So this PR adds a sentence to those notebooks that disagrees with a later cell in the same notebook. The description says "instead" was chosen to fit recipes whose validation lines carry only the selection label (06, 07, 08, 09, 10, 18, 37). Dropping the word is true for both kinds of recipe, because "is labelled a selection step" holds whether or not the check label is there as well.

  2. Validation metric lines with no selection label. These lines print a validation metric with only the check label:

    • recipes/39-document-extraction-cascade, cell evaluation-t86: validation fields at business threshold 0.86: coverage 0.7308, accuracy 1.0000, risk 0.0000 (a pipeline check, not a Jev result)
    • recipes/49-multi-agent-task-assignment, cell baseline-sweep-code: best validation score: 3/6, reached by 16 of 98 words (a pipeline check, not a Jev result) and validation: 3/6, 0 requests (a pipeline check, not a Jev result)

    These are pre-existing labelling defects, and they break CONTRIBUTING.md section 2 on their own terms. Even so, this PR puts a sentence directly above them that says they are labelled, and the description says the final wording was checked against every notebook.

What to change: drop "instead" (for example "... and a validation number is also labelled a selection step"). Then either add {selection} to those lines in 39 and 49 and re-execute both notebooks, or keep the sentence out of those two notebooks and file the labelling defect. The first option is better.

B3. The private-name guard misses private modules, which is the most direct way to import a private shared name

tests/test_no_private_shared_names.py checks only whether the imported name starts with an underscore. It never checks the module path, and SUBMODULES globs only *.py, so the jev_cookbook.fixtures package is not treated as a module root. I added each line below to a scratch copy of recipes/01-sentiment-classification/helpers.py and ran test_no_published_recipe_or_the_template_uses_a_private_shared_name against it:

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 Import or ImportFrom whose dotted module path has a segment starting with _.
  • Build SUBMODULES from package directories as well as *.py files.
  • 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 offline variable, though, is backend.mode in ("synthetic", "scripted"), so in a recorded replay no test line 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's evaluation-md already 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-diagnosis cell baselines prints the 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 reported test metric line without a label.
  • S3. The scaffolded build_fixtures.py still restates the serialization by hand. recipes/_template/build_fixtures.py:173-178 writes json.dumps(dict(sorted(...)), indent=2, ensure_ascii=False) + "\n" with a comment saying it matches dump_responses. Now that the helper is public, the scaffold could call dump_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-validation comment says route has "exactly one review reason". route has three, and the template's own test output 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.py runs the cell with check = "". 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 last n_labels tokens before it) would test what is actually printed.

What is right, verified independently

  • Notebook edits are exactly as claimed. I parsed every recipes/*/notebook.ipynb at 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 source differs (setup-md; f33940ac in 05; setup-intro in 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-md and evaluation-matrix changed. evaluation-matrix changed 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.
  • 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.py files 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.py comment lines changed.
    • Recipe 50 changes its import and two calls in build_fixtures.py, and its import, docstring and assert in tests/test_helpers.py.
    • _dump is the same object as dump_responses, pinned in tests/test_live.py. The docstring, __all__ in both modules and docs/live.md all document it.
  • Diff modes. git diff --raw -M origin/main...HEAD shows only 100644 entries. The two new test files are 000000 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_width correctly stays on label width. The test runs the cell's real source, and its mutation back to column = width is 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_threshold returns the lowest threshold whose answered subset has accuracy at least 1.0, and raises if none does. The answered subset is named (queue answers only), and route accepts confidence >= min_confidence. So "right among those routed automatically" on validation is forced, and the committed 6 of 6 agrees.
    • It is the only forced validation line in the template; the "answers that name a queue", accuracy and "routed automatically" lines are not forced.
    • The docs/recipe-template.md instruction to replace the sentence rather than copy it, and the scaffold TODO, are consistent with it.
  • 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 are tests/test_notebook_ci_sandbox.py::test_kills_every_non_zombie_process_and_returns_once_none_remain and ::test_a_process_that_will_not_die_fails_closed. Both fail only on Windows (signal.SIGKILL missing, 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>, then check_notebook_fresh.py against the committed file, then check_hygiene.py on the fresh copy. 51 of 51 OK, and git status --porcelain is 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

#209

  • Public, documented helper with its own test: met.
  • tools/new_recipe.py generates the public import, and a scaffold test proves it: met (test_the_scaffolder_emits_no_private_shared_name, both modes).
  • Every published recipe importing _dump migrated, 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.md and docs/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).

#233

  • 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.py and 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

@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

Opus reviewer (independent review 2 of 2). Model: Claude Opus 5.5 (claude-opus-5-5).
Reviewed head 7a5299fa3b9cba784dd66f9e7b73e3163090b925 against base 9a7f152202f13a1b1e4fc004d978390fc7c3232f. I did not read the other review before writing this.

As well as reading the diff, I scaffolded three throwaway recipes into a scratch folder with this head's tools/new_recipe.py and read them as a new builder would: 51 (replay), 58 (--mode scripted) and 61 (replay; the first multimodal rank). I then filled 61 with a minimal image-backed helpers.build_state (image_asset(fields["image"], root=HERE)), two 300x300 PNGs under assets/ and four ROWS, and ran its scaffolded tests.

Blocking

1. The replacement #233 sentence is false in a new way, in the template and all 44 notebooks that carry it

recipes/_template/notebook.ipynb:33 (setup-md), copied into 43 published notebooks:

In an offline run every reported metric line below (a test number) is labelled a pipeline check, not a Jev result, and a validation number is labelled a selection step instead.

"Instead" tells the reader that an offline validation number gets the selection label in place of the pipeline-check label. Every notebook that carries the sentence prints the opposite. The template's own committed output a few cells further down reads:

right among those routed automatically: 6 of 6 (a selection step, not a reported result) (a pipeline check, not a Jev result)

The template's setup cell comment says so outright ("A synthetic or scripted run prints both labels on a validation number"). So does the template's own evaluation-md: "In an offline run every number here, test and validation alike, is a check that the pipeline works, and says so". I checked the whole set: all 44 notebooks that contain "is labelled a selection step instead" also contain the double-labelled ... (a selection step, not a reported result) (a pipeline check, not a Jev result) output. Not one of them supports "instead". The parenthetical "(a test number)" also reads as though test lines are the only metric lines. In fact validation lines, the N: line in measured and the answers that name a queue line are labelled too.

#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:

In an offline run every reported metric line below is labelled a pipeline check, not a Jev result; a validation number also carries a second label saying it is a selection step, not a reported result. The individual answers and per-example lines shown along the way are not metrics and carry no label of their own; they come from the mode the header names.

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 setup-md does not contain "instead" (or matches the sentence exactly) and that every notebook carrying the sentence prints the double label.

2. The private-name guard misses private modules, and #209's last criterion is about names, not only attributes

tests/test_no_private_shared_names.py:32,41-52. #209's criterion is "No recipe imports an underscore-prefixed shared name afterwards". The package has two private modules, jev_cookbook._canonical and jev_cookbook.fixtures._scan/_schema. I ran private_shared_uses on each spelling:

source flagged?
from jev_cookbook._canonical import canonical_json no ([])
import jev_cookbook._canonical as c + c.x no
import jev_cookbook._canonical (no attribute use) no
from jev_cookbook.fixtures._scan import scan no
import jev_cookbook.fixtures._schema as s no
from jev_cookbook import fixtures + fixtures._scan no: SUBMODULES globs only src/jev_cookbook/*.py, so the fixtures subpackage never becomes a root
from jev_cookbook import _canonical yes
from jev_cookbook.vision import _MAX_IMAGE_BYTES yes
import jev_cookbook + jev_cookbook.__version__ yes. This is a false positive: a dunder is not private

The ImportFrom check looks only at the leading character of the imported name (a.name[0] == "_", line 52). It never looks at any segment of node.module, and the ast.Import branch (line 41) only records aliases and never reports anything. Two of these holes are likely to be hit for real. from jev_cookbook.fixtures import … is already the import every recipe uses, so fixtures._schema is one autocomplete away. And _canonical is where the canonical-JSON helpers a multimodal or replay-key-curious builder would reach for live.

The fix is small:

  • flag any Import/ImportFrom whose dotted module path, or imported name, has a segment after jev_cookbook that starts with _ but is not a dunder;
  • build SUBMODULES from *.py stems and package directories (fixtures);
  • exclude __dunder__ attributes;
  • add the rows above to test_the_detector_flags_what_it_claims_to, so the mutation proof covers them.

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 dump_responses the writer, not just the checker, in the template and the scaffold. recipes/_template/build_fixtures.py:177 and tools/new_recipe.py:573 still hand-copy json.dumps(dict(sorted(responses.items())), indent=2, ensure_ascii=False) + "\n", with a comment saying it is "the same serialization jev_cookbook.dump_responses writes". So the first thing every scaffolded replay recipe from 51 onward does is duplicate a public shared function, which is what rule 8 discourages, and then keep a test to prove the copy still matches. Recipe 50 already calls responses_file.write_text(dump_responses(responses), encoding="utf-8", newline="\n"), which is the better precedent. Changing the template and scaffold line (not the published recipes, which G1(a) does not pin for build_fixtures.py) gives step 5 of #209 the cleaner answer: one shared serializer that both the writer and the test use. The scaffolded test still keeps its value, because it is what proves the committed responses.json is fresh against ROWS. Once the duplicate is gone, its sort-keys paragraph is the only part left that would need rewording.

S2. docs/live.md:166-170 overstates what the scaffolded test checks. It says dump_responses is "the public way for a recipe to ask 'is my committed responses.json what the recorder would write?' (the scaffolded tests/test_build_fixtures.py asserts it)". The scaffolded test actually compares the committed file with dump_responses(build_responses(ROWS)), which is the synthetic generator's output. I measured this on scaffold 61: I set one entry's model to jev-1.13.0 and rewrote the file with dump_responses itself, so the result was exactly what the recorder would write. test_the_committed_responses_are_byte_identical_to_the_recorders_writer then failed. So once a recipe merges one recorded answer, that test fails by construction, and G1(a) (byte-identical to the scaffold) forbids adapting it. That part predates this PR. The new doc sentence is this PR's, though, and should say what the test checks: "the committed file is what build_fixtures.py would write from ROWS, serialized exactly as the recorder serializes". The recorded-fixture case is worth its own issue before the first recorded recipe. The module docstring the scaffold emits (tools/new_recipe.py:742, "matches jev_cookbook.live's writer exactly") should also name jev_cookbook.dump_responses now.

S3. Multimodal: the scaffolded tests/test_build_fixtures.py cannot pass for an image-backed recipe, and G1(a) stops a builder fixing it. On scaffold 61, with build_state reading assets/*.png through image_asset(..., root=HERE), the result was 1 passed, 2 failed. test_build_fixtures_separates_inputs_labels_from_responses and test_the_generator_reproduces_the_committed_fixtures both fail with FileNotFoundError: ...\61-screenshot-error-state-recognition\assets\a.png. The cause is that the scaffold copies only ("helpers.py", "build_fixtures.py") into tmp_path (tools/new_recipe.py:795, :835), and every recipe from 61 to 100 will read assets relative to its folder. This predates the PR and #315 covers the multimodal scaffold. But #315's criteria do not name this file, and because the file is byte-pinned, each of the 40 builders would hit a test they are not allowed to edit. I recommend adding it to #315 explicitly: copy assets/ when present, or have the scaffold copy the recipe folder minus fixtures/ and tests/. dump_responses itself is fine for multimodal use. Responses stay plain JSON (the image bytes go into the replay key, not the response), and my 61 scaffold's responses.json round-tripped through it byte for byte.

S4. The _dump alias rationale does not hold up. live.py:843 keeps _dump "so a branch scaffolded before the rename ... must keep running". But the new guard fails such a branch as soon as it is rebased, and before the rebase it runs against old main, which has _dump anyway. So no recipe can use the alias, and the only remaining consumer is tests/test_live.py. Keeping it is harmless and the decision is recorded, as #209 asks. I would still call it a transitional alias to be removed after the open recipe branches have rebased, rather than "supported, not deprecated", which invites new use.

S5. docs/fixtures.md:375's worked example still writes responses.json with json.dumps(responses, indent=2) + "\n". That leaves the top level unsorted, escapes non-ASCII and has no newline="\n". The next paragraph then says the recorder "writes the same format". With a public serializer available, the example should call dump_responses. This predates the PR, but it is the page a builder reads first.

S6. The template's evaluation-validation comment says route has "exactly one review reason", but it has three. You already noted this, and it is pre-existing. A one-word fix would be worthwhile while the template is open, because recipes copy that comment too.

Things I checked and found sound:

  • The evaluation-matrix fix (column = max(width, len(str(matrix.matrix.max())))). tests/test_template_matrix_alignment.py runs the real cell source against the real confusion_matrix, and its mutation back to column = width really does go red on the wide-count case.
  • The Foundation: four set-wide gaps from the recipe 25 review (queue attribution, threshold binding, forced-metric disclosure, baseline template line) #222 gap 3 passage is correct for the template. Its selection is over queue-naming answers with target_accuracy=1.0, and route auto-routes only queue-naming answers at or above the threshold, so 6 of 6 is forced. select_confidence_threshold raises rather than returning when no threshold meets the target, so the "by construction" claim cannot silently fail.
  • The docs/recipe-template.md bullet telling recipes to replace the sentence rather than copy it, and the scaffold's evaluation TODO.
  • The migration. Every recipes/*.py change is the _dump → dump_responses rename, plus the 48 comment lines and recipe 50's two calls. Cell-by-cell parsed-JSON comparison shows the only notebook change is the setup markdown cell (setup-md in 41, f33940ac in 05, setup-intro in 20), plus evaluation-md/evaluation-matrix in the template.

Checks run (fresh Python 3.14.3 venv, pip install -e ".[dev,ml,live]" -c .github/constraints-notebooks.txt, Windows, no TYPESAFE_API_KEY)

  • git diff --raw -M origin/main...HEAD: 151 × 100644 100644 M and 2 × 000000 100644 A. No symlink, submodule or mode change.
  • ruff check .: All checks passed. ruff format --check .: 397 files already formatted.
  • pytest: 5001 passed, 56 skipped, 50 deselected, 31 xfailed, 2 failed. The 2 failures are the known Windows-only signal.SIGKILL cases in tests/test_notebook_ci_sandbox.py (Foundation: two notebook-CI sandbox tests use signal.SIGKILL and fail on Windows #302). This PR does not touch that code, so they are not findings.
  • 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.
  • All 50 recipes and the template were executed offline from a scratch copy with tools/execute_notebook.py under tools/netguard (no network namespace on Windows), followed by check_notebook_fresh.py against the committed file and check_hygiene.py on the fresh copy: 51 of 51 OK. The checkout was unchanged afterwards.
  • Probes of the guard's detector (the table above) and the scratch scaffolds for 51, 58 and 61 (S2, S3), as described.
  • gh pr checks 317: 60 of 60 pass.

Acceptance criteria

#209

  • Public, documented helper with its own test: met (dump_responses, docs/live.md, two tests in tests/test_live.py).
  • tools/new_recipe.py emits the public import and a scaffold test proves it: met (test_the_scaffolder_emits_no_private_shared_name[replay|scripted], and the emitted test file imports from jev_cookbook import dump_responses).
  • Every published recipe importing _dump is migrated, with the enumeration recorded: met (the enumeration is in the PR body; please also copy it to Foundation: the scaffold makes all sixty recipes import the private jev_cookbook.live._dump; promote it to public API #209 as the criterion asks).
  • Keep-or-remove decision for _dump recorded: met (kept; see S4 on the stated reason).
  • No recipe imports an underscore-prefixed shared name, enforced by a guard: not met. True today, but the guard does not enforce it for private modules (Blocking 2).

#233

  • The false sentence is replaced in the template and every copy: not met. The old sentence is gone from all notebooks (0 matches for "carries that mode"), but the new one is false about validation labels (Blocking 1).

#217 (template part only)

#222 gap 3 (template line only)

OPUS-REVIEW: CHANGES-NEEDED 7a5299f

CompleteDotTech and others added 2 commits October 20, 2026 18:34
…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>
@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

Opus reviewer (independent review 1 of 2, re-review): Claude Opus 5.5 (claude-opus-5-5)

Reviewed head 7e0a1bdd5485fddd200e42fe1dae39881657be26 against base 83dfc48040736d47e8b83cf3159a198abbe5bd04 (current origin/main; also the merge base). I reviewed:

I did not read the other reviewer's comments. My previous comment is 6302632394.

Summary

All 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 validation". That is false for most lines that carry the label, including the template's own. Recipe 28, which now carries the paragraph, says the opposite in its own evaluation prose. The fix is one phrase.

Status of my earlier findings

finding status evidence
B1 "per-example lines carry no label of their own" resolved The sentence is gone. The new wording, "A line with neither label, such as an individual answer or a per-example line, still comes from the mode the header names", claims nothing about which lines are labelled. It holds in all 44 notebooks.
B2 "labelled a selection step instead" / 39 and 49 resolved as worded "instead" is gone. test_the_old_false_sentence_is_gone_everywhere pins its absence. "Two labels may follow a number" makes no claim that every validation line is labelled, so the unlabelled validation lines in recipes 39 and 49 no longer contradict the sentence. The labelling gaps themselves are still there (see S-a).
B3 private-module guard resolved I re-ran my mutation script against a scratch copy of recipe 01. All five injected spellings now fail the guard: from jev_cookbook.live import _dump, from jev_cookbook._canonical import plain_json, import jev_cookbook._canonical, from jev_cookbook.fixtures._scan import ..., and from jev_cookbook import fixtures followed by fixtures._scan. SUBMODULES now includes fixtures. Dunders are excluded, and there are tests for that and for foreign packages.
S1 "offline" also covers recorded replays resolved The paragraph now says "synthetic or scripted run", and so does the template's evaluation-md. In every carrying notebook, check is defined as " (a pipeline check, not a Jev result)" if offline else "", with offline = backend.mode in ("synthetic", "scripted"). I checked this in all 44 by parsing the code cells. So "a recorded or live run does not print it" is true of the code, not only of today's synthetic outputs.
S2 recipe 47's unlabelled gap line open, now disclosed The new test's docstring names it as a known gap. See S-a.
S3 scaffold hand-serialises resolved The template and tools/new_recipe.py build_fixtures.py now call dump_responses. docs/fixtures.md and docs/live.md are updated to match.
S4 pin the sentences resolved tests/test_run_mode_sentence.py pins the paragraph verbatim in all 44 notebooks. It also checks that exactly the seven never-carriers lack it, and it 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, and tests/test_template.py pins it.
S6 alignment test with the real suffix not addressed Optional; I am not holding the PR on it.

Blocking finding

B4. "(a selection step, not a reported result) marks a number used to choose a setting on validation" is false for most lines carrying that label

Most lines with this label are not numbers that chose anything. They are what the frozen rule does on validation, reported after the choice was made. Three examples:

  • The template itself (evaluation-validation output): accuracy of the choice: 0.80 (a selection step, not a reported result) ... and routed automatically: 6 of 10 (a selection step, ...). Neither number chose the threshold; it was chosen in python-rule from the named confidences. The template's own evaluation-md describes these lines as reporting "what the frozen rule does" on validation. It also says every validation number carries the label "unconditional[ly]", which describes a label on the split, not on the numbers that chose.
  • recipes/28-skill-selection: its evaluation markdown says "validation numbers are a selection step, labelled as such even where they are not literally what chose the threshold". This PR adds a paragraph to the same notebook saying the label marks "a number used to choose a setting". The two sentences contradict each other, and that is the same kind of in-notebook contradiction I blocked on last round (recipes 16 and 39 against "instead").
  • The same applies to the validation metric lines printed after the freeze in the other recipes (for example recipe 49's validation headline: coverage 0.8333, ...).

The rest of the labelling holds. I scanned every committed output line carrying the selection label in all 44 notebooks, and none is a test-only number. So the label is consistently a validation-split label. The paragraph just describes it too narrowly.

What to change: reword that sentence to describe the split rather than the use, and update PARAGRAPH in tests/test_run_mode_sentence.py and all 44 copies. For example:

(a selection step, not a reported result) marks a number from validation, the split settings are chosen on; it is never a result to report.

Every selection-labelled line I scanned satisfies this, and it agrees with the template's and recipe 28's own prose.

Suggestions (not blocking)

  • S-a. The labelling gaps are described as tracked, but no issue tracks them. The new test's docstring says the gaps are "tracked outside this test": validation metrics without the selection label in 39 and 49, and the unlabelled gap line in 47. I found no open issue for them (I searched issues created 2026-10-20). Please file one, so that "tracked" is true and the gaps are fixed under CONTRIBUTING.md section 2.
  • S-b. One test checks less than its name says. test_the_paragraph_is_verbatim_and_its_claims_hold_in_the_committed_output checks the "recorded or live run does not print it" claim only against committed outputs, which are all synthetic, so that branch is vacuous today. The claim does hold in the code (see S1 above). A short assertion that each carrying notebook defines check conditioned on ("synthetic", "scripted") would pin the code side.
  • S-c. S6 from last round still applies, at low priority.

Re-verified

Checks I ran at this head (Python 3.14.3 venv, pip install -e ".[dev,ml,live]" -c .github/constraints-notebooks.txt, Windows, no TYPESAFE_API_KEY or JEV_COOKBOOK_LIVE)

  • ruff check .: All checks passed. ruff format --check .: 398 files already formatted.
  • python tools/render_catalog.py --check: exit 0. python tools/check_hygiene.py: passed, 607 files. python tools/notebook_ci.py fixtures: 51 recipe folders, 0 failed.
  • pytest: 5080 passed, 63 skipped, 50 deselected, 31 xfailed, 2 failed. Both failures are the known Windows-only tests/test_notebook_ci_sandbox.py cases (signal.SIGKILL, Foundation: two notebook-CI sandbox tests use signal.SIGKILL and fail on Windows #302), and this PR does not touch that code. tests/test_run_mode_sentence.py and tests/test_no_private_shared_names.py pass.
  • Notebooks: all 50 recipes and the template, 51 of 51 OK. Each was executed offline in a scratch copy under tools/netguard, then checked against the committed file with check_notebook_fresh.py, then checked with check_hygiene.py on the fresh copy. git status --porcelain was unchanged afterwards.
  • My scripts: the parsed-JSON notebook comparison, a scan of every committed numeric output line by label (per-example lines, unlabelled lines, and selection-labelled lines that are test-only), a per-notebook parse of the offline and check definitions, and the guard mutation.
  • CI (gh pr checks 317): 60 of 60 pass on this head.

Acceptance criteria

OPUS-REVIEW: CHANGES-NEEDED 7e0a1bd

CompleteDotTech and others added 2 commits October 21, 2026 16:46
…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>
@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

PR #317 review — reviewer 1 of 2 (fresh Opus)

Verdict

CHANGES-NEEDED, on two findings, neither of which is in the code: the enumeration the PR body
records for #209 is factually wrong about recipe 32, and #233 is closed while a published notebook
(recipe 36) still carries the same class of false run-mode claim #233 was opened for. Everything
substantive in the diff is correct, and all nine blocking findings from the three prior reviews are
resolved at this head. The code, tests and sweep are the best state this PR has been in; the two
items below are cheap.

Must change

M1. The #209 enumeration in the PR body is wrong about recipe 32 — and it is the record the issue asks for

PR body, "#209" bullet:

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

Recipe 32 is a replay recipe, it does have tests/test_build_fixtures.py, and this PR did
migrate it. Measured at this head:

  • recipes/32-regression-test-selection/fixtures/responses.json exists; its setup-md says
    "get_backend(fixtures=...) replays the 163 stored answers in fixtures/responses.json";
    notebook.ipynb:76 is backend = get_backend(fixtures=responses_path()).
  • recipes/32-regression-test-selection/tests/test_build_fixtures.py is in the diff
    (git diff --name-only <base>..HEAD -- 'recipes/*/tests/test_build_fixtures.py' lists it), and at
    head it reads from jev_cookbook import dump_responses.
  • Only recipe 36 has no tests/test_build_fixtures.py (it is the one scripted recipe:
    backend = get_backend(script=helpers.script, seed=helpers.SEED), and fixtures/ holds only
    inputs.jsonl and labels.jsonl).
  • I reproduced the guard's own identity check out-of-band: for all 49 recipes that have the file,
    tests/test_build_fixtures.py is byte-identical to tools/new_recipe.py::build_fixtures_test_text(recipe).
    So the 50 changed files are 49 recipes (all but 36) + the template — the claimed count 49 is right,
    the parenthetical that explains it is not (50 − {32, 36} = 48, which also contradicts the 49).

Why this is a must-change rather than a nit: #209's third acceptance criterion is "Every published
recipe importing _dump is migrated, with the enumeration recorded". The PR body is the only
place that enumeration exists (there is no such comment on #209), so the recorded enumeration has to
be right. Fix: correct the parenthetical to "all but 36, the one scripted recipe, which has none",
and post the corrected enumeration to #209 as the criterion asks. If the body is not to be edited
(CONTRIBUTING.md, "Branches, commits, pull requests": a body is written once and not edited after the
final push), a PR comment plus the enumeration comment on #209 settles it without a new head.

M2. #233 is closed while recipe 36 still tells readers every number says it is a pipeline check

recipes/36-card-game-action-selection/notebook.ipynb, setup-md (unchanged by this PR):

The header below states the mode; every number this notebook prints is a check that the pipeline
works, not a measurement of Jev, and says so.

That is the #233 defect, in the same cell, with the same counterexamples. The notebook prints 36
numeric output lines that carry neither label, including per-example and individual-answer lines
that are exactly what #233 cites:

  • [answer] Choice: bid_1 (confidence 0.81)
  • [python-rule] g999-t0: bid_1 at confidence 0.8125 -> answered (auto-played), played bid_1
  • [evaluation-wrong] g200-t2: gold bid_4, answered bid_1 (confidence 0.5500) -> review, played bid_4

Recipe 36 fell out of the sweep because the sweep was keyed on the literal string "carries that
mode" (tests/test_run_mode_sentence.py:43: "Never carried the old ... sentence, so never needed the
swap"). That comment is true of the literal sentence and false of the defect: recipe 36 did need a
swap. I checked the other six never-carriers (41, 43, 44, 45, 48, 50) — none of them makes a
label-coverage claim, so recipe 36 is the only remaining instance in the published set.

The PR explicitly sets the bar "makes only claims a script can check", and this is a markdown-only
edit of exactly the kind the PR already makes 43 times (no re-execution, no output change). Fix:
either give recipe 36 the paragraph (adapted — it is scripted, so the pipeline-check half always
applies) or at minimum replace "and says so" with something true, and add it to
tests/test_run_mode_sentence.py's coverage; or, if it is to be deferred, file it and say so in the
body rather than closing #233 with it outstanding.

Suggestions (not blocking)

S1. tests/test_run_mode_sentence.py:116-119 — "every synthetic or scripted run prints the label" cannot fail for a template-shaped notebook

if modes & {"synthetic", "scripted"}:
    assert printed_check, (
        "a synthetic or scripted run that never prints the pipeline-check label"
    )

printed_check is any(CHECK in line for line in lines) over all committed stream output, and every
carrying notebook's measured cell prints, inside if offline:,

Provenance: synthetic. Not measured live: a pipeline check, not a Jev result.

That one fixed sentence satisfies printed_check on its own, regardless of whether any metric line
carries the label. Mutation proof, on a scratch copy of
recipes/01-sentiment-classification/notebook.ipynb (never in the tree): stripping
" (a pipeline check, not a Jev result)" from every committed stream output line changed 25 lines
and left test_the_paragraph_is_verbatim_and_its_claims_hold_in_the_committed_output green
(modes={'synthetic'}, printed_check=True, one leftover line — the Provenance: sentence).
Renaming the label does turn the case red, through the labels <= {CHECK, SELECTION} assertion, so
the PR body's stated mutation holds; the "and every such run prints it" claim is the one with no
teeth. Suggest excluding the Provenance: ... Not measured live: line from the scan (or dropping the
claim from the docstring and the body). This is the same ground as prior review 3's S-b, which also
remains unaddressed.

For what it is worth, the claim is in fact true of the code: the only two code lines in any of the 44
notebooks that contain the label literal are check = " (a pipeline check, not a Jev result)" if offline else "" and the measured cell's print inside if offline:, and offline is
backend.mode in ("synthetic", "scripted") in all 50 recipes (recipe 50, a never-carrier, uses the
equivalent SYNTHETIC_MODES). Asserting that from the code cells, as review 3 suggested, would pin
something.

S2. Recipe 32 still hand-copies the serializer the PR just made public

recipes/32-regression-test-selection/build_fixtures.py:1488-1489:

text = json.dumps(dict(sorted(responses.items())), indent=2, ensure_ascii=False) + "\n"
responses_file.write_text(text, encoding="utf-8", newline="\n")

48 of 50 recipes' build_fixtures.py now call dump_responses; recipe 32 is the one that does not,
because it never named _dump and so fell outside the git grep -l _dump enumeration. #209's point 5
is explicitly about the shape all sixty recipes copy, and build_fixtures.py is not byte-pinned by
G1(a), so converging it is a two-line change with no output effect. Out of scope by the letter of the
enumeration; worth doing in the same pass.

S3. Two leftover wordings that the corrected docs/live.md already supersedes

  • tools/new_recipe.py:738-740, the scaffolded module docstring: "the committed file matches
    jev_cookbook.live's writer exactly". Every other mention in the emitted file now names
    jev_cookbook.dump_responses; this is the first line a builder reads. (Prior review 2's S2, second
    half, still open.)
  • src/jev_cookbook/live.py:834: "assert the committed file is what the recorder would write".
    docs/live.md:166-171 now states this precisely ("what the synthetic generator would write,
    serialized exactly as the recorder serializes ... once a recipe merges a recorded answer it no
    longer holds"); the docstring keeps the looser form.

S4. tests/test_run_mode_sentence.py does not cite the issue that tracks the gaps it discloses

The module docstring says the labelling gaps are "tracked outside this test". They are — #326,
opened from this PR's own reviews — but the number is not in the file or the body. One reference
makes the claim checkable.

S5. Low priority, carried over

  • Prior review 1's S6 / review 3's S-c: tests/test_template_matrix_alignment.py runs the cell with
    check: "". I looked at whether it matters: the real suffix is appended identically to the header
    and to every row after the counts, so count alignment implies suffix alignment. Immaterial; fine to
    close as "won't do".
  • tests/test_no_private_shared_names.py says the scan is syntactic and does not follow getattr or
    importlib; it also misses a plain rebinding (jev = jev_cookbook then jev.live._dump), which I
    confirmed. Worth one more clause in the docstring, not a code change.

What I verified independently and found correct

#209 — dump_responses.

  • jev_cookbook.dump_responses is jev_cookbook.live.dump_responses is jev_cookbook.live._dump is
    True; the name is in both __all__s, absent from the package namespace as _dump, and every
    __all__ entry resolves. Package __all__ keeps its classes-then-functions-then-__version__
    convention with the new name in the right place.
  • The serialization is byte-identical to the old _dump body (same expression, moved), so no fixture
    changed. tests/test_live.py's new text test pins top-level-only sorting, nested order, UTF-8,
    one final LF, no CR and non-mutation of the input, and would fail a sort_keys=True or
    ensure_ascii=True writer.
  • The migration is complete: git grep _dump over recipes/ docs/ tools/ src/ tests/ finds no
    recipe import; the only remaining mentions are the alias itself, tests/test_live.py, the guard's
    own test cases and the prose in docs/live.md.
  • The guard demonstrably fails where it must. I ran private_shared_uses from
    tests/test_no_private_shared_names.py against the base content of
    recipes/01-.../tests/test_build_fixtures.py, recipes/50-.../build_fixtures.py,
    recipes/50-.../tests/test_helpers.py and recipes/_template/tests/test_build_fixtures.py: each
    returns ['from jev_cookbook.live import _dump'] on base and [] at head. I also ran all of prior
    review 1's and 2's private-module spellings plus four of my own: 10 offending spellings flagged, 4
    public spellings clean, dunders clean, foreign-package privates clean. SUBMODULES is
    {..., _canonical, fixtures, live, vision, ...}, i.e. built from both *.py stems and package
    directories.
  • The alias decision is recorded in three places (live.py:839-842, docs/live.md,
    docs/recipe-template.md) and is now labelled transitional with a removal condition.
  • docs/fixtures.md's worked example, which no test executes, I ran end to end in a temp directory:
    it writes a file with sorted top-level keys, no CR, one final LF, byte-identical to
    dump_responses(responses).

#233 — the run-mode paragraph.

  • Counts check out. At base, git grep -l "carries that mode" -- 'recipes/*/notebook.ipynb' returns
    44 files (43 published + _template) out of 51 folders; the 7 without it are exactly
    NEVER_HAD_THE_OLD_SENTENCE. At head the phrase is gone from every notebook.
  • Markdown-only, as claimed. Parsing base and head JSON and comparing every cell field and every
    top-level key: in each of the 43 published notebooks exactly one markdown cell's source
    differs; no output, metadata, id, execution count, cell count or cell order changed anywhere. In
    the template, four cells differ in source only (setup-md, evaluation-md markdown;
    evaluation-validation, evaluation-matrix code) and no output changed.
  • The paragraph splices cleanly in all 43 (I printed the surrounding text in each); nothing reads as
    a broken sentence. Recipes 21, 25, 26 and 31 continue with their own following sentence, and those
    read correctly.
  • The claims hold where I could check them: no selection-labelled line in any carrying notebook is a
    test-only number (the only such lines I found are in recipe 48, a never-carrier, which prints
    validation and test on one line); no carrying notebook's committed output contains a
    (a ..., not a ...) label other than the two named; and the label literals in code are mode-gated
    in all of them.
  • test_the_old_false_sentence_is_gone_everywhere also pins the absence of the two wordings earlier
    rounds rejected ("labelled a selection step instead", "marks a number used to choose"), so those
    cannot come back silently.

#217, template part. column = max(width, len(str(matrix.matrix.max()))) is correct: counts are
non-negative integers, so the largest count has the most digits, and width keeps the label floor.
leader_width correctly stays on label width, and the header and the rows share column.
tests/test_template_matrix_alignment.py executes the real cell source against the real
confusion_matrix over five cases, and its mutation to column = width is reported as misaligned on
the wide-count case and harmless on the narrow one. The template's committed matrix output is
unchanged because its largest count is 6 (one digit) and the labels are wider.

#222 gap 3, template line. True for the template: select_confidence_threshold(..., target_accuracy=1.0)
is applied to the queue-naming answers only, and route auto-routes exactly a queue-naming answer at
or above the threshold, so "right among those routed automatically" on validation is forced — the
committed output is 6 of 6. The corrected comment is right too: recipes/_template/helpers.py:70-74
has three review reasons ("no option fits", "not a queue Python may use", "confidence below the
threshold"), so the old "exactly one review reason" was false; tests/test_template.py now pins both
the absence of the old claim and the presence of the new one.

The merge. git show --cc HEAD shows exactly one conflicted file, docs/recipe-template.md,
resolved by keeping both rows (main's side_effects.json row and this branch's rewritten
tests/test_build_fixtures.py row). Nothing else differs from one parent, so the merge smuggled no
edit. Commit message records the resolution and carries the Co-Authored-By: Claude Sonnet 5.5 line.
The merge is SSH-signed and verifies.

Two closing keywords. CONTRIBUTING.md's "Branches, commits, pull requests" says "One issue per
pull request", and this body carries Closes #209 and Closes #233. That reads as accepted practice
for foundation work rather than a violation: the scope rule's one-issue requirement is written for
recipe pull requests, merged PR #319 ("Foundation: shared review-queue/action-log fill helpers ...")
closes #210 and #212 the same way, and the orchestrator's own triage comment on #209 bundles #209,
#233, #217 and #222 into "foundation PR C". Refs #217 / Refs #222 are correctly non-closing, and
both issues' remaining work is named under "Not done". Noted, not a finding.

Scope and hygiene. git diff --raw -M <base>..HEAD is all 100644; the three new test files are
000000 100644 A; no symlink, submodule, mode or type change. The branch is foundation/... and
closes two non-recipe issues, so the recipe-PR allowlist does not apply. Nothing in the diff touches
the network, a key, a credential-shaped literal or a real side effect. No absolute home path appears
in anything committed.

Checks I ran

Fresh venv in my worktree, Python 3.14.7 via uv venv --python 3.14, deps installed with
uv pip install --link-mode symlink -e ".[dev,ml,live]" -c .github/constraints-notebooks.txt
(CI's install), TYPESAFE_API_KEY and JEV_COOKBOOK_LIVE unset throughout. Every heavy job ran
under flock .claude/heavy.lock nice -n 10, with pytest -p no:cacheprovider. Disk on /workspace
was at ~500 MB free; the install succeeded (exit 0) and the venv and worktree are removed at the end.

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)

  1. A public, documented helper exists with its own test — met. dump_responses in
    src/jev_cookbook/live.py:823, exported from both __all__s, documented in its docstring and
    docs/live.md, with two dedicated tests in tests/test_live.py.
  2. tools/new_recipe.py generates the public import, and a scaffold test proves it — met.
    The emitted tests/test_build_fixtures.py and build_fixtures.py both 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.
  3. Every published recipe importing _dump is 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.
  4. The keep-or-remove decision for _dump is recorded, and what it is for — met. Kept as a
    transitional alias, with the reason and the removal condition in live.py, docs/live.md and
    docs/recipe-template.md.
  5. 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:1488 hand-copies the serializer (my S2).
  • Recipe 36's setup-md honesty claim (my M2) and its exclusion from the run-mode test's coverage.
  • recipes/16-discord-moderation-triage prints 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.py says 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

@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

Opus review 2 of 2 — PR #317 (foundation/template-scaffolder)


1. Scope (clean)

git diff --raw -M origin/main...HEAD: 156 paths, 153 M + 3 A, every mode 100644. No
symlink (120000), no submodule (160000), no mode change, no type change, no deletion, no rename.
The three additions are tests/test_no_private_shared_names.py, tests/test_run_mode_sentence.py,
tests/test_template_matrix_alignment.py.

Non-recipe paths: docs/fixtures.md, docs/live.md, docs/recipe-template.md,
src/jev_cookbook/__init__.py, src/jev_cookbook/live.py, tools/new_recipe.py,
tests/test_live.py, tests/test_new_recipe.py, tests/test_template.py, plus the three new tests.
Recipe paths: 49 build_fixtures.py, 50 tests/test_build_fixtures.py, 44 notebook.ipynb,
1 tests/test_helpers.py (recipe 50). No stray file, no .claude/, no catalog, no workflow, no
README.md, no pyproject.toml.

The branch is foundation/* and neither closed issue (#209, #233) is a catalog recipe issue, so the
Scope (recipe pull requests) allowlist correctly does not apply to this pull request.

Closing keywords. The body has Closes #209, Closes #233, Refs #217, Refs #222 and
nothing else — exactly the two intended closes. The two "Not done" paragraphs keep #217 and #222
honestly open.

2. The notebook diffs are exactly the intended one-paragraph change (verified cell by cell)

I parsed every changed notebook at base and head and compared code-cell sources, serialized outputs,
notebook metadata, cell ids and order, and markdown cell contents:

  • All 43 published notebooks: exactly one markdown cell changed and nothing else — no code cell,
    no output, no metadata, no cell id, no reordering. The body's "Markdown only in all published
    notebooks" holds.
  • 42 are a single one-line hunk; recipes/21-quiz-answer-adjudication/notebook.ipynb is a 3-line
    hunk because its setup-md is wrapped and the replaced sentence spanned two source lines.
  • recipes/_template/notebook.ipynb: two markdown cells (setup-md, evaluation-md) and two code
    cells (evaluation-validation's comment, evaluation-matrix). Both code cells' committed
    outputs are byte-identical to the base
    , so "Template outputs are unchanged" holds.
  • 47 recipe build_fixtures.py diffs are comment-only. The only non-comment changes are recipe 50
    (import + 2 calls) and recipes/_template/build_fixtures.py (import block, comment, call) —
    exactly as claimed. import json is still used in the template's script, so no dead import.
  • All 49 recipes' tests/test_build_fixtures.py are byte-identical to
    tools/new_recipe.py::build_fixtures_test_text(recipe) at this head.
    I ran that comparison
    myself for every catalog entry: 49 compared, 0 mismatched. G1(a) therefore holds by construction
    and the regeneration claim is true.
  • The 7 notebooks without the new paragraph are exactly the 7 that never had the old sentence:
    git grep -L "carries that mode" origin/main -- 'recipes/*/notebook.ipynb' → 36, 41, 43, 44, 45,
    48, 50; git grep -l → 44 (43 published + template). No carries that mode survives anywhere at
    this head except as a regression assertion in tests/test_run_mode_sentence.py:87.

MUST CHANGE

M1 — The replacement paragraph's selection-label sentence is false for at least five of the 43 notebooks it was written into. This is the same defect class #233 exists to remove, and the PR body claims the opposite

Where: recipes/_template/notebook.ipynb setup-md, and the identical paragraph in all 43
published notebooks; pinned verbatim at tests/test_run_mode_sentence.py:33-40.

The sentence:

(a selection step, not a reported result) marks a number from validation, the split settings
are chosen on; it is never a result to report.

Read as written, it says every number this label marks is a validation number. In the committed
outputs of notebooks that carry this very paragraph, it is not:

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 from validation, 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.py that 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's setup-md (and define selection in the
    scaffolded setup cell, or state in docs/recipe-template.md that a recipe printing a
    validation number must add it), plus a test that the scaffolded setup-md contains PARAGRAPH,
    so the two sources cannot drift; or
  • derive the exemption in test_exactly_the_expected_notebooks_carry_the_paragraph from 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.py exists, imported _dump on
    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.py count, 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 has fixtures/responses.json. 36 is the only scripted recipe
    (Mode: offline, scripted backend, …, no responses.json, no tests/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._dump itself stays as a supported alias for code written before the rename (see its
comment in live.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.ipynb it sits inside a setup-md whose other
    source lines wrap at ~100, and the two new docs/recipe-template.md bullets (lines 185-186) are
    single long lines among wrapped prose. Nothing enforces wrapping (E501 is not in
    pyproject.toml's ruff select, 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) exceed line-length = 100. Both are inside strings, so
    ruff format leaves them and ruff check does not select E501; no check fails.
  • docs/live.md names the private alias in a public doc. Lines 169-173 tell the reader
    jev_cookbook.live._dump exists 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
    in live.py only.
  • The test's scripted branch is vacuous today. PROVENANCE matches , (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:18 says 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.py module docstring still says "matches jev_cookbook.live's
    writer exactly"
    (tools/new_recipe.py:740). Accurate, but now that dump_responses is the
    public name it is the one worth printing there.

Things that check out (verified, not taken on trust)

  • dump_responses is a real public helper with its own test. src/jev_cookbook/live.py:823-836;
    exported from live.__all__ and the package __all__; _dump is 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 by test_dump_responses_text_is_exactly_the_stored_form, which would
    fail a sort_keys=True or ensure_ascii=True writer.
  • No recipe or template file reaches a private shared name at this head. _dump survives only
    in src/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.md and
    docs/recipe-template.md.
  • The private-name guard is a real instrument — I probed it directly, importing
    private_shared_uses and 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.
    SUBMODULES contains both fixtures (package directory) and _canonical. It also scans the
    scaffolder's actual output in both --mode replay and --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.py execs the template's own cell source against the
    real jev_cookbook.evaluation.confusion_matrix over five label/count shapes, and
    test_the_harness_reports_the_old_label_width_only_sizing shows the harness red under
    column = width and still green where the old formula was correct. leader_width correctly stays
    on label width. Template outputs unchanged. tools/new_recipe.py emits 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
    with target_accuracy=1.0 over queue-naming answers only, and evaluation-validation prints
    right among those routed automatically: 6 of 6 (a selection step, …) (a pipeline check, …) —
    forced by construction; routed automatically: 6 of 10 and accuracy of the choice: 0.80 are
    not, so the paragraph's "One validation number cannot come out otherwise" is exact.
    test_the_template_prints_both_labels_on_a_validation_metric and
    test_the_scaffolder_evaluation_stub_keeps_the_forced_metric_prompt pin both halves.
  • The corrected route comment is factually right. recipes/_template/helpers.py:70,72,74 —
    route has 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.py pins 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"), and recipes/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.
  • check really is mode-conditional. check = " (a pipeline check, not a Jev result)" if offline else "" with offline = 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.md section 2, docs/notebook-style.md's "Title convention",
    docs/offline-and-live.md, docs/glossary.md and docs/evaluation.md all 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 calls dump_responses(...) with newline="\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, but ruff format --check covers 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 show U (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_uses fed 15 sources — 11 flagged,
    4 correctly not flagged (listed above).
  • tests/test_run_mode_sentence.py run 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 51 scaffolded into a scratch tree, then dropped under recipes/:
    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...HEAD mode/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, for test-only selection-labelled lines,
    and for trailing parentheticals following a number; split resolution for every M1 counterexample
    against the recipe's own fixtures/inputs.jsonl.
  • direct probing of private_shared_uses with 15 sources (listed above).
  • _dump enumeration on origin/main and 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:366 says the pinned ruff is ruff==0.16.10; pyproject.toml:24 pins
    ruff==0.17.1. Stale on origin/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) and recipes/26 (queue-preview) attach
    {selection} to numbers computed over demo and/or test examples, which CONTRIBUTING.md
    section 2 ties to the validation split. 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 own queue-build cell shows the correct labelling.

OPUS-REVIEW: CHANGES-NEEDED eff86c7

CompleteDotTech and others added 2 commits October 21, 2026 20:29
…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>
@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

PR #317 re-review — reviewer 1 of 2 (fresh Opus), round 2

  • Agent: independent Opus reviewer 1 of 2 for PR Foundation C: public dump_responses, honest run-mode sentence, matrix and forced-metric template fixes #317 (pr317-r1b), worktree
    .claude/worktrees/agent-a9b340d877b72db9e.
  • Model ID (as stated in my system prompt): claude-opus-5[1m] (Opus 5, 1M context).
  • Head reviewed: f60324a601d447ef09cdcc59ecec1f332ec5fc30. Signed:
    git log --show-signature -1 reports Good "git" signature ... ED25519 SHA256:4gVoHVYiiNpL+UmRnjalQ6GDYvwXEuuQuprjiQys2/c, %G? = G.
  • Merge shape: Merge: 91e5f0b eba0506. Parent 1 91e5f0b81da718d593e9b44996b63dcc41ae71a2
    (the second fix commit, also signed, %G? = G), parent 2 eba0506e4082637ca5b1ee1a54dba97d7c4f21b6
    = current origin/main.
  • Previous head I reviewed: eff86c79958c3725ed376e97714755c06b38b76b (review
    .claude/reviews/pr317-r1.md, verdict CHANGES-NEEDED).
  • Diff vs origin/main: 159 files, +1347 / −711.

Verdict

CHANGES-NEEDED. Both of my round-1 blockers are properly resolved, and so are all three of the
other reviewer's, and I verified each one myself rather than taking the builder's word. What stops
me approving is new and narrow, and it is in the one file this PR exists to fix: the template's own
evaluation-md still says "every number here, test and validation alike, is a check that the
pipeline works, and says so", which is false for the two numeric lines in the template's own
evaluation-wrong output — and the same sentence with the two split names swapped is on this PR's
own retired-wordings list, added for recipe 36. Fixing it is one sentence plus one list entry, with
no re-execution.

Must change

M1. The template's evaluation-md keeps the unscoped claim this PR retires everywhere else

recipes/_template/notebook.ipynb, cell evaluation-md:

In a synthetic or scripted run every number here, test and validation alike, is a check that
the pipeline works, and says so
; every validation number also carries an unconditional label …

The template's own committed output under that cell contains two numbers that say no such thing.
evaluation-wrong is the last code cell of the Evaluation section (cell order: evaluation-md,
evaluation-validation, evaluation-test, evaluation-matrix, evaluation-wrong,
evaluation-note-md, then the next ## heading), and its committed stream output is:

Test examples the choice got wrong, and what Python did with them:
  t08-lookalike: gold bug, answered billing (confidence 0.36) -> review
  t09-mixed: gold account, answered billing (confidence 0.71) -> billing-team

0.36 and 0.71 are numbers in that section with neither label. I measured it: scanning every
output line after evaluation-md for a digit and no label returns exactly 2 lines for the
template, and both are these. evaluation-note-md adds no disclaimer.

Three things make this a blocker rather than a nit:

  1. This PR's own instrument condemns the identical sentence.
    tests/test_run_mode_sentence.py's FORBIDDEN_WORDINGS contains
    "every number here, \validation` and `test` alike"— added, per its own comment, because recipe 36 "had the same defect". The template's wording is that string with the two backticked split names in the other order. I ran the forbidden list against three refs: that entry matches recipe 36 at the base and ateff86c7and nothing at this head, while the reversed spelling matches_templateand40-hierarchical-classification` at this head and is not on the list.
  2. This PR edited that very sentence. At eff86c7 its leading clause was changed from "In an
    offline run" to "In a synthetic or scripted run" (review 1 round 1, S1), and the body claims that
    fix. The unscoped quantifier in the same clause was left.
  3. "number" versus "metric" is the distinction this PR itself drew. The fix commit narrowed
    recipe 36 ("every number this notebook prints" → "each metric printed below … the individual
    turns it lists are not metrics and carry no label"), recipe 45 ("every number below" → "each
    metric line below") and recipe 50 ("every validation line" → "every validation metric line")
    for exactly this reason. By the PR's own standard "every number here" is the unscoped form, and
    the template is where the remaining 50 recipes copy it from — 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 asks be fixed
    first.

What to change. Scope the template's sentence the way recipe 36's was (for example: "… every
metric printed here, test and validation alike, is a check that the pipeline works, and its line
says so; the individual examples it lists are not metrics and carry no label"), and add
"every number here, \test` and `validation` alike"toFORBIDDEN_WORDINGS` so the reversed
spelling cannot come back. Markdown only; no output changes, no re-execution.

The same wording class, measured at this head:

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-md now says "each metric line below already being labelled a pipeline check". Its
    evaluation-matrix cell prints the confusion matrix as a bare label list and a raw array with
    no label on any line (['corroborating', …], [[4 0 0 0] …), and evaluation-test's
    "test examples: 16" is unlabelled too — 12 unlabelled numeric lines after evaluation-md in 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_closed asserts all(int(name[:2]) <= 50 for name in …). For the
    three-digit rank the catalog allows (100-…), name[:2] is "10", so 10 <= 50 passes and a
    rank-100 recipe could be exempted. int(name.split("-")[0]) <= 50 closes 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:834 says "what the recorder would write"
    where docs/live.md now states it precisely.
  • S4. build_fixtures.py convergence, 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.py still
    hand-serialize with json.dumps(dict(sorted(...)), indent=2, ensure_ascii=False) + "\n", and only
    the template and recipe 50 call dump_responses; in 47 of them the PR changed only the comment
    (verified on recipes/41-approval-scope-review/build_fixtures.py:479, where dump_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.py is 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's source, recipe 36
    changes two (setup-md and evaluation-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.py is the substance: PARAGRAPH reworded, 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_SENTENCE replaced by a closed
    LEGACY_WITHOUT_THE_PARAGRAPH with a closedness test and a stale-exemption test, the label checks
    factored into check_run_mode_claims with the parenthetical requirement, and a real mutation test
    (test_the_label_checks_fail_on_a_mutated_run) over _template and recipe 47 whose strip case
    first asserts the provenance prose survives, so the case cannot be vacuous.
  • tools/new_recipe.py gains RUN_MODE_PARAGRAPH, emits it in setup-md with a TODO, and the test
    pins squash(module.RUN_MODE_PARAGRAPH) == PARAGRAPH so the two cannot drift. The cross-reference
    to tests/test_template_scaffolder_agreement.py is 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 outside live.py's own tests imports it". Matches live.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

OPUS-REVIEW: CHANGES-NEEDED f60324a

@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

Opus re-review (round 2) — PR #317 (foundation/template-scaffolder)

  • Agent: reviewer 2 of 2, worktree .claude/worktrees/agent-af2a8bc045414eb23
  • Model: claude-opus-5[1m] (Opus 5, 1M context), as stated in my system prompt
  • Head reviewed: f60324a601d447ef09cdcc59ecec1f332ec5fc30 — signed (%G? = G), parents
    91e5f0b81da718d593e9b44996b63dcc41ae71a2 + eba0506e4082637ca5b1ee1a54dba97d7c4f21b6
  • Base: origin/main = eba0506e4082637ca5b1ee1a54dba97d7c4f21b6 — I confirmed origin/main
    is at exactly that commit, so the branch is merged up to date
  • Previously reviewed head: eff86c7 (my review pr317-r2.md, CHANGES-NEEDED: M1, M2, M3 + S1-S4)
  • Verdict: APPROVE. All three of my blockers are resolved, and I verified each by re-deriving
    it myself rather than by reading the fix. The other reviewer's two findings are resolved too.
    What remains is one wording nit in the PR body and four suggestions, none of which I would hold
    the merge for.

1. My findings from eff86c7 — status at f60324a

M1 (blocker) — the selection-label sentence was false for recipes 02, 10, 15, 25, 26 → RESOLVED

The sentence now reads:

(a selection step, not a reported result) marks a line that is not a result to report, such
as a number from validation, the split settings are chosen on, or a count of the items the
frozen rule queues or would queue
.

The universal claim is now "not a result to report"; the split is one of two illustrations
("such as"), so naming a split no longer asserts anything false. My five counterexamples are
covered by the second illustration — each was a queue count or a would-queue preview
(02 queue-final, 10 queue, 15 queue, 25 queue-preview, 26 queue-preview).

I re-derived this independently rather than trusting the builder's classifier. I parsed every
code cell of all 45 carrying notebooks with ast, found every f-string that interpolates the
selection variable (not the word "selection", which appears in prose in recipes 12, 27, 28
and would inflate a naive grep), and classified each by what it interpolates and what its cell
references:

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, validation and test alike, is a check
    that the pipeline works, and its line says so; the individual turns it lists are not metrics and
    carry no label. Each validation metric 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 a show_answer block (Score: 2.64 (confidence 0.64)) and per-example
    specificity score lines in composition, 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 (the tests/test_build_fixtures.py
    row naming dump_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 two dump_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 ≤ 50 guard has a three-digit hole.
    tests/test_run_mode_sentence.py asserts all(int(name[:2]) <= 50 for name in LEGACY_WITHOUT_THE_PARAGRAPH). CONTRIBUTING.md says recipe 100 carries a three-digit prefix,
    and int("100-slug"[:2]) is 10, 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 in tests/, 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_everywhere narrowed from the raw file to markdown
    cells only.
    At eff86c7 it read path.read_text(); it now joins markdown sources. Harmless
    today — I checked that none of the eight FORBIDDEN_WORDINGS appears 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 #326 disclosure's counts are approximate and, by my patterns, low. The body says
    "about 25 notebooks say in evaluation-md that every validation number carries a different
    label or both labels, and about 24 say every test number 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 rewrite setup-md by hand.

Out of scope — reported, not fixed

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

CompleteDotTech and others added 2 commits October 21, 2026 21:59
…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>
@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

PR #317 re-review — reviewer 1 of 2 (fresh Opus), round 3

  • Agent: independent Opus reviewer 1 of 2 for PR Foundation C: public dump_responses, honest run-mode sentence, matrix and forced-metric template fixes #317 (pr317-r1c), worktree
    .claude/worktrees/agent-a9b340d877b72db9e.
  • Model ID (as stated in my system prompt): claude-opus-5[1m] (Opus 5, 1M context).
  • Head reviewed: 98b79f5c5a12e6a671380d5eb933e5fbb9116d3a. Signed:
    Good "git" signature ... ED25519 SHA256:4gVoHVYiiNpL+UmRnjalQ6GDYvwXEuuQuprjiQys2/c, %G? = G.
  • Merge shape: Merge: b89b689 39ddfce. Parent 1 b89b689d82e8dae71c7dcbea5cb435edf48f1f82
    (the round-3 fix commit, also signed, %G? = G), parent 2 39ddfcef2f8da7a40812b46998390dde3ca7eee2
    = current origin/main.
  • Heads I reviewed before: eff86c7… (pr317-r1.md, CHANGES-NEEDED) and f60324a…
    (pr317-r1b.md, CHANGES-NEEDED on one blocker).
  • Diff vs origin/main: 159 files, git diff --raw -M = 156 × 100644 100644 M and
    3 × 000000 100644 A. No symlink, submodule, mode or type change.

Verdict

APPROVE. My single remaining blocker is resolved, and I verified the replacement line by line
against the template's own committed output rather than taking the wording on trust: the template's
Evaluation section prints 17 metric lines, every one of which carries the pipeline-check label, and
exactly 2 unlabelled numeric lines, which are the two per-example lines the new sentence now names
explicitly. The scaffolder stub is fixed the same way and both new instruments demonstrably fail at
the previous head. The recipe 40 exemption is the right call and I checked its premise myself.

My round-2 blockers: status

M1 — the template's evaluation-md claimed every number says so. Resolved.

New text (recipes/_template/notebook.ipynb, evaluation-md):

In a synthetic or scripted run each metric printed here, test and validation alike, is a
check that the pipeline works, and its line says so; the individual examples listed under
"Test examples the choice got wrong" are not metrics and carry no label.
Every validation
metric also carries an unconditional label saying it is a selection step rather than a
reported result, in every run mode, …

I audited the template's Evaluation section (the cells between evaluation-md and the next ##
heading: evaluation-validation, evaluation-test, evaluation-matrix, evaluation-wrong,
evaluation-note-md) line by line against the committed output:

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 check beside 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's evaluation-md anyway (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_needed makes it genuinely self-expiring, and I proved
    it falsifiable in both directions: pointing the exemption at 01-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 on FORBIDDEN_WORDINGS goes 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 its evaluation-md and
    the next ## heading has a digit and neither label. The data the test already loads is enough.
  • S2. RETIRED_BUT_TRUE_IN has no rank bound, unlike LEGACY_WITHOUT_THE_PARAGRAPH.
    test_the_legacy_list_is_closed bounds that list with is_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_IN and passing. Applying is_legacy_rank to its names too gives the new
    exemption the same guarantee, and is_legacy_rank is 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") and src/jev_cookbook/live.py:834 ("what the recorder would
    write") keep the looser wording that docs/live.md already states precisely;
    tests/test_template_matrix_alignment.py runs the cell with check = "";
    tests/test_no_private_shared_names.py's docstring does not mention the plain-rebinding hole;
    recipes 38 and 45's evaluation-md sentences 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, source only; 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_IN and its staleness test; RETIRED_SCAFFOLD_WORDINGS and the scaffold-stub
    test; legacy_rank/is_legacy_rank and the three-digit test. The exemption is applied inside
    test_the_old_false_sentence_is_gone_everywhere with 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_PARAGRAPH is untouched, so the
    paragraph the scaffolder emits still matches PARAGRAPH byte 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

OPUS-REVIEW: APPROVE 98b79f5

@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

Opus delta re-review (round 3) — PR #317 (foundation/template-scaffolder)

  • Agent: reviewer 2 of 2, worktree .claude/worktrees/agent-af2a8bc045414eb23
  • Model: claude-opus-5[1m] (Opus 5, 1M context), as stated in my system prompt
  • Head reviewed: 98b79f5c5a12e6a671380d5eb933e5fbb9116d3a — signed (%G? = G), parents
    b89b689d82e8dae71c7dcbea5cb435edf48f1f82 + 39ddfcef2f8da7a40812b46998390dde3ca7eee2
  • Base: origin/main = 39ddfcef2f8da7a40812b46998390dde3ca7eee2 (confirmed exactly the tip,
    so the branch is merged up to date)
  • Previously approved by me: f60324a (pr317-r2b.md). This is a delta review of
    f60324a → b89b689 plus the merge.
  • Verdict: APPROVE. I accept the delta. Nothing in it weakens anything I verified at
    f60324a, my suggestion (a) is implemented correctly, the PR body now states the existence
    check accurately, and I judge the recipe 40 exemption acceptable as it stands — see §2.

1. Scope of the delta

git diff --raw -M f60324a b89b689: 3 paths, all 100644 100644 M — no addition, deletion,
rename, mode or type change: recipes/_template/notebook.ipynb, tests/test_run_mode_sentence.py,
tools/new_recipe.py.

git show --cc 98b79f5 emits no combined-diff hunk, so the merge of #332 (answers.py,
tests/test_backends.py, docs/backends.md) introduced nothing beyond its two parents.

The template notebook change is one markdown cell (evaluation-md) and nothing else — no code
cell, output, metadata key, cell id or ordering differs from f60324a. Recipe 40's notebook is
untouched
: the exemption is a test-side allowance, not a notebook edit.

Full scope at this head vs origin/main is unchanged in shape: 159 paths, 156 M + 3 A, all
100644, same twelve non-recipe paths. Closing keywords still exactly Closes #209, Closes #233,
Refs #217, Refs #222.

2. The recipe 40 exemption — my own judgment: acceptable, do not reword here

RETIRED_BUT_TRUE_IN exempts 40-hierarchical-classification from the newly retired wording
"every number here, test and validation alike". I judged it against recipe 40's committed
output rather than the comment, by walking every output line of its evaluation section and tagging
each as labelled or not:

  • 0 unlabelled numeric lines. Every number in 40's evaluation section carries
    (a pipeline check, not a Jev result) — including the per-example lines in its
    evaluation-wrong cell (t11-…, t13-…, t15-…) and every confusion-matrix row. The only
    unlabelled lines are three pure-prose lines with no digits at all ("The full-path trap: …", and
    the "Test documents where … disagreed with gold" heading).
  • 40's two validation metric lines carry both labels.

So the sentence "every number here, test and validation alike, is a check that the pipeline
works, and says so" is true of recipe 40. It is false of the template, which is why the
template needed the rewording: the template's evaluation-wrong prints two per-example lines
(t08-lookalike: …, t09-mixed: …) with no label, and the new template sentence now names
exactly that case ("the individual examples listed under "Test examples the choice got wrong" are
not metrics and carry no label"). I confirmed the template's own output: every metric line carries
the check label, every validation metric carries both, and the only unlabelled numeric lines are
those two per-example lines. Reviewer 1's blocker was real and the fix is precise.

The exemption is also minimal and complete: I scanned all 51 notebooks for all nine retired
wordings and recipe 40 is the only notebook still carrying any of them, matching the single
exemption entry. And it is self-expiring: test_a_retired_wording_exemption_is_still_needed
requires the wording to still be on FORBIDDEN_WORDINGS and still be present in the named
notebook, so the entry cannot outlive the wording.

Forcing a reword of recipe 40 here would push a true sentence into the #326 markdown sweep's scope
for no gain, so I would not hold the merge for it.

One residual, for #326 not for this PR: recipe 40's sentence opens "In an offline run",
the conditional-scope wording reviewer 1 flagged in round 1 and which this PR already fixed in the
template ("In a synthetic or scripted run"). It is true of 40's synthetic run today, but a recorded
replay is also offline and would empty check. That belongs with the ~25-notebook markdown side of
#326 the body already discloses.

3. My suggestion (a) — correctly implemented

int(name[:2]) is replaced by legacy_rank(name) = int(name.split("-")[0]) with is_legacy_rank,
and test_a_three_digit_recipe_is_never_treated_as_legacy pins legacy_rank("100-multimodal-slug") == 100, that it is not legacy, that 51-new is not, that 50-… and 01-… are — and, nicely,
asserts the old spelling would have mis-classified it (assert int("100-multimodal-slug"[:2]) <= 50). That is the regression demonstration I asked for, written as a test rather than a comment.

4. Reviewer 1's blocker and the scaffolder stub

tools/new_recipe.py's evaluation-md stub now says "In a synthetic or scripted run print each
metric with check beside it; an individual example listed below a metric is not a metric and
needs no label" in place of "In an offline run print each number with check beside it". The new
test_the_scaffolded_evaluation_text_does_not_carry_a_retired_wording[replay|scripted] asserts the
stub contains none of RETIRED_SCAFFOLD_WORDINGS + FORBIDDEN_WORDINGS and does contain the new
phrasing — so the scaffolder cannot drift back.

Re-scaffolded 51 (replay) and 52 (--mode scripted) in a scratch copy at this head, as asked:

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 — 60 passed, 6 skipped

(60 passed vs 56 at f60324a: the four tests this round adds. A newly scaffolded recipe of either
mode still passes with nothing added to tests/.)

5. PR body — the changed claims

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

My venv (/home/agent/venv-r2c) and all scratch directories are deleted; I removed nothing I did
not create.

OPUS-REVIEW: APPROVE 98b79f5

@CompleteDotTech
CompleteDotTech merged commit b627e3c into main Oct 21, 2026
61 checks passed
@CompleteDotTech
CompleteDotTech deleted the foundation/template-scaffolder branch October 21, 2026 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant