Strict description reader, phases 4-5 and warnings (Stage 1, PR 3 of 3)

AGENTS.md

@@ -92,6 +92,18 @@ src/morphobricks_preprocessing/
                     read_confirmed_design/ConfirmedDesign (the strict reader other bricks use, §16)
   design_values.py  the design file's header check (no column twice or unnamed, #83), booleans,
                     counts, dates and DesignFormatError (pure; re-exported by design.py)
+  description_rules.py  the one table of the experiment-description rules (Stage 1, Spec
+                    experiment-v1): codes, field tables, forbidden names, enumerations; compared
+                    with the vendored text by tests/test_spec_rules_table.py
+  description_report.py DescriptionIssue (code, table path, value), DescriptionReport
+  description_read.py   the reference reader's bytes -> phases: decode (1), header (2), the
+                    driver (read_structure: phases 1-3)
+  description_structure.py phase 3: closed fields, forbidden names, exact types, ranges, enums
+  description_identity.py phase 4: Label (check_identity_text), Token, reserved, NFC, single,
+                    case-collision scopes; description_semantics.py phase 5: references,
+                    RGB rules, display names, used_ids, one folder one ID, reference graph;
+                    description_warnings.py the three warnings; description_walk.py members/units
+                    (read_description_bytes in description_read.py runs phases 1-5 and warnings)
   image_type.py     image type (P4): FLUORESCENCE/RGB, review_image_type (suggestion from
                     TiffMetadata.rgb_samples and the confirmed roles, whether RGB is possible via the
                     brick's check_rgb), image_type_issues (rgb_not_possible)

CHANGELOG.md

@@ -8,6 +8,24 @@ are in [docs/plan.md](docs/plan.md) §12. Upgrading from v0.3: see the
 
 ## Unreleased
 
+- **Experiment-description reader, phases 4-5 and warnings (Stage 1, PR 3).** New modules
+  `description_identity`, `description_semantics`, `description_warnings` and
+  `description_walk`; `description_read.read_description_bytes(data: bytes)` runs all five
+  phases and the three warnings of Spec experiment-v1 and reports every error of the first
+  failing phase. Every Spec `valid/` fixture passes with its exact warnings and every
+  `invalid/` fixture fails with its exact code set; the Spec's reference validator is the test
+  oracle. Not used by any run, notebook or CLI command yet.
+- **Experiment-description reader, phases 1-3 (Stage 1, PR 2).** New modules
+  `description_rules`, `description_report`, `description_read` and `description_structure`:
+  `read_structure(data: bytes)` runs the strict phases 1 to 3 of Spec experiment-v1 (bytes and
+  encoding, duplicate keys, header and version, structure, forbidden fields) and reports every
+  error of the first failing phase with its code, table path and value. Not used by any run,
+  notebook or CLI command yet.
+- **Spec fixtures vendored (Stage 1, PR 1).** `tests/spec_fixtures/` holds the
+  MorphoBricks-Spec conformance fixtures at a pinned commit, with a git tree-hash pin test,
+  a gate that makes a skipped `tests/test_spec_*.py` test a failure, and the "no fallback
+  rules" scan (conformance plan sections 1 and 2). Tests only: no change to the package,
+  the CLI, the notebook or any output.
 - **Group statistics per channel (#104).** `group_summaries` and the notebook's step 4 no
   longer pool distinct channels together: each group has one summary per channel, and the
   intensity curves are drawn in one panel per channel. Single-channel tables give the same

docs/development.md

@@ -101,6 +101,23 @@ is pinned to the release in `RELEASE_TAG` (plan §12 question 58);
 - Every acceptance check in the plan has a test.
 - A bug fix comes with a test that fails without it.
 
+## Spec conformance tests
+
+The experiment-description reader is tested against the fixtures of
+[MorphoBricks-Spec](https://github.com/CellMigrationLab/MorphoBricks-Spec)
+(conformance plan, sections 1 and 2). They are vendored in `tests/spec_fixtures/` at the full
+commit SHA in its `PIN.toml`, never a branch.
+
+- **Pin integrity:** `tests/test_spec_pin.py` fails when one byte of the vendored `fixtures/`
+  differs, or a file is added or missing (git tree hash against the pin), so a local edit
+  of a fixture cannot make a reader pass. Never edit the vendored files; updating the pin
+  is a reviewed change (`tests/spec_fixtures/README.md`).
+- **Skip is failure:** every test module named `tests/test_spec_*.py` is held by
+  `tests/spec_gate.py`: a skipped or xfailed test there fails the run.
+- **No second copy of the rules:** `tests/test_spec_no_fallback.py` scans the reader's
+  modules (`description*.py`) for a second table of codes or names and for lenient, older or
+  default-version paths.
+
 ## Versioning
 
 - Semantic versioning for the package.

pyproject.toml

@@ -66,7 +66,8 @@ addopts = "-ra"
 line-length = 100
 target-version = "py311"
 src = ["src", "tests", "tools"]
-extend-exclude = ["tests/guard_fixtures"]
+# The vendored Spec files are byte-exact copies (tests/spec_fixtures/README.md).
+extend-exclude = ["tests/guard_fixtures", "tests/spec_fixtures"]
 
 [tool.ruff.lint]
 select = [
@@ -171,6 +172,14 @@ baseline = "tools/code_rules_baseline.json"
 "morphobricks_preprocessing.dataset" = "pure"
 "morphobricks_preprocessing.design" = "io"
 "morphobricks_preprocessing.design_values" = "pure"
+"morphobricks_preprocessing.description_identity" = "pure"
+"morphobricks_preprocessing.description_read" = "pure"
+"morphobricks_preprocessing.description_semantics" = "pure"
+"morphobricks_preprocessing.description_walk" = "pure"
+"morphobricks_preprocessing.description_warnings" = "pure"
+"morphobricks_preprocessing.description_report" = "pure"
+"morphobricks_preprocessing.description_rules" = "pure"
+"morphobricks_preprocessing.description_structure" = "pure"
 "morphobricks_preprocessing.design_edit" = "pure"
 "morphobricks_preprocessing.discovery" = "io"
 "morphobricks_preprocessing.links" = "io"

src/morphobricks_preprocessing/description_identity.py

@@ -0,0 +1,177 @@
+"""Validation phase 4, identity (experiment-v1 sections 3 and 6).
+
+The Label rules (the brick's ``check_identity_text``, decision D-e), the Token rule, the reserved
+list, NFC, ``single`` and the case-collision scopes. Each broken rule has its own code and every
+rule an identity breaks is reported (one code per broken rule). Nothing is trimmed, normalized or
+repaired: the NFC form of a value is a comparison key only and never leaves this module.
+"""
+
+from __future__ import annotations
+
+import unicodedata
+from collections.abc import Mapping
+
+from morphobricks_normalization.records import SINGLE
+
+from .description_report import Issues, key_path
+from .description_rules import OBJECT_SET_PART, RESERVED, TOKEN, Code
+from .description_walk import Table, member_path, members, unit_path
+from .discovery import IdentityError, check_identity_text
+
+USED_KINDS = ("conditions", "channels", "repeats", "technical_repeats")
+
+
+def nfc(value: str) -> str:
+    """Return the NFC form of ``value`` (a comparison key, never stored)."""
+    return unicodedata.normalize("NFC", value)
+
+
+class Identities:
+    """The identity checks of one document, and the values of each case-collision scope."""
+
+    def __init__(self, out: Issues) -> None:
+        self.out = out
+        self.scopes: dict[str, list[tuple[str, str]]] = {}
+
+    def label(self, value: str, path: str) -> None:
+        """Apply the Label rules (``E_ID_TEXT``)."""
+        try:
+            check_identity_text(value, "identity")
+        except IdentityError:
+            self.out.add(Code.E_ID_TEXT, path, value)
+
+    def token(self, value: str, path: str) -> None:
+        """Apply the Label rules and the Token regular expression, as two separate rules."""
+        self.label(value, path)
+        if not TOKEN.fullmatch(value):
+            self.out.add(Code.E_ID_SYNTAX, path, value)
+
+    def nfc(self, value: str, path: str) -> None:
+        """Refuse a value that is not NFC (``E_ID_NOT_NFC``); it is never normalized."""
+        if nfc(value) != value:
+            self.out.add(Code.E_ID_NOT_NFC, path, value)
+
+    def single(self, value: str, path: str) -> None:
+        """Refuse ``single`` in another case (``E_SINGLE_MISUSE``)."""
+        if value.casefold() == SINGLE and value != SINGLE:
+            self.out.add(Code.E_SINGLE_MISUSE, path, value)
+
+    def reserved(self, value: str, path: str, *, object_set: bool = False) -> None:
+        """Refuse a name on the reserved list (and, for a mask, an ``in`` part)."""
+        folded = value.casefold()
+        if folded in RESERVED or (object_set and OBJECT_SET_PART in folded.split("_")):
+            self.out.add(Code.E_ID_RESERVED, path, value)
+
+    def label_id(self, value: str, path: str, scope: str) -> None:
+        """Check an identity of the Label kind and enter it in its collision scope."""
+        self.label(value, path)
+        self.nfc(value, path)
+        self.single(value, path)
+        self.scopes.setdefault(scope, []).append((value, path))
+
+    def collisions(self) -> None:
+        """Report identities of one scope that differ after NFC but not by case folding."""
+        for items in self.scopes.values():
+            spellings: dict[str, set[str]] = {}
+            for value, _ in items:
+                spellings.setdefault(nfc(value).casefold(), set()).add(nfc(value))
+            for value, path in items:
+                if len(spellings[nfc(value).casefold()]) > 1:
+                    self.out.add(Code.E_ID_CASE_COLLISION, path, value)
+
+
+def _conditions(doc: Table, ids: Identities) -> None:
+    for identifier, table in doc["conditions"].items():
+        path = key_path("conditions", identifier)
+        ids.label_id(identifier, path, "conditions")
+        ids.label_id(table["source_folder"], f"{path}.source_folder", "condition_folders")
+        if "reference" in table:  # a reference is checked for NFC only, then resolved (phase 5)
+            ids.nfc(table["reference"], f"{path}.reference")
+
+
+def _channels(doc: Table, ids: Identities) -> None:
+    for identifier in doc["channels"]:
+        path = key_path("channels", identifier)
+        ids.token(identifier, path)
+        ids.nfc(identifier, path)
+        ids.single(identifier, path)
+        ids.reserved(identifier, path)
+        ids.scopes.setdefault("channels", []).append((identifier, path))
+
+
+def _unit(
+    ids: Identities, repeat: str, condition: str, unit: str, table: Table, count: int
+) -> None:
+    path = unit_path(repeat, condition, unit)
+    ids.label_id(unit, path, "technical_repeats")
+    ids.label_id(table["source_folder"], f"{path}.source_folder", "technical_folders")
+    if unit == SINGLE and count > 1:
+        ids.out.add(Code.E_SINGLE_MISUSE, path, "single is not the only technical repeat")
+    if (unit == SINGLE) != (table["source_folder"] == SINGLE):
+        ids.out.add(Code.E_SINGLE_MISUSE, f"{path}.source_folder", table["source_folder"])
+    for index, channel in enumerate(table["channels"]):
+        ids.nfc(channel, f"{path}.channels[{index}]")
+
+
+def _repeats(doc: Table, ids: Identities) -> None:
+    """Check repeat IDs, source repeats, technical repeats and the ``single`` rules."""
+    for identifier in doc["repeats"]:
+        ids.label_id(identifier, key_path("repeats", identifier), "repeats")
+    by_condition: dict[str, list[tuple[str, str]]] = {}
+    for repeat, condition, member in members(doc):
+        path = member_path(repeat, condition)
+        ids.nfc(condition, path)  # a member key is a reference to a condition ID
+        ids.label_id(member["source_repeat"], f"{path}.source_repeat", "source_repeats")
+        by_condition.setdefault(condition, []).append((member["source_repeat"], path))
+        for unit, table in member["technical_repeats"].items():
+            _unit(ids, repeat, condition, unit, table, len(member["technical_repeats"]))
+    for entries in by_condition.values():
+        if len(entries) > 1:  # "single" must be the only member of its condition
+            for source, path in entries:
+                if source == SINGLE:
+                    message = "single is not the only member"
+                    ids.out.add(Code.E_SINGLE_MISUSE, f"{path}.source_repeat", message)
+
+
+def _masks(doc: Table, ids: Identities) -> None:
+    for identifier, table in doc.get("masks", {}).items():
+        path = key_path("masks", identifier)
+        ids.token(identifier, path)
+        ids.nfc(identifier, path)
+        ids.single(identifier, path)
+        ids.reserved(identifier, path, object_set=True)
+        ids.scopes.setdefault("masks", []).append((identifier, path))
+        ids.label_id(table["source_type"], f"{path}.source_type", "mask_types")
+        for index, channel in enumerate(table.get("source_channels", [])):
+            ids.nfc(channel, f"{path}.source_channels[{index}]")
+
+
+def _used_ids(doc: Table, ids: Identities) -> None:
+    """Entries follow the identity rules of their kind and share its collision scope.
+
+    The reserved list is not applied to ``used_ids`` entries: the text says "the identity rules
+    of their kind" and no fixture decides it (the reference validator does the same).
+    """
+    for kind in USED_KINDS:
+        for index, value in enumerate(doc["used_ids"][kind]):
+            path = f"used_ids.{kind}[{index}]"
+            if kind == "channels":
+                ids.token(value, path)
+            else:
+                ids.label(value, path)
+            ids.nfc(value, path)
+            ids.single(value, path)
+            ids.scopes.setdefault(kind, []).append((value, path))
+
+
+def check_identity(doc: Mapping[str, object]) -> Issues:
+    """Return every phase-4 error of a document that passed the structure phase."""
+    out = Issues()
+    ids = Identities(out)
+    _conditions(doc, ids)
+    _channels(doc, ids)
+    _repeats(doc, ids)
+    _masks(doc, ids)
+    _used_ids(doc, ids)
+    ids.collisions()
+    return out