From 437728b0ff63b4502a3ce999d13918c2be363f8e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 16:24:21 +0000 Subject: [PATCH 1/3] Bind masks to their fingerprints for the whole run (#160) Each mask's header is now read between two fingerprints that must agree (find_masks(bind=True), MaskFile.bound; else mask_changed_during_check), the manifest records that fingerprint instead of one taken when it is written, and every mask must still have it right before the commit (mask_changed_during_run, naming the mask and what changed; nothing is published). A verify_content run also hashes every mask after the header checks and again as the last step before the commit. No schema change: unchanged reruns keep the manifest byte for byte. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VbytvF3Gsa85ix8Ne2yMDW --- AGENTS.md | 11 +- CHANGELOG.md | 15 ++ docs/contract.md | 6 +- docs/user_guide.md | 8 +- pyproject.toml | 1 + .../mask_binding.py | 155 ++++++++++++++ src/morphobricks_preprocessing/mask_record.py | 15 +- .../masks/checks.py | 31 ++- src/morphobricks_preprocessing/masks/files.py | 8 +- src/morphobricks_preprocessing/masks/model.py | 29 +++ .../run/pipeline.py | 64 ++++-- .../run/preflight.py | 11 +- tests/test_mask_run_binding.py | 189 ++++++++++++++++++ 13 files changed, 513 insertions(+), 30 deletions(-) create mode 100644 src/morphobricks_preprocessing/mask_binding.py create mode 100644 tests/test_mask_run_binding.py diff --git a/AGENTS.md b/AGENTS.md index 27b3ed5f..99238f6a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -127,7 +127,11 @@ src/morphobricks_preprocessing/ preprocessing's discovery and pairing; RunInputs.open raises UnrecordedFilesError for them (PP-8b, contract §9.1) run_inputs_text.py the texts RunInputs refuses a run with (moved from run_inputs.py in PP-12) - mask_record.py the manifest's masks entry with fingerprints and roles (manifest v2) + mask_record.py the manifest's masks entry with fingerprints and roles (manifest v2); the + fingerprint is the one bound to the mask's header (issue #160) + mask_binding.py masks bound to their fingerprints (issues #160, #165): changed_masks, + mask_contents/content_changed_masks (verify_content), + mask_changed_issue (mask_changed_during_run) axis_check.py the axis check (contract §2): header_axis_issue, axis_disagreements, likely_axis_cause, check_header_axes, ConfirmedAxes (a confirmation covers only its files as fingerprinted, plain_stack_not_confirmed); AXIS_ERROR_CODES, AXIS_CHECK_CODES @@ -484,7 +488,10 @@ functions the tests use. into place (the output folder is fsynced right after the commit); the next holder of the lock finishes or discards an interrupted publication. Every image is fingerprinted when its header is read, before and after its pixels are - read, and before the commit (`source_changed_during_run`, issue #90). Nothing is + read, and before the commit (`source_changed_during_run`, issue #90). Every mask's header + is read between two fingerprints (`mask_changed_during_check`), that fingerprint is the one + recorded, and the mask must still have it before the commit (with `verify_content` also its + content SHA-256; `mask_changed_during_run`, issue #160). Nothing is published if any error is found. - **Exclusions:** the user may exclude images explicitly (`exclude=[...]`, e.g. damaged files). Excluded images are not read, are reported (`excluded_images` warning), are diff --git a/CHANGELOG.md b/CHANGELOG.md index feb8fa51..0f1d0c0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,21 @@ are in [docs/plan.md](docs/plan.md) §12. Upgrading from v0.3: see the New `MaskReport.hierarchy_cycles` and `mask_scan.hierarchy_cycles`. Acyclic hierarchies print as before; `mask_relationships.json` is unchanged (report version 1). +- **Masks are bound to their fingerprints for the whole run** (#160). Each mask's header is + read between two fingerprints that must agree (`find_masks(bind=True)`, + `MaskFile.bound`; else the new error `mask_changed_during_check`), the manifest records + that fingerprint (before, it was taken when the manifest was written, so a mask rewritten + during the run was recorded under headers checked on another version), and every mask + must still have it right before the commit; otherwise the new error + `mask_changed_during_run` names the mask and what changed, and nothing is published. A + `verify_content` run also hashes every mask after the header checks and again as the last + step before the commit. Content hashing stays opt-in (#159). + - *Records and migration:* no schema change. The manifest (versions 2 and 3) and its mask + entries keep their keys and values: for masks unchanged during the run the fingerprint + recorded is the same, so an unchanged rerun keeps the manifest byte for byte. The masks' + content SHA-256 is compared in memory and not recorded. Earlier manifests read as + before. + ## v0.4.0 (5 November 2026) The final v0.4.0, with the small changes since v0.4.0rc5 (#147) and the release step: the diff --git a/docs/contract.md b/docs/contract.md index edd998e1..a1964a8d 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -133,7 +133,7 @@ method is an offset and a scale, `(pixels − offset) / scale`. | Severity | Situations | Effect | |---|---|---| -| Error | Experiment design not confirmed (`design_not_confirmed`) or a repeat group holding two folders of one condition (`repeat_group_conflict`), when a design is used (§12); image type RGB for images that are not three channels of an integer type (`rgb_not_possible`, §13); mixed dtypes; images with different numbers of channels (`different_channel_counts`, §2) or different channel names at the same channel number (`channel_names_differ`, §2); unreadable TIFF (header or pixel data) or OME-Zarr folder in a leaf folder that is not excluded; OME-Zarr images without the `zarr` extra; OME-Zarr images whose declared axes do not fit the experiment's roles or that the confirmed roles contradict (`ome_zarr_axes_mismatch`, §2); OME-Zarr images that do not name their axes, and every OME-NGFF 0.1 or 0.2 store (`unreadable_ome_zarr`, §2); every image excluded; more than one image series in a file; unsupported sample format or axis; an image with more than one depth slice (`z_stack_not_supported`, §2; MorphoBricks v1 analyses 2-D planes); missing/duplicate X or Y, duplicate roles, unassigned non-singleton axis, inconsistent mapping; an image whose confirmed roles swap Y and X, contradict its header or put X before Y (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, §2); plane that cannot be read within the memory limit, not even in blocks (for example a plane stored compressed in one piece); folder named `single` in another case, or beside other folders (§1); folders of one level differing only in case, anywhere in the experiment (`case_conflict`); folder or image names with outer whitespace or reading as a missing value (`invalid_name`, §1); images in one leaf folder with the same position identifier (§1); links that leave the experiment or masks folder, loop, or reach one file twice (`link_outside_folder`, `link_loop`, `linked_twice`, §1); a mount point below the experiment or masks folder (`mounted_folder`, §1); folder below the technical-repeat level; images in a folder that also holds subfolders (`tiff_outside_leaf`, §1); output folder inside the experiment or inside the masks folder (`output_inside_masks`, §15); experiment or named-axis copies inside the masks folder (`experiment_inside_masks`, `copies_inside_masks`, §15); images named as masks of other images (`image_named_as_mask`, §1); no usable planes; no axis roles given or recorded (`axes_not_confirmed`, §9.2); another run holding the output folder's lock (`output_locked`, §9.2); a copy tree whose conversion record cannot be read (`conversion_record_unreadable`, §17), or whose copies changed, went missing or were added since the conversion (`copy_changed_after_conversion`, §17; owner decision of 2 November 2026, PP-12: the bricks would refuse the run); an image that changed or went missing while the run checked or read it (`source_changed_during_run`, §9.4; no scientific outputs are published and sources are untouched; the discovery report is written); a recorded `experiment_design.csv` that cannot be read (`design_unreadable`); a `calibration.csv` that cannot be read (`calibration_unreadable`) or that states sizes for a target that is neither a folder nor an image of the experiment (`calibration_target_not_found`, §9.3); no experiment folder given or recorded (`experiment_not_recorded`, §9.2); when masks are given, a masks folder missing or inside the experiment, mask names differing only in case, two masks of one type for an image, a mask whose name could belong to two images or types, or a mask that is unreadable, has axes other than Y, X and named or stated Z/T (`mask_not_2d`), has stated axes that do not fit it or contradict the axes its metadata names (`invalid_mask_axes`, `mask_axes_contradict_header`), is stated `YX` (`shared_mask_not_supported`, §15), is stored with X before Y (its header's or its stated axes; refused by `check` and `run` too, never recorded with empty axes) or with its image's Y and X sizes swapped (`mask_layout`; never transposed), is of another size than its image (in `run`, its image's Y and X under the confirmed roles; not reported for an image with an axis error), is not of integers or booleans, has other depth slices or time points than the image's other masks or, in `run`, than the image (§15) | Processing is blocked | +| Error | Experiment design not confirmed (`design_not_confirmed`) or a repeat group holding two folders of one condition (`repeat_group_conflict`), when a design is used (§12); image type RGB for images that are not three channels of an integer type (`rgb_not_possible`, §13); mixed dtypes; images with different numbers of channels (`different_channel_counts`, §2) or different channel names at the same channel number (`channel_names_differ`, §2); unreadable TIFF (header or pixel data) or OME-Zarr folder in a leaf folder that is not excluded; OME-Zarr images without the `zarr` extra; OME-Zarr images whose declared axes do not fit the experiment's roles or that the confirmed roles contradict (`ome_zarr_axes_mismatch`, §2); OME-Zarr images that do not name their axes, and every OME-NGFF 0.1 or 0.2 store (`unreadable_ome_zarr`, §2); every image excluded; more than one image series in a file; unsupported sample format or axis; an image with more than one depth slice (`z_stack_not_supported`, §2; MorphoBricks v1 analyses 2-D planes); missing/duplicate X or Y, duplicate roles, unassigned non-singleton axis, inconsistent mapping; an image whose confirmed roles swap Y and X, contradict its header or put X before Y (`axes_yx_swapped`, `axes_contradict_header`, `axes_x_before_y`, §2); plane that cannot be read within the memory limit, not even in blocks (for example a plane stored compressed in one piece); folder named `single` in another case, or beside other folders (§1); folders of one level differing only in case, anywhere in the experiment (`case_conflict`); folder or image names with outer whitespace or reading as a missing value (`invalid_name`, §1); images in one leaf folder with the same position identifier (§1); links that leave the experiment or masks folder, loop, or reach one file twice (`link_outside_folder`, `link_loop`, `linked_twice`, §1); a mount point below the experiment or masks folder (`mounted_folder`, §1); folder below the technical-repeat level; images in a folder that also holds subfolders (`tiff_outside_leaf`, §1); output folder inside the experiment or inside the masks folder (`output_inside_masks`, §15); experiment or named-axis copies inside the masks folder (`experiment_inside_masks`, `copies_inside_masks`, §15); images named as masks of other images (`image_named_as_mask`, §1); no usable planes; no axis roles given or recorded (`axes_not_confirmed`, §9.2); another run holding the output folder's lock (`output_locked`, §9.2); a copy tree whose conversion record cannot be read (`conversion_record_unreadable`, §17), or whose copies changed, went missing or were added since the conversion (`copy_changed_after_conversion`, §17; owner decision of 2 November 2026, PP-12: the bricks would refuse the run); an image that changed or went missing while the run checked or read it (`source_changed_during_run`, §9.4; no scientific outputs are published and sources are untouched; the discovery report is written); a mask that changed while its header was read (`mask_changed_during_check`) or after it, before the run's commit (`mask_changed_during_run`, §9.4, issue #160; the same); a recorded `experiment_design.csv` that cannot be read (`design_unreadable`); a `calibration.csv` that cannot be read (`calibration_unreadable`) or that states sizes for a target that is neither a folder nor an image of the experiment (`calibration_target_not_found`, §9.3); no experiment folder given or recorded (`experiment_not_recorded`, §9.2); when masks are given, a masks folder missing or inside the experiment, mask names differing only in case, two masks of one type for an image, a mask whose name could belong to two images or types, or a mask that is unreadable, has axes other than Y, X and named or stated Z/T (`mask_not_2d`), has stated axes that do not fit it or contradict the axes its metadata names (`invalid_mask_axes`, `mask_axes_contradict_header`), is stated `YX` (`shared_mask_not_supported`, §15), is stored with X before Y (its header's or its stated axes; refused by `check` and `run` too, never recorded with empty axes) or with its image's Y and X sizes swapped (`mask_layout`; never transposed), is of another size than its image (in `run`, its image's Y and X under the confirmed roles; not reported for an image with an axis error), is not of integers or booleans, has other depth slices or time points than the image's other masks or, in `run`, than the image (§15) | Processing is blocked | | Warning | Images whose metadata does not name their axes, read with the confirmed roles (`axes_not_named`, §2); images or masks with other names, hard links (`hard_linked_file`, §1); in a tree of named-axis copies, sources changed or gone since the conversion (`conversion_source_changed`, §17); folders named like a masks folder holding images named as masks (`mask_folder_in_experiment`, §1); intensity checks of the written table (`image_intensity_outlier`, `repeat_intensity_shift`, `saturated_image`, `no_intensity_spread`, `channel_without_finite_pixels`, §14), reported apart from the run's issues; files that are not images; empty folders; images excluded by the user, or excluded paths that match no image; non-finite pixels; different image widths or heights; different numbers of depth slices, time points or other planes; physical pixel sizes that differ or are known for only some images; a stated pixel size that differs from the header (`calibration_differs_from_header`, §9.3; the stated size is used); unbalanced numbers of images; biological repeats missing from some conditions (§11); planes without finite pixels while others remain; channels whose central intensity range covers too few histogram bins (`narrow_histogram_range`, §4); planes over the memory limit that are read in blocks; images without a mask of a type other images have, masks matching no image, masks of excluded images, files outside the mask-type folders (§15) | Processing continues; the item is reported | Every message states what happened in plain language, lists the affected paths, and @@ -218,7 +218,7 @@ interpretation. | `calibration_sha256` | SHA-256 of the output folder's `calibration.csv` as the run read it (§9.3); null when there was none (or absent in manifests written before PP-8). `RunInputs` checks it. | | Version 1 | A manifest written before v0.4.0 (`"1"`) is still read, for reporting, but no row of it is reused: its axes were never checked. The next run recomputes every row ("the earlier run was written before v0.4.0, without the axis check"). Bricks refuse it. | | Row reuse | A file's rows are reused only when its row key is the earlier run's (`fingerprint.RowKey`, one rule for "the same file, read the same way", audit D5): its fingerprint (`fingerprint.Fingerprint`: size and modification time, and for an OME-Zarr image the inventory digest), labels, axes, roles, `singleton_roles`, `axes_from` and the path it is read from (so moving or converting the experiment recomputes it), and the settings that change rows (bins, histogram range, robust quantiles, range-check limit). **The identity rule is the fingerprint, which is metadata** (design v0.4 §1): its known blind spot is a file rewritten with the same size whose modification time (for an OME-Zarr image, every changed file's) is set back to its earlier value; its earlier rows are then reused, its staged rows resumed, and `RunInputs` accepts it. | -| Content verification (opt-in, owner 3 November 2026) | `RunMode(name, verify_content=True)` (`run_experiment(mode=...)`, CLI `--verify-content`, the notebook's "Verify each image's content" box) closes that blind spot: each analysed image's `content_sha256` is taken after its fingerprint and recorded; it is part of the row key, so an earlier or staged row is reused only for the same content; an image read must still have it after its pixels are read (else `source_changed_during_run`); every recorded image (reused, resumed or read; an OME-Zarr image by the same store digest) is hashed once more as the last step before the commit, after the generation is staged, flushed and recorded and right before the manifest's rename (`staged_generation(before_commit=...)`, review of rc5), and a mismatch is `source_changed_during_run`: the publication folder is discarded and the earlier generation is intact, as for a fingerprint change; an image that cannot be hashed is `unreadable_planes`; and `RunInputs.open` of such a run checks every image's content SHA-256 once (reads after `open` check the fingerprint). It is a recorded input (`settings.verify_content`): a run given none keeps the recorded choice (false without one); turning it on or off recomputes every row ("content verification (verify_content) was turned on/off"), is reported as the change `verify_content`, and discards staged rows of the other mode with that reason. It reads every analysed image whole twice more per run (when its entry is made and before the commit), once more for each image read, and once for each `RunInputs.open`; the default run is unchanged. A strict run writes manifest version 3. Not covered: masks keep the fingerprint rule; a change between the header read and the first hashing with its metadata kept (the rows are of the hashed content, the header may be of the earlier one); and a change in the moment between the last hashing and the rename, or after the commit (`RunInputs.open` hashes again). | +| Content verification (opt-in, owner 3 November 2026) | `RunMode(name, verify_content=True)` (`run_experiment(mode=...)`, CLI `--verify-content`, the notebook's "Verify each image's content" box) closes that blind spot: each analysed image's `content_sha256` is taken after its fingerprint and recorded; it is part of the row key, so an earlier or staged row is reused only for the same content; an image read must still have it after its pixels are read (else `source_changed_during_run`); every recorded image (reused, resumed or read; an OME-Zarr image by the same store digest) is hashed once more as the last step before the commit, after the generation is staged, flushed and recorded and right before the manifest's rename (`staged_generation(before_commit=...)`, review of rc5), and a mismatch is `source_changed_during_run`: the publication folder is discarded and the earlier generation is intact, as for a fingerprint change; an image that cannot be hashed is `unreadable_planes`; and `RunInputs.open` of such a run checks every image's content SHA-256 once (reads after `open` check the fingerprint). It is a recorded input (`settings.verify_content`): a run given none keeps the recorded choice (false without one); turning it on or off recomputes every row ("content verification (verify_content) was turned on/off"), is reported as the change `verify_content`, and discards staged rows of the other mode with that reason. It reads every analysed image whole twice more per run (when its entry is made and before the commit), once more for each image read, and once for each `RunInputs.open`; the default run is unchanged. A strict run writes manifest version 3. Masks are hashed too (issue #160): each mask's content SHA-256 is taken once the header checks pass and again as the last step before the commit, and a mismatch (or a mask that cannot be hashed) is `mask_changed_during_run`; it is not recorded in the manifest (no mask has rows to reuse), so the manifest's mask entries are unchanged. Not covered: a change between the header read and the first hashing with its metadata kept (the rows are of the hashed content, the header may be of the earlier one); and a change in the moment between the last hashing and the rename, or after the commit (`RunInputs.open` hashes again). | | Removed keys | `settings.convert_tiffs`, `settings.conversion` and `files[].ome_zarr_path` were written by v0.3 runs that converted TIFF images to OME-Zarr in the output folder (removed in v0.4.0, §10); a manifest that holds them is still read, and they are ignored. The copies such a run left in `/ome_zarr` are not touched. | | Flags | `missing_as_singleton`, `verify_content`, `read_in_blocks`, `channel_names_trimmed` and `channels_from_rgb_samples` are JSON booleans (`true`/`false`); any other value (`"false"`, `0`) makes the manifest malformed, never read as true or false, and `write_manifest` refuses to write one (`ValueError`), so a written manifest always reads back. | | `versions` | Versions of `morphobricks-preprocessing`, `morphobricks-normalization` (which computes the histograms and writes the CSV; absent in manifests written before it), `numpy`, `tifffile` and Python. Information only: a different version never causes a rebuild. | @@ -343,7 +343,7 @@ the bricks' tests. |---|---| | Staging | As soon as a run has read one image, that image's rows are staged in the output folder's `.staging` folder: a `.csv` with its rows (the normalization brick's writer, the table's format) and then `.json` (staging version 1) naming the image, its staging key and the rows file's SHA-256. The staging key is the image's row key (§9: fingerprint, labels, axes, roles, `singleton_roles`, `axes_from`, read path, bins and histogram settings) with the pixel type, the table's schema version and the versions of `morphobricks-preprocessing` and `morphobricks-normalization`. A rows file without its record (killed between the two) was never staged. | | Resume | A run killed part-way (a crash, a kill, a lost Colab session) is resumed by the next run: each image that needs new rows whose staged key is the key it would have now (and whose fingerprint is still that; with content verification, whose content SHA-256, part of the key, too) takes its staged rows without its pixels being read (`RunResult.resumed`, report `resumed`). Any other staged rows are discarded and reported with the reason (report `staged_discarded`): the image changed since its rows were staged; its roles, labels, folder or row settings changed; the experiment's pixel type changed; another version staged them; they could not be read (reported under the image the record names, else the record's file name); the table already holds its rows; the image is no longer analysed; a `full` run (which reuses no staged row). Nothing is reused when identity is uncertain. A run that stops keeps `.staging`, so the next run resumes; a run that publishes removes it after the commit. Every check still runs over the whole experiment (design v0.4 §1). A resumed table is byte-identical to an uninterrupted run's. | -| Source changes during a run (issue #90) | Each image's header is read between two fingerprints that must agree (`preflight(bind_headers=True)`), and that fingerprint is the one its manifest entry records; the image must still have it just after its pixels are read (only then are its rows staged); and every analysed image (reused, resumed or read) is fingerprinted again just before the commit (in a `verify_content` run also hashed again, as the last step before the manifest's rename, above). An image that changed or went missing at any of these points is the error `source_changed_during_run` naming it: no scientific outputs are published and sources are untouched; the discovery report is written; the earlier generation is kept, and the rows staged so far stay staged (the changed image's own rows are not staged, or are discarded by the next run). Masks are fingerprinted when the manifest is written, as before. | +| Source changes during a run (issue #90) | Each image's header is read between two fingerprints that must agree (`preflight(bind_headers=True)`), and that fingerprint is the one its manifest entry records; the image must still have it just after its pixels are read (only then are its rows staged); and every analysed image (reused, resumed or read) is fingerprinted again just before the commit (in a `verify_content` run also hashed again, as the last step before the manifest's rename, above). An image that changed or went missing at any of these points is the error `source_changed_during_run` naming it: no scientific outputs are published and sources are untouched; the discovery report is written; the earlier generation is kept, and the rows staged so far stay staged (the changed image's own rows are not staged, or are discarded by the next run). Masks are bound the same way (issue #160): each mask's header is read between two fingerprints that must agree (`find_masks(bind=True)`, `MaskFile.bound`; else `mask_changed_during_check`), that fingerprint is the one its manifest entry records (before #160 the fingerprint was taken when the manifest was written), and every mask must still have it right before the commit; a mask that changed or went missing is the error `mask_changed_during_run` naming it, with what changed, and nothing is published (`mask_binding`). | | Generation | The run's published files: `plane_histograms.csv`, `experiment_design.csv` (with a design), `run_manifest.json` and the report files, and the files the generation removes (`mask_relationships.json`, §9.2). | | Publication | (1) The whole new generation is written into `.publish` in the output folder and flushed to disk, then `.publish/publication.json` (publication version 1: each file's SHA-256 and the files removed). (2) **Commit, the one atomic step:** `os.replace` of the new `run_manifest.json` over the old one, then the output folder is flushed (`fsync` of the folder, POSIX), so the commit is on the disk when it returns and a power cut after it keeps the new generation. Nothing in the output folder changes before it. (3) The other files are moved into place (`os.replace` each), the removed files removed, `.publish` deleted, then `.staging`. | | Interrupted publication | Whatever next takes the output folder's lock (`run`, `check`, `masks`, `design`, `calibrate`, the notebook's writes) first finishes a committed publication or deletes an uncommitted `.publish`, record first (the earlier generation is then untouched) (`run.generation.finish_publication`). **Committed** means the staged manifest was moved: the record is there, `.publish/run_manifest.json` is not, and the manifest in place is the record's; the remaining moves and removals are then done. Anything else is not committed, also when the manifest in place is byte-identical to the staged one (an identical rerun refused before its commit); a `.publish` whose record cannot be removed is left whole (its staged manifest keeps it uncommitted) for the next holder of the lock. A failure to move a file after the commit (for example a table held open on Windows) raises `PublicationIncompleteError`: the new generation is current, and the next holder of the lock finishes the moves. | diff --git a/docs/user_guide.md b/docs/user_guide.md index e1e872c8..43db124f 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -959,7 +959,11 @@ that program and run again. This needs temporary disk space for the new outputs. **Images that change during the run.** If an image changes or disappears while the run checks or reads it (an acquisition or a copy still writing into the experiment), the run stops with `source_changed_during_run` and publishes nothing. Wait until nothing writes to -the experiment, then run again: the images already read resume. +the experiment, then run again: the images already read resume. Masks are checked the same +way: a mask that changes while the run checks its header, or at any time before the run +publishes its results (a segmentation still writing it), stops the run with +`mask_changed_during_check` or `mask_changed_during_run`, naming the mask and what changed, +and nothing is published. With `--verify-content` the masks' content is compared too. Every output file gets the permissions a file you save yourself would get (your umask; usually readable by everyone, writable by you), so colleagues can open the results in a @@ -1070,6 +1074,8 @@ continues. | `conversion_source_changed` | Warning | An original changed or went missing after its named-axis copy was made; the copies hold the images as converted. If the originals were corrected, convert again into a new folder. | | `copy_changed_after_conversion` | Error | An image of the folder of named-axis copies changed (its size, time or SHA-256), went missing or was added after the conversion, so the conversion record does not describe it, and the bricks (`RunInputs`) would refuse a run on it. The run stops before it writes anything but the report: convert the originals again into a new folder, and run preprocessing on that one (same output folder). | | `source_changed_during_run` | Error | An image changed or went missing while the run was checking or reading it, so its rows would not describe it. No scientific outputs are published and sources are untouched; the discovery report is written. Wait until nothing writes to the experiment (an acquisition or a copy still running), then run again: the images already read resume from their staged rows. | +| `mask_changed_during_check` | Error | A mask changed while its header was read, so the check cannot tell which version it describes. Wait until nothing writes to the masks (a segmentation or a copy still running), then check or run again. | +| `mask_changed_during_run` | Error | A mask changed or went missing after the run checked its header and before it published its results (its size or time, or for an OME-Zarr label image its files; with `--verify-content` also its content). Nothing is published; the discovery report is written. Wait until nothing writes to the masks, then run again: the images already read resume from their staged rows. | | `design_unreadable` | Error | The `experiment_design.csv` an earlier run wrote cannot be read. Correct it, or delete it to confirm the design again. | | `no_usable_planes` | Error | No plane has finite pixel values. Check the images. | | `tiff_outside_leaf` | Error | Images sit in a folder that also holds subfolders, so they have no condition or repeat of their own. A folder holds either images or subfolders, never both. Move the images into a folder of their own, or out of the experiment folder. | diff --git a/pyproject.toml b/pyproject.toml index 6014909e..7c894520 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -140,6 +140,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.axis_review" = "pure" "morphobricks_preprocessing.mask_checks" = "pure" "morphobricks_preprocessing.mask_review" = "pure" +"morphobricks_preprocessing.mask_binding" = "io" "morphobricks_preprocessing.mask_record" = "io" "morphobricks_preprocessing.notebook_axes" = "ui" "morphobricks_preprocessing.notebook_axis_preview" = "ui" diff --git a/src/morphobricks_preprocessing/mask_binding.py b/src/morphobricks_preprocessing/mask_binding.py new file mode 100644 index 00000000..87ea2cce --- /dev/null +++ b/src/morphobricks_preprocessing/mask_binding.py @@ -0,0 +1,155 @@ +"""Masks bound to their fingerprints for a whole run. + +Images are bound to their fingerprints from the moment their headers are read (issue #90, +:mod:`.run.resume`); masks are bound the same way (issue #160). In a run +(:func:`.run.run_experiment`) each mask is fingerprinted before and after its header is read +(:attr:`.masks.MaskFile.bound`; ``mask_changed_during_check`` when the two differ). In a +``verify_content`` run each mask's content SHA-256 is also taken once the header checks pass +(:func:`mask_contents`). Before the commit every mask must still have that fingerprint +(:func:`changed_masks`), and in a ``verify_content`` run that content +(:func:`content_changed_masks`); otherwise the run stops with ``mask_changed_during_run`` +and nothing is published. The manifest records the fingerprint bound to the header. + +The identity is the metadata fingerprint (size, modification time, and for an OME-Zarr +label image its inventory digest); the content SHA-256 is opt-in (``verify_content``). +""" + +from __future__ import annotations + +from collections.abc import Iterable, Mapping +from pathlib import Path + +from .fingerprint import Fingerprint +from .masks import MaskFile, MaskInventory +from .source_content import current_content +from .validation import ValidationIssue, error, plural + +MASK_CHANGED_DURING_RUN = "mask_changed_during_run" +"""The code of a run refused because a mask changed after its header was read.""" + +CHANGE_TEXTS = { + "fingerprint": ( + "their size or modification time, or for an OME-Zarr label image its files, changed" + ), + "fingerprint or content": "their size, modification time, files or content (SHA-256) changed", + "content": "their content (SHA-256) changed, even if their size and time did not", +} +"""What changed, by what was compared.""" + + +def mask_fingerprint(mask: MaskFile, folder: Path | None, root: Path) -> Fingerprint | None: + """Return a mask's fingerprint now, or None when it is gone or cannot be read. + + Parameters + ---------- + mask + The mask. + folder, root + The masks folder and the experiment folder (:meth:`.masks.MaskFile.location`). + + Returns + ------- + Fingerprint or None + """ + try: + return Fingerprint.of(mask.location(folder, root)) + except OSError: + return None + + +def changed_masks(inventory: MaskInventory, root: Path) -> list[str]: + """Return the masks whose fingerprint now is not the one bound to their header. + + Parameters + ---------- + inventory + The masks, found with ``bind=True`` (:func:`.masks.find_masks`). + root + The experiment folder. + + Returns + ------- + list of str + The changed, missing or unbound masks (a mask never bound is never trusted), sorted. + """ + return sorted( + mask.path + for mask in inventory.files + if mask.bound is None or mask_fingerprint(mask, inventory.folder, root) != mask.bound + ) + + +def mask_contents(inventory: MaskInventory, root: Path) -> dict[str, str | None]: + """Return each mask's content SHA-256 (``verify_content``), None for one not readable. + + Parameters + ---------- + inventory + The masks. + root + The experiment folder. + + Returns + ------- + dict + ``{mask path: SHA-256 or None}``, as :func:`.source_content.source_content_sha256` + gives it (a TIFF's bytes; a label image's files). + """ + return { + mask.path: current_content(mask.location(inventory.folder, root)) + for mask in inventory.files + } + + +def content_changed_masks( + inventory: MaskInventory, root: Path, contents: Mapping[str, str | None] +) -> list[str]: + """Return the masks whose content SHA-256 now is not the one in ``contents``. + + Parameters + ---------- + inventory + The masks. + root + The experiment folder. + contents + Their SHA-256 as taken earlier (:func:`mask_contents`). + + Returns + ------- + list of str + The changed, unreadable or unhashed masks, sorted. + """ + return sorted( + mask.path + for mask in inventory.files + if contents.get(mask.path) is None + or current_content(mask.location(inventory.folder, root)) != contents[mask.path] + ) + + +def mask_changed_issue(paths: Iterable[str], compared: str = "fingerprint") -> ValidationIssue: + """Return the error naming the masks that changed while a run used them. + + Parameters + ---------- + paths + The masks, sorted. + compared + What was compared, a key of :data:`CHANGE_TEXTS`. + + Returns + ------- + ValidationIssue + """ + paths = sorted(paths) + count = plural(len(paths), "mask") + return error( + MASK_CHANGED_DURING_RUN, + f"{count} changed or went missing while the run was using them " + f"({CHANGE_TEXTS[compared]}), so the run's record of them would not describe them. " + "Nothing was published.", + paths, + "Wait until nothing is writing to these masks (a segmentation or a copy still " + "running), then run again: the images already read resume from their staged rows.", + ) diff --git a/src/morphobricks_preprocessing/mask_record.py b/src/morphobricks_preprocessing/mask_record.py index 77503e0e..33822f4f 100644 --- a/src/morphobricks_preprocessing/mask_record.py +++ b/src/morphobricks_preprocessing/mask_record.py @@ -2,7 +2,8 @@ Every mask entry adds to what :meth:`.masks.MaskInventory.to_manifest` gives its fingerprint (``size``, ``mtime_ns``, and ``inventory_sha256`` for an OME-Zarr label image, -issue #86), so a brick can tell a mask changed since the run, and +issue #86), so a brick can tell a mask changed since the run (the fingerprint bound to the +header the run checked, :attr:`.masks.MaskFile.bound`, issue #160), and how it is read: ``roles`` (one per axis of ``axes``) and ``axes_from`` (``"header"``: the mask's metadata names them; ``"stated"``: the user stated them for its mask type). The key ``shared`` of a 2-D mask shared by every plane of its image (v0.4.0 release candidates) is @@ -18,7 +19,7 @@ from pathlib import Path from .fingerprint import Fingerprint -from .masks import FOLDER, MaskFile, MaskInventory +from .masks import MaskInventory def mask_manifest(inventory: MaskInventory, experiment_root: str | os.PathLike) -> dict: @@ -52,7 +53,10 @@ def mask_manifest(inventory: MaskInventory, experiment_root: str | os.PathLike) by_path = {mask.path: mask for mask in inventory.files} for entry in data["files"]: mask = by_path[entry["path"]] - fingerprint = Fingerprint.of(_mask_path(mask, inventory.folder, Path(experiment_root))) + # The fingerprint bound to the header the run checked (issue #160), else as it is now. + fingerprint = mask.bound or Fingerprint.of( + mask.location(inventory.folder, Path(experiment_root)) + ) entry.update( axes=mask.header_axes or mask.axes, **fingerprint.to_data(), @@ -62,11 +66,6 @@ def mask_manifest(inventory: MaskInventory, experiment_root: str | os.PathLike) return data -def _mask_path(mask: MaskFile, folder: Path | None, root: Path) -> Path: - """Return where a mask is on disk: in the masks folder, or inside its OME-Zarr image.""" - return (folder / mask.path) if mask.source == FOLDER else (root / mask.path) - - def shared_mask_text(records: Iterable[Mapping]) -> str | None: """Say why recorded masks shared by every plane of their image are refused, or None. diff --git a/src/morphobricks_preprocessing/masks/checks.py b/src/morphobricks_preprocessing/masks/checks.py index b80fbece..cbe231e8 100644 --- a/src/morphobricks_preprocessing/masks/checks.py +++ b/src/morphobricks_preprocessing/masks/checks.py @@ -13,6 +13,7 @@ import tifffile from ..axes import AxisMapping +from ..fingerprint import Fingerprint from ..metadata import TiffMetadata, named_axes from ..validation import ValidationIssue, agree, error, plural from ..zarr_input import ( @@ -34,6 +35,7 @@ def _header_issues( root: Path, unreadable: Iterable[str] = (), stated: Mapping[str, str] | None = None, + bind: bool = False, ) -> list[ValidationIssue]: """Read each mask's header (axes, shape, pixel type) and check it against its image. @@ -42,11 +44,21 @@ def _header_issues( updated in place with them. Also check that the masks of one image have the same depth slices and time points: a 2-D mask is never shared by the planes of its image (owner decision of 5 November 2026). ``unreadable`` are paths already known not to open - (OME-Zarr ``labels`` groups). + (OME-Zarr ``labels`` groups). With ``bind``, each mask is fingerprinted before and + after its header is read (:attr:`.model.MaskFile.bound`, issue #160); a mask whose two + fingerprints differ is ``mask_changed_during_check``. """ found: list[tuple[str, str, str]] = [("unreadable_mask", path, "") for path in unreadable] for index, mask in enumerate(files): + where = mask.location(folder, root) if bind else None + before = _fingerprint(where) checked, problems = _check_header(mask, images[mask.image], folder, root, stated or {}) + if bind and checked is not None: + after = _fingerprint(where) + if before is None or before != after: + problems.append(("mask_changed_during_check", mask.path, "")) + else: + checked = replace(checked, bound=before) found.extend(problems) if checked is not None: files[index] = checked @@ -60,6 +72,16 @@ def _header_issues( return _grouped(found) +def _fingerprint(path: Path | None) -> Fingerprint | None: + """Return a mask's fingerprint now; None without a path, or when it is gone.""" + if path is None: + return None + try: + return Fingerprint.of(path) + except OSError: + return None + + def _check_header( mask: MaskFile, image: TiffMetadata, @@ -159,6 +181,13 @@ def _grouped(found: Iterable[tuple[str, str, str]]) -> list[ValidationIssue]: "Make every mask of an image with the image's time points (T): one mask plane per " "time point.", ), + "mask_changed_during_check": ( + "Some masks changed while their headers were being read (their size or time, or " + "for a label image its files, differed before and after), so the check cannot tell " + "which version of each it describes.", + "Wait until nothing is writing to these masks (a segmentation or a copy still " + "running), then check or run again.", + ), "mask_size_mismatch": ( "Some masks do not have the size (Y and X) of their image.", "Check that each mask was made from its image, at full resolution.", diff --git a/src/morphobricks_preprocessing/masks/files.py b/src/morphobricks_preprocessing/masks/files.py index 5edba668..436b0cff 100644 --- a/src/morphobricks_preprocessing/masks/files.py +++ b/src/morphobricks_preprocessing/masks/files.py @@ -34,6 +34,7 @@ def find_masks( *, mask_axes: Mapping[str, str] | None = None, output_folder: str | os.PathLike | None = None, + bind: bool = False, ) -> MaskInventory: """Pair every image with its masks and check them from their headers, without pixels. @@ -59,6 +60,11 @@ def find_masks( ``output_inside_masks`` (the run's files would be read as masks). The folder rules (:mod:`..folder_rules`) also refuse a masks folder inside the experiment, holding it or holding named-axis copies. + bind + Fingerprint each mask before and after its header is read and keep the + fingerprint with it (:attr:`.model.MaskFile.bound`, issue #160), so a run or the + mask step refuses a mask that is no longer the one its header described; a mask + that changed while its header was read is the error ``mask_changed_during_check``. Returns ------- @@ -82,7 +88,7 @@ def find_masks( files.sort(key=lambda f: (f.image, f.mask_type)) issues.extend(_duplicate_issues(files)) stated = dict(mask_axes or {}) - issues.extend(_header_issues(files, images, folder, discovery.root, unreadable, stated)) + issues.extend(_header_issues(files, images, folder, discovery.root, unreadable, stated, bind)) types = tuple(sorted({f.mask_type for f in files})) issues.extend(_unknown_type_issues(stated, types)) issues.extend(_missing_issues(types, files, images)) diff --git a/src/morphobricks_preprocessing/masks/model.py b/src/morphobricks_preprocessing/masks/model.py index 7e795ef7..e8de98ff 100644 --- a/src/morphobricks_preprocessing/masks/model.py +++ b/src/morphobricks_preprocessing/masks/model.py @@ -9,11 +9,15 @@ from dataclasses import dataclass from pathlib import Path from types import MappingProxyType +from typing import TYPE_CHECKING import numpy as np from ..validation import ValidationIssue, has_errors +if TYPE_CHECKING: + from ..fingerprint import Fingerprint + FOLDER = "folder" """Source of a mask kept as a TIFF file in the masks folder.""" @@ -69,6 +73,12 @@ class MaskFile: the user stated the axes of its mask type (a plain stack, e.g. ``TYX``); empty when unknown. A 2-D mask is never shared by several planes of its image (owner decision of 5 November 2026): every mask has the image's time points. + bound + The mask's fingerprint taken before and after its header was read, when they agree + (``find_masks(bind=True)``, issue #160): the header describes the mask as it was + then, and a run or scan refuses the mask once it no longer has it. None when the + masks were found without binding (or the header could not be bound: the error + ``mask_changed_during_check``). """ image: str @@ -81,6 +91,25 @@ class MaskFile: header_axes: str = "" axes_from: str = "" named_axes: str = "" + bound: Fingerprint | None = None + + def location(self, masks_folder: Path | None, experiment_root: Path) -> Path: + """Return where the mask is on disk: in the masks folder, or inside its OME-Zarr image. + + Parameters + ---------- + masks_folder + The masks folder (:attr:`MaskInventory.folder`) for a TIFF mask. + experiment_root + The experiment folder, holding the label images of OME-Zarr images. + + Returns + ------- + Path + """ + if self.source == FOLDER: + return Path(masks_folder) / self.path + return Path(experiment_root) / self.path @property def planes(self) -> dict[str, int]: diff --git a/src/morphobricks_preprocessing/run/pipeline.py b/src/morphobricks_preprocessing/run/pipeline.py index 9d0f790c..e1040454 100644 --- a/src/morphobricks_preprocessing/run/pipeline.py +++ b/src/morphobricks_preprocessing/run/pipeline.py @@ -13,7 +13,7 @@ import os from collections import Counter from collections.abc import Callable, Iterable, Mapping -from dataclasses import dataclass, replace +from dataclasses import dataclass, field, replace from datetime import UTC, datetime from pathlib import Path from typing import Literal @@ -49,6 +49,12 @@ write_manifest, ) from ..manifest_versions import version_for +from ..mask_binding import ( + changed_masks, + content_changed_masks, + mask_changed_issue, + mask_contents, +) from ..mask_record import mask_manifest from ..mask_scan import MASK_REPORT_FILENAME from ..planes import DEFAULT_MEMORY_LIMIT @@ -224,6 +230,8 @@ class _Run: previous: tuple[RunManifest | None, str | None] found: frozenset[str] """The names in the output folder before the run wrote anything.""" + mask_content: Mapping[str, str | None] = field(default_factory=dict) + """In a ``verify_content`` run, each mask's content SHA-256 taken after the header checks.""" def _locked_run(given: GivenInputs, output: Path, options: _Options) -> RunResult: @@ -259,6 +267,10 @@ def _locked_run(given: GivenInputs, output: Path, options: _Options) -> RunResul ) if has_errors(checked.issues): return _stop(run, base, None) + if inputs.verify_content and pre.masks is not None: + run = _with_mask_content(run) + if has_errors(checked.issues): + return _stop(run, base, None) write_report(replace(_report(run, base), provisional=True), output) settings = RunSettings( checked.bins, @@ -284,6 +296,22 @@ def _locked_run(given: GivenInputs, output: Path, options: _Options) -> RunResul return _publish(run, base, rows, settings) +def _with_mask_content(run: _Run) -> _Run: + """Return the run with each mask's content SHA-256 (``verify_content``, issue #160). + + Taken once the header checks pass, of the masks as fingerprinted with their headers: a + mask that cannot be hashed, or no longer has that fingerprint, adds + ``mask_changed_during_run`` to the run's issues. + """ + masks, root = run.pre.masks, run.pre.discovery.root + run = replace(run, mask_content=mask_contents(masks, root)) + unsettled = {path for path, sha in run.mask_content.items() if sha is None} + unsettled.update(changed_masks(masks, root)) + if unsettled: + run.checked.issues.append(mask_changed_issue(sorted(unsettled), "fingerprint or content")) + return run + + def _report( run: _Run, outcome: RunOutcome, intensity: IntensityReport | None = None ) -> DiscoveryReport: @@ -326,6 +354,7 @@ def _publish(run: _Run, base: RunOutcome, rows: Rows, settings: RunSettings) -> Every image is fingerprinted again just before the commit (issue #90), and in a strict run hashed again as the last step before it (:func:`_final_content_check`): an image changed since the run checked it stops the run, and no scientific output is published. + Every mask likewise (issue #160): ``mask_changed_during_run``. """ output, pre, design = run.output, run.pre, run.checked.design records = sorted(rows.records, key=_row_order) @@ -334,17 +363,22 @@ def _publish(run: _Run, base: RunOutcome, rows: Rows, settings: RunSettings) -> masks = None if pre.masks is None else mask_manifest(pre.masks, pre.discovery.root) outcome = replace(outcome_of(rows, base), replaced=_replaced(run, masks)) removes = [name for name in (MASK_REPORT_FILENAME,) if name in outcome.replaced] - final_check = _final_content_check(rows, pre.discovery.root, settings) + final_check = _final_content_check(run, rows, settings) try: with staged_generation(output, removes, final_check) as staged: sha256 = _write_generation(run, rows, settings, records, (masks, intensity), staged) changed = changed_since(rows.entries, pre.discovery.root) if changed: raise _SourcesChangedError(changed) + # Every mask still the one whose header the run checked (issue #160). + changed = [] if pre.masks is None else changed_masks(pre.masks, pre.discovery.root) + if changed: + raise _SourcesChangedError(changed, masks=True) report = replace(_report(run, outcome, intensity), status=WRITTEN) write_report(replace(report, manifest_sha256=sha256), staged) except _SourcesChangedError as exc: - run.checked.issues.append(changed_issue(exc.paths, exc.compared)) + issue = (mask_changed_issue if exc.masks else changed_issue)(exc.paths, exc.compared) + run.checked.issues.append(issue) return _stop(run, base, rows) clear_staging(output) logger.info( @@ -370,37 +404,43 @@ def _publish(run: _Run, base: RunOutcome, rows: Rows, settings: RunSettings) -> ) -def _final_content_check( - rows: Rows, root: Path, settings: RunSettings -) -> Callable[[], None] | None: +def _final_content_check(run: _Run, rows: Rows, settings: RunSettings) -> Callable[[], None] | None: """Return a strict run's last check before its commit; None for a default run. A ``verify_content`` run hashes every recorded image once more (:func:`content_changed`, the OME-Zarr store digest for a store) after its generation is staged, right before the manifest is replaced (:func:`.generation.staged_generation`): an image rewritten since its last check, even with its size and time set back, stops the run with - ``source_changed_during_run``, and the earlier generation is intact. It reads every - analysed image whole once more. + ``source_changed_during_run``, and the earlier generation is intact. Every mask is hashed + again too (``mask_changed_during_run``, issue #160). It reads every analysed image and + mask whole once more. """ if not settings.verify_content: return None + root, masks = run.pre.discovery.root, run.pre.masks def check() -> None: - """Raise when an image's content is not the SHA-256 its entry records.""" + """Raise when an image's or mask's content is not the SHA-256 taken earlier.""" changed = content_changed(rows.entries.values(), root) if changed: raise _SourcesChangedError(changed, "content") + changed = [] if masks is None else content_changed_masks(masks, root, run.mask_content) + if changed: + raise _SourcesChangedError(changed, "content", masks=True) return check class _SourcesChangedError(Exception): - """Raised inside the publication when images changed since the run checked them.""" + """Raised inside the publication when images or masks changed since the run checked them.""" - def __init__(self, paths: list[str], compared: str = "fingerprint") -> None: + def __init__( + self, paths: list[str], compared: str = "fingerprint", *, masks: bool = False + ) -> None: super().__init__(", ".join(paths)) self.paths = paths - self.compared = compared # a key of resume.CHANGE_TEXTS + self.compared = compared # a key of resume.CHANGE_TEXTS (mask_binding's for masks) + self.masks = masks def _write_generation( diff --git a/src/morphobricks_preprocessing/run/preflight.py b/src/morphobricks_preprocessing/run/preflight.py index 105a08ba..c1a41315 100644 --- a/src/morphobricks_preprocessing/run/preflight.py +++ b/src/morphobricks_preprocessing/run/preflight.py @@ -196,6 +196,7 @@ def preflight( Fingerprint each image before and after its header is read (:attr:`Preflight.header_fingerprints`), so a caller that writes from the headers (:func:`.conversion_plan.plan_conversion`) never mixes a header with a later file. + Each mask is bound the same way (:attr:`.masks.MaskFile.bound`, issue #160). Returns ------- @@ -216,7 +217,7 @@ def preflight( exclusion_issues=tuple(_exclusion_issues(discovery, excluded)), suggested_mapping=prefill_experiment_mapping(metadata), dataset=check_dataset(discovery, excluded=excluded), - masks=_masks(discovery, metadata, masks_folder, mask_axes, output), + masks=_masks(discovery, metadata, masks_folder, mask_axes, output, bind_headers), calibration=calibration, conversion=review_conversion(root, discovery.images), header_fingerprints=bound, @@ -274,10 +275,16 @@ def _masks( masks_folder: str | os.PathLike | None, mask_axes: Mapping[str, str] | None, output: Path, + bind: bool = False, ) -> MaskInventory | None: """Pair the analysed images with their masks; None without masks folder or label image.""" masks = find_masks( - masks_folder, discovery, metadata.files, mask_axes=mask_axes, output_folder=output + masks_folder, + discovery, + metadata.files, + mask_axes=mask_axes, + output_folder=output, + bind=bind, ) if masks_folder is None and not masks.files and not masks.issues: return None # no masks folder and no label images: the run has no masks diff --git a/tests/test_mask_run_binding.py b/tests/test_mask_run_binding.py new file mode 100644 index 00000000..828572a8 --- /dev/null +++ b/tests/test_mask_run_binding.py @@ -0,0 +1,189 @@ +"""Issue #160: every mask is bound to its fingerprint for the whole run, like the images. + +A run checks the masks from their headers and records each mask's fingerprint in the +manifest, but before #160 it took that fingerprint only when it wrote the manifest: a mask +rewritten while the run checked headers or read images was recorded as it was then, under +headers checked on another version. Now each mask is fingerprinted before and after its +header is read (``mask_changed_during_check`` when they differ), that fingerprint is the +one recorded, and every mask must still have it right before the commit +(``mask_changed_during_run``); a ``verify_content`` run also compares each mask's content +SHA-256 taken after the header checks with one taken right before the commit. +""" + +import json +import os + +import numpy as np +import pytest +import tifffile + +from morphobricks_preprocessing import RunMode, prefill_mapping, run_experiment +from morphobricks_preprocessing import run as run_module +from morphobricks_preprocessing.fingerprint import Fingerprint +from morphobricks_preprocessing.masks import checks as mask_checks +from morphobricks_preprocessing.run.generation import PUBLISH_FOLDER +from morphobricks_preprocessing.testing import make_clean_run + +STRICT = RunMode("restart", verify_content=True) +YX = prefill_mapping("YX") +MASK = "Cells/ctrl/rep1/a.tif" +OUTPUTS = ("run_manifest.json", "plane_histograms.csv") + + +@pytest.fixture +def clean(tmp_path): + return make_clean_run(tmp_path, axes="YX", shape=(8, 8), mask_types=("Cells", "Nuclei")) + + +def outputs(out): + return {name: (out / name).read_bytes() for name in OUTPUTS} + + +def rewrite_mask(path, *, keep_metadata=False): + """Write other labels into a 2-D mask (same shape and type), a new time unless kept.""" + before = path.stat() + labels = np.zeros(tifffile.imread(path).shape, np.uint16) + labels[2:5, 2:5] = 3 + tifffile.imwrite(path, labels, photometric="minisblack", metadata={"axes": "YX"}) + if keep_metadata: + os.utime(path, ns=(before.st_atime_ns, before.st_mtime_ns)) + assert path.stat().st_size == before.st_size + else: + os.utime(path, ns=(before.st_atime_ns, before.st_mtime_ns + 10**9)) + + +def mask_entry(out, path=MASK): + data = json.loads((out / "run_manifest.json").read_text(encoding="utf-8")) + (entry,) = [m for m in data["masks"]["files"] if m["path"] == path] + return entry + + +def test_a_mask_changed_while_the_images_are_read_is_refused(clean, monkeypatch): + out, mask = clean.output, clean.masks / MASK + before = outputs(out) + (clean.experiment / "ctrl/rep1/b.ome.tif").touch() # one image to read again + os.utime(clean.experiment / "ctrl/rep1/b.ome.tif", ns=(1, 10**9)) + read = run_module.resume._read_file + + def segmentation_still_writing(path, rel, *args): + rows = read(path, rel, *args) + rewrite_mask(mask) + return rows + + monkeypatch.setattr(run_module.resume, "_read_file", segmentation_still_writing) + result = run_experiment(clean.experiment, output_folder=out, masks_folder=clean.masks) + assert not result.written and result.report_written + (issue,) = [i for i in result.issues if i.is_error] + assert issue.code == "mask_changed_during_run" and issue.paths == (MASK,) + assert "size or modification time" in issue.message and "Nothing was published" in ( + issue.message + ) + assert outputs(out) == before and not (out / PUBLISH_FOLDER).exists() + monkeypatch.undo() + again = run_experiment(clean.experiment, output_folder=out, masks_folder=clean.masks) + assert again.written + assert Fingerprint.recorded(mask_entry(out)) == Fingerprint.of(mask) + + +def test_a_mask_changed_after_its_header_and_before_the_commit_is_refused(clean, monkeypatch): + # No image is read: the masks are checked again right before the commit. + out, mask = clean.output, clean.masks / MASK + changed_since = run_module.pipeline.changed_since + + def meanwhile(entries, root): + rewrite_mask(mask) + return changed_since(entries, root) + + monkeypatch.setattr(run_module.pipeline, "changed_since", meanwhile) + result = run_experiment(clean.experiment, output_folder=out, masks_folder=clean.masks) + assert not result.written + assert [(i.code, i.paths) for i in result.issues if i.is_error] == [ + ("mask_changed_during_run", (MASK,)) + ] + + +def test_a_mask_changed_while_its_header_is_read_is_refused(clean, monkeypatch): + check = mask_checks._check_header + + def rewritten_during(mask, *args): + found = check(mask, *args) + if mask.path == MASK: + rewrite_mask(clean.masks / MASK) + return found + + monkeypatch.setattr(mask_checks, "_check_header", rewritten_during) + result = run_experiment(clean.experiment, output_folder=clean.output, masks_folder=clean.masks) + assert not result.written + (issue,) = [i for i in result.issues if i.is_error] + assert issue.code == "mask_changed_during_check" and issue.paths == (MASK,) + + +def test_an_unchanged_rerun_keeps_every_output_byte_for_byte(clean): + out = clean.output + before = outputs(out) + entry = mask_entry(out) + assert Fingerprint.recorded(entry) == Fingerprint.of(clean.masks / MASK) + result = run_experiment(clean.experiment, output_folder=out, masks_folder=clean.masks) + assert result.written and result.read == () + assert outputs(out) == before and mask_entry(out) == entry + + +def test_a_strict_run_refuses_a_mask_rewritten_with_its_size_and_time_kept(clean, monkeypatch): + out, mask = clean.output, clean.masks / MASK + assert run_experiment( + clean.experiment, output_folder=out, masks_folder=clean.masks, mode=STRICT + ).written + before = outputs(out) + hashed = run_module.pipeline.content_changed + + def rewritten_before_the_commit(entries, root): + rewrite_mask(mask, keep_metadata=True) + return hashed(entries, root) + + monkeypatch.setattr(run_module.pipeline, "content_changed", rewritten_before_the_commit) + result = run_experiment(clean.experiment, output_folder=out) + assert not result.written + (issue,) = [i for i in result.issues if i.is_error] + assert issue.code == "mask_changed_during_run" and issue.paths == (MASK,) + assert "content (SHA-256)" in issue.message + assert outputs(out) == before and not (out / PUBLISH_FOLDER).exists() + + +def test_a_default_run_does_not_see_a_rewrite_with_size_and_time_kept(clean, monkeypatch): + # The stated blind spot of the metadata identity: content hashing stays opt-in (#159). + mask = clean.masks / MASK + changed_since = run_module.pipeline.changed_since + + def meanwhile(entries, root): + rewrite_mask(mask, keep_metadata=True) + return changed_since(entries, root) + + monkeypatch.setattr(run_module.pipeline, "changed_since", meanwhile) + result = run_experiment(clean.experiment, output_folder=clean.output, masks_folder=clean.masks) + assert result.written + + +def test_a_label_image_changed_during_the_run_is_refused(tmp_path, monkeypatch): + pytest.importorskip("zarr") + from zarr_tree import write_labels, write_ngff + + image = write_ngff(tmp_path / "exp/ctrl/a.ome.zarr", np.zeros((6, 8), np.uint8), "yx") + cells = np.zeros((6, 8), np.uint16) + cells[1:3, 1:3] = 1 + write_labels(image, {"cells": cells}, "yx") + changed_since = run_module.pipeline.changed_since + + def chunk_rewritten(entries, root): + label = image / "labels" / "cells" / "0" + chunk = sorted(p for p in label.rglob("*") if p.is_file() and p.name[0] != ".")[0] + stat = chunk.stat() + chunk.write_bytes(chunk.read_bytes()) + os.utime(chunk, ns=(stat.st_atime_ns, stat.st_mtime_ns + 10**6)) + return changed_since(entries, root) + + monkeypatch.setattr(run_module.pipeline, "changed_since", chunk_rewritten) + result = run_experiment(tmp_path / "exp", YX, output_folder=tmp_path / "out") + assert not result.written + assert [(i.code, i.paths) for i in result.issues if i.is_error] == [ + ("mask_changed_during_run", ("ctrl/a.ome.zarr/labels/cells",)) + ] From 6a0d62bf5a6ce37a7dde826dc5353cc02f38145a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 16:32:23 +0000 Subject: [PATCH 2/3] Refuse mask relationship results if masks change while scanned (#165) The mask step now fingerprints each mask with its header (prepare_mask_step binds the headers) and each image's masks before their first plane is read and after their last. A mask that no longer has the fingerprint taken with its header, went missing, or changed during its scan raises MaskChangedError (a ValueError; the error mask_changed_during_scan, which the masks command prints): no report is written and an earlier one is left as it was. With verify_content (prepare_mask_step(verify_content=...), masks --verify-content; by default the recorded run's setting) the content SHA-256 is compared too. The report format is unchanged and unchanged masks give the same report. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VbytvF3Gsa85ix8Ne2yMDW --- AGENTS.md | 6 +- CHANGELOG.md | 9 + docs/contract.md | 13 ++ docs/user_guide.md | 10 +- src/morphobricks_preprocessing/cli_masks.py | 15 +- .../mask_binding.py | 138 ++++++++++++- src/morphobricks_preprocessing/mask_scan.py | 18 ++ .../run/mask_step.py | 32 ++- tests/test_mask_scan_binding.py | 192 ++++++++++++++++++ 9 files changed, 423 insertions(+), 10 deletions(-) create mode 100644 tests/test_mask_scan_binding.py diff --git a/AGENTS.md b/AGENTS.md index 99238f6a..75d2426a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -131,7 +131,8 @@ src/morphobricks_preprocessing/ fingerprint is the one bound to the mask's header (issue #160) mask_binding.py masks bound to their fingerprints (issues #160, #165): changed_masks, mask_contents/content_changed_masks (verify_content), - mask_changed_issue (mask_changed_during_run) + mask_changed_issue (mask_changed_during_run, mask_changed_during_scan), + scan_fingerprints/check_scanned and MaskChangedError (scan_masks) axis_check.py the axis check (contract §2): header_axis_issue, axis_disagreements, likely_axis_cause, check_header_axes, ConfirmedAxes (a confirmation covers only its files as fingerprinted, plain_stack_not_confirmed); AXIS_ERROR_CODES, AXIS_CHECK_CODES @@ -185,7 +186,8 @@ src/morphobricks_preprocessing/ mask_axes_questions (the types whose axes must be stated, with the statements mask_roles accepts) mask_scan.py the optional mask step (Q32): piece_counts (scipy.ndimage), pair_counts, - scan_masks -> MaskReport (kinds, relations, direct-parent hierarchy, evidence), + scan_masks -> MaskReport (kinds, relations, direct-parent hierarchy, evidence; + each image's masks fingerprinted before and after they are read, #165), write_mask_report (mask_relationships.json); DEFAULT_THRESHOLDS, mask_thresholds (checked changes) mask_pairs.py pair_counts' pure helpers: best_parents (a tie between parents gives no diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f1d0c0c..203da368 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,15 @@ are in [docs/plan.md](docs/plan.md) §12. Upgrading from v0.3: see the recorded is the same, so an unchanged rerun keeps the manifest byte for byte. The masks' content SHA-256 is compared in memory and not recorded. Earlier manifests read as before. +- **No mask relationship report from masks that change while they are scanned** (#165). The + mask step fingerprints each mask with its header (`prepare_mask_step`) and each image's + masks before and after their planes are read; a mask that changed or went missing raises + `MaskChangedError` (a `ValueError`; the new error `mask_changed_during_scan`, which the + `masks` command prints), no report is written and an earlier one is left as it was. With + `verify_content` (`prepare_mask_step(verify_content=...)`, new `masks --verify-content`; + by default the recorded run's setting) the masks' content SHA-256 is compared too. + *Records:* `mask_relationships.json` is unchanged (report version 3) and unchanged masks + give the same report. ## v0.4.0 (5 November 2026) diff --git a/docs/contract.md b/docs/contract.md index a1964a8d..596f4f68 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -492,6 +492,19 @@ the user names the masks and confirms or corrects the relationships. The step ho output folder's lock (§9.2) from before the first mask is read to the write of the report; another run holding it refuses the step (`output_locked`) before any mask is read. +The masks are bound to their fingerprints (issue #165, as a run's sources, §9.4): the step +fingerprints each mask before and after its header is read (`prepare_mask_step`; +`mask_changed_during_check` when they differ), and the scan fingerprints each image's masks +before their first plane is read and again after their last. A mask that no longer has the +fingerprint taken with its header, that went missing, or whose fingerprint changed during +its scan raises `MaskChangedError` (a `ValueError`; the error `mask_changed_during_scan`, +naming the masks and what changed): no report is written, nothing half-written is left (the +report is written whole, by one rename, only after every image is scanned), and an earlier +`mask_relationships.json` is left as it was. With `verify_content` (`prepare_mask_step(..., +verify_content=True)`, CLI `masks --verify-content`; by default the recorded run's setting) +each mask's content SHA-256 is also taken before and after its scan and must not change. +Unchanged masks give the same report as before. + | Kind of mask | Rule | |---|---| | `empty` | No foreground in any image. | diff --git a/docs/user_guide.md b/docs/user_guide.md index 43db124f..03af6a6f 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -655,7 +655,7 @@ SHA-256 still accept it; the report records the rerun. Other options: | `--image-type fluorescence\|rgb` | The image type (default: the earlier run's, else the suggested one, printed) | | `--design FILE` | The confirmed experiment design (default: `experiment_design.csv` in the output folder); it is copied to the output folder | | `--mode restart\|full` | `restart` (default) reuses the rows of unchanged images; `full` recomputes every row and check. Both keep your confirmations and replace the results only when the run succeeds | -| `--verify-content`, `--no-verify-content` | Reuse rows (earlier or staged) only when each image's content SHA-256 is the one recorded with them, not only its size and modification time; every image is read once more. Default: the earlier run's choice, else off. Turning it on or off recomputes every row | +| `--verify-content`, `--no-verify-content` | Reuse rows (earlier or staged) only when each image's content SHA-256 is the one recorded with them, not only its size and modification time; every image is read once more. Default: the earlier run's choice, else off. Turning it on or off recomputes every row. With `masks`: also compare each mask's content SHA-256 before and after it is read (same default) | | `--no-masks` | Drop the masks folder an earlier run recorded, with the mask axes stated for its masks (those of OME-Zarr label images are kept) | | `--no-exclude` | Clear the exclusions an earlier run recorded, so every image is analysed again (not together with `--exclude`) | | `--remove-stale-lock` | Remove the lock left by a run that was stopped or whose computer restarted (`check`, `run`, `masks`, `design` and `calibrate`); give it only when no other run uses the output folder | @@ -727,7 +727,12 @@ objects have positive labels: a mask with a negative value stops the command wit naming the mask, and no report is written. The output folder's lock is held from before the first mask is read until the report is written: when another run holds it, the command stops with `output_locked` before reading -any mask. +any mask. A mask that changes after the command read its header or while its pixels are read +(a segmentation still writing it) stops the command with `mask_changed_during_scan`, naming +the mask: no report is written, and an earlier `mask_relationships.json` is left as it was. +`--verify-content` also compares each mask's content (SHA-256) before and after it is read +(each mask is read whole twice more); given neither it nor `--no-verify-content`, the run's +own setting is used. The rules behind each suggestion use thresholds you can change, for example `--inside-object-share 0.8` to call a kind of mask "inside" another when 80% of its objects @@ -1076,6 +1081,7 @@ continues. | `source_changed_during_run` | Error | An image changed or went missing while the run was checking or reading it, so its rows would not describe it. No scientific outputs are published and sources are untouched; the discovery report is written. Wait until nothing writes to the experiment (an acquisition or a copy still running), then run again: the images already read resume from their staged rows. | | `mask_changed_during_check` | Error | A mask changed while its header was read, so the check cannot tell which version it describes. Wait until nothing writes to the masks (a segmentation or a copy still running), then check or run again. | | `mask_changed_during_run` | Error | A mask changed or went missing after the run checked its header and before it published its results (its size or time, or for an OME-Zarr label image its files; with `--verify-content` also its content). Nothing is published; the discovery report is written. Wait until nothing writes to the masks, then run again: the images already read resume from their staged rows. | +| `mask_changed_during_scan` | Error | The `masks` command (or **Describe the masks**): a mask changed or went missing after its header was read or while its pixels were read. No report is written; an earlier `mask_relationships.json` is left as it was. Wait until nothing writes to the masks, then describe them again. | | `design_unreadable` | Error | The `experiment_design.csv` an earlier run wrote cannot be read. Correct it, or delete it to confirm the design again. | | `no_usable_planes` | Error | No plane has finite pixel values. Check the images. | | `tiff_outside_leaf` | Error | Images sit in a folder that also holds subfolders, so they have no condition or repeat of their own. A folder holds either images or subfolders, never both. Move the images into a folder of their own, or out of the experiment folder. | diff --git a/src/morphobricks_preprocessing/cli_masks.py b/src/morphobricks_preprocessing/cli_masks.py index 9e5dddfe..7d97b1b4 100644 --- a/src/morphobricks_preprocessing/cli_masks.py +++ b/src/morphobricks_preprocessing/cli_masks.py @@ -8,13 +8,16 @@ axes and the exclusions; a value given that differs is used and named. The pairing, the scan and the write are :mod:`.run.mask_step`'s, shared with the notebook: the output folder's lock is held from before the first mask is read to the write, so a locked folder -is refused before any mask is read. +is refused before any mask is read. A mask that changes after its header is read or while it +is read refuses the step (``mask_changed_during_scan``, issue #165); ``--verify-content`` +also compares each mask's content SHA-256. """ from __future__ import annotations import argparse +from .mask_binding import MaskChangedError from .mask_scan import DEFAULT_THRESHOLDS, mask_thresholds from .mask_text import mask_report_summary, mask_summary from .report import format_issues @@ -46,6 +49,12 @@ def add_masks_arguments(masks: argparse.ArgumentParser) -> None: help="clear the exclusions of the earlier run for this scan (not with --exclude)", ) masks.add_argument("--log", metavar="FILE", help="write technical details to this file") + masks.add_argument( + "--verify-content", + action=argparse.BooleanOptionalAction, + help="also compare each mask's content SHA-256 before and after it is read (reads " + "each mask whole twice more; default: as in the earlier run, else no)", + ) limits = masks.add_argument_group( "relationship thresholds", "change the defaults (contract §15.1); shares are 0 to 1" ) @@ -73,6 +82,7 @@ def masks_command(args: argparse.Namespace) -> int: exclude=() if args.no_exclude else (tuple(args.exclude) or None), masks_folder=args.masks, mask_axes=args.mask_axes or None, + verify_content=args.verify_content, ) if step.refusal is not None: print(format_issues([step.refusal])) @@ -96,6 +106,9 @@ def progress(done: int, total: int, path: str) -> None: print() try: result = describe_masks(step, progress=progress, thresholds=thresholds) + except MaskChangedError as exc: # changed since its header was read, or while read + print(format_issues([exc.issue])) + return 1 except ValueError as exc: # masks whose pixels cannot be used (planes, labels) print(f"The masks cannot be used: {exc}") return 1 diff --git a/src/morphobricks_preprocessing/mask_binding.py b/src/morphobricks_preprocessing/mask_binding.py index 87ea2cce..17267dca 100644 --- a/src/morphobricks_preprocessing/mask_binding.py +++ b/src/morphobricks_preprocessing/mask_binding.py @@ -1,4 +1,4 @@ -"""Masks bound to their fingerprints for a whole run. +"""Masks bound to their fingerprints for a whole run and for the mask step's scan. Images are bound to their fingerprints from the moment their headers are read (issue #90, :mod:`.run.resume`); masks are bound the same way (issue #160). In a run @@ -10,6 +10,12 @@ (:func:`content_changed_masks`); otherwise the run stops with ``mask_changed_during_run`` and nothing is published. The manifest records the fingerprint bound to the header. +The mask step (:func:`.mask_scan.scan_masks`, issue #165) binds them too: each image's masks +are fingerprinted (and, with ``verify_content``, hashed) before their pixels are read and +again after; a mask whose fingerprint is not the one its header was bound to, or that changes +during its scan, raises :class:`MaskChangedError` (``mask_changed_during_scan``), and no +report is written. + The identity is the metadata fingerprint (size, modification time, and for an OME-Zarr label image its inventory digest); the content SHA-256 is opt-in (``verify_content``). """ @@ -26,6 +32,8 @@ MASK_CHANGED_DURING_RUN = "mask_changed_during_run" """The code of a run refused because a mask changed after its header was read.""" +MASK_CHANGED_DURING_SCAN = "mask_changed_during_scan" +"""The code of a mask step refused because a mask changed before or while it was read.""" CHANGE_TEXTS = { "fingerprint": ( @@ -128,8 +136,10 @@ def content_changed_masks( ) -def mask_changed_issue(paths: Iterable[str], compared: str = "fingerprint") -> ValidationIssue: - """Return the error naming the masks that changed while a run used them. +def mask_changed_issue( + paths: Iterable[str], compared: str = "fingerprint", *, scan: bool = False +) -> ValidationIssue: + """Return the error naming the masks that changed while a run or the mask step used them. Parameters ---------- @@ -137,6 +147,9 @@ def mask_changed_issue(paths: Iterable[str], compared: str = "fingerprint") -> V The masks, sorted. compared What was compared, a key of :data:`CHANGE_TEXTS`. + scan + The mask step's scan (``mask_changed_during_scan``) rather than a run + (``mask_changed_during_run``). Returns ------- @@ -144,6 +157,17 @@ def mask_changed_issue(paths: Iterable[str], compared: str = "fingerprint") -> V """ paths = sorted(paths) count = plural(len(paths), "mask") + if scan: + return error( + MASK_CHANGED_DURING_SCAN, + f"{count} changed or went missing before or while the mask step read them " + f"({CHANGE_TEXTS[compared]}), so a relationship report could mix versions of " + "them. No report was written; an earlier mask_relationships.json is left as it " + "was and describes the masks as they were when it was written.", + paths, + "Wait until nothing is writing to these masks (a segmentation or a copy still " + "running), then describe the masks again.", + ) return error( MASK_CHANGED_DURING_RUN, f"{count} changed or went missing while the run was using them " @@ -153,3 +177,111 @@ def mask_changed_issue(paths: Iterable[str], compared: str = "fingerprint") -> V "Wait until nothing is writing to these masks (a segmentation or a copy still " "running), then run again: the images already read resume from their staged rows.", ) + + +def scan_fingerprints( + masks: Iterable[MaskFile], folder: Path | None, root: Path, verify_content: bool +) -> dict[str, tuple]: + """Return each mask's fingerprint (and content) before the scan reads it (issue #165). + + A mask found with ``bind=True`` must still have the fingerprint taken with its header: + its header (planes, axes) describes it only as it was then. + + Parameters + ---------- + masks + The masks of one image. + folder, root + The masks folder and the experiment folder. + verify_content + Also take each mask's content SHA-256. + + Returns + ------- + dict + ``{mask path: (fingerprint, SHA-256 or None)}``, for :func:`check_scanned`. + + Raises + ------ + MaskChangedError + For masks gone, unreadable, or no longer with the fingerprint bound to their header. + """ + masks = list(masks) + found = { + mask.path: ( + mask_fingerprint(mask, folder, root), + current_content(mask.location(folder, root)) if verify_content else None, + ) + for mask in masks + } + changed = [ + mask.path + for mask in masks + if found[mask.path][0] is None + or (mask.bound is not None and found[mask.path][0] != mask.bound) + or (verify_content and found[mask.path][1] is None) + ] + if changed: + raise MaskChangedError( + changed, "fingerprint or content" if verify_content else "fingerprint" + ) + return found + + +def check_scanned( + masks: Iterable[MaskFile], + folder: Path | None, + root: Path, + before: Mapping[str, tuple], + hashed: bool, +) -> None: + """Raise :class:`MaskChangedError` for the masks that changed while the scan read them. + + Parameters + ---------- + masks + The masks of one image, as given to :func:`scan_fingerprints`. + folder, root + The masks folder and the experiment folder. + before + :func:`scan_fingerprints`'s answer. + hashed + Compare the content SHA-256 too (``verify_content``). + + Raises + ------ + MaskChangedError + For the masks whose fingerprint (or content) is not the one taken before. + """ + changed = [ + mask.path + for mask in masks + if mask_fingerprint(mask, folder, root) != before[mask.path][0] + or (hashed and current_content(mask.location(folder, root)) != before[mask.path][1]) + ] + if changed: + raise MaskChangedError(changed, "fingerprint or content" if hashed else "fingerprint") + + +class MaskChangedError(ValueError): + """Raised when masks changed before or while the mask step read them (issue #165). + + A ``ValueError``, as every other mask that cannot be used in the scan. + + Attributes + ---------- + paths + The masks, sorted. + compared + What was compared, a key of :data:`CHANGE_TEXTS`. + issue + The error ``mask_changed_during_scan``. + """ + + def __init__(self, paths: Iterable[str], compared: str = "fingerprint") -> None: + self.paths = sorted(paths) + self.compared = compared + self.issue = mask_changed_issue(self.paths, compared, scan=True) + super().__init__( + f"{self.issue.message} ({MASK_CHANGED_DURING_SCAN}: {', '.join(self.paths)})" + ) diff --git a/src/morphobricks_preprocessing/mask_scan.py b/src/morphobricks_preprocessing/mask_scan.py index b59c0ed0..6108b786 100644 --- a/src/morphobricks_preprocessing/mask_scan.py +++ b/src/morphobricks_preprocessing/mask_scan.py @@ -59,6 +59,8 @@ from ._io import write_text_atomic from .manifest import package_versions +from .mask_binding import MaskChangedError as MaskChangedError # raised by the scan +from .mask_binding import check_scanned, scan_fingerprints from .mask_pairs import best_parents, match_counts from .mask_pairs import one_to_one_matches as one_to_one_matches # moved there from .masks import MaskInventory, check_labels, read_mask @@ -399,9 +401,18 @@ def scan_masks( *, progress: Callable[[int, int, str], None] | None = None, thresholds: Mapping[str, float] | None = None, + verify_content: bool = False, ) -> MaskReport: """Read every image's masks and describe the mask types and their relationships. + Each image's masks are bound to their fingerprints while they are read (issue #165): + every mask is fingerprinted before its first plane is read and again after its last, + and must have the same fingerprint both times, the one taken with its header when it + was found with ``bind=True`` (:attr:`.masks.MaskFile.bound`). With ``verify_content`` + its content SHA-256 is also taken before and after and must not change. A mask that + changed raises :class:`.mask_binding.MaskChangedError` (``mask_changed_during_scan``): + no report is returned, so none is written from masks of mixed versions. + Parameters ---------- inventory @@ -412,6 +423,8 @@ def scan_masks( Called as ``progress(done, total, image)`` before each image is read. thresholds Thresholds to change from their defaults, by name (:func:`mask_thresholds`). + verify_content + Also compare each mask's content SHA-256 before and after it is read. Returns ------- @@ -419,6 +432,9 @@ def scan_masks( Raises ------ + MaskChangedError + A ``ValueError``: for masks that changed or went missing before or while their image's + planes were read. ValueError For an invalid threshold, before any mask is read; for an image whose masks have different depth slices or time points; for a mask @@ -443,6 +459,7 @@ def scan_masks( "(mask_planes_differ)" ) counts_of = dict(next(iter(planes))) + before = scan_fingerprints(found_masks.values(), inventory.folder, root, verify_content) for name in found_masks: seen[name] += 1 # A mask with depth slices or time points is scanned one plane at a time, as if @@ -460,6 +477,7 @@ def scan_masks( for name, mask in found_masks.items() } _scan_plane(masks, pieces, single, totals, limits) + check_scanned(found_masks.values(), inventory.folder, root, before, verify_content) if progress is not None: progress(len(images), len(images), "") types = { diff --git a/src/morphobricks_preprocessing/run/mask_step.py b/src/morphobricks_preprocessing/run/mask_step.py index 3f8d460b..9875e051 100644 --- a/src/morphobricks_preprocessing/run/mask_step.py +++ b/src/morphobricks_preprocessing/run/mask_step.py @@ -14,6 +14,12 @@ report, holding the output folder's lock from the first mask read to the write: a run may be replacing or removing the same file. A locked folder is refused before any mask is read. + +Every mask is bound to its fingerprint from its header read to the end of its scan (issue +#165, :mod:`..mask_binding`): :func:`prepare_mask_step` fingerprints each mask before and +after its header is read, and the scan refuses a mask that no longer has that fingerprint +or changes while it is read (``mask_changed_during_scan``, also its content SHA-256 when +``verify_content`` is on: given, else the run's recorded setting). Nothing is written then. """ from __future__ import annotations @@ -54,6 +60,9 @@ class MaskStep: exclusions' and the masks'. refusal ``experiment_not_recorded`` when no experiment folder was given or recorded. + verify_content + Compare each mask's content SHA-256 before and after it is read, not only its + fingerprint (given, else the run's recorded setting). """ preflight: Preflight | None @@ -61,6 +70,7 @@ class MaskStep: changes: tuple[InputChange, ...] = () issues: tuple[ValidationIssue, ...] = () refusal: ValidationIssue | None = None + verify_content: bool = False @property def masks(self) -> MaskInventory: @@ -127,9 +137,14 @@ def prepare_mask_step( exclude: Sequence[str] | None = None, masks_folder: str | os.PathLike | None = None, mask_axes: Mapping[str, str] | None = None, + verify_content: bool | None = None, ) -> MaskStep: """Pair the masks the mask step reads, with the inputs the earlier run recorded. + Each mask is fingerprinted before and after its header is read + (:attr:`.masks.MaskFile.bound`); :func:`describe_masks` reads only masks that still + have that fingerprint. + Parameters ---------- experiment_root @@ -142,6 +157,9 @@ def prepare_mask_step( The masks folder; None takes the recorded one. mask_axes Mask axes stated per mask type; they add to (and replace per type) the recorded ones. + verify_content + Also compare each mask's content SHA-256 while it is read; None takes the run's + recorded setting (False without one). Returns ------- @@ -162,6 +180,7 @@ def prepare_mask_step( exclude=None if exclude is None else tuple(exclude), masks_folder=Path(masks_folder).resolve() if masks_folder else None, mask_axes=mask_axes or None, + verify_content=verify_content, ) resolved = resolve_inputs(given, read_previous(output.resolve())[0]) changes = tuple( @@ -173,6 +192,7 @@ def prepare_mask_step( exclude=list(resolved.exclude), masks_folder=resolved.masks_folder, mask_axes=resolved.mask_axes, + bind_headers=True, ) pre = replace(pre, masks=pre.masks or MaskInventory(folder=None)) # none found return MaskStep( @@ -180,6 +200,7 @@ def prepare_mask_step( output_folder=output, changes=changes, issues=(*pre.discovery.issues, *pre.exclusion_issues, *pre.masks.issues), + verify_content=resolved.verify_content, ) @@ -211,7 +232,10 @@ def describe_masks( ------ ValueError For a step that is not ready, and for masks whose pixels cannot be used (planes, - labels; :func:`.mask_scan.scan_masks`). + labels; :func:`.mask_scan.scan_masks`); :class:`..mask_binding.MaskChangedError` + (``mask_changed_during_scan``) for masks that changed since their headers were read + or while they were read. No report is written then, and an earlier one is left as + it was. """ if not step.ready or step.preflight is None or step.output_folder is None: raise ValueError("the masks cannot be described: the step has errors or no masks") @@ -222,7 +246,11 @@ def describe_masks( # recorded inputs or removing the report; a locked folder is refused before any read. with output_lock(output): report = scan_masks( - step.masks, step.preflight.discovery.root, progress=progress, thresholds=thresholds + step.masks, + step.preflight.discovery.root, + progress=progress, + thresholds=thresholds, + verify_content=step.verify_content, ) path = write_mask_report(report, output / MASK_REPORT_FILENAME) except OutputLockedError as exc: diff --git a/tests/test_mask_scan_binding.py b/tests/test_mask_scan_binding.py new file mode 100644 index 00000000..8edd1787 --- /dev/null +++ b/tests/test_mask_scan_binding.py @@ -0,0 +1,192 @@ +"""Issue #165: no mask relationship report from masks that change while they are scanned. + +The mask step reads every image's masks and writes ``mask_relationships.json``. Before +#165 nothing checked the masks around the scan: a segmentation still writing a mask could +be analysed mid-write, and planes of one mask could come from two versions of the file. +Now each mask is fingerprinted with its header (:func:`prepare_mask_step`), before its +image's planes are read and after; with ``verify_content`` also its content SHA-256. A +change raises ``mask_changed_during_scan``: no report is written, an earlier one is left as +it was, and nothing half-written is left in the output folder. +""" + +import itertools +import os + +import numpy as np +import pytest +import tifffile + +from morphobricks_preprocessing import mask_scan, prefill_mapping, run_experiment +from morphobricks_preprocessing.cli import main +from morphobricks_preprocessing.mask_binding import MaskChangedError +from morphobricks_preprocessing.mask_scan import MASK_REPORT_FILENAME +from morphobricks_preprocessing.run import describe_masks, prepare_mask_step +from morphobricks_preprocessing.testing import make_clean_run + +MASK = "Cells/ctrl/rep1/b.tif" +LABELS = itertools.count(3) + + +@pytest.fixture +def clean(tmp_path): + return make_clean_run(tmp_path, axes="YX", shape=(8, 8), mask_types=("Cells", "Nuclei")) + + +def rewrite_mask(path, *, keep_metadata=False): + """Write other labels into a 2-D mask (same shape and type), a new time unless kept.""" + before = path.stat() + labels = np.zeros(tifffile.imread(path).shape, np.uint16) + labels[2:5, 2:5] = next(LABELS) # other content at each rewrite + tifffile.imwrite(path, labels, photometric="minisblack", metadata={"axes": "YX"}) + shift = 0 if keep_metadata else 10**9 + os.utime(path, ns=(before.st_atime_ns, before.st_mtime_ns + shift)) + if keep_metadata: + assert path.stat().st_size == before.st_size + + +def rewrite_while_read(monkeypatch, path, mask_path=MASK, **how): + """Rewrite ``path`` right after the scan reads ``mask_path`` (a segmentation writing it).""" + read = mask_scan.read_mask + + def read_then_rewrite(mask, *args, **kwargs): + pixels = read(mask, *args, **kwargs) + if mask.path == mask_path: + rewrite_mask(path, **how) + return pixels + + monkeypatch.setattr(mask_scan, "read_mask", read_then_rewrite) + + +def leftovers(out): + return sorted(p.name for p in out.iterdir() if p.name.startswith(f".{MASK_REPORT_FILENAME}")) + + +def test_a_mask_changed_while_it_is_scanned_writes_no_report(clean, monkeypatch): + out = clean.output + step = prepare_mask_step(None, out) + assert step.ready + rewrite_while_read(monkeypatch, clean.masks / MASK) + with pytest.raises(MaskChangedError) as raised: + describe_masks(step) + assert isinstance(raised.value, ValueError) # as every mask the scan cannot use + assert raised.value.paths == [MASK] + issue = raised.value.issue + assert issue.code == "mask_changed_during_scan" and issue.is_error + assert issue.paths == (MASK,) and "No report was written" in issue.message + assert not (out / MASK_REPORT_FILENAME).exists() and leftovers(out) == [] + assert not (out / ".morphobricks-preprocessing.lock").exists() + + +def test_an_earlier_report_is_left_as_it_was(clean, monkeypatch): + out = clean.output + first = describe_masks(prepare_mask_step(None, out)) + written = first.path.read_bytes() + step = prepare_mask_step(None, out) + rewrite_while_read(monkeypatch, clean.masks / MASK) + with pytest.raises(MaskChangedError): + describe_masks(step) + assert (out / MASK_REPORT_FILENAME).read_bytes() == written and leftovers(out) == [] + + +def test_a_mask_changed_since_its_header_was_read_is_not_scanned(clean): + # The header (planes, axes) describes the mask only as it was when the step paired it. + step = prepare_mask_step(None, clean.output) + rewrite_mask(clean.masks / MASK) + with pytest.raises(MaskChangedError) as raised: + describe_masks(step) + assert raised.value.paths == [MASK] + assert not (clean.output / MASK_REPORT_FILENAME).exists() + + +def test_a_mask_gone_before_its_scan_is_refused(clean): + step = prepare_mask_step(None, clean.output) + (clean.masks / MASK).unlink() + with pytest.raises(MaskChangedError) as raised: + describe_masks(step) + assert raised.value.paths == [MASK] + + +def test_content_is_compared_only_with_verify_content(clean, monkeypatch): + # A rewrite keeping size and time is the metadata identity's stated blind spot. + out = clean.output + rewrite_while_read(monkeypatch, clean.masks / MASK, keep_metadata=True) + assert describe_masks(prepare_mask_step(None, out)).path.exists() + (out / MASK_REPORT_FILENAME).unlink() + step = prepare_mask_step(None, out, verify_content=True) + assert step.verify_content + with pytest.raises(MaskChangedError) as raised: + describe_masks(step) + assert raised.value.compared == "fingerprint or content" + assert "content (SHA-256)" in raised.value.issue.message + assert not (out / MASK_REPORT_FILENAME).exists() + + +def test_the_step_verifies_content_when_the_run_did(tmp_path): + from morphobricks_preprocessing import RunMode + + clean = make_clean_run(tmp_path, axes="YX", shape=(8, 8), mask_types=("Cells",)) + assert not prepare_mask_step(None, clean.output).verify_content + strict = run_experiment( + clean.experiment, + output_folder=clean.output, + masks_folder=clean.masks, + mode=RunMode("restart", verify_content=True), + ) + assert strict.written + assert prepare_mask_step(None, clean.output).verify_content + assert not prepare_mask_step(None, clean.output, verify_content=False).verify_content + + +def test_unchanged_masks_give_the_same_report_with_and_without_content(clean): + out = clean.output + plain = describe_masks(prepare_mask_step(None, out)).path.read_bytes() + strict = describe_masks(prepare_mask_step(None, out, verify_content=True)).path.read_bytes() + assert strict == plain + + +def test_the_masks_command_names_the_changed_mask(clean, monkeypatch, capsys): + rewrite_while_read(monkeypatch, clean.masks / MASK) + assert main(["masks", "--output", str(clean.output)]) == 1 + printed = capsys.readouterr().out + assert "changed or went missing before or while the mask step read them" in printed + assert MASK in printed + assert "Report written" not in printed + assert not (clean.output / MASK_REPORT_FILENAME).exists() + + +def test_a_label_image_changed_while_it_is_scanned_writes_no_report(tmp_path, monkeypatch): + pytest.importorskip("zarr") + from zarr_tree import write_labels, write_ngff + + image = write_ngff(tmp_path / "exp/ctrl/a.ome.zarr", np.zeros((2, 6, 8), np.uint8), "tyx") + cells = np.zeros((2, 6, 8), np.uint16) + cells[:, 1:3, 1:3] = 1 + write_labels(image, {"cells": cells}, "tyx") + out = tmp_path / "out" + assert run_experiment(tmp_path / "exp", prefill_mapping("TYX"), output_folder=out).written + step = prepare_mask_step(None, out) + assert step.ready + read, path = mask_scan.read_mask, "ctrl/a.ome.zarr/labels/cells" + + def chunk_rewritten(mask, *args, **kwargs): + pixels = read(mask, *args, **kwargs) + label = image / "labels" / "cells" / "0" + chunk = sorted(p for p in label.rglob("*") if p.is_file() and p.name[0] != ".")[0] + stat = chunk.stat() + chunk.write_bytes(chunk.read_bytes()) + os.utime(chunk, ns=(stat.st_atime_ns, stat.st_mtime_ns + 10**6)) + return pixels + + monkeypatch.setattr(mask_scan, "read_mask", chunk_rewritten) + with pytest.raises(MaskChangedError) as raised: + describe_masks(step) + assert raised.value.paths == [path] + assert not (out / MASK_REPORT_FILENAME).exists() + + +def test_the_masks_command_takes_verify_content(clean, monkeypatch, capsys): + rewrite_while_read(monkeypatch, clean.masks / MASK, keep_metadata=True) + out = str(clean.output) + assert main(["masks", "--output", out, "--no-verify-content"]) == 0 + assert main(["masks", "--output", out, "--verify-content"]) == 1 + assert "content (SHA-256)" in capsys.readouterr().out From 65b78619009c4b9f5cf80788cff7cb029c902fba Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 16:57:13 +0000 Subject: [PATCH 3/3] Notebook: a mask changed while described says to wait, not to correct it (#165) describe_masks raises MaskChangedError (a ValueError) when a mask changes while it is read. The notebook's Describe the masks caught it as an unusable mask and told the user to correct or replace the mask. It now shows the error card of mask_changed_during_scan, which names the mask and says to wait until nothing writes to it, as the masks command does. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VbytvF3Gsa85ix8Ne2yMDW --- CHANGELOG.md | 2 +- .../notebook_mask_step.py | 12 +++++++- tests/test_notebook_mask_step.py | 28 +++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 203da368..d600ff79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,7 +55,7 @@ are in [docs/plan.md](docs/plan.md) §12. Upgrading from v0.3: see the mask step fingerprints each mask with its header (`prepare_mask_step`) and each image's masks before and after their planes are read; a mask that changed or went missing raises `MaskChangedError` (a `ValueError`; the new error `mask_changed_during_scan`, which the - `masks` command prints), no report is written and an earlier one is left as it was. With + `masks` command and the notebook's **Describe the masks** show with what to do), no report is written and an earlier one is left as it was. With `verify_content` (`prepare_mask_step(verify_content=...)`, new `masks --verify-content`; by default the recorded run's setting) the masks' content SHA-256 is compared too. *Records:* `mask_relationships.json` is unchanged (report version 3) and unchanged masks diff --git a/src/morphobricks_preprocessing/notebook_mask_step.py b/src/morphobricks_preprocessing/notebook_mask_step.py index 3d15565e..21fdf997 100644 --- a/src/morphobricks_preprocessing/notebook_mask_step.py +++ b/src/morphobricks_preprocessing/notebook_mask_step.py @@ -10,7 +10,9 @@ hierarchy, as suggestions to check (contract §15). The thresholds are the defaults; the command line's ``masks`` can change them. A refusal is shown on the card (the locked output folder with :class:`.notebook_output._LockCard`), and an embedding brick that hides the -output folder (``show_output=False``) does not get the card. +output folder (``show_output=False``) does not get the card. Masks that changed while they +were read (``mask_changed_during_scan``, issue #165) get their error card, which says to wait +until nothing writes to them, not to correct them. """ from __future__ import annotations @@ -25,6 +27,7 @@ from .discovery import default_output_folder from .fingerprint import current_fingerprint from .manifest import MANIFEST_FILENAME +from .mask_binding import MaskChangedError from .mask_scan import MASK_REPORT_FILENAME from .mask_text import ( mask_ambiguous_lines, @@ -173,6 +176,13 @@ def _describe_recorded_masks(self) -> MaskStepResult | None: self.mask_step_progress.value, self.mask_step_progress.bar_style = 0, "info" try: result = mask_step.describe_masks(step, progress=self._mask_step_progressed) + except MaskChangedError as exc: # still being written: wait, nothing to correct + self._mask_step_failed( + "The masks changed while they were read: no report was written.", + ["Read the message below; it names the masks and what to do."], + ) + self.mask_step_result.value = issues_html([exc.issue]) + return None except ValueError as exc: # masks whose pixels cannot be used (planes, labels) self._mask_step_failed( "The masks cannot be used.", diff --git a/tests/test_notebook_mask_step.py b/tests/test_notebook_mask_step.py index 83fb673e..2d3df463 100644 --- a/tests/test_notebook_mask_step.py +++ b/tests/test_notebook_mask_step.py @@ -9,10 +9,12 @@ import importlib import json +import os import numpy as np import pytest +from morphobricks_preprocessing import mask_scan from morphobricks_preprocessing.cli import main from morphobricks_preprocessing.mask_scan import MASK_REPORT_FILENAME from morphobricks_preprocessing.run.lock import output_lock @@ -188,6 +190,32 @@ def test_masks_whose_labels_cannot_be_used_get_a_red_card(tmp_path): assert nb.mask_step_progress.bar_style == "danger" +def test_a_mask_changed_while_it_is_described_says_to_wait_not_to_correct_it(tmp_path, monkeypatch): + # Issue #165 (review of #180): a mask still being written is not a mask to correct. + root = experiment(tmp_path) + masks = nested_masks(tmp_path) + nb, _ = ran(root, masks) + assert nb.run().written + read, changed = mask_scan.read_mask, masks / "cells" / IMAGES[0] + + def read_then_rewrite(mask, *args, **kwargs): + pixels = read(mask, *args, **kwargs) + stat = changed.stat() + write_data(changed, np.zeros((16, 16), np.uint16)) + os.utime(changed, ns=(stat.st_atime_ns, stat.st_mtime_ns + 10**9)) + return pixels + + monkeypatch.setattr(mask_scan, "read_mask", read_then_rewrite) + assert nb._on_describe_masks() is None + status = nb.mask_step_status.value + assert "changed while they were read" in status and "Nothing was written." in status + assert "Correct or replace" not in status + assert "mask changed during scan" in nb.mask_step_result.value + assert "Wait until nothing is writing to these masks" in nb.mask_step_result.value + assert nb.mask_step_progress.bar_style == "danger" + assert not (output_of(root) / MASK_REPORT_FILENAME).exists() + + def test_a_new_run_clears_the_description(tmp_path): root = experiment(tmp_path) nb, _ = ran(root, nested_masks(tmp_path))