Handle steps that run nothing, and stop the driver degrading a protocol

CLAUDE.md

@@ -272,7 +272,7 @@ handles stock sequences and third-party ones are what force a manual rebuild.
   scan a person should look at, which is what that verdict means. No shipped
   example does it; the branch exists so a future one is visible rather than
   quietly resolved.
-- The corpus stands at 677 third-party, 276 stock and 31 unrecognized, pinned by
+- The corpus stands at 701 third-party, 299 stock and 31 unrecognized, pinned by
   `test_the_shipped_examples_are_accounted_for_apart_from_a_pinned_few`.
   `tse_crusher` (`Flair axial low SAR`) is labelled `USER` and so reports
   third-party; `fl3d_rd` (`vessels_head`) is labelled `SIEMENS` and is also in
@@ -515,6 +515,24 @@ before/after pair is the only way to learn where a printed value is stored --
   real difference is hardware: `aRxCoilSelectData[1].aFFT_SCALE` has 58
   entries on P1 and 20 on P2, on the one scan using a second coil array. A
   generator moving a protocol between scanners must not copy that across.
+- **A step in the running order need not run anything.** `EdfPauseStep` is an
+  instruction an operator put between scans -- "Count down with RA to start of
+  scan", "Pause for saliva collection", "Do NOT add Raw Filter to 3D MPR" --
+  carrying an `EdfMeasurementStepContent` with injector fields and no protocol
+  child. Eleven of `CHR-MDD`'s thirty-four steps are pauses, and the reader
+  raised on all three archives that arrived with them. They are named, they are
+  in the chain, and the PDF does not print them as scans, so anything walking
+  *scans* skips them: `Step.is_pause` reads the instance kind, `runs_a_protocol`
+  reads the content, and a test asserts the two always agree because either
+  alone could be wrong.
+- **A printout carries fewer digits than the protocol.** One scan prints
+  `TE 1 = 54 ms` for a stored `54.16`, so writing the printed value back drops
+  0.16 ms. `build.agrees_at_printed_precision` treats a printed value as
+  matching when the stored one rounds to it at the precision actually printed;
+  without it, driving an archive from its own PDF degrades it.
+- **Strip a printed unit only after whitespace.** Matching it anywhere turns
+  `RMS` into `R`, because `MS` is a unit and the comparison is
+  case-insensitive -- which then fails to resolve as an `Averaging` choice.
 - **A readable archive can hold no protocols at all.** Exporting an empty
   folder node rather than the protocol tree yields a valid SQLite file with
   the directory scaffolding, a `Root` label and nothing else -- five

src/siemens_protocol/exar/archive.py

@@ -48,6 +48,16 @@
 #: Instance types, as ``Instance.InstanceType`` spells them.
 PROGRAM = "EdfProgram"
 MEASUREMENT_STEP = "EdfMeasurementStep"
+
+#: A step in the running order that acquires nothing. Operators put pause and
+#: instruction steps between scans -- "Count down with RA to start of scan",
+#: "Pause for saliva collection" -- and the console gives them their own
+#: instance type. They are common in real clinical trees, so the running order
+#: mixes both kinds and anything walking *scans* has to skip these.
+PAUSE_STEP = "EdfPauseStep"
+
+#: Both kinds that appear in a program's running order.
+STEP_KINDS = (MEASUREMENT_STEP, PAUSE_STEP)
 PROTOCOL = "EdfProtocol"
 STRING = "EdfString"
 
@@ -239,6 +249,36 @@ class Step:
     name: str
     protocols: list[Protocol] = field(default_factory=list)
 
