Re-check every mask fingerprint right before the manifest replacement

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

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")

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()