diff --git a/CHANGELOG.md b/CHANGELOG.md index e6d5d043..643f82fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ are in [docs/plan.md](docs/plan.md) ยง12. Upgrading from v0.3: see the ## Unreleased +- **A mask changed after the last check is no longer published (#198).** Every run, also one + with `verify_content` off, fingerprints each mask again in the commit's `before_commit` + callback, right before the manifest is replaced; a changed mask stops the run with + `mask_changed_during_run` and the earlier generation is intact. Strict runs do this plus their + existing SHA-256 check. - **Derived `experiment_design.csv` and the cross-version parity suite (Stage 1, PR 10).** New module `design_derive`: `derive_design(description)` and `write_derived_design(description, path)` derive the legacy design file from a published description (Spec section 11): source diff --git a/src/morphobricks_preprocessing/run/pipeline.py b/src/morphobricks_preprocessing/run/pipeline.py index e1040454..802c7f8f 100644 --- a/src/morphobricks_preprocessing/run/pipeline.py +++ b/src/morphobricks_preprocessing/run/pipeline.py @@ -405,22 +405,31 @@ def _publish(run: _Run, base: RunOutcome, rows: Rows, settings: RunSettings) -> 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. Every mask is hashed - again too (``mask_changed_during_run``, issue #160). It reads every analysed image and - mask whole once more. + """Return a run's last check before its commit, or None when there is nothing to check. + + Every run fingerprints each mask once more after its generation is staged, right before + the manifest is replaced (:func:`.generation.staged_generation`; issue #198): a mask + changed since the check in :func:`_publish` stops the run with + ``mask_changed_during_run``, also when ``verify_content`` is off. + + A ``verify_content`` run also hashes every recorded image (:func:`content_changed`, the + OME-Zarr store digest for a store) and every mask (``mask_changed_during_run``, issue + #160): one 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 and mask whole once more. """ - if not settings.verify_content: - return None root, masks = run.pre.discovery.root, run.pre.masks + if masks is None and not settings.verify_content: + return None def check() -> None: - """Raise when an image's or mask's content is not the SHA-256 taken earlier.""" + """Raise when an image's or mask's fingerprint or content is not the one taken earlier.""" + if masks is not None: + changed = changed_masks(masks, root) + if changed: + raise _SourcesChangedError(changed, masks=True) + if not settings.verify_content: + return changed = content_changed(rows.entries.values(), root) if changed: raise _SourcesChangedError(changed, "content") diff --git a/tests/test_mask_run_binding.py b/tests/test_mask_run_binding.py index 4c8f9dc0..ae079335 100644 --- a/tests/test_mask_run_binding.py +++ b/tests/test_mask_run_binding.py @@ -193,3 +193,35 @@ def chunk_rewritten(entries, root): assert [(i.code, i.paths) for i in result.issues if i.is_error] == [ ("mask_changed_during_run", ("ctrl/a.ome.zarr/labels/cells",)) ] + + +def rewrite_after_the_staged_body(monkeypatch, mask, *, keep_metadata=False): + """Rewrite ``mask`` once the generation is staged, right before the manifest is replaced. + + The publication record is the last staged file; the staged-body checks of the pipeline + are all done by then, and only the commit's own ``before_commit`` callback is left. + """ + write = run_module.generation.write_text_atomic + + def record_then_rewrite(path, text, *args, **kwargs): + write(path, text, *args, **kwargs) + if path.name == run_module.generation.PUBLICATION_RECORD: + rewrite_mask(mask, keep_metadata=keep_metadata) + + monkeypatch.setattr(run_module.generation, "write_text_atomic", record_then_rewrite) + + +@pytest.mark.parametrize("mode", [RunMode("restart", verify_content=False), STRICT]) +def test_a_mask_rewritten_after_the_staged_checks_is_not_published(clean, monkeypatch, mode): + # Issue #198: the last fingerprint check ran before the commit; with the content hashing + # opted out there was no callback after it, and the manifest was replaced regardless. + out, mask = clean.output, clean.masks / MASK + before = outputs(out) + rewrite_after_the_staged_body(monkeypatch, mask) + result = run_experiment( + clean.experiment, output_folder=out, masks_folder=clean.masks, mode=mode + ) + 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 outputs(out) == before and not (out / PUBLISH_FOLDER).exists()