Skip to content

Audit batches 33, 34a, 34b: publish only what was answered, of the image checked - #254

Merged
guijacquemet merged 2 commits into
mainfrom
claude/batch33-fixes
Nov 9, 2026
Merged

guijacquemet merged 2 commits into
mainfrom
claude/batch33-fixes

Conversation

@guijacquemet

Copy link
Copy Markdown
Contributor

Fixes the findings of the external audits batch 33 (reviewed at old commit 6dc8c26), 34a and 34b, each verified against current main bf733ee first. Every fix has a regression test that fails on bf733ee (checked by running the new tests against the bf733ee src/).

Batch 33

BLOCKER 1. Layout confirm saves a tag that was never shown: STILL HOLDS. At bf733ee TagGrid._confirm_layout() (notebook_channels_grid.py:238-252) filled every untagged position from the raw header proposal. Reproduced: with labels DNA, ACTIN, PHASE declared, header names DNA/ACTIN/PHASE, and the user tagging C1 = DNA, the block confirm saved (DNA, DNA, PHASE) for all four folders. apply_answers() did not check uniqueness. "Tag like the saved group" copies a saved, fully tagged block only, so it has no hidden proposal. Its uniqueness is now enforced by the same gate.
Fix: confirm saves effective(), the tags shown. A hidden proposal stays untagged and the block stays incomplete. apply_answers() (and so save_draft, the notebook's _answer and publish_draft) refuses a unit that tags one label twice, with E_DUPLICATE_CHANNEL_IN_UNIT.
Tests: test_confirm_saves_only_the_tags_the_block_shows (also covers the 34b test b), test_a_label_at_two_positions_is_never_saved_whatever_writes_it, test_a_label_at_two_positions_of_a_unit_is_never_saved.

BLOCKER 2. An unanswered image type is published as ordinary channels: STILL HOLDS. missing_confirmations() ignored image_type and _repeat() mapped None to an absent type. The shared test fixture complete() had no type at all and was published by the existing tests. Spec §4.4b/§5: in the published file an absent image_type means an ordinary channel image, so that format is unchanged ("channels" is still written as absent). The draft now requires an explicit answer. The notebook's Image type step and the #249 all_colour logic already used saved answers only. A resumed published file maps an absent type to "channels" (published_answers), so reopening is unaffected.
Fix: new open question unit_image_type_unanswered for every unit whose type is not "channels" or "rgb". It is worded on Check and save ("the image type of … is not answered (Image type)").
Tests: test_an_unanswered_image_type_is_an_open_question, test_publish_refuses_an_unanswered_type_and_writes_nothing, test_only_an_explicit_type_answers_it_and_channels_is_published_absent, and the notebook-level test_an_unanswered_image_type_keeps_save_and_run_closed_until_answered (34b test a: save refused, no morphobricks.toml, Run unavailable, then "channels" opens the gate).

BLOCKER 3. A stale channel-count mapping is hidden but publishable: STILL HOLDS. tags_of() hid it. missing_confirmations(), description_from_draft() and publish_draft() never compared the tags with the image. check_candidate().can_save ignores the run's R_CHANNEL_COUNT_MISMATCH.
Fix: publish_draft(..., channel_counts=...) is now a required keyword: each unit's channel count read through the confirmed axis roles. missing_confirmations and description_from_draft take the same counts. A saved tag list of another length is unit_channels_stale, and the user retags it. It is never stretched or dropped silently. A unit without a count is unit_channel_count_unknown. The notebook passes its UnitLayout counts, and its save is also refused when any analysed image changed since the folder check.
Tests: test_tags_saved_for_another_channel_count_are_refused_until_retagged (publish_draft) and test_tags_saved_for_another_channel_count_are_not_published (notebook). That notebook test runs a valid 3-channel draft, replaces the images with 2-channel images at the same paths, and checks that the save is refused before a re-check (image changed) and after it (stale tags) until the folders are retagged, then saves.

BLOCKER 4. Thumbnail evidence is not bound to the checked revision: STILL HOLDS. The notebook's preflight did not bind headers. Thumbnails._read() gave no StripOptions.expected. The channel-step cache key had no fingerprints. The #250 Step 1 pictures (header_preview), the native-pixel crop and the mask evidence had the same gap.
Fix: the notebook's folder check uses preflight(bind_headers=True). Thumbnails takes Preflight.header_fingerprints, passes expected, re-checks the fingerprint before returning a cached picture or a crop, and the channel-step key holds the representative images' fingerprints. header_preview(..., bound=...) (a required keyword), mask_evidence(..., options, bound) (image bound, and the mask's own MaskFile.bound or, for an unbound mask, no change across the read) behave the same way. A mismatch, or an image the check did not bind, is refused with "… changed since the folder check read it: check the folder again." New helpers: preview.check_unchanged, preview.checked_fingerprint, fingerprint.changed_since_check.
Tests: test_a_thumbnail_of_an_image_changed_after_the_check_is_refused (same shape and header names; a re-check draws again), test_the_step_1_picture_of_an_image_changed_after_the_check_is_refused, test_the_mask_evidence_of_an_image_changed_after_the_check_is_refused.

FILE 1. testing_described.py count guard: STILL HOLDS. A negative count slipped past base == 0, and zero technical repeats raised ZeroDivisionError. The generator also validated lazily, so write_described_tree could write earlier members first.
Fix: every member is checked up front: no technical repeat, or image_count < units, raises ValueError before anything is written.
Test: test_make_described_run_refuses_impossible_counts_and_writes_nothing (-1, -3, 0, no units).

FILE 2. _answer() acknowledges an unsaved answer: STILL HOLDS. _save_description() swallowed lock, draft-file and OS errors, the last of them silently.
Fix: _save_description() returns a bool and the reason. _answer() restores the previous answers and refuses with AnswerNotSaved. save_experiment() stops if the draft could not be saved first. With no output folder at all, the answers are still kept in memory, as the page already says.
Test: test_an_answer_refused_by_the_output_lock_is_not_acknowledged.

Batch 34a

  1. BLOCKER: the records writer does not validate new records. STILL HOLDS. _append_records({"type": "reconfirmation"}) wrote a line the reader refuses. Fix: _candidate_records() runs check_records() on the complete candidate bytes, and publish_locked calls it before the description is written. Any issue raises RecordsFileError (R_RECORDS_FILE_INVALID) and leaves both files untouched. Tests: missing keys, a bad date and a malformed SHA, each for a direct append and for a publication (snapshot unchanged).
  2. BLOCKER: test precondition. STILL HOLDS. test_units_of_one_layout_that_disagree_are_still_refused_by_the_run now asserts pre.ok before the run and keeps the exact code and not result.written. Checked locally, not committed: with an unrelated broken TIFF injected, the strengthened test fails in all 3 cases while the old version passed in all 3.
  3. FILE: invariant wording. HOLDS. CHANGELOG (Described runs with several layouts: inconsistent_mapping and the missing suggested mapping hide the real refusal #245 entry), docs/contract.md §3, the unit_consistency docstring and the test module docstring now say "a previously blocking input stays blocked; diagnostic codes may change". The optional differential test was not added.
  4. FILE: RecordsFileError reported as description_unreadable. STILL HOLDS. Fix: a separate, local except branch in load_description reports R_RECORDS_FILE_INVALID with its E_RECORDS_* reasons. The existing run test expected description_unreadable and now expects the Spec code. New test: a malformed record (E_RECORDS_KEY_SET) stops the run.
  5. FILE: all_colour() length-only check. STILL HOLDS. Fix: the tags must be exactly ("R", "G", "B"). Tests: wrong orders and tokens, plus a resumed malformed draft with B, G, R.

Batch 34b (items assigned here)

  • Unanswered image type published as channels: the same defect as batch 33 B2 above. Fixed, with test (a).
  • Block confirm saves a suppressed suggestion: the same defect as batch 33 B1 above. Fixed, with test (b).
  • FILE 2: tag blocks built from the first image only. STILL HOLDS. Fix: UnitLayout.uniform is False when another image of the unit has another signature (count, raw names, colour). Such a unit is never grouped, and its block line says "its images do not all have this header, so it is tagged on its own." Test: test_a_folder_whose_later_image_differs_is_never_grouped_on_its_first_image (two images per folder; the second image of one folder disagrees).

What else changes

  • publish_draft has a new required channel_counts keyword (an API change; its only caller is the notebook). header_preview needs bound=. mask_evidence takes StripOptions and bound in place of cache, memory_limit.
  • The notebook's folder check now binds headers, so it fingerprints each image (and mask) before and after its header is read, as the run already does.
  • Test fixtures now give an explicit "channels" type and the channel counts. No Spec fixture or golden changed. The 21 tests/test_spec_* modules all pass with 0 skips.

Gate

python tools/check.py: ruff check clean, ruff format clean, code rules match the baseline (not grown), pytest 5041 passed, 4 skipped (the 4 skips are the existing platform skips: case-insensitive file system ×3 and file modes ×1). After that run, the only change was a one-line ruff format fix. The new and tag-block tests were re-run after it: 23 passed.

Not touched, to avoid conflicts: preflight/run P21/P29 wiring and the batch-34b areas of the other agent (notebook_axes_quick.py, notebook_axis_preview.py, the reopen/fold parts of notebook_check_save.py, preview_crop.py). The axis-step preview has the same unbound-read gap. I left it for that agent.


Generated by Claude Code

… image checked

Batch 33:
- B1: block confirm saves exactly the tags shown (effective()); a unit tagging one
  label twice is refused before it is saved (E_DUPLICATE_CHANNEL_IN_UNIT).
- B2: an unanswered image type is an open question (unit_image_type_unanswered).
- B3: publish_draft requires the current, axis-confirmed channel counts; stale tags
  (unit_channels_stale) or a unit without a count are refused. The notebook save is
  refused when an image changed since the folder check.
- B4: the folder check binds every header to its file; tag pictures, colour
  composites, Step 1 pictures and mask evidence are drawn only of that revision.
- FILE 1: make_described_run refuses impossible counts before writing anything.
- FILE 2: an answer that cannot be saved is refused and rolled back.

Batch 34a:
- the records writer checks the complete candidate bytes before writing;
- the run reports R_RECORDS_FILE_INVALID with its E_RECORDS_* reasons;
- the cross-unit test asserts pre.ok; invariant wording corrected;
- all_colour requires exactly R, G, B.

Batch 34b:
- a unit whose later images have another header is never grouped in a tag block.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
…rk stale tags

- remasked() binds the masks paired again when the check bound the headers, so
  the mask evidence of a mask changed after that pairing is refused.
- The masks screen reads its kept evidence again (and so refuses it) once the
  representative image or its bound mask changed since the check.
- Check and save no longer shows tags saved for another channel count as the
  folder's chips; it says they are stale, as the to-do list does.
- CHANGELOG: the axis preview is not bound yet (it was claimed); the API changes
  of publish_draft, header_preview, mask_evidence and Thumbnails are listed.
  docs/contract.md lists the new open questions and channel_counts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
@guijacquemet
guijacquemet merged commit ac105b6 into main Nov 9, 2026
7 checks passed
@guijacquemet
guijacquemet deleted the claude/batch33-fixes branch November 9, 2026 17:03
guijacquemet pushed a commit that referenced this pull request Nov 9, 2026
guijacquemet pushed a commit that referenced this pull request Nov 9, 2026
The axis preview compared Fingerprint.of itself; it now uses
preview.checked_fingerprint and preview.check_unchanged, as the channel
thumbnails, header previews and mask evidence do, before and after the
read. A PreviewRefused is a failed preview. Adds a regression test for a
file rewritten during the read (fails without the after-read check).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
guijacquemet added a commit that referenced this pull request Nov 9, 2026
guijacquemet pushed a commit that referenced this pull request Nov 10, 2026
Brings the audit batch 33/34a/34b fixes and the pre-release follow-ups into the v0.5.0
release candidate, keeping the P21/P29 description_in_use wiring.

Conflicts:
- notebook_folder.py: the folder check passes both with_description (P21/P29) and
  bind_headers=True (audit batch 33).
- cli_describe.py: the module docstring says both that describe reads image headers when it
  compares a published description and that a non-NFC folder label is refused; the imports
  of both sides are kept.
- CHANGELOG.md: main's Unreleased entries are folded into the 0.5.0 section, after the
  P21/P29 fix; the 0.5.0 introduction names #254, #255 and #257.

Also: the P30 test's answers (added on this branch) now answer the image type as
"channels", since an unanswered image type is an open question since audit batch 33
(unit_image_type_unanswered). tools/check.py: ruff clean, 415 files formatted,
5105 passed, 4 skipped (environment), code rules passed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
guijacquemet pushed a commit that referenced this pull request Nov 10, 2026
RELEASE_REF (and the test asserting it) now names 0ea3254, the merge of origin/main
(#254, #255, #257) into claude/release-0.5.0, which carries the P21/P29 fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
guijacquemet pushed a commit that referenced this pull request Nov 10, 2026
Only docs/conformance.md: the measured commit (ba415de, whose RELEASE_REF is the merge
0ea3254), the counts (5109 collected, 5105 passed, 4 skipped; Spec modules 1671 passed,
0 skipped), tools/check.py (415 files, 5105 passed), the P30 answers now stating the image
type, and the fixes merged from main: audit batches 33/34a (#254), 34b (#255) and the
pre-release follow-ups (#257).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc
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.

2 participants