Skip to content

Foundation: image provenance manifest enforced by the fixture validator (#308) - #330

Open
CompleteDotTech wants to merge 3 commits into
mainfrom
foundation/image-provenance
Open

CompleteDotTech wants to merge 3 commits into
mainfrom
foundation/image-provenance

Conversation

@CompleteDotTech

@CompleteDotTech CompleteDotTech commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Closes #308

What this adds

A per-recipe image provenance manifest and its enforcement in the fixture validator. Foundation work (no recipe touched); this defines a file format every multimodal recipe copies, so it needs two independent reviews.

  • recipes/<slug>/assets-manifest.json, one per recipe, required exactly when the recipe has files under assets/. Simple JSON: {"schema": 1, "assets": [entry, ...]}. Each entry: path, sha256, format, width, height, origin (generated only), generator (build_fixtures.py:draw_scene or jev_cookbook.<module>:<function>), parameters (object, seed included), licence (MIT or CC0-1.0), description, attestations (synthetic true; real_photo, real_brand, real_person, third_party_content false), optional shared_across_splits.
  • JSON Schema src/jev_cookbook/fixtures/assets-manifest.schema.json (packaged; _schema.py gains const and minimum).
  • src/jev_cookbook/fixtures/_assets.py, called from validate_recipe, so python -m jev_cookbook.fixtures validate <recipe>, --all and tools/notebook_ci.py fixtures all enforce it. Errors are path: [rule] message; the rules are listed in docs/fixtures.md ("Image provenance manifest", with a worked example and a generator helper for Foundation: no shared way to generate synthetic test images, so each of forty recipes will hand-roll one, or reach for real photos, brands and screenshots #314).
  • Docs: docs/fixtures.md, docs/recipe-template.md, one bullet in CONTRIBUTING.md section 5.

Decisions for the reviewers

  1. Location differs from the issue's proposal. The issue proposed assets/manifest.json. tools/check_recipe_assets.py (merged in Foundation: image-file contract, checker, scope rule and notebook image policy (#307) #322) says assets/ holds image files only (non-image-in-assets), and that file is outside this issue's paths, so the manifest sits beside the folder as recipes/<slug>/assets-manifest.json. A recipe PR can edit it (it is inside the recipe folder).
  2. synthetic: true attestation added to the issue's four false ones, so the manifest says "synthetic" explicitly as well as "no real photo/brand/person/third party".
  3. Licence is a closed list (MIT, CC0-1.0) because third-party material is banned; widen it if a real need appears.
  4. fields image-path convention: any string value in fields (any depth, any key) with no whitespace that starts with assets/ or ends .png/.jpg/.jpeg. Each must equal a manifest path exactly and pass resolve_asset_path(root=recipe).
  5. Split leak compares manifest SHA-256 of images named by train/validation/test examples (demo exempt), under one path or two. It does not decode pixels, so two encodings of one picture are two hashes (stated in the docs). The allow-list is shared_across_splits: true on every entry carrying those bytes.
  6. "An image that nothing references" is not enforced: an image may be used only by question criteria, which the validator cannot see. It must still be listed.
  7. pyproject.toml (package-data for the new schema file) and tests/test_multimodal_backends.py / tests/test_fixtures.py are outside the issue's list but needed: the schema file must ship; the multimodal test built an image recipe with no manifest (now correctly rejected) so it gained one; the unsupported-keyword test used minimum as its example of an unsupported keyword and now uses maximum.
  8. docs/multimodal-track.md now points to the manifest section (allowed by the orchestrator after the PR opened).
  9. The positive multimodal fixture is built in tmp_path by the tests (Pillow-drawn images) rather than committed binaries under tests/.

How it was verified

Fresh venv, pip install -e ".[dev]", Windows:

  • ruff check .: All checks passed. ruff format --check .: 399 files already formatted (this also formats the docs code blocks).
  • Merged origin/main (e998eb5, Foundation: enforce the Images API's 280 px minimum, hash image bytes in replay keys, bound tiles (#306) #321, which changed vision.py) with a signed merge commit, then re-ran everything on the merged head. pytest full run: 5457 passed, 60 skipped, 31 xfailed; 2 failures, the known Windows-only tests/test_notebook_ci_sandbox.py ones (signal.SIGKILL, Foundation: two notebook-CI sandbox tests use signal.SIGKILL and fail on Windows #302); no Tests: kernel tests in tests/test_new_recipe.py flake on Windows under load (ZMQError: Address in use, exit-time noise) #328 kernel flakes appeared. Before the merge, test_image_metadata_fixtures_validate_box_answers was broken by the new rule and fixed by giving it a manifest. The 2 skips in tests/test_fixtures_assets.py are symlink tests (this Windows account cannot create symlinks); the containment branch is covered by a monkeypatched test.
  • python tools/render_catalog.py --check: exit 0. python tools/check_hygiene.py: passed. python tools/check_recipe_assets.py: passed. python tools/notebook_ci.py fixtures: 51 recipe folders, 0 failed. python -m jev_cookbook.fixtures validate --all: all valid (no recipe has assets/, so unchanged).
  • Notebook execution: no notebook or recipe changed, so none run.
  • New tests tests/test_fixtures_assets.py (147): each rule positive and negative: missing manifest, unlisted file, orphan entry, hash/format/size mismatch, missing/invalid licence, every attestation true/false/missing/non-boolean, required and unknown fields, path traversal in a listed path (13 shapes) and in a fields value, symlink, generator, duplicate entry, split leak and allow-list, unreadable manifest, plus checks that the docs example and helper pass the schema/validator and that the schema patterns equal the code's.

🤖 Generated with Claude Code

CompleteDotTech and others added 3 commits October 20, 2026 21:37
… by the fixture validator (#308)

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

Copy link
Copy Markdown
Contributor Author

Opus reviewer (independent review 2 of 2). Model: Claude Opus 5.5 (claude-opus-5-5).
Reviewed head b9844a2238e9df47f275dd9a5164fffb5325434d, base e998eb59d78ee06b32c8346c9932a09e6605a45e. I wrote this without reading the other review.

How I tested it as a recipe builder

I scaffolded a throwaway recipe 61 in a scratch folder (tools/new_recipe.py 61 --recipes-dir <scratch>). In build_fixtures.py I drew four 400 x 300 PNGs with Pillow into assets/, referenced them from fields (one of them also from a nested list), and computed replay keys with assets=. I wrote the manifest using only docs/fixtures.md, copying its manifest_entry helper word for word. Then I ran the validator and ran check_recipe_assets.check_entries over every file in the scratch recipe.

Blocking

B1. CI is red: Tests (py3.10) and Tests (py3.14) both fail on Linux. The failing test is tests/test_fixtures_assets.py:663 with the parameter C:/Windows/x.png:
no [asset-path] line with ('C:/Windows/x.png', 'e0') in [... [unlisted-asset] fields image path 'C:/Windows/x.png' has no entry ...].
On POSIX that string is a relative path, so resolve_asset_path accepts it. The PR was only verified on Windows. This is more than a test bug: the rule a fixture fails depends on the operating system that runs the validator. In src/jev_cookbook/fixtures/_assets.py:290-302, classify a fields image path without reference to the OS, before calling resolve_asset_path. Suggested fix: any detected image path that does not _ASSET_PATH.fullmatch is [asset-path] ... must be an assets/... path exactly as listed in assets-manifest.json. That covers drive letters, backslashes, ./assets/... and bare filenames the same way everywhere. Keep resolve_asset_path as the containment check for paths that match.

B2. The fields image-path convention also applies to text-only recipes, and a recipe cannot opt out. See _assets.py:96-105 and 287-302. Any whitespace-free string ending in .png, .jpg or .jpeg, under any key, must be a manifest entry. That applies even when the recipe has no assets/ folder. I copied recipes/09-file-organization and changed one fixture filename to IMG_2041.JPG, which is a natural hard case for a file-organization recipe. Validation then fails:
text09/fixtures/inputs.jsonl (id 'v01-invoice'): [unlisted-asset] fields image path 'IMG_2041.JPG' has no entry in assets-manifest.json.
There is no way round it: the manifest needs at least one entry, and each entry needs a real file under assets/. Main's 51 folders pass only because none of them happens to contain such a string. Ranks 52–60 can still contain one. So can any multimodal recipe that keeps an original upload name or a source link as text metadata, for example "uploaded_as": "receipt.jpg" or "source": "https://tracker.example/att/12.png"; that second one fails as [asset-path]. A valid image path can only have one form, assets/.... Fix: only strings that start with assets/ are image paths. If you want to keep a suffix-only check to catch typos, apply it only when the recipe has assets/, and report it with a hint ("did you mean assets/…?"). Document the rule in docs/fixtures.md:459, and say there that text recipes are unaffected. Fix this now: every multimodal recipe will be built against this behaviour.

Suggestions (not blocking)

  • S1. The helper assumes the current directory; the scaffold does not (docs/fixtures.md:482). The scaffolded build_fixtures.py says "from anywhere; it writes next to this file". The documented manifest_entry reads Path(path) relative to the current directory. Copied word for word and run from the repository root, it raised FileNotFoundError: 'assets\\screen-error.png'. Change the signature to manifest_entry(recipe_dir, path, generator, parameters, description, licence=...) and read Path(recipe_dir) / path; Foundation: no shared way to generate synthetic test images, so each of forty recipes will hand-roll one, or reach for real photos, brands and screenshots #314 needs exactly that signature. Also recommend writing entries sorted by path, so regenerated manifests diff cleanly.
  • S2. Too many lines when the manifest is missing or invalid. With no manifest, or a trailing comma, each fields reference adds an [unlisted-asset] line. My four examples produced six lines under the real cause; a recipe with 40 examples would bury it. Skip the fields-to-entry check when the manifest could not be read. Also report each path once per example: t02 names the same path twice and got two identical lines.
  • S3. Pattern failures print the raw regex. Examples: generator build_fixtures:draw_screen or tools/gen.py:draw_screen, and path ./assets/... or assets\\.... Add a sentence such as "expected script.py:function (a file at the recipe root) or jev_cookbook.<module>:<function>" or "write assets/<name>.png with forward slashes".
  • S4. Hint when the manifest is at assets/manifest.json. The issue itself proposes that location, so builders will make this mistake. Today it reports manifest-missing ... 5 file(s) under assets/, counting the manifest as an asset. When that file exists, say "move it to <recipe>/assets-manifest.json".
  • S5. jev_cookbook.<module>:<function> generators are not checked at all. importlib.util.find_spec plus an attribute lookup is cheap, and package code is trusted. This matters for Foundation: no shared way to generate synthetic test images, so each of forty recipes will hand-roll one, or reach for real photos, brands and screenshots #314, whose shared drawing functions will be recorded this way.
  • S6. state examples are not scanned (test_state_examples_are_not_scanned_for_images). The docs say an image example uses fields. Spell out the consequence: an image recipe that uses state rows gets no reference or leak checks.

Decisions in the PR body

  1. Manifest beside assets/: agree. non-image-in-assets would reject it inside the folder.
  2. synthetic: true: agree. It duplicates origin: generated, but it is harmless and explicit.
  3. Closed licence list: agree.
  4. fields convention: disagree as written; see B1 and B2.
  5. Leak check by bytes: agree. It is documented, and the allow-list rule (every entry carrying the bytes must be flagged) is tested.
  6. No unreferenced-image rule: agree. It is not in the acceptance criteria, and criteria-only images are invisible to the validator.
  7. Out-of-scope files: each is justified and small. minimum replaced maximum in the unsupported-keyword test.
  8. docs/multimodal-track.md pointer: fine.
  9. Positive fixture built in tmp_path: acceptable. Committed binaries under tests/ would sit outside the recipes/-scoped image checker.

Checks run (fresh venv, pip install -e ".[dev]", Windows, no TYPESAFE_API_KEY)

  • ruff check .: all passed. ruff format --check .: 399 files already formatted.
  • python tools/render_catalog.py --check: exit 0. python tools/check_hygiene.py: passed (609 files). python tools/check_recipe_assets.py: passed (461 files).
  • python tools/notebook_ci.py fixtures: 51 folders, 0 failed. python -m jev_cookbook.fixtures validate --all: all valid.
  • pytest: 5457 passed, 60 skipped, 31 xfailed, 2 failed. The 2 failures are the known Windows-only test_notebook_ci_sandbox.py ones (Foundation: two notebook-CI sandbox tests use signal.SIGKILL and fail on Windows #302), not from this PR.
  • Mutation run on _assets.py / _schema.py, with the files restored afterwards: 9 of 9 mutants killed by tests/test_fixtures_assets.py. The mutants were: the allow-list all changed to any; the split filter removed; the leak threshold; the hash, size, generator and orphan checks disabled; the const type check removed; the duplicate-entry check.
  • git diff --raw -M origin/main...HEAD: 12 files, all 100644, no mode or type change, no symlink or submodule. No keys, no live calls, no real data.
  • No notebook changed, so none were executed.
  • gh pr checks 330: Tests (py3.10) and Tests (py3.14) fail (B1). Lint, Catalog, Hygiene, Fixtures and Notebook jobs pass.

Acceptance criteria (#308)

  • Manifest format and JSON Schema defined: met (the location change is justified).
  • validate (and --all) fails, naming the file, for each listed fault: met. I reproduced every one.
  • fields image-path convention defined: met but needs changing (B1, B2).
  • Negative tests for each failure, plus a positive multimodal fixture: met.
  • Text-only recipes validate unchanged on main's 51 folders: met as written, but B2 changes the contract for future text fixtures.
  • CONTRIBUTING.md §5 bullet: met.
  • CI green: not met (B1).

OPUS-REVIEW: CHANGES-NEEDED b9844a2

@CompleteDotTech

Copy link
Copy Markdown
Contributor Author

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

Reviewed head b9844a2238e9df47f275dd9a5164fffb5325434d against base e998eb59d78ee06b32c8346c9932a09e6605a45e (#308). I did not read the other review before writing this one.

The design is sound and the validator held up well when I tried to break it. It fails closed on every non-compliant recipe I built that follows the documented fields convention, and every check I mutated was caught by a test. One blocker: both required test jobs are red on Linux.

Blocking

  1. Tests (py3.10) and Tests (py3.14) fail on CI. The failing test is tests/test_fixtures_assets.py::test_fields_path_traversal_is_an_asset_path_problem[C:/Windows/x.png] (parametrize at tests/test_fixtures_assets.py:663). On POSIX, C:/Windows/x.png has no anchor, so resolve_asset_path treats it as a relative path inside the recipe. The validator then reports it as [unlisted-asset] instead of [asset-path]. The recipe is still rejected, so this is not a bypass, but the rule name now depends on the OS and the PR's "pytest passed" claim only holds on Windows. Fix it in _assets.py rather than by dropping the case. Around _assets.py:290-296, before calling resolve_asset_path, reject a fields value as [asset-path] when PureWindowsPath(value).anchor or PurePosixPath(value).anchor is non-empty, or when it contains a backslash. The result is then the same on every OS, which matters because builders run Windows and CI runs Linux. Keep the test as it is.

Suggestions (not blocking; the first two matter for the 40 recipes that copy this)

  1. The split-leak rule only sees images named in fields. I built each of these and each passed with 0 problems while validation and test shared identical image bytes:

    • a state row that names the image (test_state_examples_are_not_scanned_for_images pins this on purpose);
    • paths derived from the id (assets/{id}.png, no image in fields), which a builder may well write;
    • the path as a dict key;
    • the path split across two values.

    "Store image paths in fields" is guidance in docs/fixtures.md that nothing enforces. A cheap guard: in a recipe that has assets/, report every scored example (train/validation/test) whose fields name no listed image, plus every state row. Allow a documented opt-out, such as "assets-free": true in fields, for deliberately text-only examples. At minimum, make "every example that sends an image names it in fields" a stated MUST, and add it to the recipe review checklist.

  2. Decision 4, the fields heuristic: the assets/ prefix test is right, but the bare-suffix test is too broad and has no opt-out. A text-only recipe with no assets/ folder whose fields hold {"filename": "holiday_0.jpg"} fails with [unlisted-asset] ... has no entry in assets-manifest.json (I reproduced this). The same can happen in a multimodal recipe whose data includes a file name, such as 93 bug-report attachments or 99 screenshot histories. Manifest paths must start with assets/, and check_recipe_assets.py already rejects images outside assets/. So a string that only ends in .png can never be a valid image path, and the only thing the suffix branch adds is catching typos. Limit it to strings that contain / or \. That still catches ../x.png and fixtures/x.png but leaves bare file names alone.

  3. Decision 5, hash-based leak detection: acceptable as a floor, and cheap to strengthen.

    • Pillow is already a hard dependency (pyproject.toml) and fixtures are at most 1 MiB each. Also hashing the decoded pixels (mode, size and raw bytes) would catch lossless re-encodes. I confirmed that the same picture saved with another compress_level passes today.
    • Treating an identical (generator, parameters) pair in two scored splits as a leak would also catch a PNG/JPEG pair of the same drawing. The data for that is already in the manifest.
  4. shared_across_splits: setting it on every entry silently disables the leak rule (tested, 0 problems). That is visible in review, which is probably enough. A one-line note in the docs would help: each true needs a sentence in the PR saying why the image is a shared reference.

  5. generator with jev_cookbook.<module>:<function> is never checked. jev_cookbook.nope:nothing passes. importlib.util.find_spec plus an ast scan for the function name would close this without importing anything.

  6. Uppercase suffix: assets/x.PNG is rejected by both checkers, under suffix-case and under [asset-path] ... does not match the pattern plus unlisted-asset. That is consistent, but the second message could say "rename to lowercase".

  7. (Unverified, low priority.) On POSIX, Path.resolve() of a string with an embedded NUL can raise ValueError rather than OSError, so a fields value containing \u0000 might crash with a traceback instead of reporting a problem. It would still fail closed. On Windows it is reported cleanly as [unlisted-asset].

The "Decisions for the reviewers" list

  1. Manifest beside assets/, not inside it. Agree. check_recipe_assets.py (Foundation: image-file contract, checker, scope rule and notebook image policy (#307) #322) bans non-images in assets/, and check_recipe_scope.py allows any 100644 file under recipes/<slug>/, so a recipe PR can edit the manifest.
  2. synthetic: true. Agree. It overlaps with origin: generated, but a positive statement next to the four negatives costs nothing, and it is const: true with a type check (1 and "true" are refused).
  3. Licence allowlist of MIT and CC0-1.0. Agree. Third-party material is banned, so nothing else is needed, and a closed enum is the right default.
  4. fields path heuristic. Partly agree; see suggestion 2. The exact-match rule against manifest path combined with resolve_asset_path is strong. Traversal, ./, backslash, case variants, UNC, drive-relative, URL and zero-width-space forms are all rejected.
  5. Hash-based split leak. Agree as a floor; see suggestions 1 and 3. The blind spot that matters is images not named in fields, not re-encoding.
  6. Not enforcing "referenced by nothing". Agree. Criteria-only images still have to be listed, and an image placed outside assets/ is caught by check_recipe_assets.py in the Hygiene job, not by this validator.
  7. to 9. Out-of-list files, the docs pointer, and the positive fixture built in tmp_path. All justified. The minimum to maximum swap in tests/test_fixtures.py keeps the "unsupported keyword" test meaningful, so nothing was weakened.

Attempts to get a non-compliant recipe past the validator (all built in a temp dir)

Attempt Result
Unlisted image, including one in a subfolder caught (unlisted-asset)
Orphan entry, or manifest with no assets/ folder caught (missing-file)
Hash, format or size mismatch; boolean width caught
Missing, true or non-boolean attestation; synthetic false caught (attestation)
Licence missing or not allowed caught (licence)
Traversal in a listed path (17 shapes in the tests, plus mine) caught (asset-path), outside file never read
Traversal in fields: ../, assets/../, ./, \, ASSETS/, /etc, C:x, UNC, file://, zero-width space, NUL all reported (rule name OS-dependent for drive paths; see blocker)
Same image in two scored splits, one path or two caught (split-leak); allow-list must cover every entry
Same picture re-encoded with different bytes passes (documented)
Image named in a state row, by id convention, as a dict key, or split across values passes (suggestion 1)
Image used only by criteria must be listed (caught as unlisted); outside assets/ it is caught by check_recipe_assets.py only
PNG with a valid header and garbage body passes this validator; caught by check_recipe_assets.py (png-structure)
Generator named only in a comment; fake jev_cookbook.* generator passes (documented as reviewer's job / suggestion 5)

Mutation check on src/jev_cookbook/fixtures/_assets.py, run against tests/test_fixtures_assets.py. Every mutant was killed:

  • the leak rule ignoring the split filter;
  • the leak rule keyed by path instead of hash;
  • the allow-list accepting any instead of all;
  • the unlisted check removed;
  • the generator check removed;
  • the size check removed;
  • the containment check removed;
  • fields resolution skipped.

Checks run (fresh venv, pip install -e ".[dev]", Windows, no TYPESAFE_API_KEY)

  • git diff --raw -M origin/main...HEAD: 12 files, every mode 100644. No symlinks, no submodules, no mode or type changes.
  • ruff check .: all checks passed. ruff format --check .: 399 files already formatted.
  • pytest: 2 failed, 5457 passed, 60 skipped, 31 xfailed. The 2 failures are the known Windows-only tests/test_notebook_ci_sandbox.py ones (Foundation: two notebook-CI sandbox tests use signal.SIGKILL and fail on Windows #302).
  • python tools/render_catalog.py --check: exit 0. python tools/check_hygiene.py: passed (609 files). python tools/check_recipe_assets.py: passed (461 files).
  • python -m jev_cookbook.fixtures validate --all: all valid. python tools/notebook_ci.py fixtures: 51 recipe folders, 0 failed.
  • Notebook execution: none run, because no notebook, recipe or loader changed.
  • gh pr checks 330: Lint, Catalog, Hygiene, Scope, Fixtures and all Notebook jobs pass. Tests (py3.10) and Tests (py3.14) fail, each with only the one test in blocker 1.

Acceptance criteria (#308)

  • Manifest format defined, with a JSON schema: met. The location differs from the proposal, with justification.
  • validate <recipe> and --all fail and name the file for: unlisted or orphan, hash, size or format, attestation or licence, fields path resolution, cross-split hash: met for recipes that name images in fields. Leak detection is blind to other ways of referencing an image (suggestion 1).
  • Negative tests for each failure, plus a positive multimodal fixture under tests/: met. The positive fixture is built by the tests in tmp_path, which I accept. But one test fails on Linux CI (blocker 1).
  • Text-only recipes validate unchanged (--all green on 51 folders): met.
  • CONTRIBUTING.md section 5 bullet: met.

OPUS-REVIEW: CHANGES-NEEDED b9844a2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Foundation: image provenance and licence are not recorded anywhere, and the fixture validator never looks at a recipe's images

1 participant