diff --git a/CHANGELOG.md b/CHANGELOG.md index e35fd32a..26b3936e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ are in [docs/plan.md](docs/plan.md) §12. Upgrading from v0.3: see the ## Unreleased +- **A described run writes manifest version `"4"` (Stage 2 design question 9, owner answer 9).** + When the run manifest carries `experiment_description`, `manifest_version` is `"4"`; an + undescribed run keeps `"2"` or `"3"`, byte for byte (the v0.4.0 parity suite is unchanged). + A v0.4.0 reader skips keys it does not know, so it opened a described run and silently ignored + the description (channels by header name, pairing by folder name, no mask checks); it now + refuses version `"4"` (`ManifestFormatError`, tested with the code of the `v0.4.0` tag). + `read_manifest` and `RunInputs.open` read version 4. They refuse a version 4 manifest without + `experiment_description` and a version 1, 2 or 3 manifest with one; the writer applies the + same rule. Version 4 keeps the content rule of 2 and 3: a `content_sha256` in every entry + exactly when `settings.verify_content` is true. Contract §9 (`manifest_version`). - **Images without header channel names are listed in a described run (follow-up of #218, owner decision 2026-11-08).** An image whose header names no channel, or not every one, still passes the per-unit header checks (§4.3, fixture v12: no evidence needed, the confirmed layout is diff --git a/docs/contract.md b/docs/contract.md index bdc98b18..181fa4e0 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -199,7 +199,7 @@ interpretation. | Key | Meaning | |---|---| -| `manifest_version`, `schema_version` | Manifest format and CSV format (`"4"`), written as JSON **strings**, not numbers. The manifest format is `"3"` (v0.4.0rc5, release review) for a content-verification run (`verify_content`, below), which every run is unless it opts out (#159), and `"2"` (since v0.4.0) for a run that opted out: version 3 is version 2 with `settings.verify_content` `true` and a `content_sha256` in every file entry (`manifest_versions`). The version, not only those keys, marks a strict run, because readers of v0.4.0rc4 and earlier ignore keys they do not know but refuse a version they do not know (`ManifestFormatError: Manifest version '3' is not supported`, tested by running v0.4.0rc4's code on a strict manifest): they never open a strict run as a fingerprint-only one. The reader and the writer refuse a version 3 manifest without `verify_content` or without a file's `content_sha256`, and a version 1 or 2 manifest that holds either (a strict manifest written as version 2, only by unreleased rc5 builds before this rule). A run that opted out writes version 2, as before except for `settings.verify_content: false` (tested against the code of the commit before version 3). This version reads `"1"`, `"2"` and `"3"`; `RunInputs`, `restart` reuse and the recorded inputs treat 2 and 3 alike, and switching between a strict and a default run recomputes every row as before. A manifest whose `schema_version` is not the current one makes the next run recompute every row. | +| `manifest_version`, `schema_version` | Manifest format and CSV format (`"4"`), written as JSON **strings**, not numbers. The manifest format is `"3"` (v0.4.0rc5, release review) for a content-verification run (`verify_content`, below), which every run is unless it opts out (#159), and `"2"` (since v0.4.0) for a run that opted out: version 3 is version 2 with `settings.verify_content` `true` and a `content_sha256` in every file entry (`manifest_versions`). The version, not only those keys, marks a strict run, because readers of v0.4.0rc4 and earlier ignore keys they do not know but refuse a version they do not know (`ManifestFormatError: Manifest version '3' is not supported`, tested by running v0.4.0rc4's code on a strict manifest): they never open a strict run as a fingerprint-only one. The reader and the writer refuse a version 3 manifest without `verify_content` or without a file's `content_sha256`, and a version 1 or 2 manifest that holds either (a strict manifest written as version 2, only by unreleased rc5 builds before this rule). A run that opted out writes version 2, as before except for `settings.verify_content: false` (tested against the code of the commit before version 3). Switching between a strict and a default run recomputes every row as before. A manifest whose `schema_version` is not the current one makes the next run recompute every row. A run with an `experiment_description` writes version `"4"` (v0.5.0, Stage 2 design question 9): version 2 or 3 as `settings.verify_content` says, plus the description; v0.4.0 readers skip keys they do not know and would have ignored the description, so they refuse version 4 (tested with the code of the `v0.4.0` tag). A version 4 manifest must carry `experiment_description` and a version 1, 2 or 3 manifest must not (reader and writer refuse the mismatch); version 4 holds a `content_sha256` in every entry exactly when `verify_content` is true. A run without a description keeps version 2 or 3 byte for byte. This version reads `"1"` to `"4"`; `RunInputs`, `restart` reuse and the recorded inputs treat 2, 3 and 4 alike. | | `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. | diff --git a/src/morphobricks_preprocessing/manifest.py b/src/morphobricks_preprocessing/manifest.py index 110edf58..81854be1 100644 --- a/src/morphobricks_preprocessing/manifest.py +++ b/src/morphobricks_preprocessing/manifest.py @@ -308,6 +308,7 @@ def to_dict(self) -> dict: self.manifest_version, recorded_flag(self.settings.verify_content, "verify_content"), [(f.relative_path, f.content_sha256) for f in self.files], + self.experiment_description is not None, ) data = { "manifest_version": self.manifest_version, diff --git a/src/morphobricks_preprocessing/manifest_versions.py b/src/morphobricks_preprocessing/manifest_versions.py index 0e8325a3..2ec564c8 100644 --- a/src/morphobricks_preprocessing/manifest_versions.py +++ b/src/morphobricks_preprocessing/manifest_versions.py @@ -11,7 +11,12 @@ do not know, and refuse a version they do not know (``ManifestFormatError``), so they never open a strict run as a fingerprint-only one (release review of rc5). -A default run still writes version 2, byte for byte as before. The writer and the reader +- ``"4"`` (v0.5.0): a run with an ``experiment_description`` (Stage 2 design question 9). It is + version 2 or 3 (content verification as ``settings.verify_content`` says) plus the + description. A v0.4.0 reader skips keys it does not know, so it would open a described run + and name channels by header and pair by folder name; it refuses version ``"4"`` instead. + +A default run still writes version 2 (or 3), byte for byte as before. The writer and the reader apply the same rule (:func:`check_version`), so a written manifest always reads back. """ @@ -19,17 +24,25 @@ from collections.abc import Iterable +from .description_record import KEY as DESCRIPTION_KEY from .manifest_fields import recorded_flag MANIFEST_VERSION = "2" """The version of a default run's manifest (fingerprint identity).""" STRICT_MANIFEST_VERSION = "3" """The version of a ``verify_content`` run's manifest (content SHA-256 identity).""" +DESCRIBED_MANIFEST_VERSION = "4" +"""The version of a described run's manifest (``experiment_description`` present).""" LEGACY_MANIFEST_VERSION = "1" """Manifests written before v0.4.0: read for reporting, never trusted (no row is reused).""" CONTENT_KEY = "content_sha256" """The key of a source's content SHA-256 in a manifest entry (``verify_content`` runs).""" -READ_VERSIONS = (LEGACY_MANIFEST_VERSION, MANIFEST_VERSION, STRICT_MANIFEST_VERSION) +READ_VERSIONS = ( + LEGACY_MANIFEST_VERSION, + MANIFEST_VERSION, + STRICT_MANIFEST_VERSION, + DESCRIBED_MANIFEST_VERSION, +) """The versions this package reads.""" @@ -37,24 +50,32 @@ class UnsupportedVersionError(ValueError): """Raised for a ``manifest_version`` this package does not read (a later version's).""" -def version_for(verify_content: bool) -> str: +def version_for(verify_content: bool, described: bool = False) -> str: """Return the manifest version a run with these settings writes. Parameters ---------- verify_content Whether the run verifies each source's content (:attr:`.manifest.RunSettings`). + described + Whether the run carries an ``experiment_description``. Returns ------- str + :data:`DESCRIBED_MANIFEST_VERSION` for a described run, else :data:`STRICT_MANIFEST_VERSION` for a strict run, else :data:`MANIFEST_VERSION`. """ + if described: + return DESCRIBED_MANIFEST_VERSION return STRICT_MANIFEST_VERSION if verify_content else MANIFEST_VERSION def check_version( - version: object, verify_content: bool, content: Iterable[tuple[str, str | None]] + version: object, + verify_content: bool, + content: Iterable[tuple[str, str | None]], + described: bool = False, ) -> None: """Refuse a manifest whose version does not match its content verification. @@ -66,6 +87,8 @@ def check_version( Its ``settings.verify_content``. content ``(relative path, content_sha256 or None)`` of each file entry. + described + Whether the manifest carries an ``experiment_description``. Raises ------ @@ -75,7 +98,9 @@ def check_version( If a version 3 manifest does not verify content or lacks a file's ``content_sha256``; or if an older version records content verification (a strict run must be version 3, which older - readers refuse). + readers refuse); or if version 4 and the description do not go together (a described + run must be version 4, so that older readers refuse it, and version 4 needs one); or + if a version 4 manifest's content hashes do not match its ``verify_content``. """ if version not in READ_VERSIONS: raise UnsupportedVersionError( @@ -83,9 +108,23 @@ def check_version( f"{', '.join(repr(v) for v in READ_VERSIONS)})." ) content = list(content) + if (version == DESCRIBED_MANIFEST_VERSION) != described: + raise ValueError( + f"a version {version} manifest " + + ("carries" if described else "lacks") + + " experiment_description; a described run writes version " + f"{DESCRIBED_MANIFEST_VERSION}, and version {DESCRIBED_MANIFEST_VERSION} needs one" + ) hashed = [path for path, sha in content if sha is not None] unhashed = [path for path, sha in content if sha is None] - if version == STRICT_MANIFEST_VERSION: + if version == DESCRIBED_MANIFEST_VERSION: + stray = unhashed if verify_content else hashed + if stray: + raise ValueError( + f"a version {version} manifest holds a content_sha256 in every entry exactly " + f"when verify_content is true; the entry of {stray[0]!r} does not" + ) + elif version == STRICT_MANIFEST_VERSION: if not verify_content: raise ValueError(f"a version {version} manifest must have verify_content true") if unhashed: @@ -128,5 +167,6 @@ def recorded_version(data: dict) -> str: data["manifest_version"], recorded_flag(data["settings"].get("verify_content", False)), [(entry.get("relative_path"), entry.get(CONTENT_KEY)) for entry in files], + data.get(DESCRIPTION_KEY) is not None, ) return data["manifest_version"] diff --git a/src/morphobricks_preprocessing/run/pipeline.py b/src/morphobricks_preprocessing/run/pipeline.py index 9c84e57c..ba3fe5f3 100644 --- a/src/morphobricks_preprocessing/run/pipeline.py +++ b/src/morphobricks_preprocessing/run/pipeline.py @@ -507,7 +507,7 @@ def _write_generation( if described is None else manifest_entry(described, masks, run.output, len(rows.entries)) ), - manifest_version=version_for(settings.verify_content), + manifest_version=version_for(settings.verify_content, described is not None), ) manifest = _kept_time(manifest, run.previous[0]) return file_sha256(write_manifest(manifest, staged / MANIFEST_FILENAME)) diff --git a/tests/test_described_manifest_version.py b/tests/test_described_manifest_version.py new file mode 100644 index 00000000..07bef711 --- /dev/null +++ b/tests/test_described_manifest_version.py @@ -0,0 +1,128 @@ +"""A described run writes manifest version "4", so that older readers refuse it (Stage 2 Q9). + +A v0.4.0 reader skips keys it does not know, so without a new version it would open a run with +an ``experiment_description`` and silently ignore it. The old reader here is the code of the +``v0.4.0`` tag (``released_code``): it refuses version "4" and reads the undescribed run. +""" + +from __future__ import annotations + +import json +import warnings + +import pytest + +from description_tree import MAPPING, build_experiment +from morphobricks_preprocessing import RunInputs, run_experiment +from morphobricks_preprocessing.description import description_from_bytes +from morphobricks_preprocessing.manifest import ManifestFormatError, read_manifest +from morphobricks_preprocessing.manifest_versions import check_version, version_for +from morphobricks_preprocessing.run import RunMode +from released_code import PRELUDE, extract, run_with +from spec_pin import fixtures + +V01 = next(f for f in fixtures("valid") if f.name.startswith("v01_")) +DESCRIPTION = description_from_bytes(V01.data) + +OLD_READER = ( + PRELUDE + + """ +from morphobricks_preprocessing.manifest import ManifestFormatError, read_manifest +try: + manifest = read_manifest(Path(sys.argv[2])) +except ManifestFormatError as exc: + print(json.dumps({"refused": str(exc)})) +else: + print(json.dumps({"read": manifest.manifest_version})) +""" +) + + +def _run(tmp_path, *, described, verify_content=True): + root, out = tmp_path / "exp", tmp_path / "out" + out.mkdir(parents=True) + build_experiment(root, DESCRIPTION) + if described: + (out / "morphobricks.toml").write_bytes(V01.data) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + mode = RunMode("restart", verify_content=verify_content) + result = run_experiment(root, MAPPING, output_folder=out, mode=mode) + assert result.written, result.issues + return out + + +def _version(out): + return json.loads((out / "run_manifest.json").read_text(encoding="utf-8"))["manifest_version"] + + +@pytest.mark.parametrize("verify_content", [True, False]) +def test_a_described_run_writes_version_4_and_reads_back(tmp_path, verify_content): + out = _run(tmp_path, described=True, verify_content=verify_content) + assert _version(out) == "4" + manifest = read_manifest(out / "run_manifest.json") + assert manifest.manifest_version == "4" + assert manifest.experiment_description is not None + assert manifest.settings.verify_content is verify_content + assert RunInputs.open(out).fovs() + + +@pytest.mark.parametrize(("verify_content", "version"), [(True, "3"), (False, "2")]) +def test_an_undescribed_run_keeps_its_version(tmp_path, verify_content, version): + out = _run(tmp_path, described=False, verify_content=verify_content) + assert _version(out) == version + assert "experiment_description" not in json.loads((out / "run_manifest.json").read_text()) + + +@pytest.fixture(scope="module") +def released(tmp_path_factory): + return extract("v0.4.0", tmp_path_factory.mktemp("released")) + + +def test_a_v0_4_0_reader_refuses_a_described_run_and_reads_an_undescribed_one(tmp_path, released): + described = _run(tmp_path / "d", described=True) + plain = _run(tmp_path / "u", described=False) + refused = run_with(released, OLD_READER, described / "run_manifest.json") + assert "Manifest version '4' is not supported" in refused["refused"] + assert run_with(released, OLD_READER, plain / "run_manifest.json") == {"read": "3"} + + +def _mismatched(out, **change): + path = out / "run_manifest.json" + data = json.loads(path.read_text(encoding="utf-8")) + for key, value in change.items(): + if value is None: + data.pop(key) + else: + data[key] = value + path.write_text(json.dumps(data), encoding="utf-8") + return path + + +def test_a_version_4_manifest_without_a_description_is_refused(tmp_path): + path = _mismatched(_run(tmp_path, described=False), manifest_version="4") + with pytest.raises(ManifestFormatError, match="version 4 manifest lacks experiment_desc"): + read_manifest(path) + with pytest.raises(ManifestFormatError): + RunInputs.open(path.parent) + + +@pytest.mark.parametrize("version", ["1", "2", "3"]) +def test_a_lower_version_with_a_description_is_refused(tmp_path, version): + path = _mismatched(_run(tmp_path, described=True), manifest_version=version) + with pytest.raises(ManifestFormatError, match=f"version {version} manifest carries"): + read_manifest(path) + with pytest.raises(ManifestFormatError): + RunInputs.open(path.parent) + + +def test_version_4_needs_content_hashes_exactly_when_verify_content(): + assert version_for(False, True) == version_for(True, True) == "4" + check_version("4", True, [("a.tif", "0" * 64)], True) + check_version("4", False, [("a.tif", None)], True) + with pytest.raises(ValueError, match="'a.tif'"): + check_version("4", True, [("a.tif", None)], True) + with pytest.raises(ValueError, match="'a.tif'"): + check_version("4", False, [("a.tif", "0" * 64)], True) + with pytest.raises(ValueError, match="writes version 4"): + check_version("2", False, [], True) diff --git a/tests/test_strict_manifest.py b/tests/test_strict_manifest.py index 38ee677d..acfab760 100644 --- a/tests/test_strict_manifest.py +++ b/tests/test_strict_manifest.py @@ -181,8 +181,8 @@ def test_the_reader_refuses_a_version_that_does_not_match_its_content_check(expe del data["settings"]["verify_content"] # a content_sha256 left in a version 2 entry cases.append((data, "version 2 manifest records content verification")) data = _strict_data(out) - data["manifest_version"] = "4" - cases.append((data, "(?i)manifest version '4' is not supported")) + data["manifest_version"] = "5" + cases.append((data, "(?i)manifest version '5' is not supported")) for data, message in cases: path.write_text(json.dumps(data), encoding="utf-8") with pytest.raises(ManifestFormatError, match=message): @@ -219,12 +219,12 @@ def test_an_unknown_later_version_is_plainly_not_supported(experiment, files): root, out = experiment assert run_experiment(root, TYX, output_folder=out, mode=STRICT).written data = _strict_data(out) - data["manifest_version"] = "4" + data["manifest_version"] = "5" data["files"] = files path = out / "run_manifest.json" path.write_text(json.dumps(data), encoding="utf-8") with pytest.raises(ManifestFormatError) as refused: read_manifest(path) assert str(refused.value) == ( - "Manifest version '4' is not supported (this version reads '1', '2', '3')." + "Manifest version '5' is not supported (this version reads '1', '2', '3', '4')." )