[CompleteDotTech]

Closes #329

What this adds

Three changes to the image-asset checker, plus the doc cross-reference item on the issue. Files: tools/check_recipe_assets.py, tests/test_check_recipe_assets.py, docs/vision.md.

• Pin test. test_the_checker_limits_match_the_shared_image_limits (renamed; the "when the package exports them" qualifier is gone) reads vision.IMAGE_LIMITS by plain attribute access and always compares. A rename now errors instead of passing with nothing compared.
• zlib end check. test_a_complete_deflate_stream_must_still_end_with_its_final_block_and_checksum builds two IDAT streams that inflate to exactly the right rows with nothing left over: zlib.compress(rows)[:-4] (no Adler-32) and a Z_SYNC_FLUSH-ended stream (no final block). Only the "does not end cleanly" check can reject them, and the test asserts that rule and message.
• Decompression cap. inspect_png now computes the expected raw length first and calls decompress(idat, expected + 1), so a bomb allocates about one image, not up to 256 MiB. A stream that reaches expected + 1 is reported as IDAT inflates past the N bytes the image header implies. The or inflater.unconsumed_tail term is dropped from the end check because the over-length check now covers it. _MAX_INFLATED (256 MiB) stays only as a backstop: a header that implies more than that is refused before any inflating, because otherwise a header with absurd dimensions would raise the cap with it.
• Docs. docs/vision.md says the loader's 16 MiB limit applies to the loader, and that committed fixtures are held to the stricter 1 MiB, no-metadata rule in docs/fixtures.md.

New tests: test_idat_is_inflated_no_further_than_the_length_the_header_implies (a 100 MiB zero bomb, about 100 KiB compressed, peak traced memory under 2 MiB), test_the_inflate_limit_is_exactly_one_byte_past_the_expected_length, test_a_header_implying_more_than_the_backstop_is_refused_without_inflating.

Decisions for the reviewers

