Skip to content

Foundation: image preparation, tile inverse, multi-level box mapping and a 280 px minimum check (#310) - #341

Merged
CompleteDotTech merged 4 commits into
mainfrom
foundation/image-transforms
Oct 22, 2026
Merged

CompleteDotTech merged 4 commits into
mainfrom
foundation/image-transforms

Conversation

@CompleteDotTech

@CompleteDotTech CompleteDotTech commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Closes #310

What this adds

Everything is in src/jev_cookbook/vision.py, tests/test_vision.py and docs/vision.md.

  • prepare_image(image, *, min_side=280, max_side=1568, pad_color=(128, 128, 128), upscale=False) returns (prepared, transform). It scales a too-large image down (aspect kept, exact ratio rounded half up, Lanczos), optionally scales a small one up (upscale=True, never past max_side), then pads a side still under min_side with pad_color, content centered (odd pixel right/bottom). The result is RGB with both sides in [min_side, max_side]. Transparency or an unusual pixel mode raises.
  • ImageTransform (frozen dataclass): crop(bounds, width, height), to_original(box), to_derived(box), visible_fraction(box) and then(other) for chaining any number of levels. Arithmetic uses fractions.Fraction and rounds to float once, so a multi-level mapping is the true position rounded once. A normalized edge that is the float nearest k / width is read as pixel k, so a box that touches a tile edge gives None, not a 1e-17 sliver. A mapped box too small for a float rectangle raises instead of collapsing.
  • image_to_tile(box, tile_bounds, width, height): the inverse of tile_to_image, returning the clipped tile-local box or None.
  • crop_bounds(box, width, height, *, expand=0.0): the pixel bounds crop_box cuts, so a crop can be given a transform. crop_box now uses it.
  • generate_tiles(..., min_side=None) and crop_box(..., min_side=None): with min_side set, a tile or crop with a side below it raises ValueError before anything is cropped. The default None checks nothing.
  • docs/vision.md: new sections "Minimum size for tiles and crops", "Preparing an image for the API", "Mapping boxes between an image and what you sent" and a runnable "Worked example: tile, choose, crop, prepare, Box, map back". The example is executed and its printed output compared by a test. The old sentence "Padding or further cropping requires its own transform" is replaced.
  • tile_to_image is unchanged apart from its docstring.

Decisions for the reviewers

  1. min_side defaults to None (opt-in), not 280, because existing tests and the geometry functions legitimately run on tiny images; changing the default would force edits to existing tests. The docs tell a recipe that sends tiles or crops to pass min_side=IMAGE_LIMITS["min_side"] or use prepare_image. Should the default be 280 in a follow-up?
  2. crop_box and generate_tiles refuse rather than pad. Padding changes the image and needs a transform returned, which is what prepare_image does.
  3. upscale=False by default (pad, do not invent pixels). The issue's quoted docs allow either.
  4. Mid-gray pad_color default: a neutral choice that has not been measured against Jev; the docs say so.
  5. prepare_image centers the content and returns RGB; an image with an alpha channel, a palette with transparency, or a mode such as F/I;16 raises rather than silently flattening.
  6. One transform type for crop, pad/resize and chains (original window and derived window as exact fractions) instead of separate crop and pad types, so a.then(b).then(c) works for any mix.
  7. Edge snapping (above) follows the existing normalized_to_pixels rule but applies to the box edge as the float k / width; it changes only boxes within one float step of a pixel edge.
  8. Earlier unmerged branches (foundation/vision-utilities, foundation/multimodal-api, foundation/multimodal-base, foundation/multimodal-integration) were read; none contain a resize/pad/inverse helper, so nothing was reused.

How it was verified

Python 3.14 venv, under the heavy-job lock:

  • ruff check . exit 0; ruff format --check . exit 0 (405 files formatted, including the code blocks in docs/vision.md).
  • Full pytest: 6305 passed, 55 skipped, 50 deselected, 31 xfailed (the skips, deselects and xfails are all pre-existing), exit 0.
  • python tools/check_hygiene.py, python tools/check_recipe_assets.py, python tools/render_catalog.py --check, python tools/notebook_ci.py fixtures: each exit 0.
  • Python 3.10 venv: pytest tests/test_vision.py 389 passed.
  • Tests: expected values are worked out by hand in a comment beside each (pixel counts, padding rows, tile origins). Pixel-level tests cover prepared and cropped images, and a three-level chain (tile, crop, pad) is checked against a blue rectangle drawn at known pixels.
  • Red check: nine hand-made mutations of the new code were each caught by tests/test_vision.py (no pixel snapping, ceil padding, strict min_side, half-down rounding, wrong window in then, touching counted as overlap, missing tile check, upscale ignoring max_side, float error in the remap).

Not changed

docs/evaluation.md line 766 still names tile_to_image (correct as it stands). No other file was touched.

After the two reviews (head 2401cba; supersedes the figures and statements above where they differ)

  • then()'s derived-window remap is now pinned (three assertions in test_then_refuses_mismatched_sizes_and_content_that_is_all_padding); the earlier "nine mutations each caught" did not cover that branch.
  • prepare_image returns a metadata-free RGB image on every path (a clean Image.frombytes copy): its .info is empty and a saved PNG has only IHDR, IDAT, IEND, so it passes tools/check_recipe_assets.py; the docs say image_asset does not check PNG chunks. A zero-sized image raises ValueError. A test pins that the resample filter is averaging, not nearest-neighbour (it does not pin Lanczos itself).
  • Docs: docs/vision.md and docs/evaluation.md (four files touched, not three) say a None from ImageTransform.to_original is reported as ("review", None), never ("absent", None), and when to pass upscale=True (a 10x10 crop padded to 280 is 0.13% content). image_to_tile maps a gold box into a tile; it is not a source of the per-image answer rectangle.
  • Full pytest: 6312 passed, 55 skipped, 50 deselected, 31 xfailed on 3.14; tests/test_vision.py 396 passed on 3.10. foundation/multimodal-base, named above as an earlier branch, no longer exists on the remote.

…and a 280 px minimum check (#310)

Adds prepare_image (resize/pad to the Images API limits, returning an ImageTransform),
ImageTransform (crop/then/to_original/to_derived/visible_fraction, exact rational
arithmetic), image_to_tile, crop_bounds, and an opt-in min_side for generate_tiles and
crop_box. Documents the helpers with a runnable tile-choose-crop-prepare-Box-map-back example.

Refs #310

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

Review of PR #341 — reviewer 2 of 2 (independent)


Verdict in one paragraph

The implementation is correct as far as I can test it, the API is a good fit for the recipes that
will copy it, the documentation is accurate (I recomputed the whole prepare_image size table
independently, including its third column, and ran the worked example myself: it prints exactly the
five lines the docs show), and every acceptance criterion of #310 is met. I found one thing that
must change
: the then() composition's derived-window remapping — the line that makes a box found
inside a second crop that overlaps padding come back as None rather than as a plausible wrong
rectangle — is not pinned by any test. I mutated that one line to the obvious-looking simplification
and the whole of tests/test_vision.py still passed (389 passed), while the mutated code silently
returns a wrong box for a padding-only answer. Eight of the nine other mutations I wrote were caught,
so the rest of the new test coverage is strong; this is a two-line gap in the one branch of then()
that recipe 84's expand-and-re-crop loop will exercise, and it also means the PR body's claim that a
"wrong window in then" mutation was caught does not hold for that mutation class.

Must change

M1. No test pins then()'s derived-window remap; the obvious wrong version passes the suite

src/jev_cookbook/vision.py:698-703 — the composite's derived_window is
_remap(shared, other.original_window, other.derived_window). That remap only matters when shared
is a strict subset of other.original_window, i.e. when the second step's window reaches outside the
first step's content (a crop that takes in some of prepare_image's padding). Every other chaining
test has the second step fully inside the first step's content, where the remap is the identity, so
nothing in the file distinguishes the correct line from other.derived_window.

I replaced that one line with other.derived_window, and ran the file:
389 passed in 3.06s, exit 0. The behavioural difference (both runs on the same copied tree, only
that line changed):

_, pad = prepare_image(Image.new("RGB", (200, 100)))        # content rows 90..190 of 280
inner   = ImageTransform.crop((0, 0, 280, 100), 280, 280)   # a crop that is mostly padding
chain   = pad.then(inner)

as written  derived_window (1/7, 9/10, 6/7, 1)   to_original(top half of the crop) -> None
mutated     derived_window (0, 0, 1, 1)          to_original(top half of the crop) -> {'top': 0.0, 'bottom': 0.05, 'left': 0.0, 'right': 1.0}

The existing assertion in tests/test_vision.py:1978-1980
(partial = pad.then(...), then partial.to_original(FULL) == nb(0.0, 0.0, 1.0, 0.1)) is invariant under the mutation because the
box is the whole derived image. A regression here does not raise; it returns a well-formed rectangle
in the wrong place, which is the exact failure mode this whole PR exists to prevent, and it is the
case rank 84 ("ask whether it continues past each crop edge … adjust the rectangle") will hit as soon
as it expands a crop that was padded.

What to change: extend test_then_refuses_mismatched_sizes_and_content_that_is_all_padding (or
add a sibling) with the two assertions that fail under the mutation, e.g.

assert partial.derived_window == (Fraction(1, 7), Fraction(9, 10), Fraction(6, 7), Fraction(1))
assert partial.to_original(nb(0.0, 0.0, 1.0, 0.5)) is None  # that half of the crop is only padding

and correct the PR body's red-check bullet so it does not claim the window-remap branch as a whole is
covered.

Should change (small, in files this PR already edits)

S1. The reporting guidance still says the per-image outcome comes from tile_to_image alone

docs/vision.md:254-256 ("Tiled and hierarchical recipes", from #311/#331) says the headline
per-image outcome is "the final rectangle after tile_to_image". After this PR, the pipeline the new
worked example teaches (tile → crop → prepare → Box) produces its final rectangle from
chain.to_original(...); tile_to_image on a box that is relative to a padded crop returns a
well-formed but wrong rectangle. The new "Mapping boxes…" section says the right thing, and the
image_to_tile bullet links up to this section, but the link back is missing, and the reporting
section is where a builder goes to find out what to report. One clause ("the final rectangle after
tile_to_image, or after the ImageTransform chain when the recipe cropped or padded inside the
tile") closes the loop. The same clause applies to docs/evaluation.md:766, which the PR body calls
"correct as it stands": it is not wrong for a tile-only recipe, but it is incomplete for exactly the
hierarchical recipes the issue names, so the body's wording overstates it slightly.

S2. Nothing says which outcome a None from to_original becomes

src/jev_cookbook/vision.py:655-665 and docs/vision.md:404-407 explain that a box wholly in
padding returns None and that one partly in padding "is worth a look", but never say what a recipe
reports for it. I checked what happens if a builder passes it on: localization_report raises
ValueError: example 0: box needs top, bottom, left and right edges — loud, but the docs should name
the right answer, which under CONTRIBUTING §4 and the localization_outcome policy at
docs/vision.md:146-156 is ("review", None) and never ("absent", None) (a padding-only answer is
not evidence the target is absent). One sentence in the "Mapping boxes…" bullet list.

S3. No guidance on when to pass upscale=True

docs/vision.md:353-356 states the default and its rationale ("no pixel is invented") but never says
which to choose. With the default, a small crop is padded, so the target keeps the pixel size it had
in the source: a crop-and-re-ask recipe (78, 82, 84) gains context removal but no extra detail, and a
very small crop is almost all padding — I measured a 10 × 10 crop coming back as 280 × 280 with the
content covering 0.13% of what is sent. That is a real decision for ranks 78/82/84, and the
builder has nothing to go on. Suggest one sentence: pad when the crop only needs to clear the
minimum, pass upscale=True when the point of the crop is to show the target larger; either way keep
the choice fixed across a recipe's images (as the pad_color sentence already says).

Suggestions (non-blocking)

  1. prepare_image does not validate image.size the way its neighbours do. With upscale=True
    a degenerate image raises a bare ZeroDivisionError
    (src/jev_cookbook/vision.py:784-789; at line 787 numerator, denominator = min_side, shortest with
    shortest == 0), while the same input with upscale=False raises a clean
    ValueError: original_size width must be a positive integer and generate_tiles raises
    ValueError: image width must be a positive integer (vision.py:488-489). Not reachable from any
    of the module's own outputs (only from a hand-made Image.new("RGB", (0, 0))), but one
    _positive_int(width, "image width") pair at the top of prepare_image would make it the only
    function in the module that can raise something other than ValueError.
  2. One error message serves two unrelated causes. vision.py:774-778 tells the caller of an
    F or I;16 image to "flatten it onto an explicit background", which is advice for alpha, not
    for a float or 16-bit mode. Splitting the message (transparency → flatten; unusual mode → convert
    to RGB/L yourself, deciding the scaling) would tell a builder what to actually do. Verified text:
    cannot prepare a 'F' image with transparency or an unusual pixel mode; flatten it onto an explicit background and convert it to RGB first.
  3. _pixel_bounds says "tile bounds" when the caller passed crop bounds.
    ImageTransform.crop(crop_bounds(...), w, h) with the wrong image size reports
    tile bounds must have positive area inside the whole image (vision.py:411-415), and the docs'
    own table (docs/vision.md:389) shows ImageTransform.crop being fed crop_bounds. "bounds"
    instead of "tile bounds" would fit both callers.
  4. Three crop names with three return types. crop_bounds → pixels, crop_box → an image,
    ImageTransform.crop → a transform. Since 11 recipes are about to copy this, now is the moment to
    consider ImageTransform.from_crop(...) (or for_crop); after the recipes land, renaming is a
    breaking change. Cosmetic, and I would not hold the PR for it.
  5. Getting a crop and its transform takes two calls that recompute the same bounds. The worked
    example has to bypass crop_box entirely (docs/vision.md:467-468:
    bounds = crop_bounds(...), then chosen.image.crop(bounds)) because crop_box returns only the
    image. That is defensible (and crop_bounds' docstring points at it), but a reader may take
    "call .crop yourself" as the way to dodge the min_side check. An ImageTile.transform
    property and/or a crop_box variant returning (image, transform) would remove the copy-paste.
    Equally, image_to_tile(..., min_visible=) would save rebuilding the transform just to apply the
    minimum-visible-fraction rule the per-tile report has to state.
  6. A one-ULP sliver is still a non-None overlap. I checked the boundary the docs claim: a box
    whose edge is exactly k/width is read as pixel k and gives None (verified), but a box one
    float step beyond it returns a 7.8e-17-wide rectangle. That is arguably the honest answer (it is a
    real overlap) and visible_fraction + a threshold disposes of it, which the docs already tell
    builders to do — no change needed, but it is a reason S2/visible_fraction guidance matters.
  7. Long lines. docs/vision.md:307 is 129 characters because the sentence was not re-wrapped
    after the inserted clause (the rest of the paragraph is ~90); vision.py:371, 380 and 597 are
    102–114. E501 is not in the selected ruff rules so nothing fails, but the base files hold ~100
    almost everywhere.
  8. Docs justify an API default by the test suite. docs/vision.md:337-338 explains
    min_side=None with "because the geometry functions are also used on tiny synthetic images in
    tests". The next sentence gives the reason a recipe author actually cares about (a crop around a
    found region is legitimately small). Leading with that one would read better in a contributor-
    facing doc.

On the questions I was asked to judge

Is this the API ranks 63–84 and 98 need? Yes. The ranks that matter here are the spatial ones
(77–84) plus 98; 63–76 are Choice/Noul/Score over whole images and need only the pre-existing
loaders. The unmerged recipe/83-hierarchical-visual-search branch is direct evidence: its
helpers.py hand-rolls exactly what this PR adds — it carries View(image, bounds) with absolute
root-image bounds and composes levels itself (tile_to_image(box, current.bounds, *root.image.size)),
and it hand-checks min(size) < 256 (the stale pre-#306 minimum). ImageTransform.then replaces
that bookkeeping, extends it to pads and resizes, and min_side=IMAGE_LIMITS["min_side"] replaces
the hand-written constant with the right number. image_to_tile is what docs/fixtures.md:141-146
already promised ("Python derives which tiles count as correct from the box and the tile bounds"),
and visible_fraction is exactly the "minimum visible fraction" rule docs/vision.md:260-263
requires a per-tile diagnostic to state. Naming is consistent with the existing pair
(tile_to_image/image_to_tile), to_original/to_derived/then read well, and the return shapes
(dict | None for a mapping, (image, transform) for preparation) match the module's conventions.
My only API reservations are the cosmetic ones in suggestions 4–5.

Should min_side default to 280? No — opt-in is the right call, and #310's criterion explicitly
allows either ("can be asked to enforce min_side (or prepare_image is documented as the required
step)"). Two things make it safe: a recipe that forgets is still caught before any request, because
the only way a PIL image becomes an asset is image_asset/image_reference →
validate_image_asset, which refuses an under-280 image (verified: a 100 × 100 tile gives
image width and height must each be 280 to 1568 pixels); and crop_box is legitimately used on
small regions that then go through prepare_image, so a 280 default would make the common path raise.
The asymmetry (prepare_image defaults to 280, the geometry helpers to None) is explained in the
docs and is right for each function. The one polish I would add is to the other message: the
asset-stage rejection does not mention prepare_image, so a builder who forgets gets the less
helpful of the two errors — that message lives in #306/#321 code, outside this PR.

Pad colour and upscale as decisions. Mid-gray with an explicit "has not been measured against
Jev" is the right shape of claim under CONTRIBUTING §2 (it asserts nothing about model behaviour),
it is visually distinct from both document white and UI dark, and it is overridable with a
"keep it the same across a recipe's images" instruction. upscale=False is defensible (it keeps the
original pixels, so the mapping involves no resampling) and the Images docs allow either; my only
complaint is the missing "which should I pick" sentence (S3).

The three new sections and the worked example. I ran the example standalone, outside the test
that checks it, and its five printed lines match the docs byte for byte. The arithmetic the prose
quotes is right (target at page x 840–960/y 330–390; crop bounds (200, 300, 400, 420); padded box
80–200 × 110–170; offsets 40/80, 200/300, 600/0). The size table is true: I recomputed all seven rows
and the content rectangles independently ((0,40,300,240), (40,90,240,190), (39,90,240,190),
(0,120,1568,159)), and the content column's end values are exclusive, consistent with the
"exclusive right/bottom" rule stated at docs/vision.md:103. No contradiction with the tiles,
Box-label or frames sections: the per-image headline stays the full-image label (fixtures.md:141-146),
the per-tile use is labelled a stage diagnostic, the 280/1568 numbers agree with
docs/fixtures.md:352 and vision.IMAGE_LIMITS, the "save and validate with image_asset" rule is
preserved, and the frames section is untouched and unaffected. S1 is the one loose end.

Checks I ran

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

Check Result
ruff check . All checks passed!, exit 0
ruff format --check . 405 files already formatted, exit 0 (the body's count is right)
pytest (full) 6305 passed, 55 skipped, 50 deselected, 31 xfailed in 250.74s, exit 0 — identical to the body's numbers, and to CI's Tests (py3.10) row (the body labels this a 3.14 venv; with [dev] only, tests/test_live_sdk.py is skipped, so 3.14 gives the same counts — CI's .[dev,live] 3.14 job reports 6368/53)
pytest tests/test_vision.py ×2 389 passed in 4.62s then 389 passed in 3.10s — same count, no flakiness, fast
python tools/check_hygiene.py Hygiene check passed: 615 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 exit 0
Worked example, run outside its test prints the documented five lines, MATCH: True
12 independent probes (size table, forgotten min_side, error texts, exactness of a 3-level chain, degenerate inputs, ULP sliver, identity case) as reported above
9 hand-made mutations of the new code 8 caught (strict_min, no_snap, pad_odd_left, round_half_down, no_tile_check, touch_overlaps, upscale_unbounded, no_crop_check); 1 survived (then_wrong_window, M1)
Commit hygiene head is signed (G), one commit on ebf2413, Refs #310 in the message, Co-Authored-By: Claude Sonnet 5.5 present
CI on this exact head all rows pass, including Tests (py3.10) 3m16s and Tests (py3.14) 5m11s (read, not assumed)

No notebook executes any of this code (no recipe uses the new helpers yet), so there was nothing to
execute from a scratch copy; Notebooks (execute) is green on the head and Fixtures (validate)
passes. No network call, no API key, no live inference.

Scope

git diff --raw -M origin/main...HEAD:

:100644 100644 57990b5 6bcd020 M	docs/vision.md
:100644 100644 959bfe9 a813094 M	src/jev_cookbook/vision.py
:100644 100644 fe139f1 7b1abbe M	tests/test_vision.py

Three files, exactly the three #310 names, all 100644 → 100644 modifications: no symlink (120000),
no submodule (160000), no mode or type change, no rename, no deletion. +1324/−8. This is
foundation work on a foundation/ branch, so the recipe allowlist does not apply (and Scope (recipe pull requests) passes). Nothing in catalog/, tools/, .github/ or recipes/. No
secrets, no absolute home paths, no TYPESAFE_API_KEY reference in the diff.

PR body against the diff

Accurate, with the two qualifications already noted. Verified claim by claim: the three files; the
prepare_image behaviour and signature; ImageTransform's members and the exact-Fraction,
round-once arithmetic; the pixel-edge snapping; the "too small for a float rectangle" raise;
crop_bounds being what crop_box cuts (crop_box does now call it); the opt-in min_side on both
functions raising before any crop; the three new doc sections and the executed example; the removal
of the "Padding or further cropping requires its own transform" sentence; tile_to_image changed only
in its docstring (the diff's only deletions are the old crop_box body/docstring and that one
docstring line). The ruff and pytest counts are exactly what I measured. Decision 8 checks out: I
grepped origin/foundation/vision-utilities, multimodal-api, multimodal-base and
multimodal-integration for prepare_image|pad_color|image_to_tile under src and docs and found
nothing, so there was nothing to reuse. Closes #310 is the only closing keyword anywhere in the
body (the only other # reference is the title's (#310)), and the recipe checklist is correctly
omitted — the template says to skip it for foundation work. The two qualifications: the red-check
bullet overstates coverage of the then window-remap branch (M1), and "docs/evaluation.md line 766
… correct as it stands" is better described as incomplete for multi-level recipes (S1).

Acceptance criteria of #310

Criterion Verdict
prepare_image(image, *, min_side=280, max_side=1568, pad_color=...) returns (prepared, transform) mapping normalized boxes prepared→original and back; round-trip tests at several aspect ratios including padding on one axis Met. Signature matches (plus upscale). test_boxes_round_trip_through_prepare_image_at_every_aspect_ratio covers 11 sizes × upscale ∈ {False, True} in both directions, and test_prepared_images_meet_the_api_limits_for_every_aspect_ratio re-validates each result through validate_image_asset. Pad-one-axis is (300, 200)/(200, 300), checked pixel by pixel in test_prepare_image_pads_one_axis_and_keeps_every_pixel.
image_to_tile(box, tile_bounds, width, height) returns the clipped tile-local box or None; tested Met. Exact signature; hand-computed inside/clipped/covering/disjoint/edge-touching cases, plus an inverse check against tile_to_image over every generated tile and invalid-bounds/invalid-box rejection.
Multi-level mapping documented and tested; a box two levels down maps to the full image exactly Met. test_a_box_found_two_levels_down_maps_to_the_full_image_exactly asserts ==, not approx; a three-level chain with a pad in the middle, a pixel-verified chain on a real image, associativity, and resize→crop / crop→resize+pad orders. Documented in "Mapping boxes between an image and what you sent". M1 is a gap within this criterion's testing, not a failure of it.
generate_tiles and crop_box can be asked to enforce min_side (or prepare_image documented as required), with a test that a sub-280 tile is refused or padded Met. min_side= on both, checked before any crop (proved by monkeypatching Image.Image.crop to fail), exactly 280 accepted, 279 refused, expanded crop measured after expand, message naming the size and prepare_image, invalid min_side rejected. The documented requirement is also stated in bold at docs/vision.md:338-339.
docs/vision.md worked example: tile → choose → crop → prepare → Box → map back Met. All six stages, runnable offline, output compared by a test and by me independently.

Outside my task (reported, not fixed)

OPUS-REVIEW: CHANGES-NEEDED 308cac8

@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

Review of PR #341 — reviewer 1 of 2 (independent)


Verdict in one paragraph

The coordinate maths is right. I re-derived the whole model independently in exact rationals and
cross-checked 37,362 random multi-level chains (crops and resize+pad steps in any order, image
sizes 1–2000 px): 45,279 forward and backward mappings agreed with my independent exact model
bit for bit, then was associative in every case, visible_fraction matched exact area
arithmetic in every case, and the tile_to_image / image_to_tile round trip was exact to
<= 1e-9 over 20,000 random tilings with zero failures. The snapping rule is sound: it fires
only on a coordinate exactly equal to the double float(k)/float(dimension), so it never
moved a coordinate by more than ~1e-16 px and it never hid a sliver — one float step past a tile
edge is still reported, as is a genuine 1e-9-wide box. 26 of my own 27 mutants of vision.py
were killed by tests/test_vision.py. I reproduced every number in the "How it was verified"
section exactly. The one thing I will not approve as it stands is narrow and cheap to fix:
prepare_image carries the input image's .info through on two of its three paths, so the
"prepared" image it hands you — the image docs/vision.md now tells a recipe to save as an
asset — fails the repository's own image-file contract with png-metadata (iCCP) whenever the
source carried an ICC profile, while the padding path silently does not. I confirmed that
against tools/check_recipe_assets.py itself.


Must change

M1 — prepare_image output can fail the repository's image-file contract, inconsistently by shape

src/jev_cookbook/vision.py:798-804

prepared = image.convert("RGB")
if content != (width, height):
    prepared = prepared.resize(content, Image.Resampling.LANCZOS)
if size != content:
    canvas = Image.new("RGB", size, fill)
    canvas.paste(prepared, (left, top))
    prepared = canvas

Image.convert and Image.resize both copy self.info onto the result; only the padding branch
starts from a fresh Image.new canvas and therefore drops it. So the returned image's .info
depends on which branch ran. Measured on both interpreters (Pillow 13.0.0 on 3.14, Pillow 12.3.0
on 3.10), with a source carrying icc_profile, dpi and exif:

path prepared.info keys
already fits (no resize, no pad) dpi, exif, icc_profile
resize only icc_profile
pad (fresh canvas) (empty)

Saving the first two as PNG writes an iCCP chunk. Driving the repository's own checker
(tools/check_recipe_assets.check_image) on those files:

no-pad path  (400 x 300 already fits):      [png-metadata] chunk iCCP is not allowed ...
resize path  (2000 x 1500 -> 1568 x 1176):  [png-metadata] chunk iCCP is not allowed ...
pad path     (300 x 200 -> 300 x 280):      []
docs/fixtures.md's Image.frombytes copy:    []

Why this matters rather than being a Pillow footnote:

  • docs/fixtures.md:361-363 already names this exact hazard in the project's own words — "saving
    to PNG does carry a source's ICC profile across (the iCCP chunk fails the check), so do not
    re-save the opened file itself" — and prescribes the Image.frombytes("RGB", rgb.size, rgb.tobytes()) clean copy. prepare_image's no-op path is effectively re-saving the opened
    file.
  • The new docs/vision.md section points the reader the other way: "prepare_image does not
    certify the result for the API: save the prepared image and load it with image_asset, which
    re-checks format, size and the file limit." image_asset / validate_image_asset do not
    check PNG chunks (I confirmed: the iCCP file is accepted), so a recipe builder who follows
    only vision.md gets a green local validation and a red Hygiene job.
  • Because the pad path is clean, the failure is shape-dependent: the same code produces a
    compliant asset for a small crop and a non-compliant one for an image that already fits. That
    is the hardest kind of rule to learn from, and ranks 63–84 and 98 copy this interface.

What to change — one line plus one sentence:

  • build the result from raw pixel bytes so all three paths behave alike, e.g. replace the final
    prepared with Image.frombytes("RGB", prepared.size, prepared.tobytes()) (or clear
    prepared.info), and say so in the docstring ("the result carries no metadata from the
    source");
  • in docs/vision.md's "Preparing an image for the API", add that the prepared image carries no
    metadata, pointing at the image-file contract in fixtures.md;
  • add a test: save a prepare_image result whose source has info["icc_profile"] and assert the
    PNG's chunk list is IHDR/IDAT/IEND (or has no iCCP), for the fits, resize and pad paths.
    Without such a test this regresses silently — nothing in the 6,305-test suite touches .info.

This is the only item I would hold the merge for.


Suggestions (not blocking)

  • S1 — nothing pins the resampling filter. src/jev_cookbook/vision.py:800. My mutant
    replacing Image.Resampling.LANCZOS with NEAREST was the only one of 27 to survive the
    whole of tests/test_vision.py (389 passed). The docstring and docs/vision.md both state
    "resampling is Lanczos", and the filter is what decides the bytes of every generated asset, so
    a silent change would move committed asset digests and the replay keys computed from them. A
    cheap guard that does not pin Pillow's exact implementation: downscale a hard black/white edge
    by a non-integer ratio and assert at least one intermediate grey value appears (NEAREST and
    other point filters cannot produce one).
  • S2 — upscale is unvalidated while every neighbouring argument is strict.
    vision.py:745 — prepare_image(..., upscale="yes") is accepted and treated as true, and
    upscale=0 / upscale=None as false, whereas min_side=280.0, min_side=True and
    pad_color=True are all rejected by name. A if not isinstance(upscale, bool): raise would
    match the module's own standard.
  • S3 — EXIF orientation is not mentioned. prepare_image's docstring covers transparency and
    pixel modes but not orientation, and the no-op path carries info["exif"] through.
    _image_format rejects an asset whose orientation tag is not 1, and docs/fixtures.md:376
    tells authors to run ImageOps.exif_transpose before the copy. One sentence in the docstring
    ("rotate orientation into pixels with ImageOps.exif_transpose first") closes the loop. (No
    PNG eXIf chunk is written in practice on either Pillow version I tested, so this is
    documentation, not a defect.)
  • S4 — four added lines exceed the configured line-length = 100: vision.py:371 (108),
    :380 (102), :597 (114), plus docs/vision.md:307 (129) and two 103–104-char test lines.
    ruff check is silent because E501 is not in the select list, and ruff format cannot
    reflow a docstring, so nothing flags them; origin/main has exactly one such line in this file
    and none in tests/test_vision.py. Worth reflowing for consistency, not worth a round trip on
    its own.
  • S5 — the "too small to represent as a float rectangle" ValueError escapes the
    None-returning API
    (vision.py:563, reached from to_derived, to_original and
    image_to_tile). I could not reach it from anything realistic: 47,486 image_to_tile calls
    over random realistic tilings (five page sizes, five tile widths, gold boxes written as
    decimals, as pixel ratios, and as the output of tile_to_image) produced zero raises, and
    200,000 prepare_image-shaped pad mappings produced zero. It only appeared in my generator's
    deliberately pathological pads (content of 1–39 px on a canvas up to 300 px larger), 128 times
    in 40,000 chains. So I agree with the design, but a recipe that loops image_to_tile over
    every tile to build per-tile gold has one code path that raises instead of skipping; a sentence
    in image_to_tile's docstring saying so (or in the docs bullet that already mentions the
    raise) would be kind to the eleven recipes that copy this.
  • S6 — ImageTransform's public fields are annotated with the private _Edges alias
    (vision.py:603-604). The class is exported and its constructor is documented as usable
    ("the constructor only checks that the sizes and windows are valid"), but a caller who wants to
    build one directly has no public name for the window type. Exporting the alias (or spelling it
    tuple[Fraction, Fraction, Fraction, Fraction]) would be tidier.
  • S7 — docs/evaluation.md:766 still describes the headline outcome as "the final rectangle
    after tile_to_image", which is now only the single-crop case. The PR body discloses this and
    it is not wrong as written; broadening it to "after mapping back (tile_to_image, or an
    ImageTransform chain)" would be a one-line follow-up.

What I verified myself, and how

The coordinate maths

I loaded the geometry half of vision.py verbatim (PIL and sibling imports stubbed) and wrote an
independent exact-rational model of the whole pipeline from scratch — each step modelled as an
affine map between two windows, composed step by step with clipping at every level — then
compared it with the code's compose-then-clip-once implementation.

property cases result
random chains of 1–3 crop / resize+pad steps built with then 37,362 all built; the only ValueError was the documented "shows none of the first transform's content"
to_derived against my exact model (float equality, not approx) 20,338 0 mismatches
to_original against my exact model (float equality) 24,941 0 mismatches
to_derived returning None where my model says empty 14,126 0 disagreements
then associativity: left fold == right fold 37,362 0 differences
visible_fraction against exact area arithmetic single-step subset 0 differences

So "a multi-level mapping is the true position rounded once" is not a claim I had to take on
trust: the composite result equals the exact rational position rounded once, to the last bit, in
every case I generated.

The snapping rule (_snap, vision.py:519-527)

  • Does it ever snap something that is not a pixel edge? No. 500,000 random coordinates
    against ten dimensions: every snap satisfied value == k / dimension, and no snap moved a
    coordinate by more than 1e-12 of a pixel (in fact ~1e-16).
  • Is it consistent with the pre-existing _pixel_edge rule? Yes. For every k/dimension
    with dimension in 1–399 plus 512, 777, 1000, 1024, 1568, 2048, 4000, 10000 and 99991, _snap
    snapping and _pixel_edge snapping agreed: 0 disagreements. (_snap drops _pixel_edge's
    extra abs(scaled - nearest) <= ulp guard, which the equality test implies in practice.)
  • Can it hide a real sliver? No. Pushing a box edge past a tile edge at four
    (width, pixel) combinations plus a 3-px image: exactly k/W -> None, and one float step
    past -> a sliver is reported
    (3.9e-14 px at W=1000), as are 1e-15, 1e-12, 1e-9, 1/1000 px,
    1/10 px and one whole pixel past. A genuinely 1e-9-wide box wholly inside a tile is returned
    unchanged. The same holds at a prepare_image pad boundary: exactly the content edge gives
    None, one float step past gives a clipped box. So the rule collapses only the exactly
    representable pixel edge, which is the documented intent.

Consistency with the existing semantics and the 280 px rules

  • image_to_tile / tile_to_image round trip: 20,000 random (image size, tile bounds, tile-local box) triples — 0 failures, 0 raises, max error <= 1e-9, and None never returned
    for a box the forward map produced.
  • ImageTransform.crop(...).to_original agrees with tile_to_image for generated tiles (the
    PR's own test, which I confirmed is a genuine new-vs-old cross-check, not a tautology).
  • generate_tiles's docstring claim that the min_side check is exact — "every tile has the
    same size, min(tile_width, width) by min(tile_height, height)" — is true: starts()
    only returns positions p with p + window <= length, so min(left + tile_width, width) is
    never a partial tile. The check is therefore neither conservative nor permissive.
  • prepare_image's output is always inside the limits by construction (content <= max_side
    from the clamp in scaled, size = max(content, min_side)), which also puts the area inside
    78,400–2,458,624 at the defaults. The PR's parametrised test proves it end to end by saving a
    PNG and running validate_image_asset.
  • IMAGE_LIMITS (280/1568/78,400/2,458,624) matches tools/check_recipe_assets.py:48-51
    exactly.
  • Hand-recomputed the whole prepare_image docs table independently (all seven rows and all five
    content rectangles): correct, including 1000 x 1568/1601 = 979.39 -> 979, the odd-pixel row
    201 x 100 -> columns 39..240 (39 left, 40 right) and 4000 x 100 -> rows 120..159.
  • Hand-recomputed the worked example's three-level chain in exact fractions: to_original gives
    7/10, 11/30, 4/5, 13/30, whose floats are bit-identical to 840/1200, 330/900, 960/1200, 390/900, so the example's found == gold and box_iou == 1.0 are true for the right reason,
    and normalized_to_pixels recovers (840, 330, 960, 390) only because of the pre-existing
    _pixel_edge snapping (without it the right edge would ceil to 961).

prepare_image vs the image-file contract and replay keys

  • Does it change bytes? Yes by design (it converts, resamples and/or pads); the input is not
    modified, which the PR's test proves by mutating the output.
  • Deterministic across Pillow versions? Yes, for the two versions this repository's CI uses.
    I hashed prepared.tobytes() for twelve (size, upscale) combinations on a textured synthetic
    page and the saved PNG bytes for two of them, under Pillow 13.0.0 / Python 3.14.7 and
    Pillow 12.3.0 / Python 3.10.21: every pixel digest and both PNG file digests were
    identical. Note also that the common path for a small crop (upscale=False) performs no
    resampling at all — it is a paste onto a fresh canvas — so it is trivially reproducible. The
    residual risk is a future Pillow changing its Lanczos kernel, which only S1 would catch.
  • Replay keys: replay_key hashes decoded file bytes of assets
    (src/jev_cookbook/backends.py:184-193), so a recipe that prepares an image at notebook run
    time and sends it is only as reproducible as the prepared bytes; given the measurement above
    that is currently fine, and the pad-only path needs no assumption at all. Nothing in the diff
    changes replay_key or image_reference.
  • The contract interaction is M1 above.

Degenerate, extreme and non-numeric inputs (all with the real module)

NaN, ±inf, negative, >1, zero-width, zero-height, three-edge and non-mapping boxes all raise
ValueError with a named edge, from to_derived, to_original and visible_fraction alike.
Non-integer tile bounds, float width, min_side=nan, min_side=0/-280/True/280.0/"280" and
every bad pad_color in the PR's table raise. prepare_image on 1 x 1 -> 280 x 280 with window
139/280..140/280; 1 x 1 with upscale=True -> 280 x 280; 10000 x 1 -> 1568 x 280; 279 x 279 ->
280 x 280; 1568 x 1569 -> 1567 x 1568 (aspect kept, inside the limits). A 1-px image tiles and
maps fine. A hand-built transform with a 10**9 nominal size works; 10**400 raises
OverflowError rather than ValueError, which is unreachable from any real image and not worth
changing. to_derived accepts a Box-answer object and a {"box": ...} mapping, like the rest of
the module.

Are the tests real checks? My own mutants

I wrote 27 mutations of vision.py of my own (not the PR's nine) and ran
pytest tests/test_vision.py -x -q -p no:cacheprovider against each. 26 were killed, the
survivor being S1 (LANCZOS -> NEAREST):

killed — never snap a pixel edge; always snap; _clip counts touching as overlap; _remap swaps
fx/fy; scaled uses ceil / round-half-down / floor; the odd pad pixel goes left/top; content in
the top-left instead of centred; min_side strict (<=); min_side checks the long side;
generate_tiles ignores a source smaller than the tile; generate_tiles skips the check;
crop_box skips the check; crop_box checks after cropping; then uses the wrong window in the
first _remap; then drops the empty-intersection check; upscale ignores max_side;
upscale overrides a downscale; _as_box returns a collapsed box; to_original clips to the
wrong window; visible_fraction returns an area not a share; no min_side > max_side check;
pad_color accepts 256; transparency and odd modes allowed through; crop_bounds drops the
expansion.

The worktree was restored afterwards (git status clean, git diff --stat empty).

On tautology: I found none. The expectations are hand-computed integers and fractions in comments
beside each assertion; the pixel-level tests (crop((0, 28, 300, 228)).tobytes() == image.tobytes(), the blue-rectangle getbbox() tests, crop_box(prepared, mapped).tobytes() == crop_box(image, box).tobytes()) are independent of the mapping code; and the two places that do
use the code under test to form an expectation —
test_image_to_tile_inverts_tile_to_image_for_every_generated_tile branching on
visible_fraction, and test_boxes_round_trip_... bounding the result with
transform.derived_window — both still carry an independent assertion, and the windows
themselves are pinned by hand elsewhere (window_of(transform) == (Fraction(0), Fraction(7, 64), ...)). test_generate_tiles_min_side_accepts_...'s first assertion compares the function to
itself, but the next two lines pin the tile size and the count (12) by hand.


Checks I ran

Fresh venvs under /home/agent/r1/ (uv venv --python 3.14 and --python 3.10, then
uv pip install --link-mode symlink -r a one-line requirements file holding
-e <worktree>[dev]), each heavy command under flock .claude/heavy.lock nice -n 10, pytest -p no:cacheprovider, no nested flock, available memory checked first (4.0–4.2 GB).

command result
ruff check . (0.17.1) All checks passed!, exit 0
ruff format --check . 405 files already formatted, exit 0 — matches the PR body
pytest (3.14.7, Pillow 13.0.0) 6305 passed, 55 skipped, 50 deselected, 31 xfailed in 257 s, exit 0 — matches the PR body exactly
pytest tests/test_vision.py (3.10.21, Pillow 12.3.0) 389 passed in 4.5 s — matches the PR body exactly
python tools/check_hygiene.py passed: 615 file(s) scanned, exit 0
python tools/check_recipe_assets.py passed: 461 file(s) under recipes/ checked, exit 0
python tools/render_catalog.py --check exit 0
python tools/notebook_ci.py fixtures 51 recipe folders, 0 failed, exit 0
my independent exact-model property tests 37,362 chains / 45,279 mappings, 0 problems
my 27 mutants of vision.py 26 killed, 1 survived (S1)
prepare_image byte digests, Pillow 13.0.0 vs 12.3.0 identical for all 12 cases and both PNG files
tools/check_recipe_assets.check_image on prepare_image output png-metadata (iCCP) on 2 of 3 paths — M1
the new docs/vision.md min_side code block, executed by hand ran clean

No notebooks, recipes or fixtures are touched by the diff (docs/vision.md,
src/jev_cookbook/vision.py, tests/test_vision.py only), so no notebook execution was needed
and tools/check_recipe_scope.py has nothing to judge.


PR body against the diff

Accurate throughout; I checked every factual claim and every number.

  • Signatures, defaults, step order, the "RGB with both sides in [min_side, max_side]"
    guarantee, the half-up rounding, the Lanczos filter, the odd-pixel-right/bottom centring, the
    transparency/mode refusal, the "rounds to float once" claim, the snapping rule stated as "the
    float nearest k / width is read as pixel k" (which is exactly value == float(k)/float(w)),
    the "too small for a float rectangle raises" behaviour, image_to_tile as the inverse,
    crop_bounds being what crop_box now uses, the opt-in min_side raising before any crop,
    and "tile_to_image is unchanged apart from its docstring" — all confirmed against the diff.
  • The old sentence "Padding or further cropping requires its own transform" is indeed gone, and a
    test asserts its absence.
  • "How it was verified": all four reproducible numbers (405 files; 6305/55/50/31; four tools at
    exit 0; 389 on 3.10) came out identical for me. I cannot replay the author's nine mutants, but
    all nine shapes they describe are among my 27 and all nine were killed in my run.
  • Reuse claim: I fetched foundation/vision-utilities, foundation/multimodal-api and
    foundation/multimodal-integration and grepped their src/ and tools/ for
    prepare_image|ImageTransform|image_to_tile|def .*resize|LANCZOS — no hits, so "nothing was
    reused" is correct. (foundation/multimodal-base no longer exists on the remote; harmless
    staleness in the body.)
  • "No other file was touched" — correct, three files, +1324/-8.
  • No overclaim found. The one thing the body does not mention is M1.

The decisions offered to reviewers all seem right to me: opt-in min_side (changing the default
would force edits to existing tests, and the geometry functions legitimately run on tiny images —
and generate_tiles's check is exact, so turning it on later is safe); refusing rather than
padding in crop_box/generate_tiles; upscale=False; one transform type rather than two; and
the edge-snapping rule. On decision 1 I would answer: yes, revisit the default in a follow-up
once a recipe exists to try it against, not now.


Acceptance criteria of #310

# Criterion Status
1 vision.prepare_image(image, *, min_side=280, max_side=1568, pad_color=...) returns (prepared_image, transform) mapping prepared -> original and back; round-trip tests at several aspect ratios, including padding on one axis Met. Exact signature; test_boxes_round_trip_through_prepare_image_at_every_aspect_ratio runs 11 sizes x upscale x 5 boxes, covering pad-one-axis (300 x 200, 200 x 300), pad-both (200 x 100), scale-then-pad (4000 x 100), scale-only and no-op. My own round trip over the same shapes agreed with exact arithmetic.
2 vision.image_to_tile(box, tile_bounds, width, height) returns the clipped tile-local box or None when it does not intersect; tested Met. Inside, clipped, covering, four directions of miss, edge-touching, one-pixel overlap, invalid bounds and invalid boxes are all tested; I verified the inverse relationship over 20,000 random tilings.
3 Multi-level mapping documented and tested: a box found two levels down maps to the full image exactly Met. docs/vision.md has a table of steps, the chaining rules and the executed worked example; test_a_box_found_two_levels_down_maps_to_the_full_image_exactly and test_three_levels_with_a_pad_in_the_middle_... assert ==, not approx, and test_chained_transforms_agree_with_pixels_on_real_images ties it to real pixels.
4 generate_tiles and crop_box can be asked to enforce min_side (or prepare_image is documented as the required step), with a test that a sub-280 tile is refused or padded Met. Both take min_side=; both refuse before any crop (proved by monkeypatching Image.Image.crop to fail); exactly 280 passes, 279 fails with a message naming the size and prepare_image; and docs/vision.md states in bold that a recipe sending tiles or crops passes min_side=IMAGE_LIMITS["min_side"] or runs them through prepare_image.
5 docs/vision.md worked example: tile -> choose -> crop -> prepare -> Box -> map back Met. All six stages, offline, with its printed output compared by a test; I recomputed the whole chain by hand and it is exact.

All five are met. M1 is about how the prepared image is saved, which #310 does not ask for but
docs/fixtures.md does.


Outside my task (reported, not fixed)

  • docs/evaluation.md:766 (S7) — pre-existing wording, now slightly narrow.
  • tests/test_docs_examples.py only indexes docs/evaluation.md, so docs/vision.md's code
    blocks are executed only by the two bespoke tests in test_vision.py (the new worked example,
    and the pre-existing frame walkthrough). The new "Minimum size" block is not executed by any
    test; I ran it by hand and it is clean. Widening that harness is a separate issue.
  • foundation/multimodal-base is gone from the remote, so the PR body's branch list is stale by
    one name.

OPUS-REVIEW: CHANGES-NEEDED 308cac8

…adata from prepare_image, document reporting and upscale (#310)

Review fixes for the image-transforms PR: assertions that pin then()'s second remap, a clean
metadata-free copy on every prepare_image path (checked against tools/check_recipe_assets.py),
a ValueError for a zero-sized image, a resample-filter check, and docs on reporting a mapped-back
None and on choosing upscale.

Refs #310

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

Re-review of PR #341 (round 2) — reviewer 2 of 2 (independent)

  • Agent ID: reviewer 2 of 2 for PR Foundation: image preparation, tile inverse, multi-level box mapping and a 280 px minimum check (#310) #341, worktree .claude/worktrees/agent-abbc7760162df55c1
  • Model: claude-opus-5[1m] (Opus 5, 1M context), as stated in my system prompt
  • Head reviewed: 2401cba11111960504622b9db28b5aff3ddcce6f (fetched myself with
    git fetch origin pull/341/head; signature %G? = G; its only parent is
    308cac8ba764f89d0556cb595a1525a99bdc9c61, the head of my first review)
  • Base used: origin/main = ebf2413239782817471353a8c90d91847bfb1a6e (unchanged; still the
    merge base, so no integration is needed)
  • Previous round: I gave CHANGES-NEEDED 308cac8 with one must-change (the then()
    derived-window remap unpinned) and three should-changes (S1 reporting guidance, S2 None →
    review, S3 when to upscale) plus eight suggestions.
  • I read the new diff and the changed docs myself before reading the PR body's new section, and I
    re-ran every check rather than trusting it.

Verdict in one paragraph

My blocker is properly fixed: I replayed my own mutant (_remap(shared, other.original_window, other.derived_window) → other.derived_window) against the new tree and the file now goes red on
the first of the three new assertions, and a second variant I invented for this round (the
original_window side of the same chain) is caught too — 12 of 12 mutations now fail, up from 8 of
9. S1, S2 and S3 are all addressed in both docs/vision.md and docs/evaluation.md, the zero-size
ValueError is in, and reviewer 1's metadata fix is well made and genuinely verified (the test runs
the real tools/check_recipe_assets.py and includes a negative control that the unprepared source
fails with png-metadata/iCCP). I am blocking on one new thing only, four words of new prose:
docs/vision.md:392-394 now asserts that a padded 0.13%-content crop is "too little for Jev to
use", which is an unmeasured statement about Jev's quality — forbidden by
docs/offline-and-live.md:87-93 ("Where the cookbook describes what Jev does, it describes it from
the documentation, with a link … It makes no statement about Jev's quality, latency or cost") — and
it contradicts the sourced sentence 40 lines above it (docs/vision.md:348-350), where the Images
documentation is quoted as offering padding as a remedy. Delete or reword the clause and I approve;
no test pins it.

Must change

M1. New prose makes an unmeasured claim about Jev, and contradicts the sourced paragraph above it

docs/vision.md:390-396:

Choose upscale once per recipe. The default pads, which keeps every original pixel and is
enough when a crop only has to clear the 280 pixel minimum. But a small crop is then almost all
padding: a 10 × 10 crop comes back 280 × 280 with the content covering 0.13% of the image, too
little for Jev to use.
Pass upscale=True when the point of the crop is to show the target
larger …

Two problems with the bolded clause:

  1. It is a statement about Jev's quality with no recorded inference behind it.
    docs/offline-and-live.md:87-93 states the rule for the cookbook as a whole, not only for
    recipes: what Jev does is described "from the documentation, with a link", and "It makes no
    statement about Jev's quality, latency or cost. A statement like that is allowed only when it
    comes from recorded inference on a held-out set, with the model version stated."
    CONTRIBUTING.md §2 says the same. The irony is local: the paragraph immediately above
    (docs/vision.md:379-381) is careful to say the mid-gray pad colour "has not been measured
    against Jev", and this PR's own body lists that disclosure as decision 4. No measurement exists
    for this one either.
  2. It contradicts the section's own sourced statement. docs/vision.md:348-350 says "The
    Images documentation asks for this: scale the image so its
    longest side is at most 1568, and scale up or pad an image smaller than the minimum." A
    recipe author reading both sentences is told that the vendor offers padding as a remedy and then
    that the remedy does not work, with nothing to go on. That is exactly the kind of inference
    presented as fact that the rule above exists to keep out of the docs.

What to change — the geometry is the real argument and needs no claim about the model, e.g.

But a small crop is then almost all padding: a 10 × 10 crop comes back 280 × 280 with the content
covering 0.13% of the image, and padding adds no detail — the target stays exactly the size it was
in the source.

Deleting the four words outright is equally fine. The figure itself is correct and I verified it
independently (100 / 78_400 = 0.1276%, content window (27/56, 27/56, 29/56, 29/56)), and the
test at tests/test_vision.py:2162-2187 pins only "the content covering 0.13% of the image", so no
test changes.

Should change (not blocking)

S1 (new). Stripping metadata also strips the orientation that image_asset used to refuse

The metadata rebuild (src/jev_cookbook/vision.py:821,
prepared = Image.frombytes("RGB", prepared.size, prepared.tobytes())) is the right fix for the
chunk problem, but it also drops info["exif"], and with it the one piece of evidence the asset
loader checks. I measured the before/after on the same source (a JPEG whose EXIF orientation is 6):

unprepared JPEG, orientation=6 -> validate_image_asset refuses: "rotate EXIF orientation into
                                   pixels with ImageOps.exif_transpose and save the upright image
                                   before loading it"
prepared  JPEG, orientation=6 -> ACCEPTED by validate_image_asset (pixels still sideways)

So a pipeline of load → prepare_image → save → image_asset now accepts a sideways image where
before the fix it stopped (and a prepared PNG would have failed the committed-asset contract on its
eXIf chunk — which is why the metadata had to go). The docstring and the docs both warn
("Orientation is not applied: rotate the pixels first with ImageOps.exif_transpose"), and the case
is close to unreachable in this repository, since CONTRIBUTING.md §5 requires images generated by
the recipe's own code and forbids photographs. Still, the module's house rule everywhere else is to
refuse rather than silently alter — it refuses transparency for precisely this reason ("the RGB it
hides is arbitrary"). Refusing an image whose EXIF orientation is not 1, with the same message
_image_format already uses, would keep both the clean output and the guard. Three lines, and it
would mirror the existing check rather than invent one.

S2 (carried, trivial). _asset_checker() re-execs the tool once per parametrized case

tests/test_vision.py:2094-2101 loads tools/check_recipe_assets.py inside the test body, so the
four size cases exec it four times and leave it in sys.modules. The pattern matches the house
style (tests/test_check_recipe_assets.py:28-30, same comment), so this is only a tidiness note: a
module-level load or an lru_cache would do.

First-round findings: what happened to each

Round-1 finding Status
M1 then()'s derived-window remap unpinned Resolved. tests/test_vision.py:1983-1992 adds the three assertions (derived_window == (1/7, 9/10, 6/7, 1); the padding-only top half → None; a partly-clipped box → nb(0.5, 0.0, 1.0, 0.05)), each with the arithmetic worked out in a comment. My original mutant now fails on the first of them, and a new variant that drops the original_window remap instead fails test_a_box_found_two_levels_down_maps_to_the_full_image_exactly. I also read the three values off the live objects: they are what the test says.
S1 reporting guidance named only tile_to_image Resolved, in both places. docs/vision.md:253-262 and docs/evaluation.md:765-770 now say "after tile_to_image, or, when the recipe also cropped or padded inside the tile, after ImageTransform.to_original on the chain that .then built from every step", with a link to the mapping section.
S2 no outcome named for a None from to_original Resolved and consistent (see below).
S3 no guidance on choosing upscale Addressed, and the new paragraph is the right shape — except for the clause in M1.
Suggestion 1, ZeroDivisionError on a zero-sized image Resolved. src/jev_cookbook/vision.py:785-786 adds the same _positive_int pair generate_tiles uses; I verified all six combinations of (0,0), (300,0), (0,300) × upscale raise ValueError naming the right axis, and removing the guard turns the file red.
Suggestions 2, 3, 4, 5, 6, 7, 8 (message wording, tile bounds label, from_crop naming, crop+transform ergonomics, ULP sliver, long lines, docs rationale) Not taken, all explicitly non-blocking; I do not press any of them. The only new long line is the 104-character anchor link at docs/vision.md:258.

Is the None → review guidance consistent with localization_report and the tiled section? Yes,
and it is now stated in three places that agree: docs/vision.md:257-262 (tiled section, "report
("review", None), never ("absent", None): a rectangle in padding is not evidence that the target
is missing"), docs/vision.md:421-426 (mapping section, same rule with a link back), and
docs/evaluation.md:765-770 ("a mapped-back answer that is only padding (None) as review"). It
matches localization_report's contract exactly: the three outcome kinds are located/absent/
review, ("located", None) is rejected (I checked: ValueError: example 0: box needs top, bottom, left and right edges), and a review is never counted as an absent prediction, so a padding-only
answer lands in present_review/absent_review rather than inventing a miss. It also matches
CONTRIBUTING.md §4 ("Uncertain or inconsistent results go to an explicit review outcome") and the
localization_outcome policy at docs/vision.md:146-156. The builder's choice not to present
image_to_tile as a source of the answer rectangle is correct — it maps gold into a tile, the
opposite direction — and the parenthesis saying so is a useful warning.

Keeping "after tile_to_image" because a test needs it: correct, and the reasoning holds up. A
pre-existing test from #311/#331, test_the_docs_say_a_tiled_recipe_reports_per_image_not_per_tile
(tests/test_vision.py:1217-1236), requires the literal substring after `tile_to_image` in that
section. The phrase is still true for a tile-only recipe, so extending the sentence rather than
replacing the phrase is the right move: it keeps another issue's guard intact and says the whole
truth. Nothing was weakened to make a check pass.

Review of git diff 308cac8 2401cba

Four files, +149/−6, all 100644 → 100644:

:100644 100644 155025a 8707cea M	docs/evaluation.md
:100644 100644 6bcd020 a085b49 M	docs/vision.md
:100644 100644 a813094 536b9af M	src/jev_cookbook/vision.py
:100644 100644 7b1abbe 1bac780 M	tests/test_vision.py
  • vision.py:785-786 — the zero-size guard, placed after the mode check and before any arithmetic,
    using the module's existing helper and the same message text as generate_tiles:488-489. Correct.
  • vision.py:821 — Image.frombytes("RGB", prepared.size, prepared.tobytes()) with a comment
    explaining that convert()/resize() copy .info while only the pad path starts from a fresh
    canvas. prepared is RGB on every path at that point, so the rebuild is type-safe; it costs one
    full-size buffer (7.4 MB at 1568²) and I confirmed the identity path is still pixel-identical
    (prepared.tobytes() == source.tobytes()), so the docs' "a pixel-identical copy" claim still
    holds. .info == {} on all five paths I tried (fits / resize / pad / resize+pad / upscale), and
    both a PNG and a JPEG saved from the result still pass validate_image_asset.
  • vision.py:769-772 — docstring: metadata-free on any path, orientation not applied. Accurate.
  • Docs: the four edits described above. The metadata sentences are true as written — I verified a
    saved PNG contains exactly IHDR, IDAT, IEND, and that image_asset really does not inspect PNG
    chunks (_image_format checks format, stillness, EXIF orientation, dimensions, area and size), so
    "the metadata rules are tools/check_recipe_assets.py's" is the right pointer.
  • Tests: +111 lines. The metadata test is the strongest addition — it loads the repository's actual
    checker by path (house pattern), asserts check_image(...) == [] for the prepared image on four
    size classes, and then asserts the unprepared source fails with exactly ["png-metadata"] and
    iCCP in the message. That is an instrument run against a case where it must find something.
    The resampling test pins "an averaging filter, not nearest neighbour" without pinning Lanczos's
    kernel (I see greys [2, 51, 251] in columns 780–790), which is the right amount of pinning.
    The doc-text test pins the new sentences, including the 0.13% figure, which it also re-derives.
  • Nothing was removed, loosened, skipped or xfailed; no test was weakened to accommodate the change.

PR body's new section against the diff

The appended "After the two reviews" section is accurate on every point I can check, and it corrects
the earlier round honestly: it says the "nine mutations each caught" claim did not cover the then
branch, and that four files are touched, not three. I verified its figures myself: 6312 passed, 55 skipped, 50 deselected, 31 xfailed (my run: identical, 242.81s) and tests/test_vision.py 396
passed (mine: 396, twice). foundation/multimodal-base is indeed gone from the remote
(git ls-remote --heads origin "foundation/multimodal-*" lists only multimodal-api and
multimodal-integration). Two notes, neither about truthfulness:

  • The older "Not changed" section still reads "docs/evaluation.md line 766 still names
    tile_to_image (correct as it stands). No other file was touched", which is now false in
    isolation; it is rescued only by the new section's "supersedes the figures and statements above
    where they differ". A reader of the body alone meets the contradiction before the resolution.
  • Editing the body after the final push is what CONTRIBUTING.md:121 tells contributors not to do
    ("A pull-request description is written once, at open (or with the first push), and never edited
    after the final push … Record fix-round notes and integration receipts in pull-request comments
    instead"), precisely because the edited event fires a second Scope run. It did: head 2401cba
    carries two Scope (pull_request_target) runs, 39712008075 and 39712507631. Both are
    success, so the Readiness helper: duplicate successful Scope rows on one head (synchronize + edited) fail closed; decide the rule #138 collapse rule applies and the head is still mergeable — benign here, but
    the next such edit may not be, and the note belonged in a comment.

Checks I ran on 2401cba

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

Check Result
ruff check . All checks passed!, exit 0
ruff format --check . 405 files already formatted, exit 0
pytest (full) 6312 passed, 55 skipped, 50 deselected, 31 xfailed in 242.81s, exit 0
pytest tests/test_vision.py ×2 396 passed in 4.65s, then 396 passed in 3.66s — same count, not flaky
python tools/check_hygiene.py Hygiene check passed: 615 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 exit 0
Worked example re-extracted from the new docs/vision.md and run outside its test prints the five documented lines, MATCH: True
Seven-row size table recomputed independently all seven sizes and all four quoted content rectangles match, unchanged from round 1
12 mutations (my 9 from round 1, a second then variant, the metadata rebuild, the zero-size guard) 12 of 12 caught. then_wrong_window → test_then_refuses_mismatched_sizes_and_content_that_is_all_padding; then_wrong_window_self → test_a_box_found_two_levels_down_maps_to_the_full_image_exactly; no_metadata_rebuild → the new metadata test; no_zero_size_guard → the new zero-size test; the other eight as in round 1. Baseline on the copied tree: 396 passed.
8 probes (size table, metadata on five paths incl. PNG chunks and JPEG re-validation, identity pixels, zero sizes, EXIF orientation, 0.13% figure, the pinned then windows, resampling) as reported above; the EXIF one is S1
Commit hygiene signed (G), one commit on 308cac8, Refs #310 in the message, Co-Authored-By: Claude Sonnet 5.5 present; no rebase, no force-push (the first head is still the parent)
CI on this exact head every row green: Lint (ruff), Catalog, Tests (py3.10) 3m14s, Tests (py3.14) 4m41s, Hygiene, Fixtures (validate), Notebooks (discover/execute), Scope; read, not assumed

No network call, no API key, no live inference, no notebook executed in the tree under review (no
recipe uses these helpers yet).

Acceptance criteria of #310

Unchanged from my first review: all five are met, and the multi-level criterion is now also properly
guarded by tests. The new doc sentences do not disturb any of them; M1 is a disclosure problem in
new prose, not an unmet criterion.

Scope

Four files against origin/main (docs/evaluation.md, docs/vision.md,
src/jev_cookbook/vision.py, tests/test_vision.py), all regular-file modifications, no mode or
type change, no rename or deletion, nothing in catalog/, tools/, .github/ or recipes/.
docs/evaluation.md is new to the diff this round and is a legitimate foundation edit (the
cross-reference I asked for); the body discloses it. No secrets, no absolute home paths.

OPUS-REVIEW: CHANGES-NEEDED 2401cba

@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

Re-review of PR #341 — reviewer 1 of 2 (independent), round 2

  • Agent ID: reviewer 1 of 2 for PR Foundation: image preparation, tile inverse, multi-level box mapping and a 280 px minimum check (#310) #341, worktree
    .claude/worktrees/agent-a0d672c24d82c4de1
  • Model: claude-opus-5[1m] (Opus 5, 1M context), as stated in my system prompt
  • Head reviewed: 2401cba11111960504622b9db28b5aff3ddcce6f (fetched myself with
    git fetch origin pull/341/head and checked out; signature %G? = G; one commit whose only
    parent is 308cac8ba764f89d0556cb595a1525a99bdc9c61; message ends Refs #310 and
    Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>)
  • Previous head reviewed: 308cac8ba764f89d0556cb595a1525a99bdc9c61
    (.claude/reviews/pr341-r1.md, verdict CHANGES-NEEDED with one must-change, M1)
  • Base: git merge-base HEAD origin/main = ebf2413239782817471353a8c90d91847bfb1a6e, the
    base I used for both rounds. origin/main moved during this review — see "The base moved"
    below; the merge is clean and nothing I measured is affected.
  • I checked out the head myself and did not rely on the PR description. Reviewer 2's review is
    posted on the PR; I have not read it (I saw only its header lines while checking for prior
    comments in round 1).

Verdict

M1 is resolved, and resolved properly. prepare_image now returns a metadata-free image on
every path, I verified that against the repository's own checker on all eight shape/upscale
paths with a source carrying icc_profile, exif, dpi and text, and I confirmed the fix
changed no pixel and no saved byte (all twelve pixel digests and both PNG file digests are
unchanged from 308cac8 and identical between Pillow 13.0.0 and 12.3.0). The one mutant that
survived my first round (LANCZOS -> NEAREST) is now killed, and the new test pins the filter
family only — BILINEAR, BICUBIC and HAMMING all still pass, so the claim "it does not pin
Lanczos itself" is exactly true. 33 of 33 applicable mutants of mine are now killed (0
survived).
My property tests give bit-identical results to round 1, and I added a new
pixel-level cross-check: over 400 random sizes the transform's content window equals the content
rectangle read back from the prepared image's own pixels, exactly, every time. Every number in
the PR body's new section reproduced exactly. APPROVE.


1. Is M1 resolved?

Yes. The fix (src/jev_cookbook/vision.py:818-821) is the one-liner I suggested, placed
after both branches so it covers all three paths:

# convert() and resize() copy the source's .info (an ICC profile would become an iCCP chunk
# when saved); only the pad path starts from a fresh canvas. Rebuild from the pixel bytes so
# every path returns an image with no metadata.
prepared = Image.frombytes("RGB", prepared.size, prepared.tobytes())

I replayed my own round-1 save/iCCP route rather than trusting the new test. A source carrying
icc_profile, exif, dpi and a Comment, through every path, saved as PNG and as baseline
JPEG, then run through tools/check_recipe_assets.check_image:

path result .info PNG chunks PNG findings JPEG findings
already fits, 400 x 300 400 x 300 {} IHDR, IDAT, IEND [] []
resize only, 2000 x 1500 1568 x 1176 {} IHDR, IDAT, IEND [] []
pad only, 300 x 200 300 x 280 {} IHDR, IDAT, IEND [] []
resize + pad, 4000 x 100 1568 x 280 {} IHDR, IDAT, IEND [] []
tiny pad, 10 x 10 280 x 280 {} IHDR, IDAT, IEND [] []
upscale=True, 200 x 100 / 10 x 10 / 1568 x 100 560 x 280 / 280 x 280 / 1568 x 280 {} — [] —

Round 1's failures ([png-metadata] chunk iCCP is not allowed on the fits and resize-only paths)
are gone. I also re-ran the negative control: the same source saved without preparation is
still rejected with exactly png-metadata naming iCCP, so the guard demonstrably fails where
it must — and the new test asserts that negative control itself, which is what hard rule 9 asks
for.

The fix is pixel-neutral, which matters because a changed pixel would move committed asset
digests and replay keys:

  • the padded content rows still equal the source tobytes() exactly;
  • the resize path's pixels equal a plain convert("RGB").resize((1568, 784), LANCZOS) exactly;
  • all twelve prepare_image pixel digests and both saved-PNG digests on my textured test page are
    identical to the values I measured on 308cac8 (e.g. 1a6455b3d12474ac,
    ebefabf4c7b4af29, PNG 3a245433858cbe51 / 616bcdfcd14b6528) and identical between Pillow
    13.0.0 (3.14.7) and Pillow 12.3.0 (3.10.21)
    .

The docstring and docs/vision.md now both say the result carries no metadata on any path, and —
closing the exact gap I flagged — that "image_asset does not check PNG chunks; the metadata
rules are tools/check_recipe_assets.py's (run by CI on committed assets)."

EXIF orientation: acceptable, not a trap

The new docstring says "Orientation is not applied: rotate the pixels first with
ImageOps.exif_transpose." I tested what that means in practice: I built a JPEG whose stored
pixels are landscape (a red band along the stored top) with orientation tag 6, opened it, and ran
prepare_image. The pixels come back un-rotated and the tag is gone, so the saved PNG passes
the contract and validate_image_asset accepts it — where an author who had passed the
original file to image_asset would have got the explicit refusal ("rotate EXIF orientation into
pixels with ImageOps.exif_transpose…"). So the orientation tag is laundered away.

I still judge this acceptable, for three independent reasons:

  1. It is not new and not specific to prepare_image. On 308cac8 the tag never reached a
    saved file either: Pillow's PNG writer takes exif from encoderinfo only (I confirmed no
    eXIf chunk on either Pillow version) and so does its JPEG writer. The behaviour is
    unchanged; only the .info dictionary is now consistently empty.
  2. It matches the repository's own sanctioned snippet. docs/fixtures.md's clean copy
    (Image.frombytes("RGB", rgb.size, rgb.tobytes())) drops the tag too, and that document says
    so in as many words: "Apply ImageOps.exif_transpose before the copy … because the copy drops
    the tag." prepare_image now behaves identically to the pattern the contract prescribes.
  3. The input class is banned. The only realistic source of an orientation tag is a camera
    photograph, and CONTRIBUTING.md:92 requires assets to be "generated by the recipe's own code"
    with "no real photographs". The instruction is now in three places (the docstring,
    docs/vision.md, docs/fixtures.md:376).

Optional hardening, offered as a suggestion only (S8 below): mirror _image_format and raise when
image.getexif().get(274, 1) != 1. That would turn the one silent case into a loud one for three
lines, but I would not hold the merge for it.

Mode conversions

All accepted modes convert cleanly, carry no metadata and pass the contract; everything with an
alpha channel, a transparency key or a non-8-bit channel is refused:

input result
L 90 RGB, (90, 90, 90), .info={}, findings []
1 RGB, (255, 255, 255), .info={}, findings []
CMYK RGB, (207, 198, 190), .info={}, findings []
YCbCr RGB, .info={}, findings []
P (palette index 3 -> (9, 8, 7)) RGB, (9, 8, 7), .info={}, findings []
RGBA, LA, PA, RGBa, La ValueError ("flatten it onto an explicit background…")
I, F, I;16, I;16B ValueError — 16-bit and float channels are refused, not truncated
HSV, LAB ValueError
P with info["transparency"], RGB with a colour-key transparency ValueError

2. Property tests re-run against the new code

The geometry functions are byte-identical in this diff, so I expected — and got — results
identical to round 1 (same seed, same independent exact-rational model built from scratch):

property cases result
random chains of 1–3 crop / resize+pad steps built with then 37,362 all built; only the documented "shows none of the first transform's content" raise
to_derived vs my exact model (float equality) 20,338 0 mismatches
to_original vs my exact model (float equality) 24,941 0 mismatches
to_derived returning None where my model says empty 14,126 0 disagreements
then associativity (left fold == right fold) 37,362 0 differences
visible_fraction vs exact area arithmetic single-step subset 0 differences
total problems — 0

New this round, because prepare_image changed at its end — an independent pixel-level check
of the transform rather than of the arithmetic. For 400 random sizes (1–2500 px each side) across
both upscale settings, with pad_color=0 on a white source, I read the content rectangle back
out of the prepared image with getbbox() and compared it with transform.derived_window:

  • derived_window == (bbox / prepared size) as exact fractions: all 400 match;
  • both sides in [280, 1568]: all 400;
  • area in [78,400, 2,458,624]: all 400;
  • .info == {}: all 400.

Edge cases re-run on the new head: NaN, ±inf, negative, >1, zero-width/height, three-edge, None
and non-mapping boxes all raise ValueError with a named edge from to_derived, to_original
and visible_fraction. 1 x 1 -> 280 x 280; 279 x 279 -> 280 x 280; 280 x 280 and 1568 x 1568
unchanged; 1568 x 1569 -> 1567 x 1568; 10000 x 1 -> 1568 x 280; 1 x 10000 -> 280 x 1568;
3136 x 3136 -> 1568 x 1568. The new guard fires correctly in all three orientations:
(0, 0) and (0, 300) -> "image width must be a positive integer", (300, 0) -> "image height
must be a positive integer" — the same wording generate_tiles already uses.


3. Review of git diff 308cac8 2401cba (4 files, +149/-6)

src/jev_cookbook/vision.py (+11). The frombytes copy (above), the two
_positive_int(width/height, "image …") calls, and four docstring lines. The zero-size guard sits
after the mode check and before any arithmetic, so a 0-sized image can no longer reach
scaled() (where it previously clamped to 1 px and resized from an empty image). Message wording
matches generate_tiles. Correct.

tests/test_vision.py (+111). I verified the three new then assertions by hand in exact
fractions rather than taking the comments on trust:

  • pad = prepare_image(200 x 100) -> derived_window = (1/7, 9/28, 6/7, 19/28);
    other = crop((0, 0, 280, 100), 280, 280) -> original_window = (0, 0, 1, 5/14);
    shared = (1/7, 9/28, 6/7, 5/14); the second remap gives
    (1/7, (9/28)(14/5), 6/7, (5/14)(14/5)) = (1/7, 9/10, 6/7, 1) — exactly the asserted value,
    and the comment's "using the second step's whole derived image instead would give (0, 0, 1, 1)"
    is the right counterfactual.
  • rows 0..50 of the crop: bottom = 1/2 < top = 9/10 after clipping -> None. Correct.
  • to_original(0.5, 0.9, 1.0, 0.95): edges snap on (280, 100) to (1/2, 9/10, 1, 19/20), clip
    to (1/2, 9/10, 6/7, 19/20), remap with fx = 7/5, fy = 1 onto (0, 0, 1, 1/10) ->
    (1/2, 0, 1, 1/20) = nb(0.5, 0.0, 1.0, 0.05). Exactly the asserted value, and the
    comment's arithmetic (1/2 - 1/7) / (5/7) = 1/2 is right.

The metadata test is a real check, not a restatement: it loads the repository's actual
tools/check_recipe_assets.py by path, asserts check_image(...) == [] on the prepared file, and
asserts the unprepared source is rejected with exactly ["png-metadata"] naming iCCP. Its
four parametrize ids map to the four real paths. The .info == {} and explicit
["IHDR", "IDAT", "IEND"] assertions are direct, not inferred.

The averaging-filter test is scoped exactly as advertised — see §5.

docs/vision.md (+27/-6) and docs/evaluation.md (+6/-2). Accurate. to_original returning
None is now reported as ("review", None), never ("absent", None), in both documents, with the
right justification ("a rectangle in padding is not evidence that the target is missing"); the
upscale guidance is quantitatively right (100 / 78,400 = 0.1276% -> 0.13%, and I confirmed a
10 x 10 source gives derived_window = (27/56, 27/56, 29/56, 29/56), i.e. pixels 135..145 of 280);
image_to_tile is explicitly named as the gold-labelling direction and "not a source of the
per-image answer rectangle", which keeps it consistent with docs/fixtures.md:141-146. The
docs/evaluation.md edit is exactly my round-1 S7.


4. The PR body's new section against the diff

Checked line by line; accurate.

claim my result
then()'s derived-window remap is now pinned (three assertions) Correct — I re-derived all three by hand (§3)
prepare_image returns a metadata-free RGB image on every path; .info empty; PNG only IHDR/IDAT/IEND; passes check_recipe_assets.py; docs say image_asset does not check PNG chunks Correct on all five counts, verified independently (§1)
A zero-sized image raises ValueError Correct, in all three orientations
A test pins that the filter is averaging, not nearest-neighbour, and does not pin Lanczos itself Correct and precisely stated — NEAREST fails, BILINEAR/BICUBIC/HAMMING pass (§5)
four files touched, not three Correct
Full pytest 6312 passed, 55 skipped, 50 deselected, 31 xfailed on 3.14 Reproduced exactly
tests/test_vision.py 396 passed on 3.10 Reproduced exactly
foundation/multimodal-base … no longer exists on the remote Correct; this is the round-1 staleness, now disclosed

One precision point, not an overclaim: "the earlier 'nine mutations each caught' did not cover
that branch" is a statement about the author's own mutation set, and is fair. But a reader could
take it to mean the branch was previously unconstrained, and it was not: I ran both of my
second-remap mutants (-> _FULL, and "uses the first transform's windows") against a copy of
308cac8's test file and both were already killed there (4 and 3 failures). What the new
assertions add is a direct assertion of the window's value instead of coverage-by-consequence —
worth having, and more precise, but not first coverage. (Both the temporary test copy and the
mutation were removed; git status clean.)


5. Mutation testing — 33 of 33 killed

Same 27 mutants as round 1 plus 7 new ones aimed at this commit. 34 attempted, 1 skipped (a
non-unique anchor), 33 killed, 0 survived:

  • round-1 set, all killed again: never snap / always snap; _clip counts touching; _remap swaps
    fx/fy; scaled ceil / half-down / floor; odd pad pixel left/top; content top-left; min_side
    strict; min_side on the long side; generate_tiles ignores a small source; generate_tiles
    no check; crop_box no check; crop_box checks after cropping; then wrong first remap; then
    no empty-intersection check; upscale ignores max_side; upscale overrides a downscale;
    _as_box collapses; to_original clips to the wrong window; visible_fraction returns an area;
    no min_side > max_side check; pad_color accepts 256; transparency allowed through;
    crop_bounds drops the expansion.
  • LANCZOS -> NEAREST: now KILLED (my round-1 S1, the only survivor then).
  • new, all killed: the frombytes copy removed; the frombytes copy only on the pad path; the
    zero-size check removed; then's second remap -> _FULL; then's second remap using the first
    transform's windows.
  • skipped: a width-only variant of the zero-size check (its anchor appears twice in the file);
    the full-removal variant above already covers it.

The filter test's scope, measured: with the filter swapped to NEAREST the suite fails; with
BOX it fails (incidentally, on a different test); with BILINEAR, BICUBIC and HAMMING all
396 tests pass
. So the test pins the filter family and genuinely does not pin Lanczos's kernel
— which is the right trade-off: it kills the mutant I cared about without making the suite hostage
to a Pillow kernel refinement, and the PR body says precisely that.

The worktree was restored after every mutation run (git status clean, git rev-parse HEAD =
2401cba…).


6. Checks I ran

Fresh venvs under /home/agent/r1b/ (uv venv --python 3.14 / --python 3.10, then
uv pip install --link-mode symlink -r a one-line requirements file holding -e <worktree>[dev]),
every heavy command under flock .claude/heavy.lock nice -n 10, pytest -p no:cacheprovider, no
nested flock, background wrapper + log polling for the long runs, available memory checked first
(4.5 GB).

command result
ruff check . (0.17.1) All checks passed!, exit 0
ruff format --check . 405 files already formatted, exit 0
pytest (3.14.7, Pillow 13.0.0) 6312 passed, 55 skipped, 50 deselected, 31 xfailed in 237 s, exit 0
pytest tests/test_vision.py (3.10.21, Pillow 12.3.0) 396 passed in 5.0 s
python tools/check_hygiene.py passed: 615 file(s) scanned, exit 0
python tools/check_recipe_assets.py passed: 461 file(s) under recipes/ checked, exit 0
python tools/render_catalog.py --check exit 0
python tools/notebook_ci.py fixtures 51 recipe folders, 0 failed, exit 0
my independent exact-model property tests 37,362 chains / 45,279 mappings, 0 problems
my 34 mutants 33 killed, 0 survived, 1 skipped
filter-family scope (BOX/BILINEAR/BICUBIC/HAMMING/NEAREST) family pinned, kernel not
prepare_image digests, Pillow 13.0.0 vs 12.3.0 vs 308cac8 all identical
check_recipe_assets.check_image on prepare_image output, 8 paths, PNG + JPEG [] everywhere; unprepared control still png-metadata/iCCP
400 random sizes: window vs real pixel bbox, limits, .info all match
git merge-tree 23f523e 2401cba clean, no conflicts

No notebooks, recipes or fixtures are touched by the diff, so no notebook execution was needed and
tools/check_recipe_scope.py has nothing to judge.


7. The base moved during this review

origin/main advanced from ebf2413 to 23f523e ("Foundation: bind the confidence gate to
the shared helper and a live review branch (#282, #227, #272) (#336)") while I was running the
suite — another agent fetched it into the shared object store. So:

  • git merge-base HEAD origin/main is still ebf2413: Foundation: image preparation, tile inverse, multi-level box mapping and a 280 px minimum check (#310) #341 is now one commit behind main.
  • git merge-tree --write-tree 23f523e 2401cba produced a single tree with no conflicts.
  • main's new commit touches CONTRIBUTING.md, docs/evaluation.md, docs/recipe-template.md
    and tests/test_recipe_guard.py. The only file it shares with this PR is docs/evaluation.md,
    and the two edits are far apart (its insertion is at ~line 276, this PR's at ~line 763);
    tests/test_recipe_guard.py is independent of vision.py.

I ran every check on the head alone, as my brief instructed. Given the clean merge and the
non-overlapping files I see no risk, but the orchestrator should note that a merge of origin/main
into the branch is now needed before it goes in, and may want the suite re-run on the merged tree.


8. Status of my round-1 findings

# Finding Status
M1 prepare_image propagates .info; saved PNG carries iCCP and fails png-metadata on 2 of 3 paths Resolved. Clean frombytes copy on every path; .info == {} and check_image(...) == [] on all 8 paths I tested; pixel- and byte-neutral; negative control asserted by the new test; both docs updated.
S1 Nothing pins the resampling filter (my only surviving mutant) Resolved. NEAREST is now killed; the test pins the family, not the kernel, and says so.
S2 upscale is not type-validated (upscale="yes" is truthy) Not changed (still accepted; the docs now explain when to pass it instead). Minor; leaving it is defensible.
S3 EXIF orientation unmentioned Resolved in documentation — the docstring and docs/vision.md now say orientation is not applied and to use ImageOps.exif_transpose. See §1 for why I accept the documented behaviour, and S8 for optional hardening.
S4 Four added lines over the configured line-length = 100 Not changed. vision.py:371 (108), :380 (102), :597 (114) — :140 was pre-existing — plus two 101–102-char test lines and docs/vision.md:307. E501 is not in ruff's select, so nothing flags them. Cosmetic.
S5 The "too small for a float rectangle" raise escapes a None-returning API Not changed, and I am content: docs/vision.md already carries the bullet "A mapped box too small to be a float rectangle raises ValueError, never a collapsed box", and I again could not reach it from anything realistic.
S6 ImageTransform's public fields annotated with the private _Edges alias Not changed. Cosmetic.
S7 docs/evaluation.md:766 named only tile_to_image Resolved — now names the ImageTransform.to_original chain and the None -> review rule.
S8 (new, optional) prepare_image silently drops a non-upright source's orientation tag, so the result passes validate_image_asset where the original file would have been refused Suggestion only. Mirroring _image_format (raise when image.getexif().get(274, 1) != 1) would make the one silent case loud for three lines. Not required: the behaviour is unchanged from 308cac8, identical to docs/fixtures.md's own clean-copy snippet, documented in three places, and the input class is banned by CONTRIBUTING.md:92.

All five acceptance criteria of #310 remain met (round 1, §"Acceptance criteria"), and criteria 3
and 5 are now better pinned: the then chain's derived window is asserted by value, and the docs
state how a mapped-back None is reported.

Nothing in the remaining suggestions would stop me being comfortable with this merging.

OPUS-REVIEW: APPROVE 2401cba

CompleteDotTech and others added 2 commits October 22, 2026 04:23
…nce; refuse EXIF orientation in prepare_image (#310)

The upscale paragraph now states only the arithmetic (a 10 x 10 crop padded to 280 x 280 is
0.13% content) and that the choice is a design decision not yet measured against Jev.
prepare_image drops EXIF, so it now refuses a non-identity orientation the way image_asset does
and reports damaged EXIF as a ValueError.

Refs #310

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

Re-review of PR #341 (round 3) — reviewer 2 of 2 (independent)

  • Agent ID: reviewer 2 of 2 for PR Foundation: image preparation, tile inverse, multi-level box mapping and a 280 px minimum check (#310) #341, worktree .claude/worktrees/agent-abbc7760162df55c1
  • Model: claude-opus-5[1m] (Opus 5, 1M context), as stated in my system prompt
  • Head reviewed: 3b01fc1a8cbaf754afb0ecd48a5c38dadd97dd92 (fetched with
    git fetch origin pull/341/head; signature %G? = G; a merge commit whose parents are
    b46d93570ae55417be1b2195aa8d8fa0332524a4 — one signed commit on top of 2401cba, the head of my
    round-2 review — and 717a3f0d3ffd09995350a8558637f95ee1e3160f)
  • Base used: origin/main = 717a3f0d3ffd09995350a8558637f95ee1e3160f, which is also
    git merge-base origin/main HEAD, so the branch is fully up to date with main
  • Previous rounds: CHANGES-NEEDED 308cac8 (unpinned then() remap + three docs items) and
    CHANGES-NEEDED 2401cba (one must-change: the unmeasured "too little for Jev to use" clause; one
    non-blocking note: prepare_image silently dropping an EXIF orientation that image_asset refuses)
  • I read the diff and the changed docs myself before the PR body, and re-ran every check.

Verdict in one paragraph

My blocker is resolved: the clause is gone, and the rewritten paragraph (docs/vision.md:392-398)
now states only arithmetic — padding clears the 280 px minimum and adds no detail (0.13% content for
a 10 × 10 crop, the target no larger than it was), upscale=True scales the crop up so the target
fills more of what is sent — and then says outright that "Which of the two works better for a recipe
has not been measured against Jev; treat it as a design decision to settle on validation examples".
I grepped every occurrence of "Jev" in both changed docs: all seven are API mechanics or explicit
non-measurement disclosures, and nothing now claims what Jev can or cannot do with an image or with
padding, so the section no longer contradicts the sourced Images-documentation sentence 40 lines
above it that offers padding as a remedy. The EXIF change that followed my non-blocking note is
better than the doc warning it replaces, and I verified the part that could have been a trap: the
documented remedy works (ImageOps.exif_transpose output is accepted for orientations 2, 3, 6 and 8),
orientation 1, no EXIF and EXIF without tag 274 are all accepted, an image that came through
image_asset is unaffected, and both refusals now share one _ORIENTATION_MESSAGE constant that a
test pins from both call sites. The merge brought nothing of its own, every check I ran is green
(6457 passed, 65 skipped, 50 deselected, 31 xfailed; tests/test_vision.py 402 twice), and 15 of
15 mutations
— my 12 from earlier rounds plus three aimed at the new EXIF code — turn the file red.
APPROVE. One record item for whoever merges, and it is not a code change: the PR body is stale in
the five ways listed below (a comment, not another body edit, is the right fix — a second edit would
fire another Scope run). CI on this head is now fully green: the three jobs that were still running
when I first looked (Tests (py3.10), Tests (py3.14), Notebooks (execute)) all passed when I
rechecked, so every current check has succeeded on the exact head I approve.

1. Is the blocker resolved?

Yes. docs/vision.md:392-398 now reads:

Choose upscale once per recipe. The default pads, which keeps every original pixel and clears
the 280 pixel minimum. A small crop is then almost all padding, and padding adds no detail: a
10 × 10 crop comes back 280 × 280 with the content covering 0.13% of the image, the target no
larger than it was in the source. upscale=True instead scales the crop up so the target fills
more of the image sent. Which of the two works better for a recipe has not been measured against
Jev; treat it as a design decision to settle on validation examples, then keep it, like
pad_color, the same across the recipe's images.

Every sentence is now either arithmetic I verified or a disclosure:

  • "padding adds no detail … the target no larger than it was in the source" — a statement about the
    image, not about the model. True by construction (the default path only pastes onto a canvas).
  • "0.13% of the image" — I measured 0.1276% for a 10 × 10 crop (content window
    (27/56, 27/56, 29/56, 29/56) of 280 × 280), which rounds to 0.13%; the test re-derives it.
  • "upscale=True instead scales the crop up so the target fills more of the image sent" — I checked
    the same 10 × 10 crop with upscale=True: it comes back 280 × 280 with no padding, so the
    content goes from 0.13% to 100% of what is sent. Accurate.
  • "has not been measured against Jev; treat it as a design decision to settle on validation
    examples" — the disclosure the contract asks for, and the "settle it on validation" instruction
    matches CONTRIBUTING.md §2's own method ("Choose … on validation examples and freeze them").

Any remaining claim about what Jev can or cannot do? No. Every "Jev" in the two changed files:

Where Text Verdict
docs/vision.md:344 "a recipe that sends tiles or crops to Jev passes min_side=…" mechanics
docs/vision.md:379 pad colour "has not been measured against Jev" disclosure
docs/vision.md:397 "has not been measured against Jev; treat it as a design decision" disclosure
docs/vision.md:402 "A box that Jev returns is relative to the image it was shown" mechanics (Box docs)
docs/vision.md:444 "where a recipe would call Jev" mechanics
docs/evaluation.md:847 "Jev's Choice and Noul answers carry no time" mechanics (documented)
docs/evaluation.md:943 "not a measure of Jev" disclosure

The section is now internally consistent with docs/vision.md:348-350 ("The Images documentation
asks for this: scale the image so its longest side is at most 1568, and scale up or pad an image
smaller than the minimum"): both options stay on the table, each is described by what it does to the
pixels, and neither is declared better. tests/test_vision.py:2174-2175 pins both halves — the
disclosure must be present and "too little for Jev" must be absent — so the clause cannot creep
back in unnoticed.

2. The EXIF change, judged as a recipe builder

src/jev_cookbook/vision.py:789-794:

try:
    orientation = image.getexif().get(274, 1)  # the rebuild below drops EXIF and this evidence
except (struct.error, SyntaxError, OSError) as exc:
    raise ValueError("the image's EXIF data is damaged; remove it before preparing") from exc
if orientation != 1:
    raise ValueError(_ORIENTATION_MESSAGE)

Refusing beats silently dropping, and this is the right shape of refusal. In round 2 I measured
that the metadata rebuild had quietly disarmed the loader's orientation guard: the same JPEG
(orientation 6) was refused by validate_image_asset before preparation and accepted after it, with
sideways pixels. The module's house rule elsewhere is to refuse rather than guess — it refuses
transparency because "the RGB it hides is arbitrary" — and this follows it, at the earliest point
where the information still exists. Factoring the text into _ORIENTATION_MESSAGE
(vision.py:82-86, reused at :109 and :794) means the two refusals cannot drift apart, and
tests/test_vision.py:2204-2216 pins that both prepare_image and image_asset refuse the same
file
with the same message, for orientations 2, 3, 6 and 8.

Does it break a documented workflow? No — I checked each one on the real code:

Workflow Result
ImageOps.exif_transpose(image) → prepare_image (the remedy both the docstring and docs/vision.md:384-386 prescribe) accepted for all of orientations 2, 3, 6, 8; the prepared image's .info is {} and 6/8 come back correctly rotated to (300, 320)
orientation 1 / no EXIF / EXIF carrying other tags but not 274 accepted (prepared .info == {})
image_asset → Image.open → prepare_image accepted; a committed asset cannot carry EXIF anyway (image_asset refuses a non-identity orientation and the image-file contract forbids PNG eXIf and JPEG APPn)
generate_tiles / crop_box on an EXIF-bearing source → prepare_image(tile) refused, because Pillow's crop copies .info (I verified "exif" in tile.image.info is True). The refusal is correct — the source needs rotating — and it fires before any request
docs/vision.md worked example and the "Minimum size" snippet both still run; the worked example prints the five documented lines byte for byte

The message. "rotate EXIF orientation into pixels with ImageOps.exif_transpose and save the
upright image before loading it" is actionable, and sharing it with image_asset is worth more than
tailoring it. The trailing "before loading it" is loader-flavoured: at the tile stage there may be no
loading step left, and the thing to re-save is the source. A nit, not worth a round; if the file is
touched again, "…and use the upright image" would fit both callers.

Damaged EXIF. The new catch does what it says — I exercised five malformed blobs and got
ValueError: the image's EXIF data is damaged; remove it before preparing for three of them
(header-only, bogus bytes, non-EXIF text) and plain acceptance for the two Pillow parses as empty;
nothing leaked a non-ValueError. Worth knowing: prepare_image is now stricter than
image_asset here — the same broken blobs written into a real JPEG are accepted by
validate_image_asset (Pillow warns "Corrupt EXIF data" and getexif() returns nothing). That
asymmetry is in the safe direction and only reachable with corrupt metadata that the fixture contract
already forbids, so I note it rather than ask for anything. The message tells the builder what to do
but not how (del image.info["exif"], or re-save the pixels as docs/fixtures.md:371-379 already
shows); also only a nit.

3. git diff 2401cba b46d935 and git show --cc 3b01fc1

The builder commit (b46d935, signed, Refs #310, Co-Authored-By: Claude Sonnet 5.5, parent
2401cba — no rebase, no amend) touches three files, +69/−15, all 100644 → 100644:

  • src/jev_cookbook/vision.py — import struct; the _ORIENTATION_MESSAGE constant and its reuse
    in _image_format (a pure extraction: the string is unchanged, which I diffed character by
    character); the EXIF read, damaged-EXIF guard and refusal in prepare_image, placed after the
    mode/transparency check and before the size validation, so an RGBA image still gets the
    transparency message first; the docstring sentence at :776-778. Nothing else in the module
    changed.
  • docs/vision.md — the metadata paragraph's last sentence becomes the EXIF refusal
    (:384-386), and the upscale paragraph is rewritten (:392-398).
  • tests/test_vision.py — +43/−2: the metadata test's fake EXIF becomes a real
    Image.Exif() blob carrying ImageDescription (it had to: the old placeholder
    b"Exif\x00\x00II*\x00" is now correctly diagnosed as damaged, which is itself evidence the new
    guard bites), three new tests (refusal for four orientations plus the parallel image_asset
    refusal; acceptance of orientation 1 and no EXIF; damaged EXIF → ValueError), and three new
    assertions on the docs text including "too little for Jev" not in flat.

The merge (3b01fc1) is clean and introduced nothing of its own. I checked this the hard way
rather than trusting "auto-merged":

  • git merge-base b46d935 717a3f0 = ebf2413, the old main.
  • git diff 717a3f0 3b01fc1 lists exactly the branch's four files
    (docs/evaluation.md, docs/vision.md, src/jev_cookbook/vision.py, tests/test_vision.py).
  • git diff b46d935 3b01fc1 lists exactly main's own five
    (CONTRIBUTING.md, docs/evaluation.md, docs/recipe-template.md, tests/test_evaluation.py,
    tests/test_recipe_guard.py).
  • For docs/evaluation.md, the only file both sides touch, the branch-side hunk is byte-identical
    before and after the merge
    — same -/+ lines, only the hunk header moves from @@ -763,8 to
    @@ -806,8 because main grew the file above it. So the resolution kept both changes and edited
    neither.
  • No conflict markers anywhere (ruff, the full suite and the doc tests would all fail if there
    were; I also read the merged paragraph).

4. PR body against the diff — what is stale

The body still ends with the round-2 section, written for head 2401cba. Nothing in it is a false
claim about that head, but as a description of the head being merged it is stale in five ways:

  1. The EXIF behaviour change is absent entirely. prepare_image now raises ValueError for a
    non-identity EXIF orientation and for damaged EXIF — a behaviour change to a public helper, with
    its own tests and docs sentence, and the body does not mention it. This is the one that matters:
    CONTRIBUTING.md says a body "state[s] only what the final diff contains", and a reader of the
    body alone would not know the function can now refuse an input it used to accept. (The commit
    message of b46d935 does say it, clearly.)
  2. Dropping the unmeasured claim is not recorded. The body still says the docs explain "when to
    pass upscale=True"; it does not say the "too little for Jev to use" clause was removed and
    replaced by a disclosure, which was the entire reason for this round.
  3. The counts are stale. It says "6312 passed, 55 skipped … on 3.14; tests/test_vision.py 396
    passed". On this head I measure 6457 passed, 65 skipped, 50 deselected, 31 xfailed and 402
    in tests/test_vision.py (the extra tests are this round's three plus reviewer 1's; the extra
    skips come from main, not from this branch — tests/test_vision.py skips nothing).
  4. The merge of origin/main (717a3f0) is not mentioned, and the section header still says
    "head 2401cba".
  5. The older "Not changed" section is still false in isolation ("docs/evaluation.md line 766
    still names tile_to_image (correct as it stands). No other file was touched"), rescued only by
    the newer section's "supersedes … where they differ".

Recommended fix: a pull-request comment, not another body edit. CONTRIBUTING.md:121 says the
description is written once and never edited after the final push, with fix-round notes and
integration receipts in comments — precisely because an edited event fires a second Scope run
(the earlier edit already left head 2401cba with two Scope rows; both passed, so #138's collapse
rule saved it). There is no receipt comment for this round yet: the four comments on the PR are the
four review texts. If the squash-merge message is taken from the body, whoever merges should fold in
b46d935's commit message, which describes the EXIF change correctly. I am not blocking on this —
nothing in the body misdescribes the code, and the remedy needs no new head.

5. Checks I ran on 3b01fc1

Fresh Python 3.14.7 venv under /home/agent/ (uv venv --python 3.14,
uv pip install --link-mode symlink -e ".[dev]", Pillow 13.0.0), everything inside flock .claude/heavy.lock nice -n 10 (two acquisitions, no nesting), pytest -p no:cacheprovider.

Check Result
ruff check . All checks passed!, exit 0
ruff format --check . 405 files already formatted, exit 0
pytest (full) 6457 passed, 65 skipped, 50 deselected, 31 xfailed in 252.27s, exit 0
pytest tests/test_vision.py ×2 402 passed in 4.88s, then 402 passed in 3.49s — same count, not flaky
python tools/check_hygiene.py Hygiene check passed: 615 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 exit 0
Worked example + the "Minimum size" snippet, extracted from this head's docs/vision.md and run outside their tests example prints the five documented lines, MATCH: True; snippet runs clean
Seven-row size table and its four quoted content rectangles, recomputed independently (third round running) all match, unchanged
9 EXIF/docs probes (remedy, orientation 1 / none / other tags, tile and crop inheritance, asset round trip, five damaged blobs, the shared constant, the 0.13% and upscale figures, zero-size and PNG chunks) all as reported above; PROBES_OK
15 mutations 15 of 15 caught. New this round: no_orientation_refusal → test_prepare_image_refuses_a_non_identity_exif_orientation_like_image_asset; orientation_default_zero (.get(274, 1) → .get(274, 0)) → test_prepare_image_pads_one_axis_and_keeps_every_pixel; removing the damaged-EXIF try/except → test_prepare_image_reports_damaged_exif_as_a_value_error with the raw struct.error the guard converts. The twelve from rounds 1–2 (both then windows, snapping, half-up rounding, pad parity, min_side on tiles and crops, strict minimum, touch-as-overlap, unbounded upscale, the metadata rebuild, the zero-size guard) still all fail. Baseline on the copied tree: 402 passed.
Commit hygiene both new commits signed (G); b46d935 ends Refs #310 + Co-Authored-By: Claude Sonnet 5.5; merge commit carries the co-author trailer; parentage shows no rebase, force-push or amend
CI on this exact head All green. Lint (ruff), Catalog (README is current), Hygiene, Fixtures (validate), Notebooks (discover) and Scope (recipe pull requests) passed on my first read, when Tests (py3.10), Tests (py3.14) and Notebooks (execute) were still running (the head was minutes old); I waited and re-read them: Tests (py3.10) pass 5m7s, Tests (py3.14) pass 5m18s, Notebooks (execute) pass. Read from the PR, not assumed.

No network call, no API key, no live inference, nothing executed in the tree under review beyond the
read-only doc snippets, no notebook affected (no recipe uses these helpers yet).

Scope

Against origin/main (717a3f0), four files, all regular-file modifications, no mode or type
change, no rename or deletion, nothing in catalog/, tools/, .github/ or recipes/:

:100644 100644 602a56f b90268c M	docs/evaluation.md
:100644 100644 57990b5 cdf22c1 M	docs/vision.md
:100644 100644 959bfe9 3310e29 M	src/jev_cookbook/vision.py
:100644 100644 fe139f1 7da9ae0 M	tests/test_vision.py

Foundation branch, so the recipe allowlist does not apply; Scope (recipe pull requests) passes.
No secrets, no absolute home paths, no credential reference. Nothing was skipped, xfailed, loosened
or deleted to make a check pass — and the one existing test whose text constrains this work
(test_the_docs_say_a_tiled_recipe_reports_per_image_not_per_tile, which requires the literal
after `tile_to_image` ) was satisfied by extending the sentence rather than by editing the test.

Earlier findings, final status

Finding Status
R1 M1 then() derived-window remap unpinned Resolved in 2401cba, still pinned here (my mutant fails; a second variant fails too)
R1 S1 reporting guidance named only tile_to_image Resolved in both docs/vision.md:254-262 and docs/evaluation.md:808-814, and the merge preserved the evaluation hunk byte for byte
R1 S2 no outcome named for a None from to_original Resolved; ("review", None) in three agreeing places, consistent with localization_report's three outcome kinds
R1 S3 no guidance on choosing upscale Resolved, and now stated without any claim about Jev
R1 suggestion 1 ZeroDivisionError on a zero-sized image Resolved; removing the guard turns the file red
R2 M1 "too little for Jev to use" Resolved (section 1 above)
R2 S1 silently dropping an EXIF orientation image_asset refuses Resolved, better than I asked: refused, with a shared message and tests from both call sites
R2 S2 _asset_checker() re-execs the tool per case Not taken; trivial, matches house style, I do not press it
R1 suggestions 2–8 (message wording, the "tile bounds" label, from_crop naming, crop+transform ergonomics, the ULP sliver, long lines, the min_side rationale) Not taken, all explicitly non-blocking; the only new long line is a 104-character anchor link at docs/vision.md:258. The 'F'-mode message nit from R1 suggestion 2 is now joined by the two EXIF-message nits in section 2 — one future tidy-up, nothing more

Acceptance criteria of #310

All five met, unchanged from round 1, and now more tightly guarded: the multi-level criterion by the
three then() assertions, the min_side criterion by the before-any-crop tests, the prepare_image
criterion additionally by the metadata, zero-size and EXIF tests, and the worked-example criterion by
a byte-for-byte output comparison that I also ran outside its test.

OPUS-REVIEW: APPROVE 3b01fc1

@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

Record update for the final head 3b01fc1 (a comment, not a body edit, since the head is final). Supersedes the PR body where they differ:

  • prepare_image refuses a non-identity EXIF orientation with the same message image_asset uses (shared constant _ORIENTATION_MESSAGE, pinned by a test from each call site) and reports damaged EXIF as a ValueError; orientation 1, no EXIF and EXIF without tag 274 are accepted. docs/vision.md says so.
  • The upscale paragraph in docs/vision.md states only arithmetic (a 10 x 10 crop padded to 280 x 280 is 0.13% content; upscale=True takes the same crop to 280 x 280 with no padding) and says which works better "has not been measured against Jev; treat it as a design decision". The earlier "too little for Jev to use" clause was removed (a test pins its absence).
  • Files touched: src/jev_cookbook/vision.py, tests/test_vision.py, docs/vision.md, docs/evaluation.md. Signed merge of origin/main 717a3f0 (only docs/evaluation.md shared, auto-merged).
  • Verification at 3b01fc1: full pytest 6457 passed, 65 skipped, 50 deselected, 31 xfailed on Python 3.14; tests/test_vision.py 402 passed on 3.10; hygiene, assets, catalog and fixtures checks pass; CI green per check.

@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

Re-review of PR #341 — reviewer 1 of 2 (independent), round 3

  • Agent ID: reviewer 1 of 2 for PR Foundation: image preparation, tile inverse, multi-level box mapping and a 280 px minimum check (#310) #341, worktree
    .claude/worktrees/agent-a0d672c24d82c4de1
  • Model: claude-opus-5[1m] (Opus 5, 1M context), as stated in my system prompt
  • Head reviewed: 3b01fc1a8cbaf754afb0ecd48a5c38dadd97dd92 (fetched and checked out myself with
    git fetch origin pull/341/head). A merge commit, signature %G? = G, parents
    b46d93570ae55417be1b2195aa8d8fa0332524a4 and 717a3f0d3ffd09995350a8558637f95ee1e3160f.
  • New code commit: b46d935 (signature G, Refs #310,
    Co-Authored-By: Claude Sonnet 5.5), one commit on top of 2401cba.
  • Base: origin/main = 717a3f0d3ffd09995350a8558637f95ee1e3160f, which is also
    git merge-base HEAD origin/main — the branch is now up to date with main (it was one
    commit behind at the end of round 2).
  • Previous rounds: 308cac8 -> CHANGES-NEEDED (pr341-r1.md, one must-change, M1);
    2401cba -> APPROVE (pr341-r1b.md).
  • Reviewer 2's reviews are on the PR; I have not read them.

Verdict

The merge is a pure auto-merge (its tree is bit-identical to the one git merge-tree recomputes,
and git show --cc has an empty body), and both sides' docs/evaluation.md edits survived
intact. The new code change is sound and I could not break it in anger: prepare_image and
image_asset now give the same ValueError with the same message for orientations 2–8 and for
the malformed 0, 9, 255 and 65535
, agree on little- and big-endian EXIF, on EXIF carried by PNG
and WebP, on XMP-only orientation and on the sub-IFD blind spot, and both accept orientation 1 and
no EXIF; every byte-like damaged-EXIF blob I could build becomes the documented ValueError on
Pillow 11.3.0, 12.3.0 and 13.0.0; and the documented remedy works end to end — exif_transpose
then prepare_image then save gives an asset that passes image_asset and the image-file
contract with zero findings, for all four flipped orientations. Nothing the repository's own rules
would accept is newly refused (drawn images, reopened plain PNGs, tiles and crops of upright
sources all pass untouched). 24 of 24 mutants killed, including seven aimed at the new check and
one that puts the removed "too little for Jev" claim back; property tests bit-identical to rounds 1
and 2; every check green. APPROVE.


1. The merge commit (git show --cc 3b01fc1)

  • Parents: b46d935 (the branch) and 717a3f0 (current origin/main). Signature G.
  • git show --cc 3b01fc1 prints no combined diff body: the merge resolved nothing by hand.
  • I recomputed the automatic merge independently:
    git merge-tree --write-tree b46d935 717a3f0 -> cff1e14adf23b85fa99850eea544a6b0aa646c61,
    and git rev-parse 3b01fc1^{tree} -> the same OID. So nothing was smuggled into the merge.
  • What main brought in (vs the first parent): CONTRIBUTING.md, docs/evaluation.md,
    docs/recipe-template.md, tests/test_evaluation.py, tests/test_recipe_guard.py — i.e. Foundation: bind the confidence gate to the shared helper and a live review branch (#282, #227, #272) #336
    and Foundation: Noul gate band edge is exact only in real arithmetic (#247) #344, none of which touch vision.py.
  • docs/evaluation.md was the only file both sides edited, and both edits are present in the
    result: main's "The gate comes from select_confidence_threshold…" paragraph at line 279, and
    this PR's tiled/hierarchical sentence (now naming ImageTransform.to_original and the
    None -> review rule) at lines 808–814. Auto-merged correctly; the test that asserts the
    PR's wording passes.
  • The PR as a whole now touches exactly four files against current main: docs/evaluation.md,
    docs/vision.md, src/jev_cookbook/vision.py, tests/test_vision.py (+1528/-15).

2. The code change (git diff 2401cba b46d935, 3 files, +69/-15)

_ORIENTATION_MESSAGE = (
    "rotate EXIF orientation into pixels with ImageOps.exif_transpose "
    "and save the upright image before loading it"
)
...
try:
    orientation = image.getexif().get(274, 1)  # the rebuild below drops EXIF and this evidence
except (struct.error, SyntaxError, OSError) as exc:
    raise ValueError("the image's EXIF data is damaged; remove it before preparing") from exc
if orientation != 1:
    raise ValueError(_ORIENTATION_MESSAGE)

The message _image_format already raised is extracted to a module constant and reused verbatim,
so the two refusals cannot drift apart in wording. The check sits after the mode/transparency
check and before any arithmetic or pixel work, so nothing is computed for an image that will be
refused, and import struct is used exactly once (ruff is clean, so it is not unused).

This closes the round-2 suggestion S8 in the strongest available way: in round 2 I measured that
prepare_image laundered away a non-identity orientation tag, turning a loud image_asset
refusal into a silently sideways asset. It now refuses instead, with the identical message, so the
two entry points cannot disagree. The comment on the getexif() line states exactly why the check
has to happen here ("the rebuild below drops EXIF and this evidence") — the right reason, recorded
in the right place.

3. Trying to break the EXIF handling

All measured on Pillow 13.0.0 / Python 3.14.7 and Pillow 12.3.0 and 11.3.0 / Python
3.10.21
(I swapped Pillow in my own 3.10 venv for the third data point and restored 12.3.0
afterwards).

3.1 Orientation values — prepare_image beside image_asset on the same file

orientation in a JPEG prepare_image validate_image_asset
1 ACCEPT ACCEPT
2, 3, 4, 5, 6, 7, 8 ValueError: rotate EXIF orientation into pixels… the same ValueError
0, 9, 255, 65535 (malformed) the same ValueError the same ValueError

Identical on all three Pillow versions. Refusing an out-of-range value is the same thing
_image_format has always done (!= 1), so the two cannot diverge — which is the point of the
shared constant. The agreement is behavioural, not just a shared string.

3.2 IFD layouts and byte order

case getexif() sees prepare_image image_asset
little-endian II TIFF header, hand-built, orientation 1 / 3 / 6 / 0 / 9 1 / 3 / 6 / 0 / 9 ACCEPT / refuse x4 —
big-endian MM TIFF header, hand-built, same values 1 / 3 / 6 / 0 / 9 ACCEPT / refuse x4 —
orientation only in the Exif sub-IFD (0x8769) absent in IFD0 ACCEPT ACCEPT
orientation as a RATIONAL, and an entry with a 2^31 offset a non-1 value refuse refuse
orientation with count=3 (Pillow warns) 1 ACCEPT ACCEPT

The sub-IFD case is a blind spot, but it is image_asset's blind spot too — the two still agree,
and orientation there is non-standard. Nothing diverges.

3.3 Damaged, truncated and garbage EXIF

Seventeen malformed blobs assigned to info["exif"] (header only; Exif\0\0II*\0; a partial
offset; an offset past the end; a truncated MM*; bad magic; an entry count with no entries; a
65,535 entry count; random bytes; all zeros; empty; a raw TIFF with no Exif prefix; a NUL run
after a valid head; a 2^31 entry offset; a RATIONAL orientation; a count-3 orientation;
bytearray; memoryview):

  • every byte-like blob either parses (Pillow only warns Corrupt EXIF data) or raises
    struct.error or SyntaxError, and the new except turns both into the documented
    ValueError: the image's EXIF data is damaged; remove it before preparing.
    No leak, on any of the three Pillow versions. The caught tuple is exactly right.
  • the only escapes are non-byte-like values: info["exif"] set to a str, an int or a
    list leaks TypeError/AttributeError, and on Pillow <= 12.3 a memoryview leaks
    AttributeError too. That requires caller code to assign a wrong type — Pillow always stores
    bytes there when opening a JPEG, PNG or WebP — so it is a programming error, and the
    exception type is not misleading. Suggestion S9 below, one word; not a blocker.

I also checked the neighbouring exposure, because _image_format's own try does not list
struct.error: I pushed all fifteen saveable blobs through real JPEG files and
validate_image_asset. No non-ValueError escaped — Pillow rewrites/pads the APP1 segment on
save and only warns on reopen. So there is no pre-existing hole to report there.

3.4 EXIF from other container formats, and XMP

source getexif() orientation prepare_image
PNG with an eXIf chunk (orientation 6) 6 refuse (and that PNG independently fails the contract with png-metadata)
WebP with EXIF (orientation 6) 6 refuse
XMP-only tiff:Orientation="6" (Pillow's getexif falls back to XMP) 6 refuse

The XMP case is worth calling out as a good consequence of using the same public getexif()
as _image_format: both refuse it, so they still agree. getexif() and the deprecated private
_getexif() returned the same orientation (6) for a JPEG, so the choice of API changes nothing
except that it matches _image_format exactly.

3.5 Does it refuse anything the repository's rules would accept? No.

input verdict
Image.new("RGB", …) (how every repo asset is drawn) ACCEPT
a plain PNG saved and reopened ACCEPT
EXIF present but no orientation tag; orientation 1 ACCEPT
ImageOps.exif_transpose(...) output, orientations 2/3/6/8 ACCEPT
the repo's own flow: draw a 1200x900 page -> save -> reopen -> generate_tiles(min_side=280) -> crop_box -> prepare_image ACCEPT, info == {}

And nothing that could legally be a committed asset can carry an orientation tag at all:
image_asset already refuses a non-identity orientation, and tools/check_recipe_assets.py
forbids eXIf in a PNG and every non-JFIF APPn in a JPEG. So prepare_image is no stricter
than the asset rules for anything that could be an asset; for a source image it refuses sideways
pixels, which is the intent. The full suite passing (6457 tests) is the other side of this: nothing
in the repository regressed.

3.6 The documented remedy works end to end

For each of orientations 2, 3, 6 and 8: ImageOps.exif_transpose rotates the pixels
(400x300 -> 300x400 for 6 and 8) and removes tag 274, prepare_image then accepts, and the
saved PNG gives image_asset = ACCEPT and check_recipe_assets.check_image(...) == []. A crop of
an exif_transpose'd source is accepted too. This was the check that mattered most — if the
prescribed remedy had still been refused, the change would have been a blocker.

Conversely, a crop() or a generate_tiles tile of a rotated source inherits the parent's
info["exif"] and is refused. That is the right answer (its pixels are sideways), and it means
the refusal propagates correctly through the tiling path rather than being bypassed by cropping.

3.7 Input mutation, and two notes on Pillow's _exif cache

prepare_image leaves the input's pixels, size and info byte-identical; only Pillow's private
_exif lazy cache is populated, which is not observable through the public API. The round-1 test
test_prepare_image_does_not_modify_the_input_and_returns_new_pixels still holds.

Two measured quirks of that cache, reported as observations with no change recommended:

  • Not idempotent on damaged EXIF. Three calls in a row on the same image object give
    ValueError("the image's EXIF data is damaged…"), then ACCEPT, then ACCEPT, because a failed
    Exif.load leaves _exif marked loaded. For a well-formed non-identity orientation all three
    calls refuse identically, so the case that matters is stable. Reaching this needs damaged EXIF,
    which the repository's code-generated assets cannot have, and the first call does refuse.
  • A stale cache can hide a real orientation, but only in the contrived order "call getexif()
    on an image that has no EXIF yet, then attach info['exif']". A file opened from disk and
    read first still refuses (I checked).

I deliberately do not suggest replacing image.getexif() with a fresh Image.Exif().load(...)
to fix these: that would drop the XMP fallback and make prepare_image disagree with
image_asset on the case in §3.4 — worse than the quirk. Agreement is the more valuable property
and the current code buys it.

4. Property tests re-run

Same seed, same independent exact-rational model built from scratch; the geometry functions are
untouched in this diff and the results are bit-identical to rounds 1 and 2:

property cases result
random chains of 1–3 crop / resize+pad steps built with then 37,362 all built; only the documented empty-intersection raise
to_derived vs my exact model (float equality) 20,338 0 mismatches
to_original vs my exact model (float equality) 24,941 0 mismatches
to_derived returning None where my model says empty 14,126 0 disagreements
then associativity (left fold == right fold) 37,362 0 differences
visible_fraction vs exact area arithmetic single-step subset 0 differences
total problems — 0

5. Mutation testing — 24 of 24 killed

The key mutants from rounds 1–2 plus seven new ones aimed at this commit and one aimed at the
docs guard. All 24 killed, 0 survived.

  • geometry core, killed again: never snap / always snap; _clip counts touching; _remap swaps
    fx/fy; then wrong first remap; then wrong second remap; then no empty-intersection check;
    scaled half-down; content top-left not centred; min_side strict; crop_box no check;
    to_original clips to the wrong window; visible_fraction returns an area; NEAREST instead
    of Lanczos; the frombytes copy removed; the zero-size check removed.
  • new, all killed: the whole orientation check removed; the refusal inverted (== 1); the
    default for a missing tag changed to 0 (so a no-EXIF image would be refused); the wrong tag
    number (275); the damaged-EXIF except removed so struct.error leaks; prepare_image given
    its own message instead of image_asset's; the check disabled with and False.
  • docs guard, killed: putting the removed claim back ("too little for Jev to use") fails
    test_the_docs_say_how_to_report_a_mapped_back_answer_and_when_to_upscale. So the negative
    assertion "too little for Jev" not in flat is a real guard, not decoration.

The worktree and docs/vision.md were restored after every mutation (git status clean,
git rev-parse HEAD = 3b01fc1…).

I also re-read the three new tests as tests. They are real checks, not restatements: the
orientation test asserts image.getexif()[274] == orientation first (so the fixture really
carries the tag), then asserts both prepare_image and image_asset refuse the same file
with the same message — which is exactly the property the shared constant is meant to guarantee,
checked through two independent entry points. The acceptance test covers both orientation 1 and no
EXIF. The damaged-EXIF test uses a header-only blob, which I confirmed is one of the inputs that
genuinely makes getexif() raise struct.error on all three Pillow versions, so it exercises the
new except rather than passing vacuously. The metadata test's EXIF blob is now a real
Image.Exif() with an ImageDescription — a better fixture than the old truncated header, and
one that does not accidentally depend on the damaged-EXIF path.

6. The docs paragraph (criterion 4)

Choose upscale once per recipe. The default pads, which keeps every original pixel and clears
the 280 pixel minimum. A small crop is then almost all padding, and padding adds no detail: a
10 × 10 crop comes back 280 × 280 with the content covering 0.13% of the image, the target no
larger than it was in the source. upscale=True instead scales the crop up so the target fills
more of the image sent. Which of the two works better for a recipe has not been measured against
Jev; treat it as a design decision to settle on validation examples, then keep it, like
pad_color, the same across the recipe's images.

Every clause is arithmetic or mechanical — "keeps every original pixel" (upscale=False does not
resample a small crop), "clears the 280 pixel minimum", "padding adds no detail", "0.13% of the
image" (100/78,400 = 0.1276%, pinned by a test), "the target no larger than it was in the source"
(scale factor 1), "scales the crop up so the target fills more of the image sent" — and the only
statement about which is better is explicitly "has not been measured against Jev; treat it as a
design decision to settle on validation examples". The removed claim is gone and its absence is
asserted. I grepped every added documentation line in the whole PR for "Jev": the remaining
mentions are the pad_color sentence (already hedged with "has not been measured against Jev"),
"a recipe that sends tiles or crops to Jev", "A box that Jev returns", "where a recipe would call
Jev" and two import lines — no unmeasured behavioural claim about Jev is left anywhere in the
PR's added prose.
Criterion 4 met.

7. Checks I ran

Fresh venvs under /home/agent/r1c/ (uv venv --python 3.14 / --python 3.10, then
uv pip install --link-mode symlink -r a one-line requirements file holding -e <worktree>[dev]),
every heavy command under flock .claude/heavy.lock nice -n 10, pytest -p no:cacheprovider, no
nested flock, background wrapper + log polling for the long runs, memory checked first (3.9 GB).

command result
ruff check . (0.17.1) All checks passed!, exit 0
ruff format --check . 405 files already formatted, exit 0
pytest (3.14.7, Pillow 13.0.0) 6457 passed, 65 skipped, 50 deselected, 31 xfailed in 252 s, exit 0
pytest tests/test_vision.py (3.10.21, Pillow 12.3.0) 402 passed in 5.7 s
python tools/check_hygiene.py passed: 615 file(s) scanned, exit 0
python tools/check_recipe_assets.py passed: 461 file(s) under recipes/ checked, exit 0
python tools/render_catalog.py --check exit 0
python tools/notebook_ci.py fixtures 51 recipe folders, 0 failed, exit 0
my independent exact-model property tests 37,362 chains / 45,279 mappings, 0 problems
my 24 mutants (7 new, EXIF-focused) 24 killed, 0 survived
EXIF battery: orientations, II/MM, sub-IFD, 17 damaged blobs, PNG/WebP/XMP, remedy, crops no leak for byte-like input; prepare_image and image_asset agree everywhere
the same battery on Pillow 11.3.0, 12.3.0, 13.0.0 identical behaviour for byte-like input
git merge-tree b46d935 717a3f0 vs the merge commit's tree identical OID cff1e14…

Counts move as they should: tests/test_vision.py is 396 + 6 = 402, and the full suite is
6312 (round 2) + 6 (this commit) + 139 from main's #336 and #344 = 6457, with the 10 extra
skips coming from main. No notebooks, recipes or fixtures are touched, so no notebook execution
was needed and tools/check_recipe_scope.py has nothing to judge.

8. Remaining suggestions — all minor, none blocking

# Suggestion
S9 (new) except (struct.error, SyntaxError, OSError) at src/jev_cookbook/vision.py:789 does not cover a non-byte-like info["exif"], which leaks TypeError (a str/int/list on all versions) or AttributeError (a memoryview on Pillow <= 12.3). Adding TypeError, AttributeError to the tuple would keep the module's "bad input raises ValueError" convention. Only reachable from caller code that assigns a wrong type; Pillow always stores bytes.
S2 upscale is still not type-validated (upscale="yes" is accepted as true) while min_side, max_side and pad_color are all strict. Addressed as documentation instead; defensible.
S4 src/jev_cookbook/vision.py:371 (108), :380 (102), :597 (114) exceed the configured line-length = 100 (:140 is pre-existing), plus two 101–102-char test lines. E501 is not in ruff's select, so nothing flags them. Cosmetic.
S6 ImageTransform's public fields are annotated with the private _Edges alias. Cosmetic.
note The two _exif cache quirks in §3.7, for which I recommend no change.

9. Status of every finding across the three rounds

# Finding Status
M1 (round 1) prepare_image propagated .info; a saved PNG carried iCCP and failed png-metadata on 2 of 3 paths Resolved in 2401cba, re-verified then; untouched here.
S1 (round 1) Nothing pinned the resampling filter Resolved in 2401cba; NEAREST still killed here.
S3 / S8 (rounds 1–2) EXIF orientation: first undocumented, then documented but silently laundered away Resolved here. prepare_image now refuses a non-identity orientation with image_asset's own message, via a shared constant, and reports damaged EXIF as a ValueError. Verified on three Pillow versions, and the documented remedy works end to end.
S7 (round 1) docs/evaluation.md named only tile_to_image Resolved in 2401cba; survived the merge with main's edit to the same file.
reviewer 2's blocker The upscale paragraph claimed a padded crop is "too little for Jev to use" Resolved here, and the absence of the claim is asserted by a test that my mutant proved fires.
S2, S4, S6, S9 minor, listed in §8 Open; none blocking.

All five acceptance criteria of #310 remain met (round 1, with criteria 3 and 5 better pinned in
round 2). I would be comfortable with this merging as it stands; the branch is up to date with
main and every check is green on the merged tree.

OPUS-REVIEW: APPROVE 3b01fc1

@CompleteDotTech
CompleteDotTech merged commit 812367a into main Oct 22, 2026
60 checks passed
@CompleteDotTech
CompleteDotTech deleted the foundation/image-transforms branch October 22, 2026 05:35
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