+    @property
+    def is_pause(self) -> bool:
+        """Return whether this step is an instruction rather than an acquisition.
+
+        Returns
+        -------
+        bool
+            ``True`` for an ``EdfPauseStep``.
+        """
+        return self.instance.kind == PAUSE_STEP
+
+    @property
+    def runs_a_protocol(self) -> bool:
+        """Return whether this step acquires anything.
+
+        A measurement step need not hold a protocol. Operators put pause and
+        instruction steps in the running order -- "Count down with RA to start
+        of scan", "Pause for saliva collection", "Do NOT add Raw Filter to 3D
+        MPR" -- and those carry an ``EdfMeasurementStepContent`` with injector
+        fields and no protocol child. They are common in real clinical trees
+        and the PDF does not print them as scans, so anything walking scans
+        must skip them rather than assume every step has one.
+
+        Returns
+        -------
+        bool
+            ``True`` when the step holds at least one protocol.
+        """
+        return bool(self.protocols)
+
     @property
     def protocol(self) -> Protocol:
         """Return the step's single protocol.
@@ -251,7 +291,9 @@ def protocol(self) -> Protocol:
         Raises
         ------
         ValueError
-            If the step holds no protocol at all.
+            If the step holds no protocol at all, which is a legitimate state
+            -- see :attr:`runs_a_protocol` -- so callers sweeping an archive
+            should test that first rather than catching this.
         """
         if not self.protocols:
             raise ValueError(f"measurement step {self.name!r} holds no protocol")

src/siemens_protocol/exar/build.py

@@ -32,7 +32,7 @@
 
 #: Units the card prints beside a value and the protocol does not store.
 UNIT_SUFFIX = re.compile(
-    r"\s*(ms|s|mm|cm|deg|degree|degrees|Hz|Hz/Px|kHz|%|TRs|TR|min|sec|mT/m|ppm)\s*$",
+    r"\s+(ms|s|mm|cm|deg|degree|degrees|Hz|Hz/Px|kHz|%|TRs|TR|min|sec|mT/m|ppm)\s*$",
     re.I,
 )
 
@@ -52,8 +52,10 @@ def printed_value(text: Any) -> Any:
     """
     if not isinstance(text, str):
         return text
-    # Some values carry two: "5.0 mm" is one, "24 TRs" is one, and a printed
-    # angle is "360 degrees". Strip repeatedly rather than assume a single unit.
+    # The unit has to follow whitespace. Matching it anywhere turns "RMS" into
+    # "R", because "MS" is a unit and the comparison is case-insensitive --
+    # which then fails to resolve as an Averaging choice. Strip repeatedly,
+    # since a value can carry more than one suffix.
     stripped = text.strip()
     while True:
         shorter = UNIT_SUFFIX.sub("", stripped).strip()
@@ -180,24 +182,65 @@ def apply_protocol(archive: Archive, parsed: MappingType[str, Any]) -> BuildRepo
         What was written, refused and inherited.
     """
     report = BuildReport()
+    # A pause step carries no protocol and the PDF does not print it as a scan,
+    # so it can never be the counterpart of one.
     steps: dict[str, list[Any]] = {}
     for step in archive.steps:
-        steps.setdefault(step.name, []).append(step)
+        if step.runs_a_protocol:
+            steps.setdefault(step.name, []).append(step)
 
-    seen: set[str] = set()
+    scans: dict[str, list[Any]] = {}
     for scan in parsed.get("scans", []):
-        name = scan.get("name", "")
-        found = steps.get(name, [])
-        if len(found) != 1:
-            report.unmatched.append(name)
+        scans.setdefault(scan.get("name", ""), []).append(scan)
+
+    seen: set[str] = set()
+    for name, printed in scans.items():
+        held = steps.get(name, [])
+        # A name repeats: a protocol may run four scans called Localizer. Pair
+        # them in running order, which both sides preserve -- but only when the
+        # two sides agree on how many there are. Otherwise which is which is a
+        # guess, and guessing would write one scan's values into another.
+        if not held or len(held) != len(printed):
+            report.unmatched.extend(name for _ in printed)
             continue
         seen.add(name)
-        report.matched.append(name)
-        _apply_scan(archive, found[0], scan, report)
+        for step, scan in zip(held, printed):
+            report.matched.append(name)
+            _apply_scan(archive, step, scan, report)
     report.untouched = [n for n in steps if n not in seen]
     return report
 
 
