From 4c91156b9169b6710f3476839e5941e7a42d2890 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 11:45:31 +0000 Subject: [PATCH 1/4] Refuse mount points below the experiment and masks folders; warn about hard-linked files (B1, I1) A Linux bind mount below the experiment is an ordinary folder to the walk, so discovery read files from outside the selected folder. LinkGuard now refuses any folder (or file) below the walked root that is a mount point, with the error mounted_folder naming it: a path listed in /proc/self/mountinfo (the only way to see a same-device bind mount; os.path.ismount and st_dev report an ordinary folder), or a folder on another device than the root. The rule holds inside OME-Zarr stores, the masks folder, check, run, convert and RunInputs (mounted_folder is one of LINK_CODES). The root itself may be on any mount. An image or mask whose link count is above 1 is the warning hard_linked_file (from the stat claim() already takes); reading and fingerprints are unchanged. Tests: a real bind mount (skipped by name without mount --bind as root) and a replaced mount table, which always runs. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- AGENTS.md | 4 +- CHANGELOG.md | 9 + docs/contract.md | 6 +- docs/user_guide.md | 8 + pyproject.toml | 1 + src/morphobricks_preprocessing/links.py | 99 ++++++--- src/morphobricks_preprocessing/mounts.py | 111 ++++++++++ tests/test_mounts.py | 259 +++++++++++++++++++++++ 8 files changed, 470 insertions(+), 27 deletions(-) create mode 100644 src/morphobricks_preprocessing/mounts.py create mode 100644 tests/test_mounts.py diff --git a/AGENTS.md b/AGENTS.md index 96d55c3a..ae5385d2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,7 +65,9 @@ src/morphobricks_preprocessing/ position_id per image and the duplicate check; FOV_KEY, check_identity_text, check_plane_index, check_identity_case (the shared identity rules, contract §16) links.py LinkGuard: links followed only inside the folder walked (experiment, OME-Zarr - stores, masks folder); link_outside_folder, link_loop, linked_twice (#81) + stores, masks folder); link_outside_folder, link_loop, linked_twice (#81); + mounted_folder (error), hard_linked_file (warning) + mounts.py MountCheck: mount points below a folder (/proc/self/mountinfo, st_dev) folder_rules.py which of the experiment, masks, copies and output folders may hold which: experiment_inside_masks, copies_inside_masks, masks_inside_experiment, output_inside_masks (audit B1/I1, plan §12 question 70) diff --git a/CHANGELOG.md b/CHANGELOG.md index 739f92af..5fb7754e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,6 +100,15 @@ and the bricks' migration). ### Fixed +- **Mount points below the experiment or masks folder are refused** (owner, 3 November + 2026): a bind mount is an ordinary folder to the walk, so its files from outside were + read. A folder (or file) listed in `/proc/self/mountinfo`, or on another device than the + selected folder, is the error `mounted_folder` naming it, in discovery, OME-Zarr stores, + the masks folder, `check`, `run`, `convert` and `RunInputs` (`mounts.MountCheck`). Where + no mount table can be read (macOS, Windows), only a mount of another device is seen. +- **Hard-linked images and masks are named** (owner, 3 November 2026): a file whose link + count is above 1 is the warning `hard_linked_file`; it is read as before. + - Cross-brick test of 3 November 2026 (plan §12 question 73): `masks` accepts per-plane masks with a stated shared 2-D mask (`--mask-axes Nuclei=YX`) as `run` does, instead of `mask_planes_differ` (F1); `run` needs no `--axes` when every image names its axes (F9); diff --git a/docs/contract.md b/docs/contract.md index 0d56ff7e..328cdc06 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -28,6 +28,8 @@ whole experiment; any change to them goes through the normalization brick. | Names | Every label and `position_id` passes `check_identity_text` (§16): no leading or trailing whitespace, and not `""`, `NA`, `nan`, `none`, `null`, `n/a` or `` in any case, which table readers turn into a missing value. A folder or image name that fails is the error `invalid_name` (decision D-e; accepted before D86). | | Depth | At most three folder levels below the root. A folder below the technical-repeat level is an error. | | Links | Contain (issue #81, owner 2 November 2026): a symbolic link to a file or folder (on Windows also a directory junction, which Python 3.11 does not report as a link) is followed only when its real path (`os.path.realpath`) is inside the experiment folder, itself resolved. A link that leaves it is not followed and is the error `link_outside_folder`, naming each link and its target (a broken link too); a link to a folder on its own path (the folder holding it, or one above) is `link_loop`; one file reached by two paths (a link and its target, two links, a linked folder and its target, or two hard links: the same device and inode, from the one `stat` taken per image or mask; owner, 2 November 2026) is `linked_twice`, naming both. A broken link that stays inside is kept and reported by its reader (`unreadable_tiff`, `unreadable_mask`), never skipped. Hidden and system names (§1) are skipped before any link is looked at, so a hidden link is never followed; a contained link *to* a hidden folder is followed, because that folder is itself never walked, so its files are found once, under the link's name, which the user chose. The links inside an OME-Zarr store are held to the same rule, though the store is not entered as a folder level: its folders are listed and only its links resolved, about 84 ms per 20,000 store files on each discovery (accepted by the owner, 2 November 2026). The masks folder follows the same rule (§15), with `RunInputs` (§9.1) and `convert` (§17) refusing the same links; one helper, `links.LinkGuard`, walks both folders. | +| Mount points | Refused (owner, 3 November 2026): a folder (or file) below the experiment folder or the masks folder that is a mount point is not entered and is the error `mounted_folder`, naming it. A mount point is a path `/proc/self/mountinfo` lists (Linux: the only way to see a bind mount of the same file system, for which `os.path.ismount` and the device number report an ordinary folder) or a folder on another device (`st_dev`) than the selected folder: another file system or Windows volume, or a Btrfs subvolume. The selected folder itself may be on any mount. The same rule holds inside OME-Zarr stores, and `check`, `run`, `convert` and `RunInputs` refuse the same mount points (nothing is published). Limits: where `/proc/self/mountinfo` cannot be read (macOS, BSD, Windows), a mount of the same device (BSD `nullfs`) is not seen; macOS `bindfs` (FUSE) and Windows mounted folders have another device, and a Windows mounted folder is also a mount-point reparse point, held to the link rule. The mounts are read once per walk. | +| Hard links | An image or mask whose link count is above 1 (`st_nlink`, from the one `stat` taken per image or mask; it has other names, perhaps outside the folder, through which it can change) is the warning `hard_linked_file`, naming it and its count (owner, 3 November 2026). It is read and fingerprinted as any other file; the files inside an OME-Zarr store are not counted. | | System files | Dot-files and dot-folders (`.DS_Store`, `._*`, `.ipynb_checkpoints`), `Thumbs.db`, `desktop.ini` and `__MACOSX` are ignored without a warning. | | Scope | A group is identified by its full label path, e.g. `(control, bio_rep1)`. | | Positions | Every image is exactly one position (field of view): files with several positions, scenes, series or tiles are refused (§2). Its `position_id` is the image name without its extension (`.tif`, `.tiff` or `.zarr`, any case, with a preceding `.ome`): `img_01.ome.tif` gives `img_01`; a name that is only an extension (`.tif`, `.ome.zarr`) is refused. `discovery.position_id` is the one function every MorphoBricks brick names positions with (cross-brick decision D85, 29 October 2026), so it also removes the other image extensions bricks read (`.png`, `.jpg`, `.jpeg`, `.bmp`, `.gif`, `.npy`; naming only: this brick still analyses TIFF and Zarr images only). A position is identified by `condition`, `biological_repeat`, `technical_repeat` and `position_id` (cross-brick decision D02, 26 October 2026), so equal names in two technical repeats are two positions. Two images in one leaf folder with the same `position_id`, ignoring case (`img_01.tif` and `img_01.ome.zarr`), are an error (`duplicate_position_id`): the brick does not guess which is which. | @@ -122,8 +124,8 @@ 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; 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); 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; nothing is published); 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 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); 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`, §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 | +| 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; 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; nothing is published); 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 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`, §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 suggests an action. Technical details go to the diagnostic log. The command line's log diff --git a/docs/user_guide.md b/docs/user_guide.md index 73632125..4bf804a3 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -154,6 +154,12 @@ experiment/ holds it. A link back to a folder above it is an error (`link_loop`), and so is one file reached by two paths, a link and its target or two hard links (`linked_twice`): it would count twice. +- A mounted folder inside the experiment or masks folder (another disk, a network share, or + a Linux bind mount of another folder) is an error (`mounted_folder`) naming it: its files + come from outside the folder you selected. Copy the files in, or select the mounted folder + itself. The selected folder may itself be on any disk or share. +- An image or mask that has other names (a hard link, perhaps outside the folder) is a + warning (`hard_linked_file`): it can change through a name the run does not see. To change a label, rename the folder and check again. @@ -987,6 +993,8 @@ continues. | `unreadable_folder` | Error | A folder could not be opened. Check permissions. | | `link_outside_folder` | Error | A link in the experiment or masks folder leads outside it (the message names each link and its target), and nothing outside the selected folder is read. Copy or move the files in, or select a folder that holds them, then remove the link. | | `link_loop` | Error | A link leads back to a folder that holds it. Remove it. | +| `mounted_folder` | Error | A folder (or file) in the experiment or masks folder is a mount point: another disk or share, or a bind mount of another folder, so its files come from outside the selected folder. It is named and not entered. Copy the files in, or select the mounted folder itself. Where the system's mount table cannot be read (macOS, Windows), only a mount of another device is seen. | +| `hard_linked_file` | Warning | An image or mask has other names (hard links; the message gives the number), perhaps outside the folder, so it can change through a name the run does not see. It is read as usual. Replace the hard link with a copy if it may change. | | `linked_twice` | Error | One file is reached by two paths (a link and its target, two links, or two hard links), so it would count twice under two names; both are named. Keep one path to each file. | | `reserved_folder_name` | Error | A folder is named `single` in another case (`Single`), or a folder named `single` has other folders beside it. Rename it to `single` in lower case, or give it a name of its own. | | `case_conflict` | Error | Folders of one level have names that differ only in case, next to each other or anywhere in the experiment (`control/Rep1` and `treated/rep1`). Spell each name one way, or rename or merge them. | diff --git a/pyproject.toml b/pyproject.toml index e6e11bd6..ec346077 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -176,6 +176,7 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.design_edit" = "pure" "morphobricks_preprocessing.discovery" = "io" "morphobricks_preprocessing.links" = "io" +"morphobricks_preprocessing.mounts" = "io" "morphobricks_preprocessing.folder_rules" = "io" "morphobricks_preprocessing.log_target" = "io" "morphobricks_preprocessing.image_type" = "pure" diff --git a/src/morphobricks_preprocessing/links.py b/src/morphobricks_preprocessing/links.py index 71ddc75b..f179d22f 100644 --- a/src/morphobricks_preprocessing/links.py +++ b/src/morphobricks_preprocessing/links.py @@ -10,11 +10,19 @@ followed, and is the error ``link_loop``; - two paths of one file (a link and its target, two links to it, or two hard links: the same device and inode) would count one image or mask twice under two names: the error - ``linked_twice`` names both (hard links: owner, 2 November 2026). - -Inside an OME-Zarr store, which discovery never enters as a folder level, every link is -held to the same rule (:meth:`LinkGuard.check_store`), since reading and copying the store -follow its links. + ``linked_twice`` names both (hard links: owner, 2 November 2026); +- a folder (or file) below the folder that is a mount point (another file system, or a + bind mount of another folder) brings in files from elsewhere without being a link: it is + not entered, and is the error ``mounted_folder`` naming it (:mod:`.mounts`; owner, + 3 November 2026). The folder walked may itself be on any mount; +- an image or mask whose link count is above 1 (``st_nlink``: it has other names, perhaps + outside the folder, through which it can change) is the warning ``hard_linked_file`` + naming it and its count (owner, 3 November 2026). It is read and fingerprinted as any + other file. + +Inside an OME-Zarr store, which discovery never enters as a folder level, every link and +mount point is held to the same rule (:meth:`LinkGuard.check_store`), since reading and +copying the store follow its links. A link is what Python reports as one: a symbolic link (``os.DirEntry.is_symlink``) and, on Windows, also a directory junction (a mount-point reparse point), which Python 3.11 does @@ -30,21 +38,25 @@ from collections.abc import Callable, Iterator from pathlib import Path -from .validation import ValidationIssue, agree, error, plural +from .mounts import MountCheck +from .validation import ValidationIssue, agree, error, plural, warning logger = logging.getLogger(__name__) LINK_OUTSIDE = "link_outside_folder" LINK_LOOP = "link_loop" LINKED_TWICE = "linked_twice" -LINK_CODES = frozenset({LINK_OUTSIDE, LINK_LOOP, LINKED_TWICE}) -"""The codes of the errors about links (all errors).""" +MOUNTED = "mounted_folder" +HARD_LINKED = "hard_linked_file" +LINK_CODES = frozenset({LINK_OUTSIDE, LINK_LOOP, LINKED_TWICE, MOUNTED}) +"""The codes of the errors about links and mount points (all errors; the warning +``hard_linked_file`` is not one of them).""" _JUNCTION = getattr(stat, "IO_REPARSE_TAG_MOUNT_POINT", None) class LinkRefusedError(ValueError): - """A folder holds links that are refused; the message names them.""" + """A folder holds links or mount points that are refused; the message names them.""" def is_link(entry: os.DirEntry) -> bool: @@ -146,19 +158,22 @@ def __init__(self, root: Path, what: str) -> None: # The real paths of the folders above each folder entered, the folder's own last. self._stacks: dict[Path, tuple[Path, ...]] = {root: (self.real_root,)} self._owners: dict[Path | tuple[int, int], str] = {} + self._mounts = MountCheck(self.real_root) self.outside: list[str] = [] self.loops: list[str] = [] self.twice: list[str] = [] + self.mounted: list[str] = [] + self.hard_linked: list[str] = [] def relative(self, path: Path) -> str: """Return a path of the walk relative to the root, with ``/`` separators.""" return path.relative_to(self.root).as_posix() or "." def real(self, entry: os.DirEntry, folder: Path) -> Path | None: - """Return the real path of an entry of ``folder``, or None for a refused link. + """Return the real path of an entry of ``folder``, or None when it is refused. ``folder`` is the root or a folder this guard let through. A folder let through can - be walked next. + be walked next. Refused: a link that leaves the root or loops, and a mount point. """ stack = self._stacks[folder] if not is_link(entry): @@ -172,6 +187,9 @@ def real(self, entry: os.DirEntry, folder: Path) -> Path | None: if target in stack: self.loops.append(shown) return None + if self._mounts.is_mount(target, entry): + self.mounted.append(self.relative(Path(entry.path))) + return None if entry.is_dir(): self._stacks[Path(entry.path)] = (*stack, target) return target @@ -181,17 +199,21 @@ def claim(self, path: Path, real: Path) -> str: A file is known by its device and inode (from the one ``stat`` this takes), so a hard link to an image already found is caught like a symbolic one; by its real - path when it cannot be stat'ed (a broken link) or has no inode. An OME-Zarr store - (a folder) also has its links checked (:meth:`check_store`). + path when it cannot be stat'ed (a broken link) or has no inode. A file with other + names (a link count above 1) is recorded for the warning ``hard_linked_file``. An + OME-Zarr store (a folder) also has its links and mount points checked + (:meth:`check_store`). """ try: found = os.stat(path) except OSError: # a broken link: refused or reported unreadable by its reader found = None + name = self.relative(path) if found is not None and stat.S_ISDIR(found.st_mode): self.check_store(path) + elif found is not None and found.st_nlink > 1: + self.hard_linked.append(f"{name} ({found.st_nlink} names)") key = (found.st_dev, found.st_ino) if found is not None and found.st_ino else real - name = self.relative(path) first = self._owners.setdefault(key, name) if first != name: self.twice.append(f"{name} (the same file as {first})") @@ -220,10 +242,11 @@ def files(self, folder: Path, skip: Callable[[str], bool]) -> Iterator[tuple[Pat yield Path(entry.path), real def check_store(self, store: Path) -> None: - """Hold every link inside an OME-Zarr store (a folder let through) to the rule. + """Hold every link and mount point inside an OME-Zarr store (let through) to the rule. A store can hold millions of chunk files: only its folders and its links are - followed up, so a file costs no more than its listing. + followed up, so a file costs no more than its listing (its files are looked up + among the mount points only when one is below the root). """ pending = [store] while pending: @@ -234,12 +257,9 @@ def check_store(self, store: Path) -> None: logger.debug("Cannot list %s", folder, exc_info=True) continue for entry in entries: - if is_link(entry): - if self.real(entry, folder) is not None and entry.is_dir(): - pending.append(Path(entry.path)) - elif entry.is_dir(follow_symlinks=False): - stack = self._stacks[folder] - self._stacks[Path(entry.path)] = (*stack, stack[-1] / entry.name) + if not (self._mounts.below or is_link(entry) or entry.is_dir()): + continue # a plain file: nothing to resolve + if self.real(entry, folder) is not None and entry.is_dir(): pending.append(Path(entry.path)) def issues(self) -> list[ValidationIssue]: @@ -280,11 +300,42 @@ def issues(self) -> list[ValidationIssue]: "replace it with a copy if both are really meant.", ) ) + return issues + self._mount_issues() + + def _mount_issues(self) -> list[ValidationIssue]: + """Return the error about mount points and the warning about hard-linked files.""" + issues = [] + if self.mounted: + issues.append( + error( + MOUNTED, + f"{plural(len(self.mounted), 'path')} in the {self.what} " + f"{agree(self.mounted, 'is a mount point', 'are mount points')} (another " + "file system, or a bind mount of another folder), so the files there " + "come from outside the selected folder; not entered.", + self.mounted, + f"Copy the files into the {self.what}, or select the mounted folder " + "itself, then unmount it from inside the folder.", + ) + ) + if self.hard_linked: + issues.append( + warning( + HARD_LINKED, + f"{plural(len(self.hard_linked), 'file')} in the {self.what} " + f"{agree(self.hard_linked, 'has', 'have')} other names (hard links), " + "perhaps outside it: the file can change through a name the run does not " + "see. It is read and checked as any other file.", + self.hard_linked, + "Replace the hard link with a copy if the file may change through its " + "other names.", + ) + ) return issues def refuse_links(issues: list[ValidationIssue] | tuple[ValidationIssue, ...]) -> None: - """Raise :class:`LinkRefusedError` when the issues hold an error about links. + """Raise :class:`LinkRefusedError` when the issues hold an error about links or mounts. Parameters ---------- @@ -294,7 +345,7 @@ def refuse_links(issues: list[ValidationIssue] | tuple[ValidationIssue, ...]) -> Raises ------ LinkRefusedError - Naming every refused link. + Naming every refused link and mount point. """ found = [issue for issue in issues if issue.code in LINK_CODES] if found: diff --git a/src/morphobricks_preprocessing/mounts.py b/src/morphobricks_preprocessing/mounts.py new file mode 100644 index 00000000..1656446c --- /dev/null +++ b/src/morphobricks_preprocessing/mounts.py @@ -0,0 +1,111 @@ +"""Mount points below the experiment and masks folders (owner decision, 3 November 2026). + +A mount point below the folder the user selected brings in the files of another place: +another file system, or (a Linux bind mount) another folder of the same file system. To +the walk it is an ordinary folder, not a link, so the rule of :mod:`.links` ("nothing +outside the selected folder is read") would not see it. Every folder (and file) below the +experiment folder or the masks folder that is a mount point is therefore refused, and +named: the error ``mounted_folder`` (:class:`.links.LinkGuard`). The selected folder +itself may be on any mount. + +A path is a mount point (:class:`MountCheck`) when + +- it is listed as one in ``/proc/self/mountinfo`` (Linux): this is the only way to see a + bind mount of the same file system, whose device and inode look like any folder's + (:func:`os.path.ismount` reports False for it); or +- it is a folder whose device (``st_dev``) is not the selected folder's: another file + system (on Windows, another volume), or a Btrfs subvolume, which is refused too. + +Limits: where ``/proc/self/mountinfo`` cannot be read (macOS, BSD, Windows, a Linux without +``/proc``), a mount of the same device is not detected (a BSD ``nullfs`` mount); macOS +``bindfs`` (FUSE) and Windows mounted folders have another device. On Windows a mounted +folder is also a mount-point reparse point, which :func:`.links.is_link` treats as a link. +The mounts are read once per walk: a mount made after the walk is not seen. +""" + +from __future__ import annotations + +import logging +import os +import re +from pathlib import Path + +logger = logging.getLogger(__name__) + +MOUNTINFO = "/proc/self/mountinfo" +"""The Linux kernel's list of the mounts this process sees.""" + +_OCTAL = re.compile(rb"\\([0-7]{3})") + + +def mount_points(table: str | os.PathLike = MOUNTINFO) -> frozenset[Path]: + r"""Return every mount point listed in a ``mountinfo`` table; none when it cannot be read. + + Parameters + ---------- + table + The table, ``/proc/self/mountinfo`` by default. + + Returns + ------- + frozenset of Path + The mount points (the fifth field of each line, its octal escapes (``\040`` + for a space) undone). + """ + try: + text = Path(table).read_bytes() + except OSError: + logger.debug("Cannot read %s", table, exc_info=True) + return frozenset() + points = set() + for line in text.splitlines(): + fields = line.split(b" ") + if len(fields) > 4: + points.add(Path(os.fsdecode(_OCTAL.sub(_unescape, fields[4])))) + return frozenset(points) + + +def _unescape(match: re.Match[bytes]) -> bytes: + """Return the byte an octal escape of ``mountinfo`` stands for.""" + return bytes([int(match.group(1), 8)]) + + +class MountCheck: + """Tell the mount points below one folder (resolved). + + Parameters + ---------- + real_root + The selected folder, its real path. It may itself be a mount point. + """ + + def __init__(self, real_root: Path) -> None: + self.real_root = real_root + try: + self.device: int | None = os.stat(real_root).st_dev + except OSError: # a folder that does not exist holds nothing + self.device = None + self.below = frozenset(point for point in mount_points() if real_root in point.parents) + + def is_mount(self, real: Path, entry: os.DirEntry) -> bool: + """Return True when an entry of the walk, at real path ``real``, is a mount point. + + Parameters + ---------- + real + The entry's real path, below the selected folder. + entry + The entry; a folder's device is read from it (``entry.stat()``). + + Returns + ------- + bool + """ + if real in self.below: + return True + if self.device is None or not entry.is_dir(): + return False + try: + return entry.stat().st_dev != self.device + except OSError: # cannot be read: its reader reports it + return False diff --git a/tests/test_mounts.py b/tests/test_mounts.py new file mode 100644 index 00000000..5bce0624 --- /dev/null +++ b/tests/test_mounts.py @@ -0,0 +1,259 @@ +"""Mount points below the experiment and masks folders are refused (owner, 3 November 2026). + +A Linux bind mount below the experiment is an ordinary folder to the walk, not a link, so +before the fix discovery walked into data from outside the selected folder. Hard-linked +images and masks (a link count above 1) are a warning only. + +The tests with a real bind mount need ``mount --bind`` (root on Linux) and are skipped by +name otherwise; the others replace the mount table, so they always run. +""" + +import os +import shutil +import subprocess +from pathlib import Path + +import numpy as np +import pytest + +from morphobricks_preprocessing import ( + RunInputs, + RunInputsError, + prefill_mapping, + run_experiment, +) +from morphobricks_preprocessing import mounts as mounts_module +from morphobricks_preprocessing.cli import main +from morphobricks_preprocessing.discovery import discover_experiment +from morphobricks_preprocessing.mounts import MountCheck, mount_points +from morphobricks_preprocessing.run import preflight +from morphobricks_preprocessing.testing import make_clean_run +from tiff_tree import write_data + + +def _errors(issues): + return sorted(issue.code for issue in issues if issue.is_error) + + +def _issue(issues, code): + return next(issue for issue in issues if issue.code == code) + + +@pytest.fixture +def tree(tmp_path): + """An experiment of two leaf folders, and a folder outside it holding one image.""" + root, outside = tmp_path / "exp", tmp_path / "outside" + for rel in ("ctrl/rep1/a.tif", "drug/rep1/a.tif"): + write_data(root / rel, np.zeros((4, 4), np.uint8), "YX") + write_data(outside / "far.tif", np.ones((4, 4), np.uint8), "YX") + return root, outside + + +# The mount table replaced: always runs ---------------------------------------------------- + + +@pytest.fixture +def mounted(monkeypatch): + """Make the given real paths read as mount points (as /proc/self/mountinfo lists them).""" + points: set[Path] = set() + + def table(*_args): + return frozenset(points) + + monkeypatch.setattr(mounts_module, "mount_points", table) + + def mount(path): + points.add(Path(os.path.realpath(path))) + + return mount + + +def test_a_mounted_folder_in_the_experiment_is_refused_naming_it(tree, mounted): + root, _ = tree + write_data(root / "drug/rep2/b.tif", np.zeros((4, 4), np.uint8), "YX") + mounted(root / "drug/rep2") + found = discover_experiment(root) + assert _errors(found.issues) == ["mounted_folder"] + issue = _issue(found.issues, "mounted_folder") + assert issue.paths == ("drug/rep2",) + assert "experiment folder" in issue.message and "mount point" in issue.message + assert "drug/rep2/b.tif" not in found.images + + +def test_the_experiment_folder_itself_may_be_a_mount_point(tree, mounted): + root, _ = tree + mounted(root) + found = discover_experiment(root) + assert found.ok and not found.issues + + +def test_a_folder_reached_through_a_link_onto_a_mount_point_is_refused(tree, mounted): + root, _ = tree + try: + (root / "drug/rep2").symlink_to(root / ".hidden", target_is_directory=True) + except (OSError, NotImplementedError): + pytest.skip("symbolic links cannot be made here") + write_data(root / ".hidden/b.tif", np.zeros((4, 4), np.uint8), "YX") + mounted(root / ".hidden") + found = discover_experiment(root) + assert _issue(found.issues, "mounted_folder").paths == ("drug/rep2",) + + +def test_check_and_run_refuse_a_mounted_folder_and_write_no_table(tree, mounted, tmp_path): + root, _ = tree + mounted(root / "drug/rep1") + out = tmp_path / "out" + assert main(["check", str(root), "--output", str(out)]) == 1 + result = run_experiment(root, prefill_mapping("YX"), output_folder=out) + assert not result.written + assert "mounted_folder" in _errors(result.issues) + assert not (out / "plane_histograms.csv").exists() + + +def test_a_mounted_folder_in_the_masks_folder_is_refused(tmp_path, mounted): + clean = make_clean_run(tmp_path / "run", axes="YX", shape=(16, 20), mask_types=("Cells",)) + mounted(clean.masks / "Cells/drug") + pre = preflight(clean.experiment, clean.output, masks_folder=clean.masks) + issue = _issue(pre.masks.issues, "mounted_folder") + assert issue.paths == ("Cells/drug",) and "masks folder" in issue.message + with pytest.raises(RunInputsError, match="mount point"): + RunInputs.open(clean.output) + + +def test_run_inputs_refuse_a_folder_mounted_after_the_run(tmp_path, mounted): + clean = make_clean_run(tmp_path / "run", axes="YX", shape=(16, 20)) + mounted(clean.experiment / "ctrl/rep1") + with pytest.raises(RunInputsError, match="ctrl/rep1"): + RunInputs.open(clean.output) + + +def test_a_mounted_folder_inside_an_ome_zarr_store_is_refused(tree, mounted): + pytest.importorskip("zarr") + from zarr_tree import write_ngff + + root, _ = tree + store = write_ngff(root / "ctrl/rep1/z.ome.zarr", np.zeros((1, 1, 4, 4), np.uint8)) + mounted(store / "0") + found = discover_experiment(root) + assert _issue(found.issues, "mounted_folder").paths == ("ctrl/rep1/z.ome.zarr/0",) + + +def test_a_folder_on_another_device_is_a_mount_point(tmp_path, monkeypatch): + # The portable rule, where no mount table can be read (macOS, Windows). + monkeypatch.setattr(mounts_module, "mount_points", lambda *_: frozenset()) + (tmp_path / "sub").mkdir() + check = MountCheck(tmp_path) + + class Entry: + def __init__(self, device): + self.device = device + + def is_dir(self): + return True + + def stat(self): + return os.stat_result((0o40755, 1, self.device, 2, 0, 0, 0, 0, 0, 0)) + + assert check.device is not None + assert not check.is_mount(tmp_path / "sub", Entry(check.device)) + assert check.is_mount(tmp_path / "sub", Entry(check.device + 1)) + + +def test_the_mount_table_is_read_with_its_escapes(tmp_path): + table = tmp_path / "mountinfo" + table.write_bytes( + b"22 1 0:21 / / rw - ext4 /dev/vda rw\n" + b"49 28 254:0 /data /mnt/my\\040data rw,relatime - ext4 /dev/vda rw\n" + b"short line\n" + ) + assert mount_points(table) == {Path("/"), Path("/mnt/my data")} + assert mount_points(tmp_path / "missing") == frozenset() + + +# A real bind mount (Linux, as root) -------------------------------------------------------- + + +def _bind(source, target): + """Bind-mount ``source`` on ``target``; skip by name when it cannot be done here.""" + if os.name != "posix" or shutil.which("mount") is None or os.geteuid() != 0: + pytest.skip("a bind mount needs mount --bind as root on Linux") + done = subprocess.run( + ["mount", "--bind", str(source), str(target)], capture_output=True, check=False + ) + if done.returncode != 0: + pytest.skip(f"mount --bind is not allowed here: {done.stderr.decode().strip()}") + + +@pytest.fixture +def bind(): + """Bind-mount folders for one test, and unmount them after it.""" + targets = [] + + def mount(source, target): + target.mkdir(parents=True, exist_ok=True) + _bind(source, target) + targets.append(target) + + yield mount + for target in reversed(targets): + subprocess.run(["umount", str(target)], check=False) + + +def test_a_real_bind_mount_of_the_same_file_system_is_refused(tree, bind, tmp_path): + root, outside = tree + bind(outside, root / "drug/rep2") + # The kernel's own checks see no mount point: same device, an inode of its own. + assert not os.path.ismount(root / "drug/rep2") + found = discover_experiment(root) + assert _issue(found.issues, "mounted_folder").paths == ("drug/rep2",) + assert "drug/rep2/far.tif" not in found.images + out = tmp_path / "out" + assert main(["check", str(root), "--output", str(out)]) == 1 + result = run_experiment(root, prefill_mapping("YX"), output_folder=out) + assert not result.written and "mounted_folder" in _errors(result.issues) + + +def test_a_real_bind_mount_inside_an_ome_zarr_store_is_refused(tree, bind): + pytest.importorskip("zarr") + from zarr_tree import write_ngff + + root, outside = tree + store = write_ngff(root / "ctrl/rep1/z.ome.zarr", np.zeros((1, 1, 4, 4), np.uint8)) + bind(outside, store / "extra") + found = discover_experiment(root) + assert _issue(found.issues, "mounted_folder").paths == ("ctrl/rep1/z.ome.zarr/extra",) + + +# Hard-linked images and masks: a warning only ---------------------------------------------- + + +def _hard_link(source, target): + try: + os.link(source, target) + except (OSError, NotImplementedError): + pytest.skip("hard links cannot be made here") + + +def test_an_image_with_another_name_outside_is_a_warning(tree, tmp_path): + root, outside = tree + _hard_link(root / "ctrl/rep1/a.tif", outside / "other-name.tif") + found = discover_experiment(root) + assert found.ok + issue = _issue(found.issues, "hard_linked_file") + assert not issue.is_error and issue.paths == ("ctrl/rep1/a.tif (2 names)",) + assert found.images == ("ctrl/rep1/a.tif", "drug/rep1/a.tif") + out = tmp_path / "out" + assert main(["check", str(root), "--output", str(out)]) == 0 + result = run_experiment(root, prefill_mapping("YX"), output_folder=out) + assert result.written + assert "hard_linked_file" in {issue.code for issue in result.issues} + + +def test_a_hard_linked_mask_is_a_warning(tmp_path): + clean = make_clean_run(tmp_path / "run", axes="YX", shape=(16, 20), mask_types=("Cells",)) + _hard_link(clean.masks / "Cells/ctrl/rep1/a.tif", tmp_path / "elsewhere.tif") + pre = preflight(clean.experiment, clean.output, masks_folder=clean.masks) + issue = _issue(pre.masks.issues, "hard_linked_file") + assert issue.paths == ("Cells/ctrl/rep1/a.tif (2 names)",) + assert not [issue for issue in pre.masks.issues if issue.is_error] + RunInputs.open(clean.output) # unchanged: a warning does not refuse the run From bc60d2c772e10f228196d1ebe0582ff6d47a2331 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 11:47:30 +0000 Subject: [PATCH 2/4] Flush the output folder right after the commit rename (I2) _commit() replaced the manifest without an fsync of the output folder; the flush came only from _finish_committed. The folder is now flushed in _commit, right after os.replace, so the commit is on the disk when it returns. The flush a recovering holder of the lock needs moved to finish_publication. A test records the order of renames and folder flushes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- CHANGELOG.md | 3 +++ docs/contract.md | 4 +-- .../run/generation.py | 14 ++++++---- tests/test_publication.py | 26 +++++++++++++++++++ 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fb7754e..8c8570d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -106,6 +106,9 @@ and the bricks' migration). selected folder, is the error `mounted_folder` naming it, in discovery, OME-Zarr stores, the masks folder, `check`, `run`, `convert` and `RunInputs` (`mounts.MountCheck`). Where no mount table can be read (macOS, Windows), only a mount of another device is seen. +- **The commit is flushed to disk** (review of rc4, I2): the output folder is flushed + (`fsync`, POSIX) right after the manifest's rename, so a power cut after a run returned + keeps the new generation. A run that finishes an earlier publication flushes it first. - **Hard-linked images and masks are named** (owner, 3 November 2026): a file whose link count is above 1 is the warning `hard_linked_file`; it is read as before. diff --git a/docs/contract.md b/docs/contract.md index 328cdc06..f376edf2 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -310,10 +310,10 @@ the bricks' tests. | 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) 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. An image that changed or went missing at any of these points is the error `source_changed_during_run` naming it: nothing is published, 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. | | 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. 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`. | +| 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 (the manifest in place is the record's: the remaining moves and removals are done) or deletes an uncommitted `.publish` (the earlier generation is then untouched) (`run.generation.finish_publication`). 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. | | What readers see | **Through the manifest** (`RunInputs`): exactly one generation, the old or the new one, never a mix: each file the manifest names is read by its SHA-256 from the output folder or, while a publication is unfinished, from `.publish` (`run.generation.published_path`), and the read is repeated if the manifest changed meanwhile (`read_stable`). **A plain reader of `plane_histograms.csv` alone** always reads one whole table of one generation (each move is one rename): the old one until it is moved. A reader that opens several output files without the manifest can pair files of two generations while a publication's step 3 runs or after a kill in it, until the next holder of the lock finishes it: read several files through `RunInputs`, or check `csv_sha256` and `experiment_design_sha256`. | -| Windows | The same protocol. `os.replace` replaces a file atomically on NTFS too, but fails while another program has the file open: at the commit nothing has changed then (the run raises, the earlier generation is kept); after it, `PublicationIncompleteError` as above. Folders are not flushed (Windows cannot open them); the kill tests (SIGKILL at every step) run on POSIX only. | +| Windows | The same protocol. `os.replace` replaces a file atomically on NTFS too, but fails while another program has the file open: at the commit nothing has changed then (the run raises, the earlier generation is kept); after it, `PublicationIncompleteError` as above. Folders are not flushed (Windows cannot open them; NTFS journals the rename itself, but a power cut right after the commit may bring back the earlier manifest, which is then a whole earlier generation); the kill tests (SIGKILL at every step) run on POSIX only. | ## 10. Converting TIFF images to OME-Zarr (removed in v0.4.0) diff --git a/src/morphobricks_preprocessing/run/generation.py b/src/morphobricks_preprocessing/run/generation.py index fed4486a..d7934ee0 100644 --- a/src/morphobricks_preprocessing/run/generation.py +++ b/src/morphobricks_preprocessing/run/generation.py @@ -9,9 +9,10 @@ ``.publish/publication.json`` naming every file with its SHA-256 (and the files the generation removes, such as ``mask_relationships.json``). 2. **Commit, the one atomic step:** the new ``run_manifest.json`` replaces the old one - (:func:`os.replace`). Before it nothing in the output folder has changed: a run - killed then leaves the earlier generation exactly as it was, and the next run discards - the publication folder. After it the new generation is the current one. + (:func:`os.replace`), and the output folder is flushed to disk (POSIX), so a power cut + after the commit keeps the new generation. Before it nothing in the output folder has + changed: a run killed then leaves the earlier generation exactly as it was, and the + next run discards the publication folder. After it the new generation is the current one. 3. The other files are moved into place (:func:`os.replace` each), the removed files removed, and the publication folder deleted. @@ -105,7 +106,8 @@ def _commit(output: Path, staged: Path, removes: tuple[str, ...]) -> None: """Write the publication record, then replace the manifest: the one atomic step. Nothing in the output folder changes before the replacement, which either happens - whole or not at all. + whole or not at all. The output folder is flushed right after it (POSIX), so a power + cut after the commit returns cannot bring back the earlier manifest. """ names = [name for name in (*MOVED_IN_ORDER, MANIFEST_FILENAME) if (staged / name).exists()] if MANIFEST_FILENAME not in names: @@ -122,6 +124,8 @@ def _commit(output: Path, staged: Path, removes: tuple[str, ...]) -> None: _flush_folder(staged) give_output_mode(staged / MANIFEST_FILENAME, output / MANIFEST_FILENAME) os.replace(staged / MANIFEST_FILENAME, output / MANIFEST_FILENAME) + # The rename is durable only once the output folder's entries are on the disk. + _flush_folder(output) def finish_publication(output: Path) -> str | None: @@ -163,6 +167,7 @@ def finish_publication(output: Path) -> str | None: shutil.rmtree(staged) return "discarded" logger.info("Finishing the publication an earlier run left in %s", staged) + _flush_folder(output) # the earlier commit on the disk before any move _finish_committed(output, staged) return "finished" @@ -171,7 +176,6 @@ def _finish_committed(output: Path, staged: Path) -> None: """Move a committed generation's files into place, remove what it removes, clean up.""" record = _record(staged) or {"removes": []} try: - _flush_folder(output) for name in MOVED_IN_ORDER: if (staged / name).exists(): give_output_mode(staged / name, output / name) diff --git a/tests/test_publication.py b/tests/test_publication.py index 0b9f2c75..f7af3a32 100644 --- a/tests/test_publication.py +++ b/tests/test_publication.py @@ -202,6 +202,32 @@ def broken(path, *args, **kwargs): assert "Cannot remove the publication folder" in caplog.text +def test_the_output_folder_is_flushed_right_after_the_commit(tmp_path, monkeypatch): + # Review of rc4 (I2): the manifest's rename was not followed by an fsync of the folder, + # so a power cut after a run returned could bring back the earlier manifest. + root, out = first_generation(tmp_path) + generation = run_module.generation + calls = [] + original_replace, original_flush = os.replace, generation._flush_folder + + def replace(source, destination): + calls.append(("replace", Path(destination))) + return original_replace(source, destination) + + def flush_folder(folder): + calls.append(("flush_folder", Path(folder))) + return original_flush(folder) + + monkeypatch.setattr(os, "replace", replace) + monkeypatch.setattr(generation, "_flush_folder", flush_folder) + assert run_experiment(root, output_folder=out, **NEXT).written + commit = calls.index(("replace", out / MANIFEST_FILENAME)) + assert calls[commit - 1] == ("flush_folder", out / PUBLISH_FOLDER) + assert calls[commit + 1] == ("flush_folder", out) + moves = [i for i, call in enumerate(calls) if call[0] == "replace" and i > commit] + assert moves and all(i > commit + 1 for i in moves) + + # Killed between any two steps (POSIX) ---------------------------------------------------- CHILD = """ From 14aa0c8219fba11184ec057768e88c52e7f6943e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 11:56:54 +0000 Subject: [PATCH 3/4] State the metadata-fingerprint identity rule; add opt-in content verification (I3) (a) The contract, user guide, AGENTS.md, CHANGELOG and docstrings (manifest, fingerprint, inventory, run_experiment, RunInputs) now name the identity rule for reuse, resume and RunInputs: the metadata fingerprint (size and mtime_ns; for OME-Zarr the inventory). They also name its known blind spot, a same-size rewrite with the modification time set back. (b) Opt-in strict reuse: RunMode(name, verify_content=True) for run_experiment(mode=...), CLI --verify-content / --no-verify-content, and the notebook's "Verify each image's content" box. Each image's content SHA-256 (TIFF bytes; OME-Zarr: per-file SHA-256s over the files its inventory covers) is recorded in files[].content_sha256 and joins the row key, so earlier and staged rows are reused only for the same content. An image read must keep it after the read (else source_changed_during_run). RunInputs.open of such a run checks it once. settings.verify_content is a recorded input: turning it on or off recomputes every row, is reported as an input change, and discards staged rows of the other mode with the reason. The default run is unchanged. RunMode carries the flag through mode=, because run_experiment's parameter count is at its shrink-only baseline. package_versions moved to versions.py (re-exported) to keep manifest.py under 600 lines. New code: mounts-free source_content.py and inventory.store_files. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- AGENTS.md | 20 +- CHANGELOG.md | 11 + docs/contract.md | 17 +- docs/user_guide.md | 17 +- pyproject.toml | 2 + src/morphobricks_preprocessing/__init__.py | 2 + src/morphobricks_preprocessing/cli_run.py | 14 +- src/morphobricks_preprocessing/fingerprint.py | 9 + src/morphobricks_preprocessing/inventory.py | 36 ++- src/morphobricks_preprocessing/manifest.py | 51 ++-- .../notebook_options.py | 7 + .../notebook_prefill.py | 4 + .../notebook_run.py | 14 +- .../notebook_run_plan.py | 1 + .../run/__init__.py | 3 +- src/morphobricks_preprocessing/run/modes.py | 51 ++++ .../run/pipeline.py | 19 +- .../run/recorded.py | 11 +- src/morphobricks_preprocessing/run/resume.py | 18 +- src/morphobricks_preprocessing/run/reuse.py | 33 ++- src/morphobricks_preprocessing/run/staging.py | 18 +- src/morphobricks_preprocessing/run/steps.py | 7 +- src/morphobricks_preprocessing/run_inputs.py | 8 +- .../source_content.py | 117 +++++++++ src/morphobricks_preprocessing/versions.py | 35 +++ tests/test_notebook_cards.py | 16 ++ tests/test_verify_content.py | 222 ++++++++++++++++++ 27 files changed, 681 insertions(+), 82 deletions(-) create mode 100644 src/morphobricks_preprocessing/source_content.py create mode 100644 src/morphobricks_preprocessing/versions.py create mode 100644 tests/test_verify_content.py diff --git a/AGENTS.md b/AGENTS.md index ae5385d2..b6a4c25f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -101,7 +101,11 @@ src/morphobricks_preprocessing/ applied to every file, missing axes as singletons only when confirmed) axis_review.py review_axes and check_mapping (moved from axes.py in PP-4) inventory.py source_signature/source_fingerprint: a file's size and mtime, an OME-Zarr - folder's total size, newest mtime and inventory digest (issue #86; PP-12) + folder's total size, newest mtime and inventory digest (issue #86; PP-12); + store_files (the files an OME-Zarr fingerprint covers) + source_content.py verify_content: a source's content SHA-256 (TIFF bytes; OME-Zarr per-file + SHA-256s), with_content, content_changed (RunInputs of a strict run) + versions.py package_versions (moved from manifest.py, which re-exports it) fingerprint.py Fingerprint (size, mtime, inventory) and RowKey: when a file's rows are reused (D5); confirm_axes/unchanged_confirmation (a confirmation of the files as they are) recorded_sizes.py RecordedSize: pixel sizes by role with unit and source (manifest v2): @@ -134,7 +138,7 @@ src/morphobricks_preprocessing/ run/ the run (a package since PP-3; `morphobricks_preprocessing.run` re-exports its public names): preflight.py (Preflight, preflight, sample_planes), pipeline.py (run_experiment, incremental), steps.py (header checks, - rows, outcome), modes.py (restart/full), recorded.py (inputs reused from + rows, outcome), modes.py (restart/full, RunMode with verify_content), recorded.py (inputs reused from the earlier run, InputChange), lock.py (the output folder's O_EXCL lock, remove_stale_lock), output_guard.py (the output folder against the masks folder, given or recorded, before anything is written), @@ -444,17 +448,21 @@ functions the tests use. - **A file is unchanged** when its size, mtime (an OME-Zarr image: also its inventory digest, `inventory_sha256`, issue #86), labels and axis layout match and the table still holds all its rows. Unchanged rows are reused; new and changed files are read; - deleted, renamed or excluded files lose their rows. + deleted, renamed or excluded files lose their rows. The fingerprint is metadata: a + same-size rewrite with its mtime set back is not seen (the stated blind spot). The opt-in + `RunMode(verify_content=True)` / `--verify-content` adds each source's content SHA-256 to + the entry and the row key (`source_content`); a recorded input, and changing it rebuilds. - **Full rebuild** happens in `mode="full"` (PP-7; `force_rebuild` was removed), when there is no readable manifest, when - the dtype, bin count, histogram settings (`RunSettings.histogram`) or axis mapping - changed, or when the CSV's hash no longer matches. + the dtype, bin count, histogram settings (`RunSettings.histogram`), `verify_content` or + axis mapping changed, or when the CSV's hash no longer matches. Changing only the memory limit or the exclusions recomputes nothing. - **Writes (PP-12, contract §9.4):** each image's rows are staged in `.staging` as soon as it is read (an interrupted run resumes from the staged rows whose key still matches; anything else is discarded and reported). The outputs are one generation, written into `.publish` and committed by the manifest's `os.replace` (the one atomic step), then moved - into place; the next holder of the lock finishes or discards an interrupted publication. + 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 published if any error is found. diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c8570d1..f492ba94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,6 +100,17 @@ and the bricks' migration). ### Fixed +- **The identity rule is stated, and strict reuse is offered** (review of rc4, I3; owner, 3 + November 2026): reuse, resume and `RunInputs` identify a source by its metadata + fingerprint (size and `mtime_ns`; for OME-Zarr the inventory of paths, sizes and times), + whose known blind spot is a same-size rewrite with the time set back; the contract, the + user guide and the docstrings now say so. The opt-in `RunMode(verify_content=True)` (CLI + `--verify-content`, the notebook's "Verify each image's content" box) records each + source's content SHA-256 (`files[].content_sha256`, `settings.verify_content`) and + reuses earlier or staged rows only for the same content; `RunInputs` of such a run checks + it at `open`. It is a recorded input; turning it on or off recomputes every row and is + named. The default run is unchanged. + - **Mount points below the experiment or masks folder are refused** (owner, 3 November 2026): a bind mount is an ordinary folder to the walk, so its files from outside were read. A folder (or file) listed in `/proc/self/mountinfo`, or on another device than the diff --git a/docs/contract.md b/docs/contract.md index f376edf2..d19f4bb2 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -173,10 +173,10 @@ interpretation. | `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). | +| `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: the run verified content, below; false when absent). | | `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; null when absent, in manifests written before it), `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. | +| `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; null when absent, in manifests written before it), `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`). | | `csv_sha256` | SHA-256 of the table written with this manifest. | | `experiment_design_sha256` | SHA-256 of the `experiment_design.csv` written with this run (§12), so later bricks can check that the design they read is the one used; null when the run had no design, or absent in manifests written before it. Optional key: the manifest stays version 1. | | `masks` | The masks paired with the images (§15), when the run was given a masks folder or its OME-Zarr images have label images (always used): `folder` (absolute path of the masks folder, null when only OME-Zarr label images were used), `types` (sorted mask-type names) and `files` (one entry per image and mask type, sorted by image then type: `image`, `type`, `path`, `source` `folder` or `ome-zarr`, `axes` and `shape`: the mask's axes and lengths without its length-1 axes other than Y and X, `YX` for a 2D mask (`[1, 512]` for a mask one pixel high), with `Z` and/or `T` for a mask with depth slices or time points, e.g. `TYX` and `[50, 512, 512]`; §15). Absent when the run had no masks. Optional key: the table does not depend on the masks. `axes` and `shape` are absent in manifests written before them, whose masks are all 2D; a later brick must read `axes` before using a mask and never apply a 2D mask to several planes unless `shared` is true. **Version 2:** `axes` are the mask's axes as stored (its header's letters: `IYX` for a plain stack), and each mask entry also has `size` and `mtime_ns` (its fingerprint, as `source_fingerprint` gives it: the mask file, or the label image's folder), `roles` (one per axis of `axes`, what each is read as: `TYX` for that plain stack stated `--mask-axes Cells=TYX`), `axes_from` (`header`, or `stated` when the axes were stated for its mask type) and `shared` (true only for a 2-D mask stated `YX` for its type: one plane shared by every plane of its image). A mask is read as stored, never reordered. | @@ -187,9 +187,10 @@ interpretation. | `conversion_record_sha256` | SHA-256 of the conversion record (`.morphobricks-conversion.json`) in the experiment root when the experiment is a tree of named-axis copies (design v0.4 §5, §17); null otherwise. `RunInputs` checks it (owner decision of 2 November 2026, plan question 59): a record changed or gone since the run is refused, as for `calibration_sha256`. A changed record changes no row (the copies are checked by their fingerprints like any image). | | `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). | +| 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`); 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 once more per run, and once more for each image read and each `RunInputs.open`; the default run is unchanged. Not covered: masks keep the fingerprint rule; a change between the header read and the hashing, or after the hashing of a reused image, with its metadata kept. | | 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`, `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. | +| 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. | Channel names and significant bits come from OME metadata. Significant bits are for @@ -212,7 +213,9 @@ is inside its experiment (§15's folder rules, audit B1: a run recorded before t refused as a new one would be), an image or mask changed or gone since the run, and a mask recorded without axes (a v0.4.0rc1 run wrote `axes: ""` for a mask whose header puts X before Y; the message says to save the mask again with `numpy.swapaxes` and run again, never "changed or missing"); every read checks the file's -fingerprint again. The copies' SHA-256 is checked once, when the run is opened, for every +fingerprint again (metadata: a rewrite of the same size with its time set back is not seen; +for a run with content verification, §9, `open` also checks every image's `content_sha256`). +The copies' SHA-256 is checked once, when the run is opened, for every image the run recorded (`conversion_check`): each copy is read whole then, so opening a run on named-axis copies costs one read of those copies (the excluded images' copies are not read). Checking on every read instead would read a copy whole for each of its planes. @@ -278,7 +281,7 @@ the bricks' tests. |---|---| | Modes | `run_experiment(mode=...)`, CLI `--mode`: `restart` (default) reuses the rows whose `RowKey` is unchanged (§9) and reads new or changed files; `full` recomputes every row and check and reuses no row. Both keep the user's confirmations (design, axis and mask-axis statements, stated calibration, image type) and publish only on success; a failed or stopped run leaves the last good run intact. `force_rebuild` and `--force-rebuild` were removed. | | Files a run replaces or removes | Only `plane_histograms.csv`, `run_manifest.json`, `experiment_design.csv` (when the run has a design), the report files, `mask_relationships.json`, the publication folder `.publish` and the staging folder `.staging` (§9.4); never `calibration.csv` (§9.3). `mask_relationships.json` is removed by a `full` run and by a `restart` whose masks entry (§9) differs from the earlier run's (or when no v2 manifest records it). Source images, masks and anything else are never touched. | -| Recorded inputs | One output folder belongs to one experiment. An input not given is taken from the earlier v2 manifest: experiment root, axis mapping with the per-file plain-stack confirmations at their recorded fingerprints (a new or changed plain stack must be confirmed again), exclusions, masks folder and mask-axis statements (statements given are added per mask type), image type, design (`experiment_design.csv` in the output folder), bins (not across pixel types), histogram and intensity settings, `missing_as_singleton`; the stated calibration is the output folder's `calibration.csv` (§9.3). A value given that differs is used and reported as a change with its effect (rows recomputed, or none); for the design, a design given whose answers (repeat group, confirmation, control) differ for folders in both, or `experiment_design.csv` edited since the earlier run wrote it; for the calibration, `calibration.csv` added, edited or removed since the earlier run read it (`calibration`, "no row is affected; the pixel sizes of N images change"). Masks are dropped only with `masks_folder=False` / `--no-masks`, which also drops the mask-axis statements recorded for the masks folder's masks (those of OME-Zarr label images are kept; before PP-10c the recorded statements stayed and the run was refused with `invalid_mask_axes`); exclusions are cleared with `exclude=[]` / `--no-exclude`. A version 1 manifest gives nothing. | +| Recorded inputs | One output folder belongs to one experiment. An input not given is taken from the earlier v2 manifest: experiment root, axis mapping with the per-file plain-stack confirmations at their recorded fingerprints (a new or changed plain stack must be confirmed again), exclusions, masks folder and mask-axis statements (statements given are added per mask type), image type, design (`experiment_design.csv` in the output folder), bins (not across pixel types), histogram and intensity settings, `missing_as_singleton`, `verify_content`; the stated calibration is the output folder's `calibration.csv` (§9.3). A value given that differs is used and reported as a change with its effect (rows recomputed, or none); for the design, a design given whose answers (repeat group, confirmation, control) differ for folders in both, or `experiment_design.csv` edited since the earlier run wrote it; for the calibration, `calibration.csv` added, edited or removed since the earlier run read it (`calibration`, "no row is affected; the pixel sizes of N images change"). Masks are dropped only with `masks_folder=False` / `--no-masks`, which also drops the mask-axis statements recorded for the masks folder's masks (those of OME-Zarr label images are kept; before PP-10c the recorded statements stayed and the run was refused with `invalid_mask_axes`); exclusions are cleared with `exclude=[]` / `--no-exclude`. A version 1 manifest gives nothing. | | Experiment root | `run_experiment(experiment_root=None, output_folder=...)` and CLI `run --output FOLDER` use the recorded experiment root; without one, `ValueError` (API) or `experiment_not_recorded` (CLI). Another root than the recorded one is a change: every row is read again, and the recorded plain-stack confirmations are not applied there (each plain stack must be confirmed again). | | `masks` command | Uses the recorded inputs the same way (plan §12 question 56): the experiment argument is optional when `--output` holds a v2 manifest recording it (`experiment_not_recorded` otherwise), and the masks folder, mask-axis statements and exclusions are the recorded ones unless given (`--no-exclude` clears the exclusions). A value given that differs is used for that scan and printed as a change; the manifest is not changed (the next run records only what it is given). The pairing with the recorded inputs, the scan and the write under the lock are `run.mask_step.prepare_mask_step(experiment_root, output_folder, exclude=, masks_folder=, mask_axes=)` (a `MaskStep`: the preflight, the output folder, the input changes, the blocking issues, or the `experiment_not_recorded` refusal) and `run.mask_step.describe_masks(step, thresholds=, progress=)` (a `MaskStepResult`: the report and the file written, or the `output_locked` refusal with no mask read and nothing written: the lock is taken before the first mask is read and held to the write; `ValueError` for masks whose pixels cannot be used), shared with the notebook (v0.4.0, PP-10d); the command only prints their results. | | Axis roles | `mapping=None` uses the recorded roles; without any, the run stops with `axes_not_confirmed`. CLI `--axes` is needed for the first run of an experiment with plain stacks and for plain stacks that are new or changed; when every readable image names all its axes (`header_axes`) and the suggested mapping gives each a role, the first run uses those roles (design v0.4 §3; cross-brick test F9). `convert` always needs `--axes` (§17). | @@ -307,7 +310,7 @@ the bricks' tests. | Topic | Rule | |---|---| | 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) 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. | +| 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. An image that changed or went missing at any of these points is the error `source_changed_during_run` naming it: nothing is published, 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. | | 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`. | diff --git a/docs/user_guide.md b/docs/user_guide.md index 4bf804a3..c46145a3 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -91,7 +91,11 @@ Google Drive is wiped when it disconnects (after some idle time, or after severa - **Unchanged images read again.** An image counts as unchanged when its size and modification time are the same as at the last run. If Google Drive reports a file's time differently later (for example after the file was synced again from another - computer), the image is read again. That costs time but never reuses wrong rows. + 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. ## 2. Organising the experiment folder @@ -461,6 +465,12 @@ results or comparing another grouping. or changed ones; **Full** reads every image again and recomputes every row and check. Both keep your confirmations (labels, design, axis roles, image type, stated pixel sizes) and write the table only when the whole run succeeds. + - **Verify each image's content (SHA-256) before reusing its rows** (off by default): an + image counts as unchanged when its size and modification time are, so a file rewritten + with the same size and its time set back would keep its earlier rows. With the box + ticked, the run records each image's SHA-256 and reuses rows only for the same content; + every image is read once more. The choice is recorded and kept by later runs; turning + it on or off reads every image again. - Click **Compute histograms**. A progress bar shows which image is being read. Tiles then summarise planes written, images read, kept, new, changed, removed and excluded, and the status names the mode, the files replaced and the inputs that changed. The @@ -632,6 +642,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 | | `--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 | @@ -914,8 +925,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) 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; 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 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/pyproject.toml b/pyproject.toml index ec346077..c917df58 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -182,6 +182,8 @@ baseline = "tools/code_rules_baseline.json" "morphobricks_preprocessing.image_type" = "pure" "morphobricks_preprocessing.intensity" = "io" "morphobricks_preprocessing.manifest" = "io" +"morphobricks_preprocessing.versions" = "io" +"morphobricks_preprocessing.source_content" = "io" "morphobricks_preprocessing.inventory" = "io" "morphobricks_preprocessing.fingerprint" = "io" "morphobricks_preprocessing.narrow_ranges" = "pure" diff --git a/src/morphobricks_preprocessing/__init__.py b/src/morphobricks_preprocessing/__init__.py index 54fb4e84..1836a57e 100644 --- a/src/morphobricks_preprocessing/__init__.py +++ b/src/morphobricks_preprocessing/__init__.py @@ -198,6 +198,7 @@ from .report import GroupSummary, group_summaries from .run import ( Preflight, + RunMode, RunResult, SamplePlane, edit_design, @@ -302,6 +303,7 @@ "RecordedSize", "RepeatBox", "RunManifest", + "RunMode", "RunResult", "RunInputs", "RunInputsError", diff --git a/src/morphobricks_preprocessing/cli_run.py b/src/morphobricks_preprocessing/cli_run.py index 47dbb6eb..704a70ce 100644 --- a/src/morphobricks_preprocessing/cli_run.py +++ b/src/morphobricks_preprocessing/cli_run.py @@ -4,7 +4,8 @@ ``--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. Given only ``--output``, ``run`` uses the recorded +``--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). Also the helpers every command shares: :func:`parse_size` and :func:`command_preflight`. """ @@ -51,7 +52,7 @@ mapping_summary, run_summary, ) -from .run import MODES, RESTART, Preflight, preflight, run_experiment +from .run import MODES, RESTART, Preflight, RunMode, preflight, run_experiment from .run.lock import OutputLockedError, lock_issue, output_lock from .run.output_guard import output_masks_issues from .run.recorded import usable @@ -190,6 +191,13 @@ def _run_inputs(run: argparse.ArgumentParser) -> None: help="restart (default): reuse the rows of unchanged images; full: recompute every " "row and check. Both keep your confirmations and replace the results only on success", ) + run.add_argument( + "--verify-content", + 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)", + ) def _run_settings(run: argparse.ArgumentParser) -> None: @@ -274,7 +282,7 @@ def run_command(args: argparse.Namespace) -> int: memory_limit=args.memory_limit, missing_as_singleton=args.missing_as_singleton, exclude=[] if args.no_exclude else args.exclude or None, - mode=args.mode, + mode=RunMode(args.mode, args.verify_content), progress=print_progress, design=design, masks_folder=False if args.no_masks else args.masks, diff --git a/src/morphobricks_preprocessing/fingerprint.py b/src/morphobricks_preprocessing/fingerprint.py index 9b5cc903..c14240c3 100644 --- a/src/morphobricks_preprocessing/fingerprint.py +++ b/src/morphobricks_preprocessing/fingerprint.py @@ -9,6 +9,11 @@ - :class:`RowKey`: everything a row depends on. A row is reused only when its file is unchanged and its roles, ``axes_from`` and every setting that changes rows are unchanged (bins, histogram range, robust quantiles, range-check limit, the path it is read from). + +"Unchanged" means the same fingerprint: the identity rule is metadata. Its known blind spot +is a file rewritten with the same size whose modification time (for an OME-Zarr folder, +every file's) is set back: its earlier rows would be reused. A run with ``verify_content`` +closes it: the key then also holds the content SHA-256 (:mod:`.source_content`). """ from __future__ import annotations @@ -113,6 +118,8 @@ class RowKey: Bins per plane. histogram The histogram settings (range, robust quantiles, range-check limit), as data. + content_sha256 + The file's content SHA-256 in a ``verify_content`` run; None otherwise. """ relative_path: str @@ -125,6 +132,7 @@ class RowKey: read_path: str bin_count: int histogram: tuple + content_sha256: str | None = None @classmethod def of(cls, entry: FileEntry, settings: RunSettings, experiment_root: str) -> RowKey: @@ -150,6 +158,7 @@ def of(cls, entry: FileEntry, settings: RunSettings, experiment_root: str) -> Ro read_path=(Path(experiment_root) / entry.relative_path).as_posix(), bin_count=settings.bin_count, histogram=tuple(sorted(_frozen(settings.histogram.to_dict()).items())), + content_sha256=entry.content_sha256, ) diff --git a/src/morphobricks_preprocessing/inventory.py b/src/morphobricks_preprocessing/inventory.py index 1c7511ca..c42887cd 100644 --- a/src/morphobricks_preprocessing/inventory.py +++ b/src/morphobricks_preprocessing/inventory.py @@ -10,8 +10,9 @@ time stay the same. What no metadata can see is a file rewritten with the same size whose modification time -is then set back to its earlier value (as for a TIFF file): only hashing the content would, -and design v0.4 §1 keeps the fingerprint metadata only. +is then set back to its earlier value (as for a TIFF file): only hashing the content would. +Design v0.4 §1 keeps the fingerprint metadata only by default; a run that verifies content +(``verify_content``, :mod:`.source_content`) also records and checks each source's SHA-256. Moved from :mod:`.manifest` (PP-12), which re-exports :func:`source_fingerprint`. """ @@ -57,6 +58,32 @@ def source_signature(path: str | os.PathLike) -> tuple[int, int, str | None]: if not path.is_dir(): stat = path.stat() return stat.st_size, stat.st_mtime_ns, None + newest, found = store_files(path) + digest = hashlib.sha256() + for name, stat in found: + digest.update(os.fsencode(name) + f"\0{stat.st_size}\0{stat.st_mtime_ns}\n".encode()) + return sum(stat.st_size for _, stat in found), newest, digest.hexdigest() + + +def store_files(path: Path) -> tuple[int, list[tuple[str, os.stat_result]]]: + """Return the newest time and the files of an OME-Zarr folder, as its fingerprint sees them. + + Parameters + ---------- + path + The OME-Zarr folder; its root's ``labels`` group is left out. + + Returns + ------- + tuple + ``(newest mtime_ns, files)``: the files as ``(path relative to the folder with + "/", stat)``, sorted by path; the newest time includes the folder's own. + + Raises + ------ + OSError + As :func:`source_signature`. + """ found = [] newest = path.stat().st_mtime_ns seen: set[str] = set() @@ -76,10 +103,7 @@ def source_signature(path: str | os.PathLike) -> tuple[int, int, str | None]: found.append((name if relative == "." else f"{relative}/{name}", stat)) newest = max(newest, stat.st_mtime_ns) found.sort(key=lambda item: item[0]) - digest = hashlib.sha256() - for name, stat in found: - digest.update(os.fsencode(name) + f"\0{stat.st_size}\0{stat.st_mtime_ns}\n".encode()) - return sum(stat.st_size for _, stat in found), newest, digest.hexdigest() + return newest, found def source_fingerprint(path: str | os.PathLike) -> tuple[int, int]: diff --git a/src/morphobricks_preprocessing/manifest.py b/src/morphobricks_preprocessing/manifest.py index cb38bc8d..53b16af6 100644 --- a/src/morphobricks_preprocessing/manifest.py +++ b/src/morphobricks_preprocessing/manifest.py @@ -2,10 +2,13 @@ ``run_manifest.json`` sits next to ``plane_histograms.csv``. It records the confirmed folder labels and axis mapping, the settings, the package versions, -and each source file's size and modification time. The next run compares these -with the experiment folder to find new, changed and deleted files, so unchanged -images are never read again (``docs/plan.md`` §7). The format is described in -``docs/contract.md`` §9. +and each source file's fingerprint (size and modification time, and for an +OME-Zarr folder its inventory). The next run compares these with the experiment +folder to find new, changed and deleted files, and reads again only those +(``docs/plan.md`` §7). The fingerprint is metadata: a file rewritten with the same +size and its modification time set back is not seen as changed, unless the run +verifies content (``verify_content``: each source's SHA-256 is recorded and must +match). The format is described in ``docs/contract.md`` §9. The manifest also records, for information only, each file's channel names and significant bits when its metadata provides them, the SHA-256 of the @@ -22,14 +25,9 @@ import hashlib import json import os -import platform from dataclasses import dataclass, field -from importlib.metadata import PackageNotFoundError, version from pathlib import Path -import morphobricks_normalization -import numpy as np -import tifffile from morphobricks_normalization.checks import IntensityCheckSettings from morphobricks_normalization.csv_io import SCHEMA_VERSION from morphobricks_normalization.ranges import HistogramSettings @@ -41,8 +39,11 @@ from .inventory import source_fingerprint as source_fingerprint # re-exported (moved, PP-12) from .narrow_ranges import NarrowRange from .recorded_sizes import RecordedSize, sizes_from_data, sizes_to_data +from .versions import package_versions as package_versions # re-exported (moved) MANIFEST_FILENAME = "run_manifest.json" +CONTENT_KEY = "content_sha256" +"""The key of a source's content SHA-256 in a manifest entry (``verify_content`` runs).""" AXES_FROM = ("header", "confirmed") """Where a file's roles come from (:attr:`FileEntry.axes_from`).""" MANIFEST_VERSION = "2" @@ -74,6 +75,10 @@ class RunSettings: the range check (the normalization brick's ``HistogramSettings``). Manifests written before it was recorded read as the default, the minimum to maximum. + verify_content + Strict reuse (CLI ``--verify-content``): each source's content SHA-256 is recorded + (:attr:`FileEntry.content_sha256`) and must match before its staged or earlier rows + are reused. False (the default, and in manifests without it): the fingerprint only. """ bin_count: int @@ -81,6 +86,7 @@ class RunSettings: missing_as_singleton: bool = False exclude: tuple[str, ...] = () histogram: HistogramSettings = field(default_factory=HistogramSettings) + verify_content: bool = False def __post_init__(self) -> None: object.__setattr__(self, "exclude", tuple(sorted(set(self.exclude)))) @@ -140,6 +146,10 @@ class FileEntry: size and modification time, :func:`.inventory.source_signature`); None for a TIFF file, and in manifests written before it was recorded (such an OME-Zarr image counts as changed: its fingerprint is not known to be complete). + content_sha256 + The SHA-256 of the file's content (an OME-Zarr folder: of its files' paths and + SHA-256s, :func:`.source_content.source_content_sha256`), in a ``verify_content`` + run only; None otherwise. """ relative_path: str @@ -158,6 +168,7 @@ class FileEntry: axes_from: str | None = None pixel_sizes: tuple[RecordedSize, ...] = () inventory_sha256: str | None = None + content_sha256: str | None = None def same_source(self, other: FileEntry) -> bool: """Return True if ``other`` describes the same unchanged file, read the same way.""" @@ -166,6 +177,7 @@ def same_source(self, other: FileEntry) -> bool: and self.size == other.size and self.mtime_ns == other.mtime_ns and self.inventory_sha256 == other.inventory_sha256 + and self.content_sha256 == other.content_sha256 and self.labels == other.labels and self.axes == other.axes and self.singleton_roles == other.singleton_roles @@ -278,6 +290,7 @@ def to_dict(self) -> dict: ), "exclude": list(self.settings.exclude), "histogram": self.settings.histogram.to_dict(), + "verify_content": _flag(self.settings.verify_content, "verify_content"), }, "axis_mapping": {"axes": self.mapping.axes, "roles": list(self.mapping.roles)}, "labels": [ @@ -341,6 +354,7 @@ def from_dict(cls, data: dict) -> RunManifest: if settings.get("histogram") is None else HistogramSettings.from_dict(settings["histogram"]) ), + verify_content=_flag(settings.get("verify_content", False)), ), mapping=AxisMapping(str(mapping["axes"]), tuple(mapping["roles"])), files=tuple(_file_entry(f, strict=version != "1") for f in data["files"]), @@ -443,22 +457,6 @@ def file_entry_with( ) -def package_versions() -> dict[str, str]: - """Versions of the packages that determine the output.""" - try: - own = version("morphobricks-preprocessing") - except PackageNotFoundError: # running from a source checkout without installing - own = "0.0.0+unknown" - return { - "morphobricks-preprocessing": own, - # Computes the histograms and writes the CSV. - "morphobricks-normalization": morphobricks_normalization.__version__, - "numpy": np.__version__, - "tifffile": tifffile.__version__, - "python": platform.python_version(), - } - - def file_sha256(path: str | os.PathLike) -> str: """SHA-256 of a file, read in blocks.""" digest = hashlib.sha256() @@ -533,6 +531,8 @@ def _file_dict(entry: FileEntry, *, strict: bool = True) -> dict: } if entry.inventory_sha256 is not None: data[INVENTORY_KEY] = entry.inventory_sha256 + if entry.content_sha256 is not None: + data[CONTENT_KEY] = entry.content_sha256 return data @@ -562,6 +562,7 @@ def _file_entry(data: dict, *, strict: bool = True) -> FileEntry: axes_from=_text(data.get("axes_from")), pixel_sizes=sizes_from_data(data.get("pixel_sizes")), inventory_sha256=_text(data.get(INVENTORY_KEY)), + content_sha256=_text(data.get(CONTENT_KEY)), ) diff --git a/src/morphobricks_preprocessing/notebook_options.py b/src/morphobricks_preprocessing/notebook_options.py index c59c866c..83bb588a 100644 --- a/src/morphobricks_preprocessing/notebook_options.py +++ b/src/morphobricks_preprocessing/notebook_options.py @@ -56,6 +56,13 @@ "sizes) and write the table only when the whole run succeeds: a run that stops leaves the " "earlier results as they are. Full takes longer; use it to start from scratch." ) +VERIFY_CONTENT_HELP = ( + "An image counts as unchanged when its size and modification time are (for OME-Zarr, " + "those of every file). A file rewritten with the same size and its time set back would " + "not be seen. Verifying the content also compares each image's SHA-256 before its earlier " + "rows are reused: every image is read once more. Turning it on or off reads every image " + "again." +) _MAX_PLOTTED_GROUPS = 12 # Short names of the intensity checks, for the tiles and the table's first column. diff --git a/src/morphobricks_preprocessing/notebook_prefill.py b/src/morphobricks_preprocessing/notebook_prefill.py index d759f2f6..41caf207 100644 --- a/src/morphobricks_preprocessing/notebook_prefill.py +++ b/src/morphobricks_preprocessing/notebook_prefill.py @@ -80,6 +80,7 @@ def _fill(self, recorded: ResolvedInputs) -> None: self.histogram_range.value = histogram.histogram_range self.robust_low.value, self.robust_high.value = histogram.robust_quantiles self.run_checks.use(recorded.intensity_settings or IntensityCheckSettings()) + self.verify_content.value = recorded.verify_content def _unfill(self) -> None: """Set back the values filled from another output folder that were not edited.""" @@ -91,6 +92,8 @@ def _unfill(self) -> None: self.exclude.value = "" if now["intensity"] == self._prefilled["intensity"]: self.run_checks.restore_defaults() + if now["verify_content"] == self._prefilled["verify_content"]: + self.verify_content.value = False kept = ("recommended", "bins", "histogram") unedited = "bins" not in self._edited unedited = unedited and all(now[key] == self._prefilled[key] for key in kept) @@ -180,6 +183,7 @@ def _prefill_values(self) -> dict[str, object]: ), "intensity": self.run_checks.settings(), "singleton": self.missing_as_singleton.value, + "verify_content": self.verify_content.value, } def _histogram_from( diff --git a/src/morphobricks_preprocessing/notebook_run.py b/src/morphobricks_preprocessing/notebook_run.py index a7f72511..e2ab49e8 100644 --- a/src/morphobricks_preprocessing/notebook_run.py +++ b/src/morphobricks_preprocessing/notebook_run.py @@ -23,12 +23,13 @@ HISTOGRAM_RANGE_OPTIONS, MODE_HELP, MODE_OPTIONS, + VERIFY_CONTENT_HELP, ) from .notebook_plots import IntensityPanel from .notebook_render import _bins_help, _label, change_lines, issues_html from .planes import DEFAULT_MEMORY_LIMIT from .report import issue_counts, rebuild_line -from .run import RESTART, Preflight, RunResult, run_experiment +from .run import RESTART, Preflight, RunMode, RunResult, run_experiment from .validation import has_errors, plural @@ -81,6 +82,7 @@ def _build_run_step(self) -> None: self.robust_high, self.memory_gib, self.mode, + self.verify_content, ): control.observe(lambda _: self._refresh_run_step(), names="value") self.histogram_range.observe( @@ -124,6 +126,12 @@ def _build_settings(self) -> None: self.mode = widgets.RadioButtons( options=MODE_OPTIONS, value=RESTART, layout=widgets.Layout(**_WIDE) ) + self.verify_content = widgets.Checkbox( + value=False, + description="Verify each image's content (SHA-256) before reusing its rows", + indent=False, + layout=widgets.Layout(**_WIDE), + ) self.settings_card = ui.card( "Settings", [ @@ -133,6 +141,8 @@ def _build_settings(self) -> None: widgets.HTML("Mode"), self.mode, ui.help_text(MODE_HELP), + self.verify_content, + ui.help_text(VERIFY_CONTENT_HELP), ], ) @@ -308,7 +318,7 @@ def _call_run(self, histogram_settings: HistogramSettings) -> RunResult: exclude=given.exclude, masks_folder=given.masks_folder, mask_axes=given.mask_axes, - mode=self.mode.value, + mode=RunMode(self.mode.value, given.verify_content), progress=self._progress, design=self.design, image_type=given.image_type, diff --git a/src/morphobricks_preprocessing/notebook_run_plan.py b/src/morphobricks_preprocessing/notebook_run_plan.py index da3f92a8..8220d18b 100644 --- a/src/morphobricks_preprocessing/notebook_run_plan.py +++ b/src/morphobricks_preprocessing/notebook_run_plan.py @@ -142,6 +142,7 @@ def _given_inputs(self) -> GivenInputs: missing_as_singleton=self.missing_as_singleton.value, intensity_settings=self.run_checks.settings(), histogram_settings=self._histogram_settings_or_none(), + verify_content=self.verify_content.value, ) def _plan_layouts(self, review: AxisReview) -> dict[str, PlaneLayout]: diff --git a/src/morphobricks_preprocessing/run/__init__.py b/src/morphobricks_preprocessing/run/__init__.py index 5afab4c9..ba51752f 100644 --- a/src/morphobricks_preprocessing/run/__init__.py +++ b/src/morphobricks_preprocessing/run/__init__.py @@ -34,7 +34,7 @@ prepare_mask_step, recorded_masks, ) -from .modes import FULL, MODES, RESTART +from .modes import FULL, MODES, RESTART, RunMode from .pipeline import run_experiment from .preflight import Preflight, SamplePlane, preflight, sample_planes from .result import RunResult @@ -51,6 +51,7 @@ "OutputLockedError", "Preflight", "RecordedMasks", + "RunMode", "RunResult", "SamplePlane", "describe_masks", diff --git a/src/morphobricks_preprocessing/run/modes.py b/src/morphobricks_preprocessing/run/modes.py index c5be8c86..595af1ed 100644 --- a/src/morphobricks_preprocessing/run/modes.py +++ b/src/morphobricks_preprocessing/run/modes.py @@ -7,10 +7,17 @@ Both keep the user's confirmations (design, axis and mask-axis statements, image type), publish atomically and only on success, and list in the report what they reused, recomputed and replaced. + +Either can verify content (:class:`RunMode`, CLI ``--verify-content``; owner, 3 November +2026): rows are then reused only when the source's content SHA-256 is the one recorded with +them, not only its fingerprint (:mod:`..source_content`). It is a recorded input: a run not +asking keeps the earlier run's choice, and turning it on or off recomputes every row. """ from __future__ import annotations +from dataclasses import dataclass + RESTART = "restart" """Reuse the rows of unchanged files; process new or changed ones (the default).""" @@ -41,3 +48,47 @@ def check_mode(mode: object) -> str: if mode not in MODES: raise ValueError(f"mode must be one of {', '.join(MODES)}; found {mode!r}") return str(mode) + + +@dataclass(frozen=True) +class RunMode: + """A run mode with its reuse rule, for ``run_experiment(mode=...)``. + + Attributes + ---------- + name + ``"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). + """ + + name: str = RESTART + verify_content: bool | None = None + + +def run_mode(mode: object) -> RunMode: + """Return ``mode`` as a :class:`RunMode`: a mode's name, or a :class:`RunMode`. + + Parameters + ---------- + mode + ``"restart"``, ``"full"`` or a :class:`RunMode`. + + Returns + ------- + RunMode + With its name checked (:func:`check_mode`). + + Raises + ------ + ValueError + For an unknown mode, or a ``verify_content`` that is not True, False or None. + """ + if not isinstance(mode, RunMode): + return RunMode(check_mode(mode)) + flag = mode.verify_content + if flag is not None and not isinstance(flag, bool): + raise ValueError(f"verify_content must be True, False or None; found {flag!r}") + return RunMode(check_mode(mode.name), mode.verify_content) diff --git a/src/morphobricks_preprocessing/run/pipeline.py b/src/morphobricks_preprocessing/run/pipeline.py index 8be9e980..e680cee9 100644 --- a/src/morphobricks_preprocessing/run/pipeline.py +++ b/src/morphobricks_preprocessing/run/pipeline.py @@ -55,7 +55,7 @@ from .design_file import design_for_run from .generation import staged_generation from .lock import OutputLockedError, lock_issue, output_lock -from .modes import FULL, RESTART, check_mode +from .modes import FULL, RESTART, RunMode, run_mode from .output_guard import output_masks_issues from .preflight import Preflight, fit_calibration, preflight from .publish import write_report @@ -85,7 +85,7 @@ def run_experiment( # noqa: PLR0913 memory_limit: int = DEFAULT_MEMORY_LIMIT, missing_as_singleton: bool | None = None, exclude: Iterable[str] | None = None, - mode: str = RESTART, + mode: str | RunMode = RESTART, progress: Callable[[int, int, str], None] | None = None, design: ExperimentDesign | None = None, masks_folder: str | os.PathLike | Literal[False] | None = None, @@ -125,7 +125,12 @@ def run_experiment( # noqa: PLR0913 removed. mode ``"restart"`` (default) reuses the rows of unchanged files; ``"full"`` recomputes - every row. Both keep the user's confirmations and publish only on success. + every row. Both keep the user's confirmations and publish only on success. A file + is unchanged when its fingerprint is (size and modification time; for OME-Zarr its + inventory): a rewrite of the same size with the time set back is not seen. A + :class:`.modes.RunMode` with ``verify_content=True`` also records each source's + content SHA-256 and reuses rows only for the same content (``None``: as recorded; + turning it on or off recomputes every row). progress Called as ``progress(done, total, relative_path)`` before each file is read, and as ``progress(total, total, "")`` when reading is finished. @@ -165,6 +170,8 @@ def run_experiment( # noqa: PLR0913 False, or no experiment folder given or recorded. """ _check_arguments(mode, image_type, missing_as_singleton) + chosen = run_mode(mode) + mode = chosen.name output, root = _folders(experiment_root, output_folder) inside_masks = output_masks_issues(output, masks_folder) if check_output_folder(root, output) or inside_masks: @@ -183,6 +190,7 @@ def run_experiment( # noqa: PLR0913 missing_as_singleton=missing_as_singleton, intensity_settings=intensity_settings, histogram_settings=histogram_settings, + verify_content=chosen.verify_content, ) output.mkdir(parents=True, exist_ok=True) try: @@ -256,6 +264,7 @@ def _locked_run(given: GivenInputs, output: Path, options: _Options) -> RunResul inputs.missing_as_singleton, pre.excluded, histogram=inputs.histogram_settings, + verify_content=inputs.verify_content, ) rows = compute_rows(pre, checked, settings, output, previous, options.mode, options.progress) if rows.changed: @@ -440,9 +449,9 @@ def _replaced(run: _Run, masks: dict | None) -> dict[str, str]: return replaced -def _check_arguments(mode: str, image_type: str | None, missing_as_singleton: object) -> None: +def _check_arguments(mode: object, image_type: str | None, missing_as_singleton: object) -> None: """Refuse arguments that are not allowed, before anything is read or written.""" - check_mode(mode) + run_mode(mode) if image_type is not None: check_image_type(image_type) # The manifest records it as a JSON boolean (contract §9). diff --git a/src/morphobricks_preprocessing/run/recorded.py b/src/morphobricks_preprocessing/run/recorded.py index 73d66d63..e6aa2088 100644 --- a/src/morphobricks_preprocessing/run/recorded.py +++ b/src/morphobricks_preprocessing/run/recorded.py @@ -3,7 +3,7 @@ One output folder belongs to one experiment. A run given no value for an input uses the value its folder's manifest records: the experiment root, masks folder, exclusions, plain-stack confirmations, mask-axis statements, image type, bins, histogram and -intensity settings, and ``missing_as_singleton`` (the design is the +intensity settings, ``missing_as_singleton`` and ``verify_content`` (the design is the ``experiment_design.csv`` in the folder, and the stated calibration its ``calibration.csv``, which a run never writes). A value given explicitly that differs from the recorded one is used, and an :class:`InputChange` names the change and its effect. The @@ -53,6 +53,8 @@ class GivenInputs: Mask axes stated per mask type; they add to (and replace per type) the recorded ones. image_type, bin_count, missing_as_singleton, intensity_settings, histogram_settings As :func:`.pipeline.run_experiment`. + verify_content + Strict reuse by content SHA-256 (:class:`.modes.RunMode`). """ experiment_root: Path | None = None @@ -65,6 +67,7 @@ class GivenInputs: missing_as_singleton: bool | None = None intensity_settings: IntensityCheckSettings | None = None histogram_settings: HistogramSettings | None = None + verify_content: bool | None = None @dataclass(frozen=True) @@ -111,6 +114,8 @@ class ResolvedInputs: bins_recorded_for The pixel type the recorded bin count was for, when it is the recorded one (a bin count is not reused for another pixel type); None when given or default. + verify_content + Whether rows are reused only for the same content SHA-256 (False by default). changes The inputs given that differ from the recorded ones. """ @@ -126,6 +131,7 @@ class ResolvedInputs: intensity_settings: IntensityCheckSettings | None histogram_settings: HistogramSettings bins_recorded_for: str | None = None + verify_content: bool = False changes: tuple[InputChange, ...] = field(default=()) @@ -181,6 +187,7 @@ def resolve_inputs(given: GivenInputs, previous: RunManifest | None) -> Resolved intensity_settings=given.intensity_settings or before.intensity_checks, histogram_settings=given.histogram_settings or settings.histogram, bins_recorded_for=before.dtype if given.bin_count is None else None, + verify_content=_first(given.verify_content, settings.verify_content), ) return _with_changes(resolved, given, before) @@ -362,6 +369,7 @@ 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), ) @@ -394,6 +402,7 @@ def _with_changes( "intensity_settings", before.intensity_checks, given.intensity_settings, UNAFFECTED ), _change("histogram_settings", settings.histogram, given.histogram_settings, RECOMPUTED), + _change("verify_content", settings.verify_content, given.verify_content, RECOMPUTED), ] changes = tuple(change for change in found if change is not None) return replace(resolved, changes=changes) diff --git a/src/morphobricks_preprocessing/run/resume.py b/src/morphobricks_preprocessing/run/resume.py index f7c7b0d9..428aa733 100644 --- a/src/morphobricks_preprocessing/run/resume.py +++ b/src/morphobricks_preprocessing/run/resume.py @@ -6,7 +6,9 @@ fingerprint (issue #90): its manifest entry records the fingerprint taken when its header was read (between two fingerprints that must agree), and the file must still have it just after its pixels are read. A file that changed is never staged or published: it is listed, and the -run stops with ``source_changed_during_run`` (:func:`changed_issue`). +run stops with ``source_changed_during_run`` (:func:`changed_issue`). In a ``verify_content`` +run the row key also holds the file's content SHA-256, so staged rows are resumed only for the +same content, and a file read must still have that SHA-256 after its pixels are read. """ from __future__ import annotations @@ -22,6 +24,7 @@ from ..fingerprint import Fingerprint, RowKey, current_fingerprint from ..manifest import FileEntry, NarrowRange, RunSettings from ..planes import PlaneReadError +from ..source_content import current_content from ..validation import ValidationIssue, error, plural from .preflight import Preflight from .rows import _read_file @@ -182,7 +185,10 @@ def _read_one(job: FileJob, rel: str, key: dict, found: NewRows) -> None: logger.warning("Cannot read the planes of %s", rel, exc_info=True) found.failed.append(rel) return - if current_fingerprint(root / rel) != expected: + if current_fingerprint(root / rel) != expected or ( + job.settings.verify_content + and current_content(root / rel) != job.entries[rel].content_sha256 + ): found.changed.append(rel) return stage_file(job.output, key, rows, ranges) @@ -222,6 +228,14 @@ def _why_discarded(staged: dict, now: dict, full: bool, unchanged: bool) -> str: or staged.get("row_key", {}).get("fingerprint") != now["row_key"]["fingerprint"] ): return "the image changed since its rows were staged" + content = ( + staged.get("row_key", {}).get("content_sha256"), + now["row_key"].get("content_sha256"), + ) + if content[0] != content[1]: + if None in content: + return "content verification (verify_content) was turned on or off since then" + return "the image's content (SHA-256) changed since its rows were staged" if {k: staged.get(k) for k in ("versions", "schema_version")} != { k: now[k] for k in ("versions", "schema_version") }: diff --git a/src/morphobricks_preprocessing/run/reuse.py b/src/morphobricks_preprocessing/run/reuse.py index 28f6f4a5..b706b9bc 100644 --- a/src/morphobricks_preprocessing/run/reuse.py +++ b/src/morphobricks_preprocessing/run/reuse.py @@ -118,17 +118,9 @@ def _load_previous( return {}, "the earlier table uses a different format version" if manifest.dtype != dtype: return {}, f"the pixel type changed from {manifest.dtype} to {dtype}" - if manifest.settings.bin_count != settings.bin_count: - return ( - {}, - f"the bin count changed from {manifest.settings.bin_count:,} to {settings.bin_count:,}", - ) - if manifest.settings.histogram != settings.histogram: - before = _describe_histogram(manifest.settings.histogram) - now = _describe_histogram(settings.histogram) - if before == now: - return {}, "the limit of the histogram range check changed" - return {}, f"the histogram range changed from {before} to {now}" + changed = _settings_changed(manifest.settings, settings) + if changed is not None: + return {}, changed if (manifest.mapping.axes, manifest.mapping.roles) != (mapping.axes, mapping.roles): return {}, "the axis mapping changed" if not csv_path.exists() or file_sha256(csv_path) != manifest.csv_sha256: @@ -144,6 +136,25 @@ def _load_previous( return by_file, None +def _settings_changed(before: RunSettings, now: RunSettings) -> str | None: + """Say which setting that changes every row (or how rows are reused) changed, or None. + + Turning content verification on or off recomputes every row: rows recorded without a + content SHA-256 cannot be verified, and reuse never changes its rule silently. + """ + if before.bin_count != now.bin_count: + return f"the bin count changed from {before.bin_count:,} to {now.bin_count:,}" + if before.histogram != now.histogram: + was, will = _describe_histogram(before.histogram), _describe_histogram(now.histogram) + if was == will: + return "the limit of the histogram range check changed" + return f"the histogram range changed from {was} to {will}" + if before.verify_content != now.verify_content: + state = "turned on" if now.verify_content else "turned off" + return f"content verification (verify_content) was {state}" + return None + + def _describe_histogram(settings: HistogramSettings) -> str: """Describe the histogram range in words, e.g. ``"robust (0.01-99.99 %)"``.""" if settings.robust: diff --git a/src/morphobricks_preprocessing/run/staging.py b/src/morphobricks_preprocessing/run/staging.py index 8e8f7587..5fdf488e 100644 --- a/src/morphobricks_preprocessing/run/staging.py +++ b/src/morphobricks_preprocessing/run/staging.py @@ -5,10 +5,10 @@ normalization brick's writer (the same writer and reader as the table, so a resumed row is the row the run would compute), then a JSON record naming the file, its row key (:class:`.fingerprint.RowKey`: fingerprint, labels, axes, roles, ``axes_from``, read path, -bins and histogram settings), the pixel type, the table's schema version, the versions of -the two packages that compute rows, the planes whose range check failed, and the SHA-256 of -the rows file. The record is written last, so a staged entry without one is incomplete and -never used. +bins and histogram settings, and in a ``verify_content`` run the content SHA-256), the +pixel type, the table's schema version, the versions of the two packages that compute +rows, the planes whose range check failed, and the SHA-256 of the rows file. The record is +written last, so a staged entry without one is incomplete and never used. The next run (after a crash, a kill or a lost Colab session) reuses a staged file's rows only when everything its record names is what that run would read now; anything else is @@ -83,11 +83,15 @@ def staging_key(key: RowKey, dtype: str) -> dict: Returns ------- dict - The row key, the pixel type, the table's schema version and the versions of the - packages that compute rows, normalised through JSON so it compares with a record. + The row key (with the content SHA-256 only in a ``verify_content`` run), the pixel + type, the table's schema version and the versions of the packages that compute + rows, normalised through JSON so it compares with a record. """ + row = asdict(key) + if row["content_sha256"] is None: # not a verify_content run: the key as before + del row["content_sha256"] data = { - "row_key": asdict(key), + "row_key": row, "dtype": dtype, "schema_version": SCHEMA_VERSION, "versions": dict(_row_versions()), diff --git a/src/morphobricks_preprocessing/run/steps.py b/src/morphobricks_preprocessing/run/steps.py index 28347ac1..c976fc9e 100644 --- a/src/morphobricks_preprocessing/run/steps.py +++ b/src/morphobricks_preprocessing/run/steps.py @@ -25,6 +25,7 @@ from ..manifest import FileEntry, NarrowRange, RunManifest, RunSettings from ..mask_checks import mask_issues from ..planes import check_planes +from ..source_content import with_content from ..validation import ValidationIssue, error, has_errors, plural from .entries import Comparison, compare_with_previous, file_entries from .modes import FULL @@ -219,6 +220,8 @@ def compute_rows( earlier, reason = _load_previous(output, previous, settings, review.mapping, dtype, mode) in_blocks = {p for i in checked.issues if i.code == "planes_read_in_blocks" for p in i.paths} entries = file_entries(pre, review, root, in_blocks) + # Strict reuse: each image's content SHA-256 joins its row key; an unreadable one fails. + entries, unhashed = with_content(entries, root) if settings.verify_content else (entries, []) found = compare_with_previous(entries, previous[0], earlier, reason is not None, settings, root) records: list[PlaneRecord] = [] narrow: list[NarrowRange] = [] @@ -233,7 +236,7 @@ def compute_rows( to_read = [ rel for rel in sorted([*found.new, *found.changed, *(found.unchanged if reason else ())]) - if rel not in changed + if rel not in changed and rel not in unhashed ] kept = set(found.unchanged) if not reason else set() new = new_rows(job, to_read, kept, progress) @@ -243,7 +246,7 @@ def compute_rows( found, tuple(new.read), reason, - tuple(new.failed), + tuple(sorted({*unhashed, *new.failed})), entries, resumed=tuple(new.resumed), changed=tuple(sorted({*changed, *new.changed})), diff --git a/src/morphobricks_preprocessing/run_inputs.py b/src/morphobricks_preprocessing/run_inputs.py index 14709a58..fcabca16 100644 --- a/src/morphobricks_preprocessing/run_inputs.py +++ b/src/morphobricks_preprocessing/run_inputs.py @@ -15,7 +15,10 @@ listed, no pixel is read). OME-Zarr label images are never skipped: a run holding OME-Zarr images is refused without the ``zarr`` extra, and so is a recorded image whose ``labels`` group cannot be read; -- every read checks the file's fingerprint again (:class:`.fingerprint.Fingerprint`); +- every read checks the file's fingerprint again (:class:`.fingerprint.Fingerprint`); for a + run that verified content (``verify_content``), :meth:`RunInputs.open` also checks every + image's content SHA-256 once (:func:`.source_content.content_changed`), and refuses one + whose content changed even with its fingerprint kept; - masks are read by the recorded pairing only, with preprocessing's reader and its label check (:func:`.masks.check_labels`); a stated shared 2-D mask serves every plane. @@ -61,6 +64,7 @@ _unrecorded_text, _without_axes_text, ) +from .source_content import content_changed from .unrecorded import UnrecordedFiles, unrecorded_files from .validation import plural from .zarr_input import NgffError, ZarrUnavailableError @@ -382,6 +386,8 @@ def image_type(self) -> str | None: def _check_files(self) -> None: """Refuse changed or gone files, masks without axes, unchecked labels, unrecorded files.""" changed = [e.relative_path for e in self.manifest.files if not self._unchanged(e)] + if not changed and self.manifest.settings.verify_content: + changed = content_changed(self.manifest.files, self.experiment_root) if changed: raise RunInputsError(_changed_text(changed)) recorded = (self.manifest.masks or {}).get("files", ()) diff --git a/src/morphobricks_preprocessing/source_content.py b/src/morphobricks_preprocessing/source_content.py new file mode 100644 index 00000000..a8c76e7e --- /dev/null +++ b/src/morphobricks_preprocessing/source_content.py @@ -0,0 +1,117 @@ +"""Strict reuse: a source's content SHA-256 (``verify_content``; owner, 3 November 2026). + +By default a source is identified by its fingerprint (:mod:`.inventory`): metadata only, +which does not see a file rewritten with the same size whose modification time is then +set back. A run with ``verify_content`` (CLI ``--verify-content``) also records each +source's content SHA-256 in its manifest entry, and reuses an image's staged or earlier +rows only when that SHA-256 is the one recorded with them: + +- a TIFF file: the SHA-256 of its bytes; +- an OME-Zarr folder: the SHA-256 of one line per file (its path relative to the folder, + with ``/``, and its own SHA-256), in path order, over the files its fingerprint covers + (:func:`.inventory.store_files`: the ``labels`` group left out, contained links followed). + +It reads every analysed image whole once more per run (twice for an image whose pixels +are read: its SHA-256 is taken again after the read and must not have changed). +""" + +from __future__ import annotations + +import hashlib +import logging +import os +from collections.abc import Iterable, Mapping +from dataclasses import replace +from pathlib import Path + +from .inventory import store_files +from .manifest import FileEntry, file_sha256 + +logger = logging.getLogger(__name__) + + +def source_content_sha256(path: str | os.PathLike) -> str: + """Return the content SHA-256 of a TIFF file or an OME-Zarr folder. + + Parameters + ---------- + path + The image. + + Returns + ------- + str + 64 hexadecimal digits. + + Raises + ------ + OSError + If the image or a file of it cannot be read (``FileNotFoundError`` when gone), or + its links reach one folder twice. + """ + path = Path(path) + if not path.is_dir(): + return file_sha256(path) + digest = hashlib.sha256() + for name, _ in store_files(path)[1]: + digest.update(os.fsencode(name) + f"\0{file_sha256(path / name)}\n".encode()) + return digest.hexdigest() + + +def current_content(path: Path) -> str | None: + """Return an image's content SHA-256 now, or None when it cannot be read.""" + try: + return source_content_sha256(path) + except OSError: + logger.warning("Cannot hash the content of %s", path, exc_info=True) + return None + + +def with_content( + entries: Mapping[str, FileEntry], root: Path +) -> tuple[dict[str, FileEntry], list[str]]: + """Record each image's content SHA-256 in its entry. + + Parameters + ---------- + entries + The run's entries, ``{relative path: FileEntry}``. + root + The experiment folder. + + Returns + ------- + tuple + ``(entries with content_sha256, images that could not be read)``; an image that + cannot be read keeps None, and is never reused. + """ + found, unreadable = {}, [] + for rel, entry in entries.items(): + sha = current_content(root / rel) + if sha is None: + unreadable.append(rel) + found[rel] = replace(entry, content_sha256=sha) + return found, unreadable + + +def content_changed(entries: Iterable[FileEntry], root: Path) -> list[str]: + """Return the images whose content SHA-256 now is not the one recorded (gone ones too). + + Parameters + ---------- + entries + The entries of a ``verify_content`` run. + root + The experiment folder. + + Returns + ------- + list of str + The changed, unreadable or unrecorded images, sorted. + """ + return sorted( + entry.relative_path + for entry in entries + if entry.content_sha256 is None + or current_content(root / entry.relative_path) != entry.content_sha256 + ) diff --git a/src/morphobricks_preprocessing/versions.py b/src/morphobricks_preprocessing/versions.py new file mode 100644 index 00000000..5d1d8d45 --- /dev/null +++ b/src/morphobricks_preprocessing/versions.py @@ -0,0 +1,35 @@ +"""The versions of the packages that determine a run's output (moved from :mod:`.manifest`). + +:mod:`.manifest` re-exports :func:`package_versions`. +""" + +from __future__ import annotations + +import platform +from importlib.metadata import PackageNotFoundError, version + +import morphobricks_normalization +import numpy as np +import tifffile + + +def package_versions() -> dict[str, str]: + """Return the versions of the packages that determine the output. + + Returns + ------- + dict + ``{package: version}`` for this package, the normalization brick (it computes the + histograms and writes the CSV), NumPy, tifffile and Python. + """ + try: + own = version("morphobricks-preprocessing") + except PackageNotFoundError: # running from a source checkout without installing + own = "0.0.0+unknown" + return { + "morphobricks-preprocessing": own, + "morphobricks-normalization": morphobricks_normalization.__version__, + "numpy": np.__version__, + "tifffile": tifffile.__version__, + "python": platform.python_version(), + } diff --git a/tests/test_notebook_cards.py b/tests/test_notebook_cards.py index 44f7db3b..77c1ee10 100644 --- a/tests/test_notebook_cards.py +++ b/tests/test_notebook_cards.py @@ -324,6 +324,22 @@ 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. + nb = ready(experiment) + assert not nb.verify_content.value + nb.verify_content.value = True + 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 + + def test_the_plan_reads_the_calibration_the_run_reads(experiment): """A ``calibration.csv`` edited after the check (e.g. by ``calibrate``) is in the plan.""" nb = ready(experiment) diff --git a/tests/test_verify_content.py b/tests/test_verify_content.py new file mode 100644 index 00000000..1e2ddb1b --- /dev/null +++ b/tests/test_verify_content.py @@ -0,0 +1,222 @@ +"""Strict reuse by content SHA-256, ``verify_content`` (owner decision, 3 November 2026). + +The default identity rule is the fingerprint (size and modification time; for OME-Zarr the +inventory). Its blind spot, a rewrite of the same size with the time set back, is shown +first; a ``verify_content`` run closes it for earlier rows, staged rows and ``RunInputs``. +Turning the mode on or off recomputes every row and names the change. +""" + +import json +import os + +import numpy as np +import pytest + +from morphobricks_preprocessing import ( + PlaneReadError, + RunInputs, + RunInputsError, + RunMode, + prefill_mapping, + read_manifest, + run_experiment, +) +from morphobricks_preprocessing import run as run_module +from morphobricks_preprocessing.cli import main +from morphobricks_preprocessing.discovery_report import REPORT_FILENAME +from morphobricks_preprocessing.source_content import source_content_sha256 +from morphobricks_preprocessing.testing import make_clean_run +from tiff_tree import write_data + +ZYX = prefill_mapping("ZYX") +IMAGES = ["ctrl/r1/a.tif", "ctrl/r1/b.tif", "drug/r1/a.tif"] +STRICT = RunMode("restart", verify_content=True) + + +def stack(seed): + return np.random.default_rng(seed).integers(0, 255, (2, 8, 8), dtype=np.uint8) + + +def rewrite_keeping_metadata(path, seed=99): + """Rewrite an image with other pixels, the same size, and its modification time back.""" + before = path.stat() + write_data(path, stack(seed), "ZYX") + os.utime(path, ns=(before.st_atime_ns, before.st_mtime_ns)) + after = path.stat() + assert (after.st_size, after.st_mtime_ns) == (before.st_size, before.st_mtime_ns) + + +@pytest.fixture +def experiment(tmp_path): + root = tmp_path / "exp" + for seed, rel in enumerate(IMAGES, start=10): + write_data(root / rel, stack(seed), "ZYX") + return root, tmp_path / "out" + + +def reference_table(tmp_path, root): + result = run_experiment(root, ZYX, output_folder=tmp_path / "fresh", mode="full") + assert result.written + return result.csv_path.read_bytes() + + +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. + root, out = experiment + assert run_experiment(root, ZYX, output_folder=out).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 + RunInputs.open(out) # the fingerprint is all RunInputs checks by default + 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) + + +def test_a_strict_run_records_each_content_sha256(experiment): + root, out = experiment + assert run_experiment(root, ZYX, output_folder=out, mode=STRICT).written + manifest = read_manifest(out / "run_manifest.json") + assert manifest.settings.verify_content + for entry in manifest.files: + assert entry.content_sha256 == source_content_sha256(root / entry.relative_path) + data = json.loads((out / "run_manifest.json").read_text(encoding="utf-8")) + assert data["settings"]["verify_content"] is True + + +def test_a_strict_restart_reads_an_image_rewritten_with_its_metadata_kept(tmp_path, experiment): + root, out = experiment + assert run_experiment(root, ZYX, output_folder=out, mode=STRICT).written + rewrite_keeping_metadata(root / IMAGES[0]) + # Given no mode setting, the recorded one is kept. + again = run_experiment(root, output_folder=out) + assert again.written and again.read == (IMAGES[0],) + assert again.rebuild_reason is None and again.changed == (IMAGES[0],) + assert again.csv_path.read_bytes() == reference_table(tmp_path, root) + + +def test_turning_verification_on_or_off_recomputes_every_row_and_says_so(experiment): + root, out = experiment + assert run_experiment(root, ZYX, output_folder=out).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" + (change,) = [c for c in on.input_changes if c.name == "verify_content"] + assert (change.recorded, change.given) == ("False", "True") + kept = run_experiment(root, output_folder=out) + assert kept.written and kept.read == () and kept.unchanged == tuple(IMAGES) + off = run_experiment(root, output_folder=out, mode=RunMode("restart", verify_content=False)) + assert off.written and off.read == tuple(IMAGES) + assert off.rebuild_reason == "content verification (verify_content) was turned off" + + +def test_staged_rows_are_resumed_only_for_the_same_content(experiment, monkeypatch): + root, out = experiment + read = run_module.resume._read_file + + def unreadable(path, rel, *args): + if rel == IMAGES[-1]: + raise PlaneReadError("simulated") + return read(path, rel, *args) + + with monkeypatch.context() as patch: + patch.setattr(run_module.resume, "_read_file", unreadable) + stopped = run_experiment(root, ZYX, output_folder=out, mode=STRICT) + assert not stopped.written and stopped.read == tuple(IMAGES[:-1]) + rewrite_keeping_metadata(root / IMAGES[0]) + result = run_experiment(root, ZYX, output_folder=out, mode=STRICT) + assert result.written and result.resumed == (IMAGES[1],) + assert result.read == (IMAGES[0], IMAGES[2]) + assert report(out)["staged_discarded"] == { + IMAGES[0]: "the image's content (SHA-256) changed since its rows were staged" + } + + +def test_staged_rows_of_the_other_mode_are_discarded_with_the_reason(experiment, monkeypatch): + root, out = experiment + read = run_module.resume._read_file + + def unreadable(path, rel, *args): + if rel == IMAGES[-1]: + raise PlaneReadError("simulated") + return read(path, rel, *args) + + with monkeypatch.context() as patch: + patch.setattr(run_module.resume, "_read_file", unreadable) + assert not run_experiment(root, ZYX, output_folder=out).written + result = run_experiment(root, ZYX, output_folder=out, mode=STRICT) + assert result.written and result.resumed == () and result.read == tuple(IMAGES) + assert set(report(out)["staged_discarded"].values()) == { + "content verification (verify_content) was turned on or off since then" + } + + +def test_an_image_whose_content_changes_while_it_is_read_is_never_published( + experiment, monkeypatch +): + root, out = experiment + assert run_experiment(root, ZYX, output_folder=out, mode=STRICT).written + before = (out / "plane_histograms.csv").read_bytes() + write_data(root / "drug/r2/a.tif", stack(7), "ZYX") + hashed = run_module.resume.current_content + + def changed_after_read(path): + return "0" * 64 if path.name == "a.tif" and "r2" in path.parts else hashed(path) + + monkeypatch.setattr(run_module.resume, "current_content", changed_after_read) + result = run_experiment(root, output_folder=out) + assert not result.written + assert "source_changed_during_run" in {issue.code for issue in result.issues} + assert (out / "plane_histograms.csv").read_bytes() == before + + +def test_run_inputs_of_a_strict_run_refuse_a_rewrite_with_its_metadata_kept(experiment): + root, out = experiment + assert run_experiment(root, ZYX, output_folder=out, mode=STRICT).written + RunInputs.open(out) + rewrite_keeping_metadata(root / IMAGES[1]) + with pytest.raises(RunInputsError, match="ctrl/r1/b.tif"): + RunInputs.open(out) + + +def test_an_ome_zarr_chunk_rewritten_with_its_metadata_kept_is_read_again(tmp_path): + pytest.importorskip("zarr") + from zarr_tree import write_ngff + + root, out = tmp_path / "exp", tmp_path / "out" + store = write_ngff(root / "ctrl/r1/z.ome.zarr", stack(1)[None, :, :, :]) + write_data(root / "ctrl/r1/a.tif", stack(2), "ZYX") + mapping = prefill_mapping("ZYX") + first = run_experiment(root, mapping, output_folder=out, mode=STRICT) + assert first.written, first.issues + chunks = sorted(p for p in store.rglob("*") if p.is_file() and p.stat().st_size > 32) + chunk = chunks[-1] + data, before = bytearray(chunk.read_bytes()), chunk.stat() + data[-1] ^= 0xFF + chunk.write_bytes(bytes(data)) + os.utime(chunk, ns=(before.st_atime_ns, before.st_mtime_ns)) + again = run_experiment(root, output_folder=out) + assert again.written and again.read == ("ctrl/r1/z.ome.zarr",) + + +def test_the_cli_turns_content_verification_on(tmp_path): + clean = make_clean_run(tmp_path / "run", design=True) + out = clean.output + assert main(["run", "--output", str(out), "--verify-content"]) == 0 + assert read_manifest(out / "run_manifest.json").settings.verify_content + assert main(["run", "--output", str(out)]) == 0 + assert read_manifest(out / "run_manifest.json").settings.verify_content + assert main(["run", "--output", str(out), "--no-verify-content"]) == 0 + assert not read_manifest(out / "run_manifest.json").settings.verify_content + + +def test_a_mode_with_a_verify_content_that_is_not_a_flag_is_refused(experiment): + root, out = experiment + with pytest.raises(ValueError, match="verify_content"): + run_experiment(root, ZYX, output_folder=out, mode=RunMode("restart", 1)) + with pytest.raises(ValueError, match="mode"): + run_experiment(root, ZYX, output_folder=out, mode=RunMode("again", True)) From 4d0726ca587c76869dd849ec8c41fad1b04ad543 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 12:14:32 +0000 Subject: [PATCH 4/4] Read a folder's device with os.stat; keep default manifests in rc4 shape (review of #140) - mounts.MountCheck read st_dev from os.DirEntry.stat(), which is 0 on Windows, so every folder of every experiment was refused there (637 failures on the Windows CI jobs). It now uses os.stat(entry.path); regression test with a Windows-like entry. - settings.verify_content is written only when true, so a default run's manifest keeps the v0.4.0rc4 keys (contract and changelog say so). - Tests for a bind-mounted file inside an OME-Zarr store and for a strict-run image that cannot be hashed (never read), which mutations showed untested. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01G3qp9AxjpGPw9xfWAUJNqc --- CHANGELOG.md | 7 ++-- docs/contract.md | 2 +- src/morphobricks_preprocessing/manifest.py | 7 +++- src/morphobricks_preprocessing/mounts.py | 6 ++-- tests/test_mounts.py | 40 ++++++++++++++++++---- tests/test_verify_content.py | 28 +++++++++++++++ 6 files changed, 77 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f492ba94..a931b8a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,14 +109,17 @@ and the bricks' migration). source's content SHA-256 (`files[].content_sha256`, `settings.verify_content`) and reuses earlier or staged rows only for the same content; `RunInputs` of such a run checks it at `open`. It is a recorded input; turning it on or off recomputes every row and is - named. The default run is unchanged. + named. The default run is unchanged: its manifest has neither key (`verify_content` is + written only when true). - **Mount points below the experiment or masks folder are refused** (owner, 3 November 2026): a bind mount is an ordinary folder to the walk, so its files from outside were read. A folder (or file) listed in `/proc/self/mountinfo`, or on another device than the selected folder, is the error `mounted_folder` naming it, in discovery, OME-Zarr stores, the masks folder, `check`, `run`, `convert` and `RunInputs` (`mounts.MountCheck`). Where - no mount table can be read (macOS, Windows), only a mount of another device is seen. + no mount table can be read (macOS, Windows), only a mount of another device is seen. The + device is read with `os.stat` of the path: `os.DirEntry.stat()` reports `st_dev` 0 on + Windows, which refused every folder there (review of #140). - **The commit is flushed to disk** (review of rc4, I2): the output folder is flushed (`fsync`, POSIX) right after the manifest's rename, so a power cut after a run returned keeps the new generation. A run that finishes an earlier publication flushes it first. diff --git a/docs/contract.md b/docs/contract.md index d19f4bb2..6160dc88 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -173,7 +173,7 @@ interpretation. | `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: the run verified content, below; 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 `true` when the run verified content, below; written only then, so a default run's `settings` keep the v0.4.0rc4 keys; false when absent). | | `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; null when absent, in manifests written before it), `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`). | diff --git a/src/morphobricks_preprocessing/manifest.py b/src/morphobricks_preprocessing/manifest.py index 53b16af6..daa559f7 100644 --- a/src/morphobricks_preprocessing/manifest.py +++ b/src/morphobricks_preprocessing/manifest.py @@ -290,7 +290,12 @@ def to_dict(self) -> dict: ), "exclude": list(self.settings.exclude), "histogram": self.settings.histogram.to_dict(), - "verify_content": _flag(self.settings.verify_content, "verify_content"), + # Written only when on: a default run's manifest keeps the rc4 keys. + **( + {"verify_content": True} + if _flag(self.settings.verify_content, "verify_content") + else {} + ), }, "axis_mapping": {"axes": self.mapping.axes, "roles": list(self.mapping.roles)}, "labels": [ diff --git a/src/morphobricks_preprocessing/mounts.py b/src/morphobricks_preprocessing/mounts.py index 1656446c..7ae5fbfc 100644 --- a/src/morphobricks_preprocessing/mounts.py +++ b/src/morphobricks_preprocessing/mounts.py @@ -95,7 +95,9 @@ def is_mount(self, real: Path, entry: os.DirEntry) -> bool: real The entry's real path, below the selected folder. entry - The entry; a folder's device is read from it (``entry.stat()``). + The entry; a folder's device is read with :func:`os.stat` of its path, which + follows a link (``entry.stat()`` would not do: on Windows it reports + ``st_dev`` 0 for every entry). Returns ------- @@ -106,6 +108,6 @@ def is_mount(self, real: Path, entry: os.DirEntry) -> bool: if self.device is None or not entry.is_dir(): return False try: - return entry.stat().st_dev != self.device + return os.stat(entry.path).st_dev != self.device except OSError: # cannot be read: its reader reports it return False diff --git a/tests/test_mounts.py b/tests/test_mounts.py index 5bce0624..d76398f9 100644 --- a/tests/test_mounts.py +++ b/tests/test_mounts.py @@ -138,25 +138,51 @@ def test_a_mounted_folder_inside_an_ome_zarr_store_is_refused(tree, mounted): assert _issue(found.issues, "mounted_folder").paths == ("ctrl/rep1/z.ome.zarr/0",) +def test_a_mounted_file_inside_an_ome_zarr_store_is_refused(tree, mounted): + # Review of #140: a store's plain files are looked up among the mount points too (a + # bind mount of one chunk file brings in another file's bytes). + pytest.importorskip("zarr") + from zarr_tree import write_ngff + + root, _ = tree + store = write_ngff(root / "ctrl/rep1/z.ome.zarr", np.zeros((1, 1, 4, 4), np.uint8)) + chunk = next(path for path in sorted(store.rglob("*")) if path.is_file() and "0" in path.parts) + mounted(chunk) + found = discover_experiment(root) + assert _issue(found.issues, "mounted_folder").paths == (chunk.relative_to(root).as_posix(),) + + def test_a_folder_on_another_device_is_a_mount_point(tmp_path, monkeypatch): # The portable rule, where no mount table can be read (macOS, Windows). monkeypatch.setattr(mounts_module, "mount_points", lambda *_: frozenset()) (tmp_path / "sub").mkdir() check = MountCheck(tmp_path) + (entry,) = os.scandir(tmp_path) + assert check.device is not None + assert not check.is_mount(tmp_path / "sub", entry) + check.device += 1 # the selected folder on another device than its subfolder + assert check.is_mount(tmp_path / "sub", entry) + + +def test_the_device_is_read_from_the_path_not_the_folder_entry(tmp_path, monkeypatch): + # Review of #140: on Windows os.DirEntry.stat() reports st_dev 0, so reading the + # device from the entry refused every folder of every experiment there. + monkeypatch.setattr(mounts_module, "mount_points", lambda *_: frozenset()) + (tmp_path / "sub").mkdir() + check = MountCheck(tmp_path) - class Entry: - def __init__(self, device): - self.device = device + class WindowsEntry: + """A folder entry as Windows gives it: its stat() has no device.""" + + path = str(tmp_path / "sub") def is_dir(self): return True def stat(self): - return os.stat_result((0o40755, 1, self.device, 2, 0, 0, 0, 0, 0, 0)) + return os.stat_result((0o40755, 0, 0, 0, 0, 0, 0, 0, 0, 0)) - assert check.device is not None - assert not check.is_mount(tmp_path / "sub", Entry(check.device)) - assert check.is_mount(tmp_path / "sub", Entry(check.device + 1)) + assert not check.is_mount(tmp_path / "sub", WindowsEntry()) def test_the_mount_table_is_read_with_its_escapes(tmp_path): diff --git a/tests/test_verify_content.py b/tests/test_verify_content.py index 1e2ddb1b..5a6fc504 100644 --- a/tests/test_verify_content.py +++ b/tests/test_verify_content.py @@ -20,6 +20,7 @@ prefill_mapping, read_manifest, run_experiment, + source_content, ) from morphobricks_preprocessing import run as run_module from morphobricks_preprocessing.cli import main @@ -75,6 +76,10 @@ 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. + data = json.loads((out / "run_manifest.json").read_text(encoding="utf-8")) + assert "verify_content" not in data["settings"] + assert not any("content_sha256" in entry for entry in data["files"]) def test_a_strict_run_records_each_content_sha256(experiment): @@ -174,6 +179,29 @@ def changed_after_read(path): assert (out / "plane_histograms.csv").read_bytes() == before +def test_an_image_that_cannot_be_hashed_is_unreadable_and_never_read(experiment, monkeypatch): + # Review of #140: a strict run never reads or reuses an image without its SHA-256. + root, out = experiment + hashed = source_content.current_content + + def unhashable(path): + return None if path.as_posix().endswith(IMAGES[1]) else hashed(path) + + read, opened = run_module.resume._read_file, [] + + def recording(path, rel, *args): + opened.append(rel) + return read(path, rel, *args) + + monkeypatch.setattr(source_content, "current_content", unhashable) + monkeypatch.setattr(run_module.resume, "_read_file", recording) + result = run_experiment(root, ZYX, output_folder=out, mode=STRICT) + assert not result.written and IMAGES[1] not in opened + assert sorted(opened) == [IMAGES[0], IMAGES[2]] + issue = next(issue for issue in result.issues if issue.code == "unreadable_planes") + assert issue.paths == (IMAGES[1],) + + def test_run_inputs_of_a_strict_run_refuse_a_rewrite_with_its_metadata_kept(experiment): root, out = experiment assert run_experiment(root, ZYX, output_folder=out, mode=STRICT).written