From 9f4e0a7f673523a6a12d890b61f251925559058c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 13:21:28 +0000 Subject: [PATCH] Turn content hashing of the sources on by default Without it an image edited in place under the same size and modification time was reused as unchanged. A run now records each image's content SHA-256 unless it opts out (RunMode(verify_content=False), --no-verify-content, the notebook's box unticked). The manifest always records the choice (settings.verify_content, true or false). A first run hashes; an existing output folder keeps the choice of its earlier run. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01Scma6W3LC11s3qjXHZuGDm --- CHANGELOG.md | 10 +++ docs/contract.md | 6 +- docs/user_guide.md | 12 ++-- src/morphobricks_preprocessing/cli_run.py | 9 +-- src/morphobricks_preprocessing/manifest.py | 12 +++- .../notebook_prefill.py | 2 +- .../notebook_run.py | 2 +- src/morphobricks_preprocessing/run/modes.py | 5 +- .../run/recorded.py | 4 +- tests/test_content_hash_default.py | 67 +++++++++++++++++++ tests/test_manifest_position_id.py | 5 +- tests/test_manifest_v2.py | 15 ++++- tests/test_mask_run_binding.py | 12 +++- tests/test_mask_scan_binding.py | 15 +++-- tests/test_modes_lock_report.py | 7 +- tests/test_notebook_cards.py | 19 +++--- tests/test_run.py | 2 +- tests/test_run_inputs.py | 3 + tests/test_schema_v2_equivalence.py | 5 +- tests/test_strict_final_check.py | 4 +- tests/test_strict_manifest.py | 8 ++- tests/test_verify_content.py | 21 ++++-- 22 files changed, 188 insertions(+), 57 deletions(-) create mode 100644 tests/test_content_hash_default.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d1fb4398..e7d40b83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ are in [docs/plan.md](docs/plan.md) §12. Upgrading from v0.3: see the ## Unreleased +- **Content hashing of the sources is on by default (#159; owner decision of 7 November 2026).** + A run records each image's content SHA-256 and reuses a row only for the same content, so an + image edited in place with its size and modification time kept is read again (manifest + version 3). Opt out with `RunMode(verify_content=False)`, `--no-verify-content` or by + unticking the notebook's box; the manifest then records `settings.verify_content: false` + (version 2 and 3 manifests now always write the key; one written before has none and reads + as not verified). An output folder keeps the choice of its earlier run, so an existing one + is not recomputed. Cost on 60 OME-TIFF images of 25 MB (1.5 GB), cold cache: a fresh run + 15.9 s without and 19.8 s with hashing (+25 %); a rerun that reuses every row 5.4 s without + and 8.3 s with (+3 s). The standalone `masks` command is unchanged. - **Experiment-description reader, phases 4-5 and warnings (Stage 1, PR 3).** New modules `description_identity`, `description_semantics`, `description_warnings` and `description_walk`; `description_read.read_description_bytes(data: bytes)` runs all five diff --git a/docs/contract.md b/docs/contract.md index d35d7ac4..44dd0e4f 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -199,11 +199,11 @@ interpretation. | Key | Meaning | |---|---| -| `manifest_version`, `schema_version` | Manifest format and CSV format (`"4"`), written as JSON **strings**, not numbers. The manifest format is `"2"` (since v0.4.0) for a default run and `"3"` (v0.4.0rc5, release review) for a content-verification run (`verify_content`, below): version 3 is version 2 with `settings.verify_content` `true` and a `content_sha256` in every file entry (`manifest_versions`). The version, not only those keys, marks a strict run, because readers of v0.4.0rc4 and earlier ignore keys they do not know but refuse a version they do not know (`ManifestFormatError: Manifest version '3' is not supported`, tested by running v0.4.0rc4's code on a strict manifest): they never open a strict run as a fingerprint-only one. The reader and the writer refuse a version 3 manifest without `verify_content` or without a file's `content_sha256`, and a version 1 or 2 manifest that holds either (a strict manifest written as version 2, only by unreleased rc5 builds before this rule). A default run's manifest is version 2, byte for byte as before (tested against the code of the commit before the change). This version reads `"1"`, `"2"` and `"3"`; `RunInputs`, `restart` reuse and the recorded inputs treat 2 and 3 alike, and switching between a strict and a default run recomputes every row as before. A manifest whose `schema_version` is not the current one makes the next run recompute every row. | +| `manifest_version`, `schema_version` | Manifest format and CSV format (`"4"`), written as JSON **strings**, not numbers. The manifest format is `"3"` (v0.4.0rc5, release review) for a content-verification run (`verify_content`, below), which every run is unless it opts out (#159), and `"2"` (since v0.4.0) for a run that opted out: version 3 is version 2 with `settings.verify_content` `true` and a `content_sha256` in every file entry (`manifest_versions`). The version, not only those keys, marks a strict run, because readers of v0.4.0rc4 and earlier ignore keys they do not know but refuse a version they do not know (`ManifestFormatError: Manifest version '3' is not supported`, tested by running v0.4.0rc4's code on a strict manifest): they never open a strict run as a fingerprint-only one. The reader and the writer refuse a version 3 manifest without `verify_content` or without a file's `content_sha256`, and a version 1 or 2 manifest that holds either (a strict manifest written as version 2, only by unreleased rc5 builds before this rule). A run that opted out writes version 2, as before except for `settings.verify_content: false` (tested against the code of the commit before version 3). This version reads `"1"`, `"2"` and `"3"`; `RunInputs`, `restart` reuse and the recorded inputs treat 2 and 3 alike, and switching between a strict and a default run recomputes every row as before. A manifest whose `schema_version` is not the current one makes the next run recompute every row. | | `written_at` | UTC time of writing, ISO 8601. A rerun whose manifest would differ from the earlier one only in `written_at` keeps the earlier time, so the manifest's bytes and SHA-256, which later bricks pin, stay the same (cross-brick test F10, 3 November 2026); the report still records the rerun. | | `experiment_root` | Absolute path of the experiment folder, for information. | | `dtype` | Pixel type of the experiment. | -| `settings` | `bin_count`, `memory_limit` (bytes), `missing_as_singleton`, `exclude` (sorted relative paths), `histogram` (`HistogramSettings.to_dict()` of the normalization brick: `library_version`, `histogram_range`, `robust_quantiles`, `min_central_bins`, §4; the default `minmax` range when absent, in manifests written before follow-up P6), `verify_content` (JSON boolean `true` when the run verified content, below; written only then, in a version 3 manifest, so a default run's `settings` keep the v0.4.0rc4 keys; false when absent). | +| `settings` | `bin_count`, `memory_limit` (bytes), `missing_as_singleton`, `exclude` (sorted relative paths), `histogram` (`HistogramSettings.to_dict()` of the normalization brick: `library_version`, `histogram_range`, `robust_quantiles`, `min_central_bins`, §4; the default `minmax` range when absent, in manifests written before follow-up P6), `verify_content` (JSON boolean, below: `true` when the run verified content, `false` when it opted out, written by every version 2 and 3 manifest so that an opt-out is on record (#159); a manifest written before that has no key and reads as false, not verified). | | `axis_mapping` | `axes` (file axis labels) and `roles` (`X`, `Y`, `Z`, `C`, `T` or null per axis). | | `labels` | One entry per leaf folder: `folder` (relative path, `.` for the root) and its three labels. | | `files` | One entry per analysed file, sorted by path: `relative_path`, `size` (bytes), `mtime_ns`, the three labels, `axes`, `singleton_roles`, `plane_count` (its rows in the table: XYC planes), `channel_names` (list, null for unnamed channels; empty if none in metadata), `significant_bits` (null if not in metadata), `read_in_blocks` (true when the image's planes are over the memory limit and are read in blocks; information only, false when absent), `position_id` (the image's position identifier, §1: its name without the extension, never edited; required in version 2, where an entry without it, with null, or with another value than `position_id(relative_path)` is refused naming the entry, by the reader and the writer, round-2 audit A-m5; null when absent in a version 1 manifest), `channel_names_trimmed` (true when leading or trailing whitespace was removed from the image's channel names as they were read, §2; `channel_names` holds the trimmed names; false when absent), `roles` (the role of each of the file's `axes`, `X`, `Y`, `Z`, `C`, `T` or null, from the file's own mapping: an OME-Zarr image in another order than `axis_mapping` has its own roles, so later bricks read these rather than rebuild them from `axis_mapping`; null when absent, in manifests written before them; optional key, version 1). For an OME-Zarr image, `size` is the total size of its files, `mtime_ns` the time of its newest file, and `inventory_sha256` (v0.4.0, PP-12, issue #86; only for OME-Zarr images) the SHA-256 of its inventory: one line per file (its path relative to the image with `/`, a NUL, its size, a NUL, its `mtime_ns`, a newline), sorted by path, its root `labels` group left out; links to folders inside the image are followed, as the reader follows them, and links that reach one folder twice are refused. Adding, removing, renaming, resizing or touching any chunk or metadata file changes it; an OME-Zarr image recorded without it (v0.4.0rc3 and earlier) counts as changed (recomputed; `RunInputs` refuses it). Like a TIFF file's size and time it is metadata only (design v0.4 §1): a file rewritten with the same size and its time set back is not seen, only hashing the content would. **Version 2 (v0.4.0):** `roles` is required, with `axes_from` (required): `header` when the metadata names every axis and the roles agree, `confirmed` for a plain stack whose roles the user confirmed (§2); and `pixel_sizes`, one entry per role `X`, `Y`, `Z`, `T` with `value`, `unit` (OME-NGFF name) and `source`: `header` (the image's metadata) or `stated` (`calibration.csv`, §9.3) with a value, or `pixels` (stated as deliberately uncalibrated) with `value` and `unit` null; `value` and `source` null for an axis nothing gives a size (never guessed). A `header` or `stated` size without a value, or a value with no source or with `pixels`, is refused. The reader refuses a version 2 entry without `roles` or `axes_from`, and the writer refuses to write one. `content_sha256` (only in a `verify_content` run): the SHA-256 of the image's bytes, or for an OME-Zarr image of one line per file (its path relative to the image with `/`, a NUL, the file's SHA-256, a newline) over the files its inventory covers, in path order (`source_content.source_content_sha256`). | @@ -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. 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). | +| Content verification (on by default, owner 7 November 2026, #159; opt-in before) | Every run does it unless it opts out with `RunMode(name, verify_content=False)` (`run_experiment(mode=...)`, CLI `--no-verify-content`, the notebook's "Verify each image's content" box unticked); the opt-out is recorded as `settings.verify_content: false`. It 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 (true without one: a first run, or a version 1 manifest); 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 cost is in the pull request that made it the default (about 3 seconds per 1.5 GB read, measured at 1 GB/s of SHA-256). A run that verifies 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. | diff --git a/docs/user_guide.md b/docs/user_guide.md index 03af6a6f..6e18434d 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -94,8 +94,10 @@ Google Drive is wiped when it disconnects (after some idle time, or after severa computer), the image is read again. That costs time but never reuses wrong rows. The other way round is not seen: a file rewritten with the same size and its modification time set back (by a tool that restores times) counts as unchanged, and its earlier rows - are kept. Tick **Verify each image's content** (CLI `--verify-content`) to compare each - image's SHA-256 too; every image is then read once more per run. + are kept. Every run therefore compares each image's SHA-256 too (**Verify each image's + content**, ticked by default; CLI `--verify-content`), and every image is read once more per + run. Untick it (CLI `--no-verify-content`) to compare size and time only; the manifest then + records `verify_content: false`. An output folder keeps the choice of its earlier run. ## 2. Organising the experiment folder @@ -655,7 +657,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. With `masks`: also compare each mask's content SHA-256 before and after it is read (same default) | +| `--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 on; `--no-verify-content` opts out and the manifest records it. 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 | @@ -947,8 +949,8 @@ each mask's axes and size (for example `TYX` and 50 x 512 x 512 for a mask per t hidden folder `.staging` in the output folder. If the run is interrupted (a crash, a stopped kernel, a lost Colab session, a computer switched off), run again with the same settings: the images already read are not read again, as long as they are unchanged (same -size, time and, for OME-Zarr, the same files; with `--verify-content` also the same -SHA-256) and the settings that change rows are the same. The report lists them as `resumed`, and lists any rows it did not reuse with the +size, time and, for OME-Zarr, the same files; unless the run opted out with +`--no-verify-content`, also the same SHA-256) and the settings that change rows are the same. The report lists them as `resumed`, and lists any rows it did not reuse with the reason (`staged_discarded`). A `full` run reuses none. After a killed run, remove its lock first (`--remove-stale-lock`). Once a run has written its results, `.staging` is deleted. diff --git a/src/morphobricks_preprocessing/cli_run.py b/src/morphobricks_preprocessing/cli_run.py index 3db332d8..5ee0c399 100644 --- a/src/morphobricks_preprocessing/cli_run.py +++ b/src/morphobricks_preprocessing/cli_run.py @@ -4,9 +4,9 @@ ``--axes`` is needed for the first run of plain stacks (not when every image names its axes, design v0.4 §3) and for plain stacks that are new or changed; ``--no-masks`` drops the recorded masks folder, ``--no-exclude`` the recorded exclusions; -``--mode full`` recomputes every row; ``--verify-content`` reuses rows only for the same -content SHA-256 (:class:`.run.modes.RunMode`). Given only ``--output``, ``run`` uses the recorded -experiment folder (``experiment_not_recorded`` without one). +``--mode full`` recomputes every row; rows are reused only for the same content SHA-256 unless +``--no-verify-content`` opts out (:class:`.run.modes.RunMode`). Given only ``--output``, ``run`` +uses the recorded experiment folder (``experiment_not_recorded`` without one). Also the helpers every command shares: :func:`parse_size` and :func:`command_preflight`. """ @@ -270,7 +270,8 @@ def _run_inputs(run: argparse.ArgumentParser) -> None: action=argparse.BooleanOptionalAction, help="reuse rows only when each image's content SHA-256 is the one recorded with them, " "not only its size and modification time (reads every image once more; changing it " - "recomputes every row; default: as in the earlier run, else no)", + "recomputes every row; on by default: as in the earlier run, else yes; " + "--no-verify-content opts out and the manifest records that)", ) diff --git a/src/morphobricks_preprocessing/manifest.py b/src/morphobricks_preprocessing/manifest.py index 489d76d7..f9f2f988 100644 --- a/src/morphobricks_preprocessing/manifest.py +++ b/src/morphobricks_preprocessing/manifest.py @@ -315,10 +315,16 @@ def to_dict(self) -> dict: ), "exclude": list(self.settings.exclude), "histogram": self.settings.histogram.to_dict(), - # Written only when on (version 3): a default run's manifest keeps rc4's. + # Written by every version 2 and 3 manifest (#159): a run that opted out of + # content hashing says so. Manifests written before it have no key and read + # as not verified; version 1 is unchanged. **( - {"verify_content": True} - if recorded_flag(self.settings.verify_content, "verify_content") + { + "verify_content": recorded_flag( + self.settings.verify_content, "verify_content" + ) + } + if self.manifest_version != "1" else {} ), }, diff --git a/src/morphobricks_preprocessing/notebook_prefill.py b/src/morphobricks_preprocessing/notebook_prefill.py index 41caf207..eeb9e01b 100644 --- a/src/morphobricks_preprocessing/notebook_prefill.py +++ b/src/morphobricks_preprocessing/notebook_prefill.py @@ -93,7 +93,7 @@ def _unfill(self) -> None: if now["intensity"] == self._prefilled["intensity"]: self.run_checks.restore_defaults() if now["verify_content"] == self._prefilled["verify_content"]: - self.verify_content.value = False + self.verify_content.value = True kept = ("recommended", "bins", "histogram") unedited = "bins" not in self._edited unedited = unedited and all(now[key] == self._prefilled[key] for key in kept) diff --git a/src/morphobricks_preprocessing/notebook_run.py b/src/morphobricks_preprocessing/notebook_run.py index b8c2f959..2a235f46 100644 --- a/src/morphobricks_preprocessing/notebook_run.py +++ b/src/morphobricks_preprocessing/notebook_run.py @@ -125,7 +125,7 @@ def _build_settings(self) -> None: options=MODE_OPTIONS, value=RESTART, layout=widgets.Layout(**_WIDE) ) self.verify_content = widgets.Checkbox( - value=False, + value=True, description="Verify each image's content (SHA-256) before reusing its rows", indent=False, layout=widgets.Layout(**_WIDE), diff --git a/src/morphobricks_preprocessing/run/modes.py b/src/morphobricks_preprocessing/run/modes.py index 595af1ed..ed91e8e3 100644 --- a/src/morphobricks_preprocessing/run/modes.py +++ b/src/morphobricks_preprocessing/run/modes.py @@ -60,8 +60,9 @@ class RunMode: ``"restart"`` or ``"full"``. verify_content True: record each source's content SHA-256 and reuse a row only for the same - content; False: the fingerprint only (the default); None: as the earlier run - recorded (False without one). + content; False: the fingerprint only (an explicit opt-out, recorded in the manifest's + ``settings.verify_content``); None (the default): as the earlier run recorded, True + without one (#159). """ name: str = RESTART diff --git a/src/morphobricks_preprocessing/run/recorded.py b/src/morphobricks_preprocessing/run/recorded.py index 5142f4d3..cb1db1aa 100644 --- a/src/morphobricks_preprocessing/run/recorded.py +++ b/src/morphobricks_preprocessing/run/recorded.py @@ -373,7 +373,9 @@ def _defaults(given: GivenInputs) -> ResolvedInputs: missing_as_singleton=bool(given.missing_as_singleton), intensity_settings=given.intensity_settings, histogram_settings=given.histogram_settings or HistogramSettings(), - verify_content=bool(given.verify_content), + # Content hashing is on unless the caller opted out (#159); an earlier run's choice is + # kept by _with_changes, so an existing output is not silently recomputed. + verify_content=True if given.verify_content is None else bool(given.verify_content), ) diff --git a/tests/test_content_hash_default.py b/tests/test_content_hash_default.py new file mode 100644 index 00000000..b4f3b707 --- /dev/null +++ b/tests/test_content_hash_default.py @@ -0,0 +1,67 @@ +"""Content hashing of the sources is on by default; an opt-out is recorded (#159). + +Without it an image edited in place under the same name, size and modification time is +reused as unchanged. A run hashes unless it opts out (``RunMode(verify_content=False)``, +``--no-verify-content``, the notebook's box); the manifest records the choice either way. +""" + +import json + +import pytest + +from morphobricks_preprocessing import RunMode, read_manifest, run_experiment +from morphobricks_preprocessing.cli import main +from strict_tree import IMAGES, TYX, rewrite_keeping_metadata, write_experiment +from test_cli import confirm_design + +OFF = RunMode("restart", verify_content=False) + + +@pytest.fixture +def experiment(tmp_path): + return write_experiment(tmp_path) + + +def settings_of(out): + return json.loads((out / "run_manifest.json").read_text(encoding="utf-8")) + + +def test_a_run_given_no_mode_hashes_every_source_and_records_it(experiment): + root, out = experiment + assert run_experiment(root, TYX, output_folder=out).written + data = settings_of(out) + assert data["manifest_version"] == "3" and data["settings"]["verify_content"] is True + assert all(entry["content_sha256"] for entry in data["files"]) + assert read_manifest(out / "run_manifest.json").settings.verify_content + + +def test_a_default_run_reads_an_image_rewritten_with_its_metadata_kept(experiment): + root, out = experiment + assert run_experiment(root, TYX, output_folder=out).written + rewrite_keeping_metadata(root / IMAGES[0]) + again = run_experiment(root, output_folder=out) + assert again.written and again.read == (IMAGES[0],) and again.changed == (IMAGES[0],) + + +def test_an_opt_out_is_recorded_as_false_and_the_rewrite_is_not_seen(experiment): + root, out = experiment + assert run_experiment(root, TYX, output_folder=out, mode=OFF).written + data = settings_of(out) + assert data["manifest_version"] == "2" and data["settings"]["verify_content"] is False + assert not any("content_sha256" in entry for entry in data["files"]) + rewrite_keeping_metadata(root / IMAGES[0]) + again = run_experiment(root, output_folder=out) # the earlier run's choice is kept + assert again.read == () and IMAGES[0] in again.unchanged + assert settings_of(out)["settings"]["verify_content"] is False + + +def test_the_cli_hashes_by_default_and_no_verify_content_opts_out(experiment): + root, out = experiment + off_out = out.with_name("out_off") + for folder in (out, off_out): + confirm_design(root, folder) + assert main(["run", str(root), "--output", str(out), "--axes", "TYX"]) == 0 + assert settings_of(out)["settings"]["verify_content"] is True + args = ["run", str(root), "--output", str(off_out), "--axes", "TYX", "--no-verify-content"] + assert main(args) == 0 + assert settings_of(off_out)["settings"]["verify_content"] is False diff --git a/tests/test_manifest_position_id.py b/tests/test_manifest_position_id.py index 636ac377..39f13605 100644 --- a/tests/test_manifest_position_id.py +++ b/tests/test_manifest_position_id.py @@ -19,6 +19,7 @@ from morphobricks_preprocessing import ( ManifestFormatError, RunManifest, + RunMode, prefill_mapping, read_manifest, run_experiment, @@ -39,7 +40,9 @@ def run(tmp_path): pixels = np.arange(64, dtype=np.uint8).reshape(8, 8) for name in NAMES: write_data(root / "ctrl/rep1" / name, pixels, "YX") - result = run_experiment(root, prefill_mapping("YX")) + result = run_experiment( + root, prefill_mapping("YX"), mode=RunMode("restart", verify_content=False) + ) return Path(result.output_folder), Path(result.manifest_path) diff --git a/tests/test_manifest_v2.py b/tests/test_manifest_v2.py index d85f4bb3..b088a1a0 100644 --- a/tests/test_manifest_v2.py +++ b/tests/test_manifest_v2.py @@ -16,6 +16,7 @@ from morphobricks_preprocessing import ( AxisMapping, + RunMode, prefill_mapping, read_manifest, review_axes, @@ -59,7 +60,12 @@ def test_version_2_records_roles_their_source_and_pixel_sizes(tmp_path): write_plain(root / "ctrl/b.tif", stack(1)) result = run_experiment(root, AxisMapping("TYX", "TYX")) assert not result.written # one arrangement per experiment: TYX and IYX differ - result = run_experiment(root, prefill_mapping("TYX"), exclude=["ctrl/b.tif"]) + result = run_experiment( + root, + prefill_mapping("TYX"), + exclude=["ctrl/b.tif"], + mode=RunMode("restart", verify_content=False), + ) data = json.loads(result.manifest_path.read_text(encoding="utf-8")) assert data["manifest_version"] == MANIFEST_VERSION == "2" (entry,) = data["files"] @@ -135,7 +141,9 @@ def test_recorded_sizes_are_checked(): def test_a_version_1_run_gives_no_reusable_row(tmp_path): root = tmp_path / "exp" write_data(root / "ctrl/a.tif", stack(), "TYX") - first = run_experiment(root, prefill_mapping("TYX")) + first = run_experiment( + root, prefill_mapping("TYX"), mode=RunMode("restart", verify_content=False) + ) data = json.loads(first.manifest_path.read_text(encoding="utf-8")) data["manifest_version"] = "1" for entry in data["files"]: @@ -147,7 +155,8 @@ def test_a_version_1_run_gives_no_reusable_row(tmp_path): again.rebuild_reason == "the earlier run was written before v0.4.0, without the axis check" ) assert again.read == ("ctrl/a.tif",) - assert read_manifest(again.manifest_path).manifest_version == "2" + # The rebuild hashes content, as every run does unless it opts out (#159). + assert read_manifest(again.manifest_path).manifest_version == "3" def test_rows_are_reused_only_for_the_same_row_key(tmp_path): diff --git a/tests/test_mask_run_binding.py b/tests/test_mask_run_binding.py index 828572a8..4c8f9dc0 100644 --- a/tests/test_mask_run_binding.py +++ b/tests/test_mask_run_binding.py @@ -149,8 +149,9 @@ def rewritten_before_the_commit(entries, root): 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). +def test_a_run_that_opted_out_does_not_see_a_rewrite_with_size_and_time_kept(clean, monkeypatch): + # The stated blind spot of the metadata identity, for a run that opted out of the content + # hashing that is on by default (#159). mask = clean.masks / MASK changed_since = run_module.pipeline.changed_since @@ -159,7 +160,12 @@ def meanwhile(entries, root): 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) + result = run_experiment( + clean.experiment, + output_folder=clean.output, + masks_folder=clean.masks, + mode=RunMode("restart", verify_content=False), + ) assert result.written diff --git a/tests/test_mask_scan_binding.py b/tests/test_mask_scan_binding.py index 8edd1787..6a2ebd00 100644 --- a/tests/test_mask_scan_binding.py +++ b/tests/test_mask_scan_binding.py @@ -110,7 +110,8 @@ 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() + off = prepare_mask_step(None, out, verify_content=False) + assert describe_masks(off).path.exists() (out / MASK_REPORT_FILENAME).unlink() step = prepare_mask_step(None, out, verify_content=True) assert step.verify_content @@ -125,16 +126,16 @@ 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( + assert prepare_mask_step(None, clean.output).verify_content # on by default (#159) + off = run_experiment( clean.experiment, output_folder=clean.output, masks_folder=clean.masks, - mode=RunMode("restart", verify_content=True), + mode=RunMode("restart", verify_content=False), ) - 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 + assert off.written + assert not prepare_mask_step(None, clean.output).verify_content + assert prepare_mask_step(None, clean.output, verify_content=True).verify_content def test_unchanged_masks_give_the_same_report_with_and_without_content(clean): diff --git a/tests/test_modes_lock_report.py b/tests/test_modes_lock_report.py index 5617784f..c0b7e59a 100644 --- a/tests/test_modes_lock_report.py +++ b/tests/test_modes_lock_report.py @@ -16,6 +16,7 @@ from morphobricks_preprocessing import ( AxisMapping, + RunMode, default_design, default_output_folder, prefill_mapping, @@ -174,7 +175,7 @@ def test_without_roles_given_or_recorded_the_run_stops(experiment): def test_a_version_1_manifest_gives_no_inputs(experiment): - first = run_experiment(experiment, TYX) + first = run_experiment(experiment, TYX, mode=RunMode("restart", verify_content=False)) data = json.loads(first.manifest_path.read_text(encoding="utf-8")) data["manifest_version"] = "1" first.manifest_path.write_text(json.dumps(data), encoding="utf-8") @@ -491,7 +492,9 @@ def test_without_a_recorded_experiment_the_cli_names_what_is_missing( ): out = tmp_path / "out" if folder == "version 1": - run_experiment(experiment, TYX, output_folder=out) + run_experiment( + experiment, TYX, output_folder=out, mode=RunMode("restart", verify_content=False) + ) data = json.loads((out / "run_manifest.json").read_text(encoding="utf-8")) data["manifest_version"] = "1" (out / "run_manifest.json").write_text(json.dumps(data), encoding="utf-8") diff --git a/tests/test_notebook_cards.py b/tests/test_notebook_cards.py index 87ef0828..ee86433d 100644 --- a/tests/test_notebook_cards.py +++ b/tests/test_notebook_cards.py @@ -324,20 +324,21 @@ def test_the_run_step_names_the_mode_and_the_input_changes(experiment): assert "exclude" in nb.run_status.value and "Mode: full" in nb.run_status.value -def test_the_content_verification_box_is_run_recorded_and_filled_again(experiment): - # Review of rc4 (I3): the notebook exposes the strict reuse rule of the run. +def test_the_content_verification_box_is_on_by_default_and_an_opt_out_is_recorded(experiment): + # Review of rc4 (I3): the notebook exposes the strict reuse rule of the run; on by default + # (#159), and unticking it is an explicit opt-out the manifest records. nb = ready(experiment) - assert not nb.verify_content.value - nb.verify_content.value = True + assert nb.verify_content.value + nb.verify_content.value = False assert "verify_content" not in nb.plan.value # no earlier run yet result = nb.run() assert result.written manifest = json.loads((output_of(experiment) / "run_manifest.json").read_text("utf-8")) - assert manifest["settings"]["verify_content"] is True - assert all(entry["content_sha256"] for entry in manifest["files"]) - nb.verify_content.value = False - assert "verify_content: True → False" in nb.plan.value - assert checked(experiment).verify_content.value # filled from the earlier run + assert manifest["settings"]["verify_content"] is False + assert not any("content_sha256" in entry for entry in manifest["files"]) + nb.verify_content.value = True + assert "verify_content: False → True" in nb.plan.value + assert not checked(experiment).verify_content.value # filled from the earlier run def test_the_plan_reads_the_calibration_the_run_reads(experiment): diff --git a/tests/test_run.py b/tests/test_run.py index 748bb065..737ea37c 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -299,7 +299,7 @@ def test_differing_channel_names_stop_the_run(tmp_path): def test_every_row_has_its_position_id(experiment): - result = run_experiment(experiment, TYX) + result = run_experiment(experiment, TYX, mode=run_module.RunMode("restart", False)) records = read_plane_histograms(result.csv_path) assert {r.position_id for r in records} == {"img_01", "img_02"} # The same file name in two technical repeats gives two distinct positions. diff --git a/tests/test_run_inputs.py b/tests/test_run_inputs.py index 8a004bed..aa7fa4df 100644 --- a/tests/test_run_inputs.py +++ b/tests/test_run_inputs.py @@ -88,6 +88,9 @@ def test_a_version_1_run_is_refused(clean): path = clean.output / MANIFEST_FILENAME data = json.loads(path.read_text(encoding="utf-8")) data["manifest_version"] = "1" + del data["settings"]["verify_content"] # a version 1 run never hashed content + for entry in data["files"]: + del entry["content_sha256"] path.write_text(json.dumps(data), encoding="utf-8") with pytest.raises(RunInputsError, match="before v0.4.0"): RunInputs.open(clean.output) diff --git a/tests/test_schema_v2_equivalence.py b/tests/test_schema_v2_equivalence.py index e6a285c8..085b536e 100644 --- a/tests/test_schema_v2_equivalence.py +++ b/tests/test_schema_v2_equivalence.py @@ -23,6 +23,7 @@ from morphobricks_normalization.records import PlaneRecord from morphobricks_preprocessing import ( + RunMode, plane_layout, prefill_mapping, preflight, @@ -195,7 +196,9 @@ def test_a_version_2_table_is_recomputed_on_the_next_run(tmp_path, pixels): root = tmp_path / "exp" bins, axes = write_experiment(root, "float32", pixels) mapping = prefill_mapping(axes) - first = run_experiment(root, mapping, bin_count=bins) + first = run_experiment( + root, mapping, bin_count=bins, mode=RunMode("restart", verify_content=False) + ) table = first.csv_path.read_bytes() # Leave the output as a run of version 2 did: its table, and a manifest naming it. shutil.copyfile(DATA / "v2_float32.csv", first.csv_path) diff --git a/tests/test_strict_final_check.py b/tests/test_strict_final_check.py index a6e9aeb4..aa972813 100644 --- a/tests/test_strict_final_check.py +++ b/tests/test_strict_final_check.py @@ -87,7 +87,9 @@ def test_the_next_strict_run_reads_the_rewritten_images_and_publishes(experiment def test_a_default_run_has_no_final_content_check(experiment, monkeypatch): root, out = experiment - assert run_experiment(root, TYX, output_folder=out).written + assert run_experiment( + root, TYX, output_folder=out, mode=RunMode("restart", verify_content=False) + ).written calls = rewrite_before_final_check(monkeypatch, [root / IMAGES[1]]) result = run_experiment(root, TYX, output_folder=out, mode="full") assert result.written and calls == [] diff --git a/tests/test_strict_manifest.py b/tests/test_strict_manifest.py index a9704f7b..38ee677d 100644 --- a/tests/test_strict_manifest.py +++ b/tests/test_strict_manifest.py @@ -87,7 +87,7 @@ def test_a_strict_run_writes_version_3_and_a_default_run_version_2(experiment): assert off.rebuild_reason == "content verification (verify_content) was turned off" data = json.loads((out / "run_manifest.json").read_text(encoding="utf-8")) assert data["manifest_version"] == MANIFEST_VERSION == "2" - assert "verify_content" not in data["settings"] + assert data["settings"]["verify_content"] is False # the opt-out is recorded (#159) assert not any("content_sha256" in entry for entry in data["files"]) @@ -135,7 +135,8 @@ def test_v0_4_0rc4_opened_a_strict_run_written_as_version_2(experiment, tmp_path def test_a_default_manifest_is_byte_identical_to_the_one_before_version_3(experiment, tmp_path): root, out = experiment - assert run_experiment(root, TYX, output_folder=out).written + off = RunMode("restart", verify_content=False) + assert run_experiment(root, TYX, output_folder=out, mode=off).written src = extract(BEFORE_STRICT_VERSION, tmp_path / "before") before_out = tmp_path / "before-out" assert run_with(src, DEFAULT_RUN, root, before_out) == {"written": True} @@ -144,6 +145,9 @@ def test_a_default_manifest_is_byte_identical_to_the_one_before_version_3(experi # Only the time of writing and the versions may differ (the extracted code reports the # installed package's version); everything else is the same bytes. data, earlier = json.loads(now), json.loads(before) + # The opt-out is the one new key (#159); everything else is as before. + assert data["settings"].pop("verify_content") is False + now = (json.dumps(data, indent=2, ensure_ascii=False) + "\n").encode("utf-8") assert data.keys() == earlier.keys() earlier["written_at"], earlier["versions"] = data["written_at"], data["versions"] text = json.dumps(earlier, indent=2, ensure_ascii=False) + "\n" diff --git a/tests/test_verify_content.py b/tests/test_verify_content.py index b91ce43b..e0c95207 100644 --- a/tests/test_verify_content.py +++ b/tests/test_verify_content.py @@ -45,10 +45,13 @@ def report(out): return json.loads((out / REPORT_FILENAME).read_text(encoding="utf-8")) -def test_the_default_rule_does_not_see_a_rewrite_with_its_metadata_kept(experiment): - # The documented blind spot of the metadata fingerprint: the earlier rows are reused. +def test_an_opt_out_does_not_see_a_rewrite_with_its_metadata_kept(experiment): + # The documented blind spot of the metadata fingerprint, now only for a run that opted out + # of content hashing (#159): the earlier rows are reused. root, out = experiment - assert run_experiment(root, TYX, output_folder=out).written + assert run_experiment( + root, TYX, output_folder=out, mode=RunMode("restart", verify_content=False) + ).written rewrite_keeping_metadata(root / IMAGES[0]) again = run_experiment(root, output_folder=out) assert again.written and again.read == () and IMAGES[0] in again.unchanged @@ -56,9 +59,9 @@ def test_the_default_rule_does_not_see_a_rewrite_with_its_metadata_kept(experime manifest = read_manifest(out / "run_manifest.json") assert not manifest.settings.verify_content assert all(entry.content_sha256 is None for entry in manifest.files) - # A default run's manifest has no new key (review of #140): rc4's shape is kept. + # The opt-out is recorded in the manifest, not left to be read from a missing key (#159). data = json.loads((out / "run_manifest.json").read_text(encoding="utf-8")) - assert "verify_content" not in data["settings"] + assert data["settings"]["verify_content"] is False assert not any("content_sha256" in entry for entry in data["files"]) @@ -86,7 +89,9 @@ def test_a_strict_restart_reads_an_image_rewritten_with_its_metadata_kept(tmp_pa def test_turning_verification_on_or_off_recomputes_every_row_and_says_so(experiment): root, out = experiment - assert run_experiment(root, TYX, output_folder=out).written + assert run_experiment( + root, TYX, output_folder=out, mode=RunMode("restart", verify_content=False) + ).written on = run_experiment(root, output_folder=out, mode=STRICT) assert on.written and on.read == tuple(IMAGES) assert on.rebuild_reason == "content verification (verify_content) was turned on" @@ -132,7 +137,9 @@ def unreadable(path, rel, *args): with monkeypatch.context() as patch: patch.setattr(run_module.resume, "_read_file", unreadable) - assert not run_experiment(root, TYX, output_folder=out).written + assert not run_experiment( + root, TYX, output_folder=out, mode=RunMode("restart", verify_content=False) + ).written result = run_experiment(root, TYX, output_folder=out, mode=STRICT) assert result.written and result.resumed == () and result.read == tuple(IMAGES) assert set(report(out)["staged_discarded"].values()) == {