Description reader: own rule vocabulary; pin path and value of five refusals

CHANGELOG.md

@@ -8,6 +8,14 @@ are in [docs/plan.md](docs/plan.md) §12. Upgrading from v0.3: see the
 
 ## Unreleased
 
+- **Reader owns its rule vocabulary; five refusals pinned by path (#204).**
+  `description_identity` no longer imports `check_identity_text` from `discovery` or `SINGLE`
+  from normalization (Spec §2.7): `description_rules` holds `MISSING_TEXT`, `SINGLE` and
+  `label_refused`, and `tests/test_description_rules_copy.py` keeps them equal to the shared
+  values. Behaviour is unchanged. `tests/test_spec_reader_refusal_paths.py` pins the path and
+  value of `E_SOURCE_FOLDER_REUSED`, `E_SOURCE_FOLDER_CONFLICT`, `E_SOURCE_REPEAT_REUSED`,
+  `E_MASK_SOURCE_TYPE_REUSED` and `E_REFERENCE_CYCLE`, which the oracle compares by code only.
+  The reader's paths were already correct for all five.
 - **Per-unit header evidence for a described run (Stage 1, PR 13; conformance P26).** When the
   output folder holds a published `morphobricks.toml`, `run` no longer applies the
   experiment-wide v0.4 checks `channel_names_differ` and `different_channel_counts`: units may

src/morphobricks_preprocessing/description_identity.py

@@ -1,6 +1,6 @@
 """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
+The Label rules (the reader's own ``label_refused``, 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.
@@ -11,12 +11,9 @@
 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_rules import OBJECT_SET_PART, RESERVED, SINGLE, TOKEN, Code, label_refused
 from .description_walk import Table, member_path, members, unit_path
-from .discovery import IdentityError, check_identity_text
 
 USED_KINDS = ("conditions", "channels", "repeats", "technical_repeats")
 
@@ -35,9 +32,7 @@ def __init__(self, out: Issues) -> None:
 
     def label(self, value: str, path: str) -> None:
         """Apply the Label rules (``E_ID_TEXT``)."""
-        try:
-            check_identity_text(value, "identity")
-        except IdentityError:
+        if label_refused(value):
             self.out.add(Code.E_ID_TEXT, path, value)
 
     def token(self, value: str, path: str) -> None:

src/morphobricks_preprocessing/description_rules.py

@@ -162,3 +162,20 @@ class TableSpec:
     }
 )  # fmt: skip
 """The one reserved list of channel IDs and mask IDs, compared case-folded (section 3.2)."""
+
+MISSING_TEXT = frozenset({"", "na", "nan", "none", "null", "n/a", "<na>"})
+"""The names the Label rule refuses, compared case-folded: table readers read them as a missing
+value (section 3.2). The reader's own copy; ``tests/test_description_rules_copy.py`` keeps it
+equal to the value discovery uses."""
+SINGLE = "single"
+"""The one lower-case label of a level with a single member (section 3.3). The reader's own
+copy; a test keeps it equal to the value normalization uses."""
+
+
+def label_refused(value: str) -> bool:
+    """Return whether the Label rule (section 3.2) refuses a text value.
+
+    A label is refused when it starts or ends with whitespace or, case-folded, is one of
+    :data:`MISSING_TEXT`. Nothing is trimmed or repaired.
+    """
+    return value != value.strip() or value.casefold() in MISSING_TEXT

tests/spec_gate.py

@@ -37,6 +37,7 @@
     "test_spec_publication.py",
     "test_spec_reader.py",
     "test_spec_reader_phases_1_3.py",
+    "test_spec_reader_refusal_paths.py",
     "test_spec_reconcile.py",
     "test_spec_refval_crosscheck.py",
     "test_spec_rules_table.py",

tests/test_description_rules_copy.py

@@ -0,0 +1,38 @@
+"""The reader's own copy of two rules equals the values discovery and normalization use.
+
+Spec section 2.7: the reader takes no vocabulary from outside itself at run time, so
+``description_rules`` holds its own ``MISSING_TEXT`` and ``SINGLE``. These tests keep them equal
+to the shared values, so a later change to either is caught here.
+"""
+
+from __future__ import annotations
+
+import pytest
+from morphobricks_normalization.records import SINGLE as NORMALIZATION_SINGLE
+
+from morphobricks_preprocessing import description_rules as rules
+from morphobricks_preprocessing.discovery import (
+    MISSING_TEXT as DISCOVERY_MISSING_TEXT,
+)
+from morphobricks_preprocessing.discovery import IdentityError, check_identity_text
+
+
+def test_the_reader_s_missing_text_equals_discoverys():
+    assert rules.MISSING_TEXT == DISCOVERY_MISSING_TEXT
+
+
+def test_the_reader_s_single_equals_normalizations():
+    assert rules.SINGLE == NORMALIZATION_SINGLE
+
+
+@pytest.mark.parametrize(
+    "value",
+    ["", "NA", "Nan", "NONE", "null", "N/A", "<NA>", " a", "a ", "\ta", "ok", "single", "x y"],
+)
+def test_the_label_rule_agrees_with_check_identity_text(value):
+    try:
+        check_identity_text(value, "identity")
+        shared_refuses = False
+    except IdentityError:
+        shared_refuses = True
+    assert rules.label_refused(value) is shared_refuses

tests/test_spec_reader_refusal_paths.py

@@ -0,0 +1,93 @@
+"""The reader names the offending field and value of five codes the oracle compares only by code.
+
+``tests/spec_oracle.py`` (``COARSE``) drops the paths of these codes, because the oracle names
+a table and the reader names each field. A refusal attached to the wrong field would therefore
+pass the crosscheck; these tests pin the path and the value of each, on the Spec fixtures that
+trigger them.
+"""
+
+from __future__ import annotations
+
+import pytest
+
+from morphobricks_preprocessing.description_read import read_description_bytes
+from spec_oracle import COARSE
+from spec_pin import fixtures
+
+BY_NAME = {item.name: item for item in fixtures("invalid")}
+UNIT = "repeats.rep1.members.{}.technical_repeats.acq1.source_folder"
+
+CASES = {
+    "i122_two_condition_ids_one_folder": (
+        "E_SOURCE_FOLDER_REUSED",
+        {
+            ("conditions.CTRL.source_folder", "control"),
+            ("conditions.DrugA.source_folder", "control"),
+        },
+    ),
+    "i123_two_technical_repeat_ids_one_folder": (
+        "E_SOURCE_FOLDER_REUSED",
+        {
+            (
+                "repeats.rep1.members.CTRL.technical_repeats.acq1.source_folder",
+                "acquisition_1",
+            ),
+            (
+                "repeats.rep1.members.CTRL.technical_repeats.acq2.source_folder",
+                "acquisition_1",
+            ),
+        },
+    ),
+    "i124_one_technical_repeat_id_two_folders": (
+        "E_SOURCE_FOLDER_CONFLICT",
+        {
+            (UNIT.format("CTRL"), "acquisition_1"),
+            (UNIT.format("DrugA"), "acq_1"),
+        },
+    ),
+    "i74_source_folder_in_two_repeats": (
+        "E_SOURCE_REPEAT_REUSED",
+        {
+            ("repeats.Day2.members.CTRL.source_repeat", "exp7"),
+            ("repeats.Day3.members.CTRL.source_repeat", "exp7"),
+        },
+    ),
+    "i80_mask_source_type_reused": (
+        "E_MASK_SOURCE_TYPE_REUSED",
+        {
+            ("masks.Nuclei.source_type", "nuclei"),
+            ("masks.Nuclei2.source_type", "nuclei"),
+        },
+    ),
+    "i64_reference_cycle": (
+        "E_REFERENCE_CYCLE",
+        {
+            ("conditions.CTRL.reference", ("CTRL", "DrugA")),
+            ("conditions.DrugA.reference", ("CTRL", "DrugA")),
+        },
+    ),
+    "i65_reference_cycle_long": (
+        "E_REFERENCE_CYCLE",
+        {
+            (f"conditions.{name}.reference", ("CTRL", "DrugA", "DrugB"))
+            for name in ("CTRL", "DrugA", "DrugB")
+        },
+    ),
+}
+
+
+def test_the_cases_cover_the_five_codes_the_oracle_compares_by_code():
+    assert {code for code, _ in CASES.values()} == COARSE
+
+
+@pytest.mark.parametrize("name", CASES)
+def test_each_refusal_is_attached_to_its_offending_field_with_its_value(name):
+    code, expected = CASES[name]
+    issues = read_description_bytes(BY_NAME[name].data).errors
+    found = {
+        (issue.path, tuple(issue.value) if isinstance(issue.value, list) else issue.value)
+        for issue in issues
+        if issue.code.value == code
+    }
+    assert found == expected
+    assert len([i for i in issues if i.code.value == code]) == len(expected)

tests/test_spec_rules_table.py

@@ -13,7 +13,6 @@
 
 from morphobricks_preprocessing import description_rules as rules
 from morphobricks_preprocessing.description_rules import Code, TableSpec
-from morphobricks_preprocessing.discovery import MISSING_TEXT
 from spec_text import (
     BACKTICKED,
     section,
@@ -90,6 +89,6 @@ def test_the_token_pattern_and_the_rgb_tokens_are_the_text():
 
 
 def test_the_label_rule_the_reader_calls_refuses_the_names_of_the_text():
-    # The Label rule is the brick's check_identity_text (decision D-e), not a copy: its list
-    # of refused names must be the one section 3.2 states.
-    assert MISSING_TEXT == spec_label_sentinels()
+    # The reader's own Label rule (decision D-e): its list of refused names must be the one
+    # section 3.2 states; test_description_rules_copy keeps it equal to discovery's.
+    assert rules.MISSING_TEXT == spec_label_sentinels()