+def agrees_at_printed_precision(printed: str, stored: Any) -> bool:
+    """Return whether a stored value and a printed one are the same number.
+
+    A printout carries fewer digits than the protocol: one scan prints
+    ``TE 1 = 54 ms`` for a stored 54.16. Writing the printed value back would
+    quietly drop 0.16 ms, so a printed value is treated as agreeing when the
+    stored one rounds to it at the precision actually printed.
+
+    Parameters
+    ----------
+    printed : str
+        The value as the card prints it, units already removed.
+    stored : Any
+        What the protocol holds.
+
+    Returns
+    -------
+    bool
+        ``True`` when the two are the same number to the printed precision.
+    """
+    if not isinstance(stored, (int, float)) or isinstance(stored, bool):
+        return False
+    try:
+        wanted = float(printed)
+    except (TypeError, ValueError):
+        return False
+    _whole, _, fraction = str(printed).strip().partition(".")
+    return round(float(stored), len(fraction)) == wanted
+
+
 def _moved(record: patch.Applied) -> bool:
     """Return whether a written record actually changed anything.
 
@@ -242,7 +285,16 @@ def _apply_scan(
         if mapping is None:
             report.inherited[label] += 1
             continue
-        requests[label] = printed_value(value)
+        wanted = printed_value(value)
+        entry = (
+            step.protocol.preview.get(mapping.preview_path)
+            if mapping.preview_path is not None
+            else None
+        )
+        if entry is not None and agrees_at_printed_precision(wanted, entry.value):
+            report.unchanged += 1
+            continue
+        requests[label] = wanted
     if not requests:
         return
     document, applied, skipped = patch.patch_document(step.protocol, requests, step=step.name)

src/siemens_protocol/exar/patch.py

@@ -1298,6 +1298,17 @@ def apply(archive: Archive, changes: MappingType[str, MappingType[str, Any]]) ->
                 )
             continue
         step = found[0]
+        if not step.runs_a_protocol:
+            for label, value in requests.items():
+                manifest.skipped.append(
+                    Skipped(
+                        step=name,
+                        label=label,
+                        value=value,
+                        reason="that step is a pause and holds no protocol",
+                    )
+                )
+            continue
         protocol = step.protocol
         document, applied, skipped = patch_document(protocol, requests, step=name)
         manifest.applied.extend(applied)

src/siemens_protocol/exar/validate.py

@@ -20,7 +20,7 @@
 from typing import Any
 
 from . import envelope
-from .archive import MEASUREMENT_STEP, Archive
+from .archive import STEP_KINDS, Archive
 from .generate import NO_GUID, STEP_KEYED_MAPS
 
 #: Matches a GUID as these payloads spell one.
@@ -106,7 +106,7 @@ def _running_order(archive: Archive, document: dict[str, Any]) -> list[str]:
     # derived by walking the chain, so comparing the two would be circular: a
     # chain that stops early would agree with itself and the break would show
     # up only as unrelated complaints about the maps.
-    existing = sum(1 for i in archive.instances.values() if i.kind == MEASUREMENT_STEP)
+    existing = sum(1 for i in archive.instances.values() if i.kind in STEP_KINDS)
     if len(order) != existing:
         found.append(f"link chain covers {len(order)} steps but {existing} exist")
     if order and document.get("FirstStepId") != order[0]:
@@ -183,10 +183,9 @@ def _parents(archive: Archive) -> list[str]:
     program = archive.program
     found = []
     for step in archive.steps:
-        expected = {
-            "protocol": (step.protocol.instance, step.instance.element_id),
-            "step": (step.instance, program.element_id),
-        }
+        expected = {"step": (step.instance, program.element_id)}
+        if step.runs_a_protocol:
+            expected["protocol"] = (step.protocol.instance, step.instance.element_id)
         holder = archive.by_element.get(step.instance.label_element_id)
         if holder is not None:
             expected["label"] = (holder, step.instance.element_id)