• The extra margin is exactly 1 byte. That is enough to detect "too long" and keeps the over-length message distinct from the truncated one. A larger margin (a mutant with + 2**20) is also killed, but only because of the 2 MiB peak bound; a margin of a few hundred KiB would not be. I judged that acceptable under "a small margin".
• Worst case remains bounded by the header, not by a fixed small number: a hostile header that claims, say, 256 MiB of pixel data (but is within the backstop) can still make the Hygiene job inflate up to that much. For files under recipes/*/assets/ the dimension limits are checked after inspect_png, so I did not reorder them (that would change which finding a too-large image gets). Lowering _MAX_INFLATED would risk false positives on embedded notebook figures, which have no dimension limit. Say if you want either.
• The docs/vision.md edit is outside the "checker and its tests" paths but is listed as an acceptance item on the issue.

How it was verified

Python 3.14.7 venv, and 3.10.21 for the checker and vision tests, under the heavy lock.

• ruff check . passes; ruff format --check . passes (3.14 and 3.10 venvs).
• Full pytest on 3.14: 5896 passed, 49 skipped, 50 deselected, 31 xfailed. On 3.10: tests/test_check_recipe_assets.py tests/test_vision.py 432 passed.
• python tools/check_hygiene.py: passed (611 files). python tools/check_recipe_assets.py: passed (461 files). python tools/render_catalog.py --check: exit 0. python tools/notebook_ci.py fixtures: 51 recipe folders, 0 failed.
• Red before: with tools/check_recipe_assets.py from origin/main, the new test file gives 2 failed (the bomb test and the backstop test), 163 passed; the zlib end test passes on old code, as the check already existed. The old pin test with IMAGE_LIMITS renamed in vision.py still passes (silent pass); the new one fails with AttributeError.
• Mutants, each applied alone against tests/test_check_recipe_assets.py and tests/test_vision.py, then restored:

Mutant	Result
if not inflater.eof: replaced with if False:	killed by the zlib end test
decompress(..., expected + 1) back to _MAX_INFLATED	killed (peak 219,758,182 bytes)
limit expected (no + 1)	killed by test_idat_is_inflated_no_further_than_the_length_the_header_implies and test_the_inflated_idat_length_must_match_the_image_header (not because an exact-length file is rejected, as an earlier version of this row said)
limit expected + 2**20	killed (peak 3,057,912 bytes); expected + 2 is not killed
over-length check removed	killed
backstop check removed	killed
MAX_PIXELS 2,458,625	killed
IMAGE_LIMITS renamed in vision.py	killed (AttributeError)

Corrections after review (reviewers 1 and 2 of this PR)

• One mutant in the table above survives the suite: expected + 2. Only the + 2**20 margin is killed, and only by the 2 MiB peak bound. The exact + 1 is a design choice, not a pinned property. The comment near tests/test_check_recipe_assets.py:866 gives a wrong reason for why the expected-without-+1 mutant is killed (it is killed by two other tests, named in the table).
• The memory figures understate the real peaks, because CPython's zlib output buffer roughly doubles the cap: a 300 KB bomb drove a peak of about 537 MB on the old code, and a hostile header just under the backstop still reaches about 536 MB, against about 1.6 MB for a realistic image. The bound still rests on the header.
• The value of _MAX_INFLATED is pinned by no test: mutants setting it to 16 MiB or 4 GiB, or changing > to >=, survive the suite, even though 16 MiB would reject the largest legal asset (1568x1568 RGBA 16-bit, implies 19,670,560 bytes non-interlaced or 19,671,932 interlaced).
• An IDAT that is both over-length and followed by trailing bytes now reports png-structure rather than trailing-data.

These are follow-up items, not defects in the verdicts the checker gives.

[CompleteDotTech]

PR #334 — independent Opus review 1 of 2

• Agent: reviewer 1 (worktree agent-a2647921104fb0455), fresh session; I have not seen reviewer 2's findings.
• Model: claude-opus-5[1m] (Opus 5, 1M context).
• Head reviewed: ba797d67e533e85ecc9d526ae080e78dbf3a1bad, fetched myself with
  git fetch origin pull/334/head; gh pr view 334 --json headRefOid agrees. Good ED25519 signature, as has
  its parent 2353585a0d9348cfece4710e155b2ab53db7c1c1.
• Base: 39ddfcef2f8da7a40812b46998390dde3ca7eee2 — current origin/main, and also the merge-base, so the
  merge commit brings in nothing of its own. git diff 39ddfce..HEAD is the whole PR: 3 files, +97 / −12.
• Scope: tools/check_recipe_assets.py, tests/test_check_recipe_assets.py, docs/vision.md. No mode
  change, no symlink, no submodule, no fixture, no notebook.
• Offline: no live call, no TYPESAFE_API_KEY, no credential search. Everything I built is synthetic and
  in memory.

Verdict

Approve. The mechanism is correct: I re-derived the expected-length arithmetic independently, probed zlib's
max_length semantics on two zlib versions, and could not construct a case where the cap rejects a valid
image or lets an oversized stream through. All eight mutants the PR lists really are killed, and so are seven
more of my own. Seven suggestions follow, the first three worth acting on; none of them is a reason to hold
the merge.

What I verified about the new cap, independently

1. The expected-length arithmetic is right for every header the checker accepts. I wrote my own Adam7
   accounting (len(range(x0, width, dx)) per pass, no shared code) and compared it with png_raw_length
   for all five colour types × every bit depth each allows × 51 widths × 51 heights (1–39, 63, 64, 65, 255,
   256, 280, 319, 320, 333, 1568, 4095, 100000) × interlaced and not: 0 mismatches in 25,245 combinations,
   on 3.10 and on 3.14. So the cap can only be "exactly the right length, plus one".
2. Exact / +1 / −1 at the boundary, per colour type, on hand-built PNGs (no Pillow in the loop): RGB-8,
   grey-16, RGBA-16, palette 1-bit and 4-bit, interlaced RGB-8, interlaced grey-1, interlaced RGBA-16 at
   1568×1568, 1×1 interlaced, 1×1 plain, 7×7 interlaced grey-4. The exact length is accepted every time;
   expected + 1 gives png-structure "IDAT inflates past the N bytes the image header implies";
   expected − 1 gives png-structure "IDAT inflates to N−1 bytes; the image header implies N". No
   off-by-one anywhere, interlaced or not, 16-bit or palette.
3. The cap cannot let an oversized stream through. decompress(data, expected + 1) returns at most
   expected + 1 bytes, so any stream longer than expected returns expected + 1, which line 311 refuses.
   On zlib 1.2.13 and 1.3.2 I confirmed the two cases that matter: when the output limit is hit, eof is
   False and unconsumed_tail is non-empty; when it is not hit, all input is consumed (anything after the
   stream goes to unused_data). So unconsumed_tail can only be non-empty when
   len(inflated) == expected + 1, which line 311 has already caught — dropping
   or inflater.unconsumed_tail from line 316 is behaviour-preserving, and I checked that empirically:
   putting the term back changes no test (432 passed).
4. The accept path never touches the output limit. A valid file inflates to expected, below the limit,
   so acceptance does not depend on whether zlib reports Z_STREAM_END when the buffer fills exactly. (It
   does, on both zlib versions — which is the subject of S2, because the test comment claims the opposite.)
5. Zero and absurd headers. width == 0 or height == 0 is still refused at the IHDR check, before any
   of this. png_raw_length is pure Python ints, so nothing overflows: 10000×10000 RGB (300,010,000),
   65535² (12,884,574,210) and (2³²−1)² (5.5 × 10¹⁹) all exceed the backstop and are refused before
   inflating anything. Negative Adam7 pass widths (width − x0 < 0) are guarded by pass_w > 0, which I
   checked at 1×1 and 7×7.
6. Ancillary chunks and multiple IDATs. One stream split over three IDAT chunks is accepted and the cap
   applies to the concatenation; a gAMA between two IDATs still gives "IDAT chunks are not consecutive";
   bytes after the stream still give trailing-data. The one still-reachable branch of the final
   len(inflated) != expected check (too short) is still tested — removing it fails
   test_the_inflated_idat_length_must_match_the_image_header.
7. No false positive on anything real. tools/check_recipe_assets.py passes over the 461 committed files;
   I also ran inspect_embedded over all 134 images embedded in the 51 committed notebooks with the new
   code: 0 findings, largest header-implied raw length 5,030,535 bytes. The largest asset the dimension rules
   allow — 1568×1568 RGBA 16-bit interlaced, 19,671,932 bytes raw in a 19 KB file — is accepted.

Findings

No must-change findings.

S1. The memory bound is still _MAX_INFLATED, and in practice about twice that

tools/check_recipe_assets.py:296-308. The cap is expected + 1 and expected comes from the file's own
header, so for a hostile header the real bound is the backstop. Measured against this head exactly as it
stands (traced with tracemalloc, RSS with getrusage):

case, as a file under recipes/*/assets/	compressed	traced peak	max RSS
320×320 header, 100 MiB zero bomb (the new test's case)	101,929 B	962,161 B (3.14) / 970,461 B (3.10)	+2 MB
320×320 header, 1 GiB zero bomb	1,043,644 B	3,786,986 B	+2 MB
9000×9000 header, 300 MiB zero bomb	305,767 B	486,740,812 B	494 MB

The last row is a 299 KB file that passes the 1 MiB rule and makes the Hygiene job — which runs on every pull
request, forks included, on Python 3.10 — allocate about half a gigabyte; the peak is ~2× expected because
CPython grows the output buffer by copying. This is not a regression (on main any ~1 MiB bomb reaches the
same peak with any header), it is disclosed in the PR body, and it is harmless on a 16 GB runner inside a
5-minute job, so I am not blocking on it. But the issue's own wording is "so a small compressed bomb cannot
use large memory in the Hygiene job", and that holds only for an honest header.

The cheap fix, here or in a follow-up: lower _MAX_INFLATED. The largest asset the dimension rules allow
needs 19,671,932 bytes and the largest embedded figure in the repository today needs 5,030,535, so 32 MiB
keeps 1.6× headroom over any legal asset and 6× over today's figures while cutting the worst case from
~490 MB to ~64 MB. (Passing the asset path a tighter cap than the embedded path would be stricter still, at
the cost of a parameter on inspect_png.)

S2. A test comment — and one row of the PR body — states something that is not true

tests/test_check_recipe_assets.py:866-867:

One byte too many is refused and the exact length is accepted, so neither a limit that is one too small
(which would reject every good file) nor one that is too large is possible.

Both halves are false, and I checked each by mutating the tool myself:

• A limit one too small does not reject good files, and this test does not detect it. With
  decompress(bytes(idat), expected), test_the_inflate_limit_is_exactly_one_byte_past_the_expected_length
  passes (1 passed). zlib returns Z_STREAM_END when it reads the final block and the trailer even though
  the output buffer filled exactly, so eof is True and a good file is accepted. Verified directly on zlib
  1.2.13 and 1.3.2: maxlen=1000 on a 1000-byte payload gives len(out)=1000, eof=True, unconsumed=0. The
  mutant is killed, but by test_idat_is_inflated_no_further_than_the_length_the_header_implies and
  test_the_inflated_idat_length_must_match_the_image_header — i.e. because an over-long stream then reports
  "does not end cleanly" instead of "inflates past", by the message, not by a rejected good file. The body's
  row
  "limit expected (no + 1) | killed (exact-length file rejected)" has the right verdict and the wrong
  reason.
• A limit that is too large is possible. expected + 2 survives the whole file (432 passed). Only margins
  of order a megabyte are caught, and only through the 2 MiB peak bound — which the body does say honestly.

What the + 1 actually buys is worth writing down instead, because it is a real argument: it keeps the accept
path off the output limit entirely, so a valid file never depends on zlib's exact-fill behaviour, and one
spare byte is what makes "too long" distinguishable from "does not end cleanly". Please reword the comment
and that table row, and consider renaming the test: what it pins is "at least expected, and over-length is
reported as over-length", not "exactly one".

S3. _MAX_INFLATED is pinned by nothing, and the new refusal path can become a false positive

tools/check_recipe_assets.py:156, 297-302. Lowering the constant to 16 MiB passes all 432 tests, yet it
then rejects the largest asset the dimension rules allow — 1568×1568 RGBA 16-bit interlaced, 19,671,932 bytes
raw in a 19 KB file — with "the image header implies 19,671,932 bytes of pixel data; the most inflated is
16,777,216". I built that file and ran it. Before this PR a wrong value there could only widen a limit; now
it can reject a legal image, so it deserves a test: four lines next to the backstop test that build that
19 KB file and assert it is accepted. That also records why the constant has to stay above ~20 MB — useful if
anyone acts on S1.

S4. The peak measurement is sound; two lines would make it robust

tests/test_check_recipe_assets.py:853-862. Choosing tracemalloc over RSS is right: it is deterministic, it
needs no resource module (so it works on Windows, which contributors here use), and the two interpreters
agree to within 1% (962,161 vs 970,461 bytes) while the uncapped code peaks at 219,758,182 — a 200× margin, so
the 2 MiB bound is in no danger of flaking on its own. The hole is the surrounding bookkeeping:
tracemalloc.start() is a no-op when tracing is already on, get_traced_memory() then returns the peak of
the whole session, and the finally: tracemalloc.stop() switches off tracing that something else turned on.
Under PYTHONTRACEMALLOC=1 the test fails for that reason alone — I reproduced it, "peak 4,851,256 bytes".
Guard it:

    started = not tracemalloc.is_tracing()
    tracemalloc.start()
    tracemalloc.reset_peak()
    try:
        ...
    finally:
        if started:
            tracemalloc.stop()

S5 (minor). "without inflating" is not observed

tests/test_check_recipe_assets.py:874-879 proves the rule and the message, not the absence of the inflate.
If the name is to carry that claim, monkeypatch.setattr(assets.zlib, "decompressobj", boom) would make it
literal.

S6 (minor). png_with_idat replaces only the first IDAT

tests/test_check_recipe_assets.py:814-819. Correct today — Pillow 13 writes one IDAT for the flat 320×320
png(); I checked the chunk list is [('IHDR', 13), ('IDAT', 954), ('IEND', 0)] — but on a base image Pillow
splits, the helper would leave a second IDAT after the spliced body and the tests built on it would be
measuring something else. Dropping every other IDAT in the same pass makes it unconditional.

S7 (minor). The PR body overstates the docs/vision.md item

The body says that edit "is listed as an acceptance item on the issue". Issue #329 has exactly three
acceptance criteria and none mentions docs/vision.md; neither linked review comment asks for it either (the
#321 comment only observes that the 1 MiB and 16 MiB caps coexist on purpose). The body also says the doc
now "says the loader's 16 MiB limit applies to the loader", which is slightly ahead of the text: the diff adds
a parenthetical pointing committed fixtures at the 1 MiB rule. The edit itself is accurate, one sentence long,
in the spirit of the follow-up and consistent with docs/fixtures.md:351, and the relative link resolves — I
would keep it and fix the sentence in the body.

Status of the prior findings this issue tracks

Prior finding	Status
#321 comment 6305791542, suggestion 1 — "make the cross-pin unconditional … replace the getattr with a direct attribute access and delete the comment that says 'until it does'"	Resolved. tests/test_check_recipe_assets.py:1124-1131 uses plain vision.IMAGE_LIMITS[k] and the comment is gone. It bites on the checker side alone: renaming IMAGE_LIMITS throughout vision.py fails exactly one test in that file with AttributeError, and max_area = 2_458_625 in vision.py fails exactly one test in that file.
#322 comment 6305113842, S9 — "removing if not inflater.eof or inflater.unconsumed_tail: now survives … a test with zlib.compress(rows)[:-4] would pin it"	Resolved. The new test uses both the missing-Adler stream and a Z_SYNC_FLUSH stream, each asserted (in the test itself, so a zlib version difference would fail loudly rather than weaken it) to inflate to exactly the right rows. if not inflater.eof: → if False: is killed by this test and by nothing else.
#322 comment 6305113842, S10 — "inflater.decompress(idat, expected + 1) bounds memory to about 20 MB for the largest legal asset"	Resolved as to the code; the stated bound holds only for an honest header. The call is exactly what was asked for and is tested, but see S1: "about 20 MB" is not the worst case, and no test shows the residual.

Acceptance criteria of #329

1. Compare the checker's limits with vision.IMAGE_LIMITS by plain attribute access, not a skip-if-missing
   lookup, so a rename cannot turn it into a silent pass — met
   (tests/test_check_recipe_assets.py:1124-1131). The old form was a silent pass by construction
   (getattr(..., None) then if limits is not None); the new form raises AttributeError on a rename, which
   I demonstrated, and also catches a value divergence.
2. A test exercises the "zlib stream ended cleanly" check on its own (a complete IDAT stream whose final
   block or checksum is missing); the mutant that removes the check is killed — met. Two independent
   streams, exact rule and message asserted, mutant killed by this test alone.
3. IDAT decompression stops at the expected decompressed length (plus a small margin) instead of allowing up
   to 256 MiB, so a small compressed bomb cannot use large memory in the Hygiene job; tested — met, with
   the caveat in S1. The stop is implemented and tested (962,161 bytes peak against 219,758,182 without it),
   but a header claiming large dimensions still raises the bound to the backstop (measured peak
   486,740,812 bytes). The PR body discloses this and asks reviewers to choose; I am content to see it
   tightened in a follow-up.

The docs/vision.md item the body claims is not an acceptance criterion (S7); the edit itself is accurate.

The PR body against the diff

Accurate except where S2 and S7 say otherwise — in particular every row of the mutation table has the right
verdict, and the "worst case remains bounded by the header, not by a fixed small number" paragraph is an
honest statement of S1. Numbers I reproduced:

• "Full pytest on 3.14: 5896 passed, 49 skipped, 50 deselected, 31 xfailed" — I get 5897 passed, 49
  skipped, 50 deselected, 31 xfailed, 0 failed, in 195 s. One more passing test than the body says; the
  likeliest explanation is that the body's run predates the merge of origin/main into the branch. Nothing
  fails either way.
• "On 3.10: tests/test_check_recipe_assets.py tests/test_vision.py 432 passed" — reproduced exactly, 432
  passed on 3.10.21; tests/test_check_recipe_assets.py alone is 165 passed on 3.10.21.
• ruff check . and ruff format --check . — both pass (401 files formatted).
• "check_hygiene.py: passed (611 files)", "check_recipe_assets.py: passed (461 files)",
  "render_catalog.py --check: exit 0" — all three reproduced exactly.
• "Red before: with tools/check_recipe_assets.py from origin/main, the new test file gives 2 failed (the
  bomb test and the backstop test), 163 passed; the zlib end test passes on old code" — reproduced exactly:
  with tools/check_recipe_assets.py from 39ddfce, 2 failed, 163 passed, and the two are those tests.
  One detail the body does not mention: red-before, the bomb test fails on its message assertion (the old
  code inflates the lot and says "IDAT inflates to … bytes; the image header implies 307,520"), not on the
  peak, because the message is asserted first — the 2 MiB peak bound
  is what kills the _MAX_INFLATED mutant, not what makes the test red against main. Both are covered, but
  they are different assertions.

Mutants I applied myself

Each applied alone to the worktree and run against tests/test_check_recipe_assets.py +
tests/test_vision.py (432 tests), then the file restored byte for byte; git status --porcelain empty
afterwards.

Mutant	Result
if not inflater.eof: → if False:	killed — test_a_complete_deflate_stream_must_still_end_with_its_final_block_and_checksum
limit → _MAX_INFLATED	killed — bomb test, "peak 219,758,182 bytes"
limit → expected (no + 1)	killed (2 tests) — but not by the off-by-one test; see S2
limit → expected + 2**20	killed — bomb test, "peak 3,057,912 bytes"
limit → expected + 2	survived (see S2)
if len(inflated) > expected: → if False:	killed (2 tests)
if len(inflated) > expected: → >=	killed (42 tests)
if expected > _MAX_INFLATED: → if False:	killed — the new backstop test
if expected > _MAX_INFLATED: → >=	survived (unreachable boundary, harmless)
if len(inflated) != expected: → if False:	killed — the short-stream case
if inflater.unused_data: → if False:	killed — test_idat_must_be_one_clean_zlib_stream
interlace → 0 in the expected computation	killed — test_sixteen_bit_and_interlaced_pngs_are_accepted
restore or inflater.unconsumed_tail	survived — confirms the removal is behaviour-preserving
_MAX_INFLATED → 16 MiB	survived (see S3)
vision.IMAGE_LIMITS["max_area"] → 2,458,625	killed, including one test inside the checker's own file
IMAGE_LIMITS renamed throughout vision.py	killed with AttributeError, including one test in the checker's file

Checks I ran

Fresh venvs under /home/agent/r1-pr334/ (uv venv --python 3.14 and --python 3.10,
uv pip install --link-mode symlink -e ".[dev]"), every heavy command under
flock .claude/heavy.lock nice -n 10, pytest -p no:cacheprovider, no nested flock, long runs backgrounded
with log polling, no network, no key.

• ruff check . — All checks passed.
• ruff format --check . — 401 files already formatted.
• pytest (full, Python 3.14.7) — 5897 passed, 49 skipped, 50 deselected, 31 xfailed, 0 failed, 195 s.
• pytest tests/test_check_recipe_assets.py tests/test_vision.py — 432 passed on 3.14.7;
  432 passed on 3.10.21. tests/test_check_recipe_assets.py alone — 165 passed on 3.10.21, the floor the Hygiene
  job runs on.
• python tools/check_hygiene.py — passed, 611 files.
• python tools/check_recipe_assets.py — passed, 461 files under recipes/.
• python tools/render_catalog.py --check — exit 0.
• My own probes (standard library only, synthetic in-memory PNGs, run on 3.10.21 and 3.14.7): the
  25,245-combination png_raw_length cross-check; the exact/±1 boundary per colour type and bit depth,
  interlaced and not; zlib max_length semantics on zlib 1.2.13 and 1.3.2; the three bomb measurements in S1;
  inspect_embedded over all 134 images in the 51 committed notebooks; the largest legal asset; 16 mutants.
• No notebook executed. The diff changes no notebook, no fixture and nothing a notebook imports: no recipe
  imports jev_cookbook.vision, and tools/check_recipe_assets.py is loaded only by the checkers
  (tools/check_notebook_fresh.py:172-176). The notebook-facing effect of this change is inspect_embedded,
  so I exercised that over every image in every committed notebook instead, with no finding; CI's
  Notebook (…) jobs were green on this head where they had finished when I looked.
• git status --porcelain empty at the end. The only files I created are under /home/agent/r1-pr334/
  (deleted when I finished) and this review.

Outside this PR, noticed but not to be fixed here

• check_image records file-too-large and then parses the file anyway, and tracked_entries has already
  read every tracked byte into memory, so the per-file 1 MiB rule does not by itself bound the checker's
  memory. Pre-existing, unrelated to this diff.
• A PNG whose only IDAT chunk has zero length reports "no IDAT chunk" rather than an empty-stream error.
  Pre-existing, cosmetic.

OPUS-REVIEW: APPROVE ba797d6

[CompleteDotTech]

PR #334 — Opus review 2 of 2 (independent)

• Agent ID: agent-a16b174310531b11e (worktree .claude/worktrees/agent-a16b174310531b11e)
• Model: Opus 5 (1M context), resolved model ID claude-opus-5[1m]
• Head reviewed: ba797d67e533e85ecc9d526ae080e78dbf3a1bad — fetched with git fetch origin pull/334/head,
  confirmed against gh pr view 334 --json headRefOid. Merge commit, good ED25519 signature
  (Good "git" signature for timothy.gregg@complete.tech, key SHA256:4gVoHVYiiNpL+UmRnjalQ6GDYvwXEuuQuprjiQys2/c).
• Base used: 39ddfce (the PR's merge-base and origin/main at the time I fetched). Parents of the head are
  2353585 (the single work commit, also signed) and 39ddfce.
• Issue: Foundation: image-check hardening follow-ups from the #321/#322 reviews (pin test, zlib end check, decompression cap) #329, plus the review items it lists: PR Foundation: enforce the Images API's 280 px minimum, hash image bytes in replay keys, bound tiles (#306) #321 comment 6305791542 (suggestion 1) and PR Foundation: image-file contract, checker, scope rule and notebook image policy (#307) #322 comment
  6305113842 (S9, S10), plus issue comment 6305811981 (the docs/vision.md cross-reference).
• Independence and safety: I formed my view of the diff before reading any prior review comment, and I have not
  read the other Foundation: image-check hardening follow-ups (pin test, zlib end check, inflate cap) (#329) #334 reviewer's findings. No live call, no TYPESAFE_API_KEY, no credential search.
• Note on origin/main: it advanced to b627e3c (Foundation C: public dump_responses, honest run-mode sentence, matrix and forced-metric template fixes #317) during this review. git diff --name-only 39ddfce b627e3c
  touches none of this PR's three files, and GitHub still reports mergeable: MERGEABLE, mergeStateStatus: CLEAN,
  so no re-merge is needed on technical grounds.

Verdict

No must-change findings. Approve. The change is a strict improvement, it is verdict-preserving on everything I
could build, and the one residual (the memory bound rests on an attacker-controlled header) is disclosed in the PR
body. The suggestions below are for a follow-up; the one thing I would like on the record is that the PR body's
memory figures are understated by roughly a factor of two (S1), which does not change the conclusion but does change
how large the residual is.

Scope

Three files, all 100644 → 100644 modifications, no add, delete, rename, mode or type change:

:100644 100644 cd91bf8 57990b5 M  docs/vision.md
:100644 100644 a3eb45b 615304b M  tests/test_check_recipe_assets.py
:100644 100644 fc79a29 0752db4 M  tools/check_recipe_assets.py

git show --remerge-diff ba797d6 is empty: the merge resolved nothing by hand. The diff of the work commit alone
(2353585^..2353585) has the same stat as the diff against the new base, +97 −12, which matches gh pr view's
additions/deletions exactly. The docs/vision.md edit is outside "the checker and its tests", and the PR body
flags that; it is in scope, because issue comment 6305811981 lists it as an acceptance item.

Branch name foundation/image-check-hardening, Closes #329 and no other issue reference anywhere in the body
(I scanned for every spelling of close/fix/resolve: the only other hit is the ordinary English "a fixed small
number", which GitHub links nothing from). The work commit's message says Refs #329. Both commits carry
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>. The recipe checklist is correctly skipped for
foundation work, as .github/pull_request_template.md instructs.

The central question: does the new checker ever disagree with the old one?

This file is the gate in the Hygiene job on every PR, so a verdict change on a real or plausible image is the thing
that matters. I built the old checker from 39ddfce (md5 aa34b50f9a1daa27da6cf598647049a7, byte-identical to what
git show 39ddfce:tools/check_recipe_assets.py gives) and ran both side by side.

Reasoned first. The accept condition is unchanged in substance. OLD accepts iff eof, no unused_data and
len(inflated) == expected (which forces expected ≤ 256 MiB, since the output was capped there). NEW accepts iff
expected ≤ 256 MiB, eof, no unused_data and len(inflated) == expected. Each implies the other, so the two
agree on every input; only the message on some already-failing inputs can move, because the smaller max_length
makes NEW stop earlier and report the over-length defect instead of whatever the old code noticed afterwards.

Then measured. 33 hand-built cases (valid RGB8 / RGB16 / interlaced / palette / 1-bit grey / split-IDAT; short
by one; long by one, by 1 MiB, by 100 MiB and by 300 MiB; missing Adler-32; Z_SYNC_FLUSH-ended with short, exact
and long data; truncated mid-stream; trailing bytes; non-zlib IDAT; headers of 9000², 20000², 9450², 9460² and
0xFFFFFFFF² at 16-bit RGBA; under- and over-size dimensions; and six embedded-notebook shapes including a
matplotlib Software chunk, a 3000×2400 figure, a 2000×2000 RGBA figure and a bomb):

• Pass/fail verdict changes: 0. Rule-class changes: 0 on all 33.
• 10 message changes, every one on a file that failed under both checkers. They are all of the expected shape, e.g.
  IDAT inflates to 104,857,600 bytes; the image header implies 307,520 →
  IDAT inflates past the 307,520 bytes the image header implies, and for an absurd header
  IDAT inflates to 307,520 bytes; the image header implies 1,200,020,000 →
  the image header implies 1,200,020,000 bytes of pixel data; the most inflated is 268,435,456.
• Identical results on Python 3.10.21 and 3.14.7 (diff of the two corpus outputs is empty).

One rule-class change exists, outside that corpus. A PNG whose IDAT both inflates past expected and carries
bytes after the zlib stream now reports png-structure instead of trailing-data:

case	OLD	NEW
zlib.compress(bytes(expected + 1)) + b"junkjunk"	trailing-data, "8 byte(s) after the zlib stream in IDAT"	png-structure, "IDAT inflates past the 307,520 bytes the image header implies"

Still a failure either way, and arguably the better message: the over-length is the real defect and the trailing byte
count was the incidental one. I report it because it is the only rule-label difference I found and the brief asks for
every difference to be explained.

The backstop boundary is exact and inclusive. I solved for a header whose implied length is exactly
_MAX_INFLATED (width 1, height 67,108,864 → 268,435,456) and for one byte past it. The first goes through the
inflate path under both checkers and gets the identical old message; the second is the first case the new backstop
refuses. That matches > rather than >= and matches the wording "the most inflated is 268,435,456".

The whole real tree. Both checkers print Image-asset check passed: 461 file(s) under recipes/ checked. and
diff of the two outputs is empty. Worth stating plainly: git ls-files recipes | grep -icE '\.(png|jpg|jpeg)$'
is 0 — there is no committed image in the repository today, so inspect_png is never reached on the real tree
and the Hygiene job's behaviour is literally unchanged at this moment.

Real notebook figures. I decoded every image embedded in every committed notebook — 51 notebooks, 134 embedded
images — and ran inspect_embedded with both checkers: 0 verdict differences, all accepted. I then generated
15 fresh matplotlib figures (3 plot kinds × (6,4)@72, (6,4)@100, (10,8)@200, (16,12)@300, (2,1)@50, up to
4800×3600 and 625 KB): 0 differences, all accepted. So the new cap raises no false positive on the figures CI
actually produces, which was S10's worry in reverse.

Memory and wall time

Peak traced allocation while checking a 305,767-byte deflate bomb that inflates to 300 MiB (identical on 3.10.21 and
3.14.7; the 3.14 numbers are shown):

header in the file	OLD peak	NEW peak
320 × 320 (honest-looking)	537,568,850 B	1,572,843 B
9450 × 9450 → implies 267,916,950 (just under the backstop)	537,568,538 B	536,532,031 B
20000 × 20000 → implies 1,200,020,000 (over the backstop)	537,568,538 B	613,538 B

So the realistic case improves by a factor of ~342, and the absurd-header case by ~876. The middle row is the
residual, and see S1 for what the PR body says about it.

Wall time is neutral to better. Over the whole tree, in-process, 7 runs each: OLD median 1.6 ms, NEW median 1.6 ms
(461 entries, 0 findings) — no image is reached, as noted. Per image, where the change can show up, NEW is faster,
because decompress no longer grows its output buffer toward 256 MiB:

image	OLD	NEW
flat 1568 × 1568 RGB8	23.73 ms	13.79 ms
noisy 560 × 480, 807 KB file	0.54 ms	0.60 ms
flat 320 × 320	0.56 ms	0.54 ms
noisy 3000 × 2400 embedded figure (21.6 MB)	50.78 ms	32.42 ms

The Hygiene job's 5-minute timeout is in no danger from this change.

Docs

The docs/vision.md edit reads: "The accepted local files are PNG or JPEG stills, at most 16 MiB (a committed
fixture is held to the stricter 1 MiB, no-metadata asset rule in docs/fixtures.md), …".

• The 16 MiB claim is true of the loader. src/jev_cookbook/vision.py:63 sets IMAGE_LIMITS["max_bytes"] = 16 * 1024 * 1024 and lines 78 and 123 raise on it.
• The 1 MiB rule lives in docs/fixtures.md, in the "The image-file contract" table under "Writing fixtures →
  Image fixtures": the Size row says "At most 1 MiB (1,048,576 bytes) per file, and at most 8 MiB in all in one
  recipe's assets/ folder. These caps are this repository's, far below the API's 16 MiB per file".
• The "no-metadata" half is also there, in the Metadata row of the same table (PNG limited to IHDR, PLTE,
  IDAT, IEND, tRNS, gAMA, cHRM, sRGB, sBIT, pHYs, bKGD; JPEG limited to a plain JFIF APP0).
  "No-metadata" is shorthand — rendering chunks and a JFIF header are allowed — but the row it points at says so.
• The relative link fixtures.md resolves from docs/. See S5 for the anchor nit.

The constants in tools/check_recipe_assets.py are unchanged, and nothing in docs/, CONTRIBUTING.md or
AGENTS.md mentions _MAX_INFLATED or a 256 MiB cap, so the new finding message introduces no documentation drift.

Findings

Must change

None.

Suggestions

S1 (medium, for the record and for a follow-up). The PR body understates the memory numbers by about 2×, and the
bound still rests on the header. tools/check_recipe_assets.py:297-308. The body says a bomb now "allocates about
one image, not up to 256 MiB", and that a hostile under-backstop header "can still make the Hygiene job inflate up to
that much". Both are directionally right and the second is an honest disclosure, but the measured peaks above are
~512 MiB, not ~256 MiB, in both the old worst case and the surviving one. The cause is CPython's zlib output buffer:
it grows toward max_length and the returned bytes is a separate object, so the peak allocation is roughly twice
the cap. _MAX_INFLATED bounds the returned bytes, not the process's peak. Concretely, a 300 KB file whose IHDR
says 1 × 67,108,864 (or 9450 × 9450) still drives the Hygiene job to ~512 MiB, which is the full pre-PR worst case —
so issue #329's purpose clause, "a small compressed bomb cannot use large memory in the Hygiene job", holds only for
a file whose header is honest.

The builder asked for a decision on this ("Say if you want either"). My preference, for a follow-up rather than this
PR: check the dimension limits before inspect_png inflates, for non-embedded files only. That caps expected
at png_raw_length(1568, 1568, 6, 16, 0) = 19,677,824 for every file the Hygiene job reads — which is exactly S10's
"about 20 MB for the largest legal asset" — and leaves the embedded path, where there is no dimension limit,
unchanged. The builder's reason for not reordering was that it "would change which finding a too-large image gets";
that is a message change on an already-failing file, the same class of change this PR already makes (see the
trailing-data → png-structure row above), so I do not think it is a reason to avoid the reorder. Lowering
_MAX_INFLATED instead is the worse option, for the false-positive reason the builder gives.

Per CONTRIBUTING.md ("A pull-request description is written once, at open … and never edited after the final push"),
do not edit the description to fix the figures; put the corrected numbers in a PR comment or on the follow-up
issue.

S2 (low). The backstop constant's value is unpinned in both directions, and lowering it would be a silent
false-positive regression. tools/check_recipe_assets.py:156. Against tests/test_check_recipe_assets.py,
tests/test_vision.py and tests/test_notebook_ci_fresh.py (506 passed at baseline), these mutants survive:

mutant	result	what it would break
_MAX_INFLATED = 16 * 1024 * 1024	survived (506 passed)	rejects the largest legal asset — 1568 × 1568 RGBA 16-bit implies 19,677,824 — and any embedded figure implying over 16 MiB (a 3000 × 2400 RGBA figure implies 28,802,400)
_MAX_INFLATED = 4 * 1024**3	survived (506 passed)	the backstop stops bounding anything
if expected >= _MAX_INFLATED:	survived (506 passed)	the boundary the message states is not pinned

The reason is that the largest image anywhere in the suite is png(1568, 1568) at
tests/test_check_recipe_assets.py:108, a flat RGB8 file that implies only 7,377,440 bytes, and the new backstop test
monkeypatches the constant away (correctly, to test the mechanism). Two lines would close it: assert
assets._MAX_INFLATED > assets.png_raw_length(1568, 1568, 6, 16, 0), and accept one 1568 × 1568 16-bit RGBA file.
This is precisely the risk the builder names in "Decisions for the reviewers" ("Lowering _MAX_INFLATED would risk
false positives on embedded notebook figures"), so it is the thing worth pinning.

S3 (low). test_a_header_implying_more_than_the_backstop_is_refused_without_inflating does not test the "without
inflating" in its name. tests/test_check_recipe_assets.py:873-879. It asserts the rule and the message, both of
which would still hold if the backstop check were moved below zlib.decompressobj(). The ordering in inspect_png
makes the name true today, and nothing would fail if an edit broke it. Monkeypatching assets.zlib.decompressobj to
raise, or adding a tracemalloc bound as the bomb test does, would earn the name.

S4 (informational, PR body). The stated reason for dropping or inflater.unconsumed_tail is not the real one.
The body says the term was dropped "because the over-length check now covers it". The outcome is correct but the
reason is not: unconsumed_tail is non-empty only when the stream did not end, so the surviving not inflater.eof
already covered it, and the removal is a pure simplification. I probed six zlib cases on 3.10.21 and 3.14.7 (stopping
at max_length mid-stream, stopping exactly at the end of the data, one past the data, trailing bytes with and
without hitting the limit, and a missing Adler-32) and never saw eof=True with a non-empty unconsumed_tail.
Relatedly, the over-length check adds no new rejection: with it deleted (mutant M2) every bomb is still rejected by
not inflater.eof; what it adds is the accurate message, which is what kills the mutant.

S5 (low). Link the section, not the page. docs/vision.md:42-43 links fixtures.md with no anchor, while
docs/development.md:276 uses fixtures.md#image-fixtures. #image-fixtures would land the reader on the table that
actually carries the 1 MiB and metadata rows.

S6 (informational). One test-count digit in the PR body. The body reports "5896 passed, 49 skipped, 50
deselected, 31 xfailed" on 3.14; my full run at this exact head gives 5897 passed, with the other three numbers
identical and 0 failed. The diff's own contribution is exactly as claimed: tests/test_check_recipe_assets.py goes
from 161 to 165 collected tests. Also, the "New tests:" sentence names three, but four tests are new — the zlib end
test is new too; it has its own bullet above, so this is wording rather than an overclaim. Neither is worth a change.

The PR body against the diff

Everything I could check holds.

• Closes #329, once, and no other issue reference. ✓
• "+97 −12", three files, the files named. ✓
• The pin test is renamed and the "when the package exports them" qualifier is gone; plain attribute access. ✓
• The zlib end test builds exactly the two streams described, and both inflate to exactly rows (the test asserts
  that itself before checking the finding), so only the "does not end cleanly" rule can reject them. ✓
• "_MAX_INFLATED (256 MiB) stays only as a backstop: a header that implies more than that is refused before any
  inflating." ✓ — the raise precedes zlib.decompressobj().
• "The extra margin is exactly 1 byte … A larger margin (a mutant with + 2**20) is also killed … but a margin of a
  few hundred KiB would not be." ✓ and ✓ — I reproduced both: expected + 2**20 is killed, and expected + 100_000
  survives, exactly as disclosed. Honest.
• "Red before: … 2 failed (the bomb test and the backstop test), 163 passed." ✓ — I ran the PR's test file against
  39ddfce's checker and got precisely 2 failed, 163 passed, the two being
  test_idat_is_inflated_no_further_than_the_length_the_header_implies and
  test_a_header_implying_more_than_the_backstop_is_refused_without_inflating.
• "The old pin test with IMAGE_LIMITS renamed in vision.py still passes (silent pass); the new one fails." ✓ —
  with origin/main's test file and IMAGE_LIMITS renamed, the old pin test reports 1 passed; the new one dies
  with AttributeError.
• Every mutant in the body's table, re-run independently (see "Checks run"): 8 of 8 killed, matching the body
  including its reported cause in each row.
• Overstated: the memory magnitudes (S1), the reason for the unconsumed_tail removal (S4), one test count (S6).

Acceptance criteria of #329

criterion	verdict	evidence
tests/test_check_recipe_assets.py compares the checker's limits with vision.IMAGE_LIMITS by plain attribute access, not a skip-if-missing lookup, so a rename cannot turn it into a silent pass	met	tests/test_check_recipe_assets.py:1124-1131; getattr(..., None) and the if limits is not None guard are gone, the stale "until it does" comment with them. Renaming IMAGE_LIMITS in vision.py kills it (AttributeError); the old test passed silently under the same rename. A renamed key would also fail, via KeyError.
A test exercises the PNG "zlib stream ended cleanly" check on its own (a complete IDAT stream whose final block or checksum is missing); the mutant that removes the check is killed	met	tests/test_check_recipe_assets.py:822-845. Both streams inflate to exactly the implied row bytes, so neither the length check nor the over-length check can reach them; the test asserts the rule and the message, and accepts the same rows in a properly ended stream. Mutant if not inflater.eof: → if False: is killed (1 failed).
IDAT decompression stops at the expected decompressed length (plus a small margin) instead of allowing up to 256 MiB, so a small compressed bomb cannot use large memory in the Hygiene job; tested	met, with the residual in S1	tools/check_recipe_assets.py:296-315: expected is computed before any inflating, a header over the backstop is refused outright, and the inflate limit is expected + 1. Measured: a 300 KB / 300 MiB bomb behind a 320 × 320 header drops from a 537 MB peak to 1.57 MB. Three new tests cover it, including a tracemalloc bound. The "small compressed bomb" clause holds for an honest header; a header claiming just under 256 MiB still reaches ~512 MiB, which the PR body discloses (with understated figures).
(issue comment 6305811981) docs/vision.md cross-references docs/fixtures.md where it says the loader accepts files up to 16 MiB: committed fixtures are held to the stricter 1 MiB, no-metadata asset rule	met	docs/vision.md:42-43, quoted above. Both halves of the claim are in docs/fixtures.md's contract table (Size row, Metadata row), and the loader really does enforce 16 MiB (vision.py:63, 78, 123).

Status of the prior findings the issue lists

• Foundation: enforce the Images API's 280 px minimum, hash image bytes in replay keys, bound tiles (#306) #321 comment 6305791542, suggestion 1 ("make the cross-pin unconditional … replace the getattr with a direct
  attribute access and delete the comment that says 'until it does'"): resolved, literally as asked.
• Foundation: image-file contract, checker, scope rule and notebook image policy (#307) #322 comment 6305113842, S9 ("removing if not inflater.eof or inflater.unconsumed_tail: now survives … a test
  with zlib.compress(rows)[:-4] would pin it"): resolved. The test uses exactly that construction plus a
  Z_SYNC_FLUSH variant, and I confirmed the mutant is now killed.
• Foundation: image-file contract, checker, scope rule and notebook image policy (#307) #322 comment 6305113842, S10 ("inflater.decompress(idat, expected + 1) bounds memory to about 20 MB for the
  largest legal asset … stops a 1 MiB deflate bomb from allocating 256 MiB"): resolved as written — the call is
  exactly decompress(bytes(idat), expected + 1). The "about 20 MB for the largest legal asset" part of S10's
  rationale is not yet what the code guarantees, because expected comes from the header and the dimension limits are
  still applied after inspect_png; see S1.

Checks run

Fresh venvs under /home/agent/r2-pr334/, created with uv venv --python 3.14 / --python 3.10 and
uv pip install --link-mode symlink -e ".[dev]". Every heavy command ran under
flock .claude/heavy.lock nice -n 10, one at a time, with pytest -p no:cacheprovider and no nested flock. No
TYPESAFE_API_KEY in the environment. Python 3.14.7 and 3.10.21.

Python 3.14.7 (.[dev]), at the head

• ruff check . → All checks passed!
• ruff format --check . → 401 files already formatted
• python tools/check_hygiene.py → Hygiene check passed: 611 file(s) scanned. (body: 611 ✓)
• python tools/check_recipe_assets.py → Image-asset check passed: 461 file(s) under recipes/ checked. (body: 461 ✓)
• python tools/render_catalog.py --check → exit 0
• full pytest → 5897 passed, 49 skipped, 50 deselected, 31 xfailed in 197.28s, 0 failed
• python tools/notebook_ci.py fixtures → 51 recipe folders, 0 failed
• python -m jev_cookbook.fixtures validate --all → exit 0, all 51 folders valid (mode replay)

Python 3.10.21 (.[dev]) — the Hygiene interpreter and the Tests (py3.10) job

• pytest tests/test_check_recipe_assets.py tests/test_vision.py → 432 passed
• python tools/check_recipe_assets.py → passed (461 files); python tools/check_hygiene.py → passed (611 files)
• the edge-case corpus and the memory probe give results identical to 3.14

Behavioural comparison (old vs new)

• 33-case hand-built PNG corpus, check_image and inspect_embedded: 0 verdict changes, 0 rule changes, 10 message
  changes on already-failing files; identical on 3.10 and 3.14
• 7 further cases targeting the over-length/trailing interaction and the exact backstop boundary: 1 rule-label change
  (trailing-data → png-structure, documented above), 0 verdict changes
• whole tree with both checkers: identical stdout, diff empty
• 134 embedded images from all 51 committed notebooks: 0 differences, all accepted
• 15 freshly generated matplotlib figures up to 4800 × 3600: 0 differences, all accepted
• peak-memory table and per-image timing table above

Mutation runs (each applied alone, worktree restored byte-for-byte, git status clean after every run; against
tests/test_check_recipe_assets.py + tests/test_vision.py, baseline 432 passed)

mutant	result
if not inflater.eof: → if False:	killed
over-length check removed	killed
decompress(..., expected + 1) → _MAX_INFLATED	killed
limit expected (no margin)	killed
limit expected + 2**20	killed
limit expected + 100_000	survived (disclosed in the PR body)
backstop check removed	killed
MAX_PIXELS 2,458,625	killed
final len(inflated) != expected check removed	killed
IMAGE_LIMITS renamed in vision.py	killed (AttributeError)
_MAX_INFLATED = 16 MiB	survived (S2)
_MAX_INFLATED = 4 GiB	survived (S2)
expected >= _MAX_INFLATED	survived (S2)
trailing-data check removed	killed

Red-before

• the PR's test file against 39ddfce's checker: 2 failed, 163 passed — exactly the two the body names
• origin/main's pin test with IMAGE_LIMITS renamed: 1 passed (the silent pass this PR removes)

Notebooks. I executed none. No notebook can change: the diff touches the checker, its tests and one sentence of
docs/vision.md; no recipe imports jev_cookbook.vision or the checker; and the only path from this diff to notebook
CI is tools/check_notebook_fresh.py → inspect_embedded, which I verified gives identical verdicts on all 134
committed embedded images and on 15 fresh matplotlib figures. All 51 Notebook (<recipe>) jobs are green on this head.

Repository state. git status clean at the end; the three mutation harnesses each restore and assert
byte-equality. gh pr checks 334: 60 of 60 pass; check-runs for ba797d6 has 0 non-success conclusions;
mergeable: MERGEABLE, mergeStateStatus: CLEAN. My venvs and scratch files live only under
/home/agent/r2-pr334/ and will be deleted; I removed nothing I did not create.

Outside my task, reported not fixed

• There is no committed image anywhere under recipes/ (git ls-files recipes has zero .png/.jpg/.jpeg
  entries). The image-file contract, this checker and all of Foundation: no contract or check for a recipe's image files (location, format, size, metadata), and the hygiene, scope and freshness checks treat binary assets inconsistently #307/Foundation: enforce the Images API's 280 px minimum, hash image bytes in replay keys, bound tiles (#306) #321/Foundation: image-file contract, checker, scope rule and notebook image policy (#307) #322/Foundation: image-check hardening follow-ups from the #321/#322 reviews (pin test, zlib end check, decompression cap) #329 are therefore enforced today only
  against notebook-embedded output and against whatever a future PR adds. That is fine and probably intended, but it
  means the Hygiene job has never actually run inspect_png on a tracked file, so no amount of green CI on main
  exercises this path — the test suite is the only thing that does.
• origin/main moved to b627e3c (Foundation C: public dump_responses, honest run-mode sentence, matrix and forced-metric template fixes #317) mid-review, as noted at the top.

OPUS-REVIEW: APPROVE ba797d6

[CompleteDotTech]

PR #334 — delta re-review (reviewer 1 of 2)

• Agent: reviewer 1 (worktree agent-a2647921104fb0455), same session that wrote
  .claude/reviews/pr334-r1.md; I have still not seen reviewer 2's review.
• Model: claude-opus-5[1m] (Opus 5, 1M context).
• New head: cfb4fb0d9dbea1e5b938a9e21579a2aad25dc512 (fetched with git fetch origin pull/334/head;
  gh pr view 334 --json headRefOid agrees). Good ED25519 signature.
• Base: b627e3c2dd723f5b38b2e67a3c1ad0e0c3c94134 — current origin/main (Foundation C: public dump_responses, honest run-mode sentence, matrix and forced-metric template fixes #317), and the merge-base.
• Previously approved head: ba797d67e533e85ecc9d526ae080e78dbf3a1bad.

I accept the delta

The delta is the merge commit alone and it carries no content:

• git show --cc cfb4fb0 prints the commit message and no diff hunk; git show --remerge-diff cfb4fb0
  is likewise empty, so nothing was resolved by hand. Parents are exactly ba797d6 and b627e3c.
• git diff --raw -M origin/main...HEAD is still the PR's three files, all 100644 -> 100644:
  docs/vision.md, tests/test_check_recipe_assets.py, tools/check_recipe_assets.py.
• The three blobs are byte-identical to the head I approved: 57990b50…, 615304b4…, 0752db4e… at
  both ba797d6 and cfb4fb0. git diff --stat b627e3c..HEAD is the same +97 / −12.
• #317's 159-file delta (39ddfce..b627e3c) touches none of the PR's three files.

So nothing I reviewed has changed, and every finding and measurement in pr334-r1.md still stands as written.
My verdict on the code is unchanged.

The body's "Corrections after review": one of the four is wrong

Body correction	My measurement
"Two mutants in the table above survive the suite: expected without the + 1, and expected + 2." And the table row for expected (no + 1) now reads "not killed".	Half wrong, and it needs fixing. expected + 2 does survive (confirmed again at this head: 432 passed). But expected without the + 1 is killed: at this head it gives 2 failed, 430 passed. My original S2 said exactly this — "The mutant is killed, but by test_idat_is_inflated_no_further_than_the_length_the_header_implies and test_the_inflated_idat_length_must_match_the_image_header … by the message, not by a rejected good file." What is wrong is only the reason the first table version gave ("exact-length file rejected"); the verdict "killed" was right. The rest of that bullet is correct: good files are still accepted under that mutant, the off-by-one test still passes, and the comment near tests/test_check_recipe_assets.py:866 is wrong.
"a 300 KB bomb drove a peak of about 537 MB on the old code, and a hostile header just under the backstop still reaches about 536 MB, against about 1.6 MB for a realistic image. The bound still rests on the header."	Consistent with mine. I measured the doubling directly: a 9000×9000 header (implying 243,009,000 B) with a 300 MiB zero bomb peaks at 486,740,812 traced bytes / 494 MB RSS; a header just under the 256 MiB backstop would therefore land near 536 MB, as the body says. My realistic-image figure was 962,161 B traced (3.14) / 970,461 B (3.10) for the test's own case; "about 1.6 MB" is a different case but the same order and still inside the test's 2 MiB bound.
"_MAX_INFLATED is pinned by no test: 16 MiB or 4 GiB, or > to >=, survive, even though 16 MiB would reject the largest legal asset (1568×1568 RGBA 16-bit, implies 19,677,824 bytes)."	True, with one wrong number. All four survivals confirmed (16 MiB in my first review; 4 GiB and > → >= re-run here, 432 passed each). The largest legal asset implies 19,670,560 bytes non-interlaced and 19,671,932 interlaced, not 19,677,824 — I built both. The conclusion is unaffected: both exceed 16,777,216.
"An IDAT that is both over-length and followed by trailing bytes now reports png-structure rather than trailing-data."	True, verified: over-length plus 8 trailing bytes gives png-structure "IDAT inflates past the 307,520 bytes…", while an exact-length stream plus 8 trailing bytes still gives trailing-data "8 byte(s) after the zlib stream in IDAT".

The one thing to fix before merging (body only — no code change, no new commit, so no further re-review):
the mutant table row and the first correction bullet must not say the expected-without-+1 mutant survives.
It is killed, by two tests, on message assertions. Saying a gate's test suite leaves that mutant alive is both
untrue and more pessimistic than the facts. Suggested wording for the row: killed, but by the two message
assertions, not by the off-by-one test (good files are still accepted); the exact + 1 is a design choice, not
a pinned property.

Checks I ran at this head (fresh venv under /home/agent/r1b-pr334/, each heavy command under flock .claude/heavy.lock nice -n 10, pytest -p no:cacheprovider, no nested flock, long runs backgrounded with log polling, no network, no key)

• ruff check . — All checks passed. ruff format --check . — 404 files already formatted.
• pytest (full, Python 3.14.7) — 5985 passed, 55 skipped, 50 deselected, 31 xfailed, 0 failed, 220 s.
  (More tests than at ba797d6 because Foundation C: public dump_responses, honest run-mode sentence, matrix and forced-metric template fixes #317 brought its own; nothing fails.)
• pytest tests/test_check_recipe_assets.py tests/test_vision.py — 432 passed on 3.14.7 and 432 on 3.10.21.
• python tools/check_hygiene.py — passed, 614 files. python tools/check_recipe_assets.py — passed, 461
  files under recipes/. python tools/render_catalog.py --check — exit 0.
• Four mutants re-applied at this head, each alone, file restored byte for byte (diff identical afterwards,
  git status --porcelain empty): expected (no + 1) → killed, 2 failed; expected + 2 → survived;
  _MAX_INFLATED = 4 GiB → survived; backstop > → >= → survived.
• Re-measured for the body's figures: the largest legal asset's raw length (19,670,560 / 19,671,932), and the
  over-length-plus-trailing-bytes rule label.
• No notebook executed: the diff is unchanged and touches no notebook, fixture or anything a notebook
  imports; the 51 Notebook (…) CI jobs at this head are green.

CI

• PR Foundation: image-check hardening follow-ups (pin test, zlib end check, inflate cap) (#329) #334 at cfb4fb0: complete and all green. gh pr checks 334 gives 60 rows, 60 pass, 0 pending,
  0 failing; per check, gh pr view 334 --json statusCheckRollup is SUCCESS/COMPLETED for all 60 (Lint,
  Catalog, Tests py3.10, Tests py3.14, Hygiene, Scope, Notebooks discover, Notebooks execute, Fixtures
  validate, and the 51 per-recipe notebook jobs). headRefOid re-confirmed as cfb4fb0 after the run.
• main at b627e3c: gh run list --branch main shows the three workflows for Foundation C: public dump_responses, honest run-mode sentence, matrix and forced-metric template fixes #317 (CI, Hygiene,
  Notebooks) all completed success.

OPUS-REVIEW: APPROVE cfb4fb0

[CompleteDotTech]

PR #334 — Opus delta re-review (reviewer 2 of 2)

• Agent ID: agent-a16b174310531b11e (worktree .claude/worktrees/agent-a16b174310531b11e)
• Model: Opus 5 (1M context), resolved model ID claude-opus-5[1m]
• New head: cfb4fb0d9dbea1e5b938a9e21579a2aad25dc512
• Previously approved head: ba797d67e533e85ecc9d526ae080e78dbf3a1bad (my review pr334-r2.md)
• Base: origin/main = b627e3c2dd723f5b38b2e67a3c1ad0e0c3c94134, which is also git merge-base origin/main HEAD
• Parents of the head: ba797d6 and b627e3c. Good ED25519 signature
  (timothy.gregg@complete.tech, key SHA256:4gVoHVYiiNpL+UmRnjalQ6GDYvwXEuuQuprjiQys2/c).
• No live call, no TYPESAFE_API_KEY. I have still not read reviewer 1's review of this PR.

I accept the code delta

The merge itself is clean and I accept it without reservation. My approval of the diff at ba797d6 carries over
unchanged to cfb4fb0:

• git show --cc cfb4fb0 is content-free (empty combined diff), and git show --remerge-diff cfb4fb0 is empty
  too: nothing was resolved by hand.
• git diff --raw -M origin/main...HEAD is still exactly the PR's three files, all 100644 → 100644 M:
  docs/vision.md (cd91bf8 → 57990b5), tests/test_check_recipe_assets.py (a3eb45b → 615304b),
  tools/check_recipe_assets.py (fc79a29 → 0752db4). The two-dot diff is identical. The blob SHAs are the same
  ones I reviewed at ba797d6, and git diff ba797d6 cfb4fb0 -- <those three files> is empty, so the reviewed
  content is byte-identical.
• git diff --stat ba797d6 cfb4fb0 is exactly 159 files changed, 1420 insertions(+), 712 deletions(-) — precisely
  b627e3c's own change (git diff --name-only 39ddfce b627e3c is the same 159 files) and nothing else.
• No overlap: none of docs/vision.md, tests/test_check_recipe_assets.py, tools/check_recipe_assets.py
  appears in b627e3c's own change.

One blocking problem, in the new body text only

The first bullet of "Corrections after review" is false. It says:

Two mutants in the table above survive the suite: expected without the + 1, and expected + 2. Only the
+ 2**20 margin is killed … The comment near tests/test_check_recipe_assets.py:866 that says the first is
killed is wrong.

The expected (no + 1) mutant is killed, not surviving. I measured it twice at this head, against
tests/test_check_recipe_assets.py + tests/test_vision.py (the scope the table itself names; baseline 432 passed),
and captured the failing tests and their assertion text:

FAILED tests/test_check_recipe_assets.py::test_idat_is_inflated_no_further_than_the_length_the_header_implies
  tests/test_check_recipe_assets.py:859: AssertionError:
  assert [('png-structure', 'IDAT zlib stream does not end cleanly')]
      == [('png-structure', 'IDAT inflates past the 307,520 bytes the image header implies')]
FAILED tests/test_check_recipe_assets.py::test_the_inflated_idat_length_must_match_the_image_header
  tests/test_check_recipe_assets.py:965: AssertionError:
  assert 'the image header implies' in 'IDAT zlib stream does not end cleanly'

So the original table row was right ("limit expected (no + 1) | killed"), and the correction wrongly retracts
it. Only the row's stated reason was wrong: it said "exact-length file rejected", whereas good files are still
accepted (zlib reaches Z_STREAM_END when max_length equals the output length exactly — I probed this on 3.10.21
and 3.14.7). The mutant dies on the two message assertions above, one of them in a pre-existing test.

Consequently "Only the + 2**20 margin is killed" is also false — two margin mutants are killed, expected and
expected + 2**20.

The third clause needs a smaller fix. The comment at tests/test_check_recipe_assets.py:865-866 does not say the
mutant is killed; what it says is:

# One byte too many is refused and the exact length is accepted, so neither a limit that is
# one too small (which would reject every good file) nor one that is too large is possible.

Its parenthetical "(which would reject every good file)" is the wrong part, for the Z_STREAM_END reason above, and
it is fair to record that. Its conclusion is not what the correction attributes to it.

What to change (body text only, no code): rewrite that bullet to say that expected + 2 and
expected + 100_000 survive while expected (no margin) and expected + 2**20 are killed, and that the wrong thing
at :866 is the comment's reasoning, not the table's verdict. I would approve on that single edit.

I note two consequences of editing the description again: CONTRIBUTING.md says a description "is written once, at open
… and never edited after the final push", so this correction trail is already an exception the orchestrator has
chosen; and each edit fires another Scope (recipe pull requests) edited run. That is survivable — see below — but
it argues for getting the bullet right in one more edit rather than several.

The other three corrections are exactly true

Each matches my own measurements to the figure:

correction	my measurement	verdict
"a 300 KB bomb drove a peak of about 537 MB on the old code … a hostile header just under the backstop still reaches about 536 MB … about 1.6 MB for a realistic image. The bound still rests on the header."	537,568,850 B (old, 320×320 header); 536,532,031 B (9450×9450, just under the backstop); 1,572,843 B (new, 320×320). Same on 3.10.21 and 3.14.7.	exactly true
"_MAX_INFLATED is pinned by no test: 16 MiB or 4 GiB, or changing > to >=, survive … 16 MiB would reject the largest legal asset (1568x1568 RGBA 16-bit, implies 19,677,824 bytes)"	all three mutants survived (506 passed); png_raw_length(1568, 1568, 6, 16, 0) = 19,677,824	exactly true
"An IDAT that is both over-length and followed by trailing bytes now reports png-structure rather than trailing-data."	reproduced: zlib.compress(bytes(expected + 1)) + b"junkjunk" → OLD trailing-data, NEW png-structure	exactly true
"These are follow-up items, not defects in the verdicts the checker gives."	0 pass/fail verdict changes across 40 built cases, the whole tree, 134 committed embedded notebook images and 15 generated figures	consistent

The rest of the body is unchanged from the head I approved, and my pr334-r2.md findings on it stand (S4's
unconsumed_tail wording and S6's one-digit test count are still there; neither is worth another edit).

CI

• Head cfb4fb0: complete and green, per check. 61 check runs; [.check_runs[] | select(.status != "completed" or .conclusion != "success")] | length → 0. gh pr checks 334 collapses them to 60 pass, none pending,
  failed, cancelled or skipped. mergeable: MERGEABLE, mergeStateStatus: CLEAN.
• The 61st row is a duplicate Scope (recipe pull requests) (ids 119540544007 at 23:14:34 and 119546803467
  at 23:31:48, both app=github-actions, both completed/success) — the second fired by the body edit, exactly the
  Foundation: recipe 01 sync with the #124 template (#129, #130) #133/Readiness helper: duplicate successful Scope rows on one head (synchronize + edited) fail closed; decide the rule #138 scenario. It is benign: Scope (recipe pull requests) is not one of the five BASELINE_CHECKS
  (tools/check_merge_readiness.py:44-50), and an all-success same-app pair for a non-baseline name collapses into
  one success (tools/check_merge_readiness.py:314, documented at docs/merge-readiness.md:76 and :145). A
  further body edit keeps that property only while every Scope row stays success; an edit overlapping a run still
  in progress would produce a cancelled row that cannot be collapsed.
• main at b627e3c is green: gh run list --branch main shows Hygiene (19 s), CI (4 m 38 s) and
  Notebooks (5 m 11 s) all completed success for "Foundation C: public dump_responses … (Foundation C: public dump_responses, honest run-mode sentence, matrix and forced-metric template fixes #317)".

Checks run locally at cfb4fb0

Fresh Python 3.14.7 venv at /home/agent/r2b-pr334/venv (uv venv --python 3.14,
uv pip install --link-mode symlink -e ".[dev]"), everything under
flock .claude/heavy.lock nice -n 10, one at a time, no nested flock, pytest -p no:cacheprovider.

• ruff check . → All checks passed!; ruff format --check . → 404 files already formatted
• python tools/check_hygiene.py → Hygiene check passed: 614 file(s) scanned.
• python tools/check_recipe_assets.py → Image-asset check passed: 461 file(s) under recipes/ checked.
• python tools/render_catalog.py --check → exit 0
• python tools/notebook_ci.py fixtures → 51 recipe folders, 0 failed
• full pytest → 5985 passed, 55 skipped, 50 deselected, 31 xfailed in 216.25s, 0 failed
  (up from 5897/49 at ba797d6, which is b627e3c's added tests)
• margin mutants re-verified: expected killed (two named tests above), expected + 2 survived,
  expected + 2**20 killed, expected + 100_000 survived; tools/check_recipe_assets.py restored and
  cmp-verified byte-identical after each, git status clean

No notebook executed: the PR's three files are byte-identical to the head I already cleared, and all 51
Notebook (<recipe>) jobs are green on cfb4fb0.

Scratch and venv live only under /home/agent/r2b-pr334/ and are deleted. I removed nothing I did not create and
posted nothing to GitHub.

Verdict

I accept the delta — the merge is content-free, correctly scoped, signed, and every check on the head and on
main is complete and successful. I am blocking only on the one false sentence the body edit introduced, because it
records a test-coverage gap that does not exist and retracts a table row that was correct. No code change is needed;
one more body edit resolves it.

OPUS-REVIEW: CHANGES-NEEDED cfb4fb0

[CompleteDotTech]

PR #334 — Opus addendum (reviewer 2 of 2): body corrections verified

• Agent ID: agent-a16b174310531b11e (worktree .claude/worktrees/agent-a16b174310531b11e)
• Model: Opus 5 (1M context), resolved model ID claude-opus-5[1m]
• Head: cfb4fb0d9dbea1e5b938a9e21579a2aad25dc512 (unchanged since my pr334-r2b.md; base origin/main =
  b627e3c2dd723f5b38b2e67a3c1ad0e0c3c94134). No new commit, so nothing in the code delta I already accepted has
  moved. No live call, no GitHub post; I have still not read reviewer 1's review.

My sole blocker is resolved, and I checked every sentence of the rewritten "Corrections after review" section and the
amended mutant table row against my own measurements: all of it is now exactly true. The table row for limit expected (no + 1) now reads "killed by test_idat_is_inflated_no_further_than_the_length_the_header_implies and
test_the_inflated_idat_length_must_match_the_image_header (not because an exact-length file is rejected, as an
earlier version of this row said)" — those are precisely the two tests and the precise distinction I measured, twice,
with the captured assertion text. The surviving-mutant claim is now correct and no longer self-contradictory: of the
mutants in the table only expected + 2 survives, and I confirmed that "killed … only by the 2 MiB peak bound" is
literally true of expected + 2**20 — with that mutant the bomb test's message assertion, the off-by-one test and
both halves of the length-mismatch test all still pass, and only assert peak < 2 * 2**20 fails, at a traced peak of
3,058,504 bytes against the body's cited 3,057,912 (the same figure to four significant digits; the difference is
run-to-run allocator noise). The restated reason for the :866 comment being wrong is right: zlib reaches
Z_STREAM_END when max_length equals the output length exactly, which I probed on both 3.10.21 and 3.14.7, so
"would reject every good file" is the wrong part while the mutant dies on the two message assertions instead. The
memory bullet matches my numbers exactly (537,568,850 B old / 536,532,031 B for a header just under the backstop /
1,572,843 B realistic, with the output-buffer doubling as the cause and the bound still resting on the header), the
_MAX_INFLATED bullet matches my mutation run (16 MiB, 4 GiB and >= all survive), and the trailing-data →
png-structure bullet is the change I reproduced. On the largest-legal-asset figure I was wrong and the correction
is right: png_raw_length(1568, 1568, 6, 16, 0) = 19,670,560, which I re-derived by hand as
1568 * (1 + 1568 * 8) = 1568 * 12545 = 19,670,560, and the Adam7 sum is 19,671,932; my earlier 19,677,824 was my
own arithmetic slip, I withdraw it, and the substantive point is unaffected since both figures exceed 16 MiB
(16,777,216), so a 16 MiB backstop would indeed reject that asset. The closing "These are follow-up items, not defects
in the verdicts the checker gives" remains consistent with my result of zero pass/fail verdict changes across 40 built
cases, the whole tree, all 134 committed embedded notebook images and 15 generated figures. The two minor body points
from my first review (the unconsumed_tail rationale, the one-digit test count) are untouched and are not worth a
further edit, which would only fire another Scope edited run. Nothing in the tree was modified for this check — I
mutated only a copy of the tool outside the repository — and git status is clean.

OPUS-REVIEW: APPROVE cfb4fb0