From c491af07ce6eed8ac965d359ceaaa586597d7f56 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 18:10:18 +0000 Subject: [PATCH] Re-pin the vendored Spec to 7dbae0c; records file lines end at LF only Fixtures (goldens in expected/, t13), experiment-v1.md and refval at MorphoBricks-Spec 7dbae0c. Reader and reference validator agree on all fixtures and mutations; the oracle no longer differs on image_count. New byte-level tests for section digests (9.3) and morphobricks.records.jsonl (4.6). read_records split at U+2028/U+0085 as well as LF; now LF only. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Scma6W3LC11s3qjXHZuGDm --- CHANGELOG.md | 11 ++ .../run/description_file.py | 3 +- tests/spec_fixtures/PIN.toml | 8 +- tests/spec_fixtures/docs/experiment-v1.md | 30 ++++- tests/spec_fixtures/fixtures/README.md | 19 ++- ...3_homogeneous_legacy.experiment_design.csv | 10 ++ ...er_on_forbidden_list.experiment_design.csv | 5 + ...er_on_forbidden_list.experiment_design.csv | 5 + ...13_confirmed_on_with_image_count_only.toml | 71 +++++++++++ tests/spec_fixtures/tools/audit/refval.py | 114 +++++++++++++----- tests/spec_pin.py | 2 +- tests/test_spec_encoding.py | 84 +++++++++++++ tests/test_spec_reader.py | 2 +- tests/test_spec_transitions.py | 22 +--- 14 files changed, 326 insertions(+), 60 deletions(-) create mode 100644 tests/spec_fixtures/fixtures/expected/v13_homogeneous_legacy.experiment_design.csv create mode 100644 tests/spec_fixtures/fixtures/expected/v33_condition_folder_on_forbidden_list.experiment_design.csv create mode 100644 tests/spec_fixtures/fixtures/expected/v34_technical_repeat_folder_on_forbidden_list.experiment_design.csv create mode 100644 tests/spec_fixtures/fixtures/transitions/t13_confirmed_on_with_image_count_only.toml create mode 100644 tests/test_spec_encoding.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0dc9de87..4fdf1c1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,17 @@ are in [docs/plan.md](docs/plan.md) §12. Upgrading from v0.3: see the ## Unreleased +- **Re-pin the vendored Spec to 7dbae0c (MorphoBricks-Spec PR 9).** New in the pin: the byte + rules and golden files of the derived `experiment_design.csv` (section 11; `fixtures/expected/` + for `v13`, `v33`, `v34`, not yet derived by any code: Stage 1 PR 10), the byte form of one + section digest (9.3), `t13` (a changed `image_count` alone does not reopen a confirmation) and + the encoding of `morphobricks.records.jsonl` (4.6). The reader, writer and publication agree + with every fixture and the reference validator (0 differences, no exception for `image_count` + any more; the oracle's section digests equal ours on every fixture). Fix: `read_records` split + the records file with `str.splitlines`, which also cuts at U+2028 and U+0085, so a record + holding such a character (written unescaped as the Spec requires) was refused as "not JSON"; + lines now end at LF only. Test helper: the catalogue no longer reads the golden-file rows as + fixtures. - **Discovery and header additions for the experiment description (Stage 1, PR 6).** Additive; a run without a description behaves as before. (1) `preflight` and `remasked` take `with_description=False`; with it, a condition, repeat or technical-repeat folder that diff --git a/src/morphobricks_preprocessing/run/description_file.py b/src/morphobricks_preprocessing/run/description_file.py index 08b5c239..ec159bc7 100644 --- a/src/morphobricks_preprocessing/run/description_file.py +++ b/src/morphobricks_preprocessing/run/description_file.py @@ -126,7 +126,8 @@ def read_records(output_folder: str | os.PathLike) -> tuple[dict, ...]: if not data.endswith(b"\n"): raise RecordsFileError(f"{RECORDS_FILENAME} ends in the middle of a line") try: - rows = tuple(json.loads(line) for line in data.decode("utf-8").splitlines()) + # Lines end at LF only (Spec 4.6): str.splitlines would also cut at U+2028, U+0085, ... + rows = tuple(json.loads(line) for line in data.decode("utf-8").split("\n")[:-1]) except ValueError as error: raise RecordsFileError(f"{RECORDS_FILENAME} holds a line that is not JSON") from error if not all(isinstance(row, dict) for row in rows): diff --git a/tests/spec_fixtures/PIN.toml b/tests/spec_fixtures/PIN.toml index df928d16..b43b39a9 100644 --- a/tests/spec_fixtures/PIN.toml +++ b/tests/spec_fixtures/PIN.toml @@ -1,13 +1,13 @@ # The MorphoBricks-Spec commit these files are copied from (conformance plan section 1). # Updating the pin is a reviewed change: see README.md in this folder. repository = "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/CellMigrationLab/MorphoBricks-Spec" -commit = "745259b72d4252f3ddaa563302d3343a1fa2ed49" +commit = "7dbae0c600ac051b492f4d5fe41dd0b6008649db" # `git rev-parse :fixtures`, the git tree-object ID of the fixtures/ folder. The # vendored folder must hash to it, so one changed, added or missing byte fails a test. -fixtures_tree = "d26daf758c914bdb0f6bfde1bba278d99cc33bb8" +fixtures_tree = "bb336c9494c371e0ba188f9be31832fdf19b0a14" # `git rev-parse :`, the git blob ID of each other vendored file. [files] -"docs/experiment-v1.md" = "6de05a197caf7b57c6ce0409f9593a9c56d23b7c" -"tools/audit/refval.py" = "2ce3a512fecafeb954aff32308f925ce23b8cd59" +"docs/experiment-v1.md" = "73662c0b76a18250958beb885b016e4ced56d019" +"tools/audit/refval.py" = "0c8274429919108bf013e32e8dd0563e081c47b3" diff --git a/tests/spec_fixtures/docs/experiment-v1.md b/tests/spec_fixtures/docs/experiment-v1.md index 6de05a19..73662c0b 100644 --- a/tests/spec_fixtures/docs/experiment-v1.md +++ b/tests/spec_fixtures/docs/experiment-v1.md @@ -386,6 +386,14 @@ stays in the brick that owns it. `morphobricks.toml`, and copies the records unchanged into the manifest of the next run. The records file is never rewritten or truncated, and nothing about a re-confirmation is stored in `morphobricks.toml`. Products keyed on that ID are invalidated. +- **Encoding of the records file.** UTF-8, one JSON object per line, each line ending in LF, keys + in code point order, non-ASCII characters unescaped. Keys: `type` (`"reconfirmation"`), + `id_kind` (`conditions`, `channels`, `repeats` or `technical_repeats`), `old_id`, `new_id` + (`null` when the same ID lost or was reassigned something), `confirmed_on` (`YYYY-MM-DD`, the + date of the user's action), `old_sha256` (of the published file replaced, or `absent`) and + `new_sha256` (of the file written). The records are appended after the file is replaced. A records + file whose last line lacks its LF, or that holds a line that is not a JSON object, is refused + before anything is published. - **A case-only correction needs a new ID (AQ2).** `ACTIN` cannot be corrected to `Actin`: the retired spelling stays in `used_ids` and the new one collides with it (`E_ID_CASE_COLLISION`, §4.9). The correction is a new, non-colliding ID (for example @@ -702,7 +710,11 @@ standard/inline/dotted tables, never changes meaning. The whole-file SHA-256 is provenance. Preprocessing SHOULD also record per-section digests (SHA-256 of the canonical serialization of `conditions`, `channels`, `repeats`, `masks`) so that consumers -key derived products only on the sections they use. `source_folder` is part of the `conditions` and +key derived products only on the sections they use. **The bytes of one section digest** are the +section's table blocks exactly as the canonical writer emits them in the file (§9.1), each block +followed by one blank line, with `display_name`, `target` and `treatment_key` left out and nothing +before or after; a section with no tables (no `masks`) is the SHA-256 of the empty byte string. +`source_folder` is part of the `conditions` and `repeats` digests (it fixes which folder an ID names; a reassignment changes them). Digests are computed with every `display_name`, `target` and `treatment_key` omitted (descriptive fields are never cache keys, §4.2a), and `used_ids` is not a section: a change to any of them MUST NOT change any digest, a change to `reference` MUST NOT @@ -736,7 +748,9 @@ silently; edits go to a separate draft and are published atomically after valida - **N35 reopening.** The confirmation of a repeat is reopened (draft `confirmed = false`) only when channels, layout or grouping change: a new or removed unit or member, a unit's ordered channel list, or the pairing of folders into repeats. New FOVs (or removed ones) inside an existing unit - whose confirmed layout still holds do **not** reopen it; they are reported and recorded. + whose confirmed layout still holds do **not** reopen it; they are reported and recorded. A + changed `image_count` alone is therefore not a reopening: a new `confirmed_on` beside only a + changed `image_count` is `R_CONFIRMED_ON_CHANGED` (§4.4a). - **Adding a member is allowed (AQ1).** A new member folder or unit joins an existing repeat or condition without a changed-ID refusal (§4.6); it is a grouping change, so it reopens the N35 confirmation of that repeat, and it never changes an existing ID. Only removing or reassigning @@ -774,7 +788,13 @@ deterministically from a published file, with the contract's byte rules: | `schema_version` | `1` | One row per member, sorted by `condition` (the source folder label), then `biological_repeat`, in -code point order. The CSV carries the **source folder labels exactly as today** and no technical +code point order. **Bytes** (preprocessing contract §12, rule `Rows`, and the `confirmed_on` +row): UTF-8, LF line endings, one header row naming the nine columns in the order of the table +above, booleans written `true` and `false`, `confirmed_on` written as the repeat's `confirmed_on` +(`YYYY-MM-DD`). The contract does not state the delimiter, the quoting, the final line terminator +or a byte-order mark, so these four are not yet normative; the golden files use a comma, no +quoting (no golden value needs it), LF after every row including the last, and no byte-order +mark, as today's writer does. The CSV carries the **source folder labels exactly as today** and no technical repeat or condition ID, so the choice of a condition or technical repeat ID (for example `ctrl` for the folder `control`) never changes it. For a legacy dataset whose `repeat_group` values migrate to the canonical repeat IDs and whose controls migrate deterministically, the derived file MUST be @@ -786,8 +806,8 @@ latest, on every row. **Byte-identity is guaranteed only for repeat groups with `confirmed_on`**; for a group with several dates the earlier per-row dates are the only difference and are recorded in the migration note, which is their only record. -Expected `experiment_design.csv` outputs (golden files) are **deferred to Stage 1**; Stage 0 fixes -only the derivation rules above. +Expected `experiment_design.csv` outputs (golden files) for `v13`, `v33` and `v34` are in +`fixtures/expected/`; a derived file MUST equal the golden file byte for byte. ## 12. Drafts diff --git a/tests/spec_fixtures/fixtures/README.md b/tests/spec_fixtures/fixtures/README.md index 657e854b..de26ffc7 100644 --- a/tests/spec_fixtures/fixtures/README.md +++ b/tests/spec_fixtures/fixtures/README.md @@ -28,7 +28,7 @@ is executed by this repository. fixtures that do not exercise the rule it equals the ID (the key), `single` for `single`; `v33`, `v34` and `t10`-`t12` separate the two. The eight undecodable or unparsable phase-1 fixtures (`i01`-`i08`) are unchanged and fail before it matters. -- `transitions/` holds twelve files that are valid alone and are checked against a named published +- `transitions/` holds thirteen files that are valid alone and are checked against a named published predecessor: refused with an exact set of publication codes (`R_CONFIRMED_ID_CHANGED`, `R_USED_ID_REUSED`, `R_USED_ID_DROPPED`, `R_CONFIRMED_ON_CHANGED`; experiment-v1 §4.4a, §4.6, §4.9; a changed or reassigned `source_folder` counts, §4.6a) or, for `t06`, **accepted** (adding members is not a changed entity, §4.6). They need the @@ -40,7 +40,8 @@ is executed by this repository. plan on a named fixture, listed in the coverage tables below. - A copy of this tree vendored by a reader MUST hash to the pinned SHA's `fixtures` tree (conformance plan §1). -- Expected `experiment_design.csv` outputs (golden files) are deferred to Stage 1. The fixed-SHA +- Expected `experiment_design.csv` outputs (golden files, experiment-v1 §11) are in `expected/`, + one per file listed under "Expected outputs" below; the other valid files have none yet. The fixed-SHA consumer audit (roadmap S0.1) is done (PR #5); its questions are answered by the third round of owner decisions (ADR 0001) and the fixtures `v28`-`v32`, `t06`-`t09`, `i100`-`i119` follow from them; the fourth round (FR1, FR2) adds `v33`, `v34`, `t10`-`t12` and `i120`-`i127`. @@ -255,7 +256,18 @@ This is a check of the fixtures against the text, not a conformance claim for an | [`v35_flat_experiment`](valid/v35_flat_experiment.toml) | Matrix audit 2026-11-07, case F01 | pass | yes | The experiment folder is itself the leaf: condition, repeat and technical repeat are all `single`. The condition `single` has `source_folder = "single"` ("no condition folder level", 4.2, 3.2) and is the only condition (owner decision SQ6, matrix audit MS-1). | | [`v36_repeat_folder_forbidden_name`](valid/v36_repeat_folder_forbidden_name.toml) | Matrix audit 2026-11-07, case F16 | pass | yes | Biological-repeat folders `z` paired into repeat `R1`; `source_repeat` is never restricted. The folder label `z` is not an allowed repeat ID (`i130`), so no ID is proposed and the user names `R1` (4.6a; owner decision SQ6, matrix audit MS-2). | -## Transition fixtures (12) +## Expected outputs (3) + +Golden `experiment_design.csv` files (experiment-v1 §11), byte for byte. Each is the derivation of the +named valid file by the stated rules; `tools/audit/refval.py` compares them. + +| Golden file | Derived from | Notes | +|---|---|---| +| [`v13_homogeneous_legacy.experiment_design.csv`](expected/v13_homogeneous_legacy.experiment_design.csv) | `valid/v13` | One control (`CTRL`, referenced by DrugA and DrugB); every `technical_repeat_count` 0 (only `single`). | +| [`v33_condition_folder_on_forbidden_list.experiment_design.csv`](expected/v33_condition_folder_on_forbidden_list.experiment_design.csv) | `valid/v33` | `condition` holds the folder label `control`, never the ID `ctrl`; `DrugA` sorts before `control` (code point order). | +| [`v34_technical_repeat_folder_on_forbidden_list.experiment_design.csv`](expected/v34_technical_repeat_folder_on_forbidden_list.experiment_design.csv) | `valid/v34` | Two technical repeats (folders `files`, `overrides`) give `technical_repeat_count` 2; `rep2` has only `single`, so 0. | + +## Transition fixtures (13) Valid alone; checked against the named published predecessor (refused with the listed set, or accepted). @@ -273,6 +285,7 @@ Valid alone; checked against the named published predecessor (refused with the l | [`t10_condition_id_changed_same_folder`](transitions/t10_condition_id_changed_same_folder.toml) | `valid/v33` | R_CONFIRMED_ID_CHANGED (against predecessor valid/v33, without a recorded re-confirmation) | The folder `control` was confirmed as condition `ctrl` in v33 and now carries the ID `control_cells`. A confirmed ID never changes in place, also when the folder is the same (experiment-v1 4.6, 4.6a). `ctrl` stays listed in used_ids, so only the changed ID is refused. Valid alone. | | [`t11_technical_repeat_id_changed_same_folder`](transitions/t11_technical_repeat_id_changed_same_folder.toml) | `valid/v34` | R_CONFIRMED_ID_CHANGED (against predecessor valid/v34, without a recorded re-confirmation) | The folder `files` was confirmed as technical repeat `acq1` in v34 and now carries the ID `acqA` in both members. A confirmed technical repeat ID never changes in place (experiment-v1 4.6, 4.6a). `acq1` stays listed in used_ids. Valid alone. | | [`t12_condition_folder_reassigned`](transitions/t12_condition_folder_reassigned.toml) | `valid/v33` | R_CONFIRMED_ID_CHANGED (against predecessor valid/v33, without a recorded confirmation) | The ID `ctrl` held the folder `control` in v33 and now holds the folder `ctl` (a folder renamed on disk and attached to the same ID without a recorded confirmation). A folder change for an existing ID is a reassignment, so a changed entity (experiment-v1 4.6, 7). Valid alone. | +| [`t13_confirmed_on_with_image_count_only`](transitions/t13_confirmed_on_with_image_count_only.toml) | `valid/v01` | `R_CONFIRMED_ON_CHANGED` (N35 not reopened) | Against v01 only CTRL/rep1 `image_count` (12 to 17, new FOVs) and rep1 `confirmed_on` (2026-11-07) change: new FOVs do not reopen the confirmation (experiment-v1 10), so the new date is refused. | ## Invalid fixtures (134) diff --git a/tests/spec_fixtures/fixtures/expected/v13_homogeneous_legacy.experiment_design.csv b/tests/spec_fixtures/fixtures/expected/v13_homogeneous_legacy.experiment_design.csv new file mode 100644 index 00000000..d7c316b6 --- /dev/null +++ b/tests/spec_fixtures/fixtures/expected/v13_homogeneous_legacy.experiment_design.csv @@ -0,0 +1,10 @@ +condition,biological_repeat,repeat_group,control,image_count,technical_repeat_count,confirmed,confirmed_on,schema_version +CTRL,rep1,rep1,true,10,0,true,2026-10-30,1 +CTRL,rep2,rep2,true,10,0,true,2026-10-30,1 +CTRL,rep3,rep3,true,10,0,true,2026-10-30,1 +DrugA,rep1,rep1,false,10,0,true,2026-10-30,1 +DrugA,rep2,rep2,false,10,0,true,2026-10-30,1 +DrugA,rep3,rep3,false,10,0,true,2026-10-30,1 +DrugB,rep1,rep1,false,10,0,true,2026-10-30,1 +DrugB,rep2,rep2,false,10,0,true,2026-10-30,1 +DrugB,rep3,rep3,false,10,0,true,2026-10-30,1 diff --git a/tests/spec_fixtures/fixtures/expected/v33_condition_folder_on_forbidden_list.experiment_design.csv b/tests/spec_fixtures/fixtures/expected/v33_condition_folder_on_forbidden_list.experiment_design.csv new file mode 100644 index 00000000..d5b2cd81 --- /dev/null +++ b/tests/spec_fixtures/fixtures/expected/v33_condition_folder_on_forbidden_list.experiment_design.csv @@ -0,0 +1,5 @@ +condition,biological_repeat,repeat_group,control,image_count,technical_repeat_count,confirmed,confirmed_on,schema_version +DrugA,rep1,rep1,false,11,0,true,2026-11-06,1 +DrugA,rep2,rep2,false,10,0,true,2026-11-06,1 +control,rep1,rep1,true,12,0,true,2026-11-06,1 +control,rep2,rep2,true,10,0,true,2026-11-06,1 diff --git a/tests/spec_fixtures/fixtures/expected/v34_technical_repeat_folder_on_forbidden_list.experiment_design.csv b/tests/spec_fixtures/fixtures/expected/v34_technical_repeat_folder_on_forbidden_list.experiment_design.csv new file mode 100644 index 00000000..39c2b443 --- /dev/null +++ b/tests/spec_fixtures/fixtures/expected/v34_technical_repeat_folder_on_forbidden_list.experiment_design.csv @@ -0,0 +1,5 @@ +condition,biological_repeat,repeat_group,control,image_count,technical_repeat_count,confirmed,confirmed_on,schema_version +CTRL,rep1,rep1,true,12,2,true,2026-11-06,1 +CTRL,rep2,rep2,true,10,0,true,2026-11-06,1 +DrugA,rep1,rep1,false,11,2,true,2026-11-06,1 +DrugA,rep2,rep2,false,10,0,true,2026-11-06,1 diff --git a/tests/spec_fixtures/fixtures/transitions/t13_confirmed_on_with_image_count_only.toml b/tests/spec_fixtures/fixtures/transitions/t13_confirmed_on_with_image_count_only.toml new file mode 100644 index 00000000..8f608518 --- /dev/null +++ b/tests/spec_fixtures/fixtures/transitions/t13_confirmed_on_with_image_count_only.toml @@ -0,0 +1,71 @@ +# Fixture t13_confirmed_on_with_image_count_only +# Expected: R_CONFIRMED_ON_CHANGED (against predecessor valid/v01, N35 not reopened) +# Source: Preprocessing PR 188 (Stage 1 publication); experiment-v1 10 +# Valid on its own. Against v01, rep1 has the same members, units and channel lists and only two things change: CTRL/rep1 `image_count` 12 to 17 (new FOVs in an existing unit) and `confirmed_on` 2026-11-06 to 2026-11-07. New FOVs do not reopen the N35 confirmation (experiment-v1 10), so the new date is refused: a changed `image_count` alone never makes a new `confirmed_on` acceptable. +schema = "morphobricks-experiment" +schema_version = 1 + +[conditions.CTRL] +source_folder = "CTRL" +display_name = "Control" + +[conditions.DrugA] +source_folder = "DrugA" +display_name = "Drug A" +reference = "CTRL" +target = "SRC" +treatment_key = "saracatinib" + +[channels.DNA] +display_name = "DAPI" +header_names = ["DAPI"] + +[channels.ACTIN] +display_name = "F-actin" +header_names = ["Phalloidin-488"] + +[repeats.rep1] +confirmed = true +confirmed_on = "2026-11-07" + +[repeats.rep1.members.CTRL] +source_repeat = "rep1" +image_count = 17 + +[repeats.rep1.members.CTRL.technical_repeats.single] +source_folder = "single" +channels = ["DNA", "ACTIN"] + +[repeats.rep1.members.DrugA] +source_repeat = "rep1" +image_count = 11 + +[repeats.rep1.members.DrugA.technical_repeats.single] +source_folder = "single" +channels = ["DNA", "ACTIN"] + +[repeats.rep2] +confirmed = true +confirmed_on = "2026-11-06" + +[repeats.rep2.members.CTRL] +source_repeat = "rep2" +image_count = 10 + +[repeats.rep2.members.CTRL.technical_repeats.single] +source_folder = "single" +channels = ["DNA", "ACTIN"] + +[repeats.rep2.members.DrugA] +source_repeat = "rep2" +image_count = 10 + +[repeats.rep2.members.DrugA.technical_repeats.single] +source_folder = "single" +channels = ["DNA", "ACTIN"] + +[used_ids] +conditions = ["CTRL", "DrugA"] +channels = ["ACTIN", "DNA"] +repeats = ["rep1", "rep2"] +technical_repeats = ["single"] diff --git a/tests/spec_fixtures/tools/audit/refval.py b/tests/spec_fixtures/tools/audit/refval.py index 2ce3a512..0c827442 100644 --- a/tests/spec_fixtures/tools/audit/refval.py +++ b/tests/spec_fixtures/tools/audit/refval.py @@ -14,7 +14,9 @@ from __future__ import annotations +import csv import hashlib +import io import json import re import sys @@ -625,26 +627,39 @@ def _block(header: str, tbl: dict, order, sort_arrays=()) -> list[str]: return lines + [""] +def section_lines(doc: dict, sec: str) -> list[str]: + """The table blocks of one section, each followed by a blank line (section 9.1).""" + out: list[str] = [] + if sec == "conditions": + for cid in sorted(doc["conditions"]): + out += _block("conditions." + _key(cid), doc["conditions"][cid], COND) + elif sec == "channels": + for chid in sorted(doc["channels"]): + out += _block("channels." + _key(chid), doc["channels"][chid], CHAN, + ("header_names",)) + elif sec == "repeats": + for rid in sorted(doc["repeats"]): + rep = doc["repeats"][rid] + rh = "repeats." + _key(rid) + out += _block(rh, rep, REP) + for cid in sorted(rep["members"]): + m = rep["members"][cid] + mh = rh + ".members." + _key(cid) + out += _block(mh, m, MEM) + for tid in sorted(m["technical_repeats"]): + out += _block(mh + ".technical_repeats." + _key(tid), + m["technical_repeats"][tid], UNIT) + else: + for mid in sorted(doc.get("masks", {})): + out += _block("masks." + _key(mid), doc["masks"][mid], MASK, ("source_channels",)) + return out + + def write(doc: dict) -> bytes: """Canonical bytes of a valid document (experiment-v1 section 9.1).""" out = [f"schema = {_str(doc['schema'])}", f"schema_version = {doc['schema_version']}", ""] - for cid in sorted(doc["conditions"]): - out += _block("conditions." + _key(cid), doc["conditions"][cid], COND) - for chid in sorted(doc["channels"]): - out += _block("channels." + _key(chid), doc["channels"][chid], CHAN, ("header_names",)) - for rid in sorted(doc["repeats"]): - rep = doc["repeats"][rid] - rh = "repeats." + _key(rid) - out += _block(rh, rep, REP) - for cid in sorted(rep["members"]): - m = rep["members"][cid] - mh = rh + ".members." + _key(cid) - out += _block(mh, m, MEM) - for tid in sorted(m["technical_repeats"]): - out += _block(mh + ".technical_repeats." + _key(tid), - m["technical_repeats"][tid], UNIT) - for mid in sorted(doc.get("masks", {})): - out += _block("masks." + _key(mid), doc["masks"][mid], MASK, ("source_channels",)) + for sec in ("conditions", "channels", "repeats", "masks"): + out += section_lines(doc, sec) out.append("[used_ids]") for k in USED: out.append(f"{k} = {_val(sorted(doc['used_ids'][k]))}") @@ -677,19 +692,45 @@ def _strip(x): def section_digests(doc: dict) -> dict[str, str]: - """SHA-256 of the canonical serialization of each stripped section. + """SHA-256 of one section's table blocks as the canonical writer emits them (section 9.3). - AMBIGUITY: section 9.3 does not define the byte form of a section alone; the canonical writer - is applied to a document holding only that section (other required sections empty). + The blocks of the stripped section (no `display_name`, `target`, `treatment_key`), each + followed by one blank line, nothing before or after; an absent section is the empty bytes. """ - out = {} s = _strip(doc) - for sec in ("conditions", "channels", "repeats", "masks"): - part = {"schema": "", "schema_version": 1, "conditions": {}, "channels": {}, - "repeats": {}, "masks": {}, "used_ids": {k: [] for k in USED}} - part[sec] = s.get(sec, {}) - out[sec] = hashlib.sha256(write(part)).hexdigest() - return out + return {sec: hashlib.sha256("".join(f"{line}\n" for line in section_lines(s, sec)) + .encode("utf-8")).hexdigest() + for sec in ("conditions", "channels", "repeats", "masks")} + + +# ------------------------------------------------------------------ derived CSV (section 11) +CSV_COLUMNS = ("condition", "biological_repeat", "repeat_group", "control", "image_count", + "technical_repeat_count", "confirmed", "confirmed_on", "schema_version") + + +def derive_csv(doc: dict) -> bytes: + """The derived experiment_design.csv of a valid document (experiment-v1 section 11). + + AMBIGUITY: the contract (section 12) fixes UTF-8, LF, the header row, the column order, + `true`/`false` and the sort; it does not state the delimiter, the quoting, a final line + terminator or a byte-order mark. Here: comma, minimal quoting, LF after every row including + the last, no byte-order mark (as today's writer does). + """ + referenced = {c["reference"] for c in doc["conditions"].values() if "reference" in c} + rows = [] + for rid, cid, m in members(doc): + tr = m["technical_repeats"] + single = len(tr) == 1 and next(iter(tr.values()))["source_folder"] == "single" + rows.append([doc["conditions"][cid]["source_folder"], m["source_repeat"], rid, + "true" if cid in referenced else "false", str(m["image_count"]), + "0" if single else str(len(tr)), "true", + doc["repeats"][rid]["confirmed_on"], "1"]) + rows.sort(key=lambda r: (r[0], r[1])) + buf = io.StringIO() + w = csv.writer(buf, lineterminator="\n") + w.writerow(CSV_COLUMNS) + w.writerows(rows) + return buf.getvalue().encode("utf-8") # ------------------------------------------------------------------ transitions (R_*) @@ -772,10 +813,15 @@ def positions(doc): not set(old["channels"][ch].get("header_names", [])) <= \ set(new["channels"][ch].get("header_names", [])): codes.add("R_CONFIRMED_ID_CHANGED") - # confirmed_on of a repeat whose confirmation is not reopened + # confirmed_on of a repeat whose confirmation is not reopened (section 10: members, units and + # channel lists unchanged; a changed image_count alone does not reopen it) + def layout(rep): + return {c: {k: v for k, v in _strip(m).items() if k != "image_count"} + for c, m in rep["members"].items()} + for rid in set(old["repeats"]) & set(new["repeats"]): o, n = old["repeats"][rid], new["repeats"][rid] - if _strip(o["members"]) == _strip(n["members"]) and o["confirmed_on"] != n["confirmed_on"]: + if layout(o) == layout(n) and o["confirmed_on"] != n["confirmed_on"]: codes.add("R_CONFIRMED_ON_CHANGED") return codes @@ -832,6 +878,16 @@ def run_fixtures(root: Path) -> int: bad += 1 print(f"MISMATCH transition {f.stem} vs {pred}: got {sorted(got)} expected {sorted(exp)}") counts["transition-checks"] = counts.get("transition-checks", 0) + 1 + # derived CSV golden files (section 11): each is listed in the README and equals the derivation + for f in sorted((fx / "expected").glob("*.csv")): + key = f.stem.split("_")[0] + if f"`{f.name}`" not in readme: + bad += 1 + print(f"NO-CATALOGUE-ROW expected/{f.name}") + if key not in docs or derive_csv(docs[key]) != f.read_bytes(): + bad += 1 + print(f"MISMATCH expected/{f.name}: not the derivation of {key}") + counts["expected"] = counts.get("expected", 0) + 1 # round trip and digests for k, d in docs.items(): if k.startswith("v") or k.startswith("t"): diff --git a/tests/spec_pin.py b/tests/spec_pin.py index 8461e00a..118b8901 100644 --- a/tests/spec_pin.py +++ b/tests/spec_pin.py @@ -20,7 +20,7 @@ ROOT_ENV = "MORPHOBRICKS_SPEC_ROOT" KINDS = ("valid", "invalid", "transitions") CODE = re.compile(r"\b([EWRA]_[A-Z0-9_]+)\b") -ROW = re.compile(r"\| \[`([vit]\d+[^`]*)`\]\([^)]+\) \|(.*)") +ROW = re.compile(r"\| \[`([vit]\d+[^`]*)`\]\((?:valid|invalid|transitions)/[^)]+\) \|(.*)") class SpecPinError(Exception): diff --git a/tests/test_spec_encoding.py b/tests/test_spec_encoding.py new file mode 100644 index 00000000..6005ccd8 --- /dev/null +++ b/tests/test_spec_encoding.py @@ -0,0 +1,84 @@ +"""Byte-level rules the Spec 7dbae0c states: section digests (9.3) and the records file (4.6). + +The digests of every vendored fixture equal the oracle's; the records file is UTF-8, one JSON +object per line, LF after every line, keys in code point order, non-ASCII characters unescaped, +and a torn or foreign file is refused before anything is published. +""" + +from __future__ import annotations + +import hashlib +import tomllib + +import pytest + +from morphobricks_preprocessing.description import description_from_bytes +from morphobricks_preprocessing.description_file import DESCRIPTION_FILENAME, RECORDS_FILENAME +from morphobricks_preprocessing.description_transition import Reconfirmation +from morphobricks_preprocessing.description_write import canonical_bytes, section_digests +from morphobricks_preprocessing.run.description_file import ( + RecordsFileError, + _append_records, + publish_description, + read_records, +) +from spec_oracle import refval +from spec_pin import fixtures + +ALL = [*fixtures("valid"), *fixtures("transitions")] +BY_NAME = {item.name.split("_")[0]: item for item in ALL} + + +@pytest.mark.parametrize("item", ALL, ids=lambda item: item.name) +def test_the_section_digests_equal_the_oracles(item): + oracle = refval() + expected = oracle.section_digests(oracle.model(tomllib.loads(item.data.decode("utf-8")))) + assert section_digests(description_from_bytes(item.data)) == expected + + +def test_a_section_with_no_tables_digests_the_empty_byte_string(): + model = description_from_bytes(BY_NAME["v01"].data) + assert not model.masks + assert section_digests(model)["masks"] == hashlib.sha256(b"").hexdigest() + + +def test_the_records_file_bytes_are_those_of_section_4_6(tmp_path): + base = hashlib.sha256(BY_NAME["v01"].data).hexdigest() + (tmp_path / DESCRIPTION_FILENAME).write_bytes(BY_NAME["v01"].data) + candidate = description_from_bytes(BY_NAME["t01"].data) + both = ( + Reconfirmation("repeats", "rep1", "Day1", "2026-11-07"), + Reconfirmation("repeats", "rep2", "Day2", "2026-11-07"), + ) + done = publish_description(tmp_path, candidate, base_sha256=base, confirmations=both) + new = hashlib.sha256(canonical_bytes(candidate)).hexdigest() + line = ( + '{"confirmed_on": "2026-11-07", "id_kind": "repeats", "new_id": "%s", "new_sha256": "%s", ' + '"old_id": "%s", "old_sha256": "%s", "type": "reconfirmation"}\n' + ) + expected = (line % ("Day1", new, "rep1", base) + line % ("Day2", new, "rep2", base)).encode() + assert (tmp_path / RECORDS_FILENAME).read_bytes() == expected + assert done.sha256 == new + + +def test_non_ascii_text_is_written_unescaped_and_read_back_whole(tmp_path): + # U+2028 and U+0085 are line breaks to str.splitlines but not to the file format. + record = {"type": "reconfirmation", "old_id": "café
x\u0085y", "new_id": None} + _append_records(tmp_path, (record,)) + data = (tmp_path / RECORDS_FILENAME).read_bytes() + assert "café
x\u0085y".encode() in data and b"\\u" not in data + assert data.count(b"\n") == 1 and data.endswith(b"\n") + assert read_records(tmp_path) == (record,) + + +@pytest.mark.parametrize( + "content", + [b'{"a": 1}', b'{"a": 1}\n[1]\n', b'{"a": 1}\nnot json\n', b'{"a": 1}\n\n', b"\xff\xfe\n"], +) +def test_a_torn_or_foreign_records_file_is_refused_before_anything_is_written(tmp_path, content): + (tmp_path / RECORDS_FILENAME).write_bytes(content) + with pytest.raises(RecordsFileError): + read_records(tmp_path) + with pytest.raises(RecordsFileError): + _append_records(tmp_path, ({"type": "reconfirmation"},)) + assert (tmp_path / RECORDS_FILENAME).read_bytes() == content diff --git a/tests/test_spec_reader.py b/tests/test_spec_reader.py index 5d4f68d6..0a08922a 100644 --- a/tests/test_spec_reader.py +++ b/tests/test_spec_reader.py @@ -24,7 +24,7 @@ def _name(item): def test_the_fixture_counts_are_the_catalogue_s(): - assert (len(VALID), len(INVALID), len(TRANSITIONS)) == (36, 134, 12) + assert (len(VALID), len(INVALID), len(TRANSITIONS)) == (36, 134, 13) @pytest.mark.parametrize("item", VALID, ids=_name) diff --git a/tests/test_spec_transitions.py b/tests/test_spec_transitions.py index be458adc..c421bbe5 100644 --- a/tests/test_spec_transitions.py +++ b/tests/test_spec_transitions.py @@ -1,14 +1,12 @@ -"""The transition fixtures t01-t12 and the Spec's oracle, for the ``R_*`` comparison. +"""The transition fixtures t01-t13 and the Spec's oracle, for the ``R_*`` comparison. Each transition file is offered over its named predecessor and must give exactly the listed set of ``R_*`` codes (or none); the oracle (``refval.transition``) must agree on every ordered pair of valid and transition files, which also covers the used-ID and ``confirmed_on`` rules in -combinations the 12 fixtures do not reach. +combinations the 13 fixtures do not reach. -One deliberate difference: the oracle takes a changed ``image_count`` as reopening a repeat's -confirmation (so it accepts a new ``confirmed_on`` beside it). Section 10 says new FOVs in an -existing unit do not reopen it, so the reader refuses that date change (``R_CONFIRMED_ON_CHANGED``); -the pairs are compared with every ``image_count`` set to 1 and the difference has its own test. +Since Spec 7dbae0c the oracle agrees that a changed ``image_count`` alone does not reopen a +confirmation (section 10, t13), so the pairs are compared as they are. """ from __future__ import annotations @@ -37,14 +35,6 @@ PREDECESSOR = re.compile(r"predecessor valid/(v\d+)") -def _counts_equal(doc: dict) -> dict: - """Return the oracle's model with every member's image count set to 1.""" - for repeat in doc["repeats"].values(): - for member in repeat["members"].values(): - member["image_count"] = 1 - return doc - - def _codes(old, new) -> set[str]: return {issue.code.value for issue in transition_issues(old, new)} @@ -75,10 +65,10 @@ def test_nothing_is_refused_when_the_candidate_is_the_published_file(): def test_the_oracle_agrees_on_every_ordered_pair(old): oracle = refval() old_model = description_from_bytes(old.data) - old_doc = _counts_equal(oracle.model(tomllib.loads(old.data.decode("utf-8")))) + old_doc = oracle.model(tomllib.loads(old.data.decode("utf-8"))) differences = [] for new in ALL: - new_doc = _counts_equal(oracle.model(tomllib.loads(new.data.decode("utf-8")))) + new_doc = oracle.model(tomllib.loads(new.data.decode("utf-8"))) ours = _codes(old_model, description_from_bytes(new.data)) theirs = oracle.transition(old_doc, new_doc) if ours != theirs: