Make row packing toggleable: pad (default) or strict abort

A mis-triggered row cannot be written as it arrived — v6 declares n_frames per
row in the header and has no per-row length field, so a short or long row
would shift every later row in the file. Until now the only policy was to
square it up, which keeps the scan running but leaves the affected row
indistinguishable from a good one afterwards: nothing in the file records that
it was padded.

strict_rows selects the other trade. On any frame-count mismatch the scan
stops instead of writing the row, so a data run either produces rows that mean
what the header says they mean or fails loudly. Default stays pad, so existing
behaviour is unchanged.

_warn_frame_delta becomes _check_frame_delta, since it now decides rather than
just reports. Both acquisition paths already call it before writing anything
for the row (CH1 leads SCAN_CHANNELS, and the burst path checks every row up
front), so an abort leaves the file on a whole-row boundary rather than a
half-written row — test_strict_row_packing_writes_nothing_for_the_failed_row
pins that.

Plumbed through QtScanController to a checkbox in the scan panel, persisted in
ScanDefaults alongside burst_mode. scan_format.md documents both policies and
notes that the choice is not recorded in the file.

The row-clipping setup in the padding test is now a _clip_one_row helper,
reused by the strict tests. 92 tests passing, ruff clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

core/config.py

@@ -24,6 +24,7 @@ class ScanDefaults:
     save_dir: str = str(DEFAULTS_PATH.parent / "scans")
     helios_port: str = "/dev/ttyUSB2"
     burst_mode: bool = False
+    strict_rows: bool = False
 
     @classmethod
     def load(cls, path: Path = DEFAULTS_PATH) -> "ScanDefaults":

core/scan_engine.py

@@ -96,7 +96,7 @@ class ScanEngine:
                  plan: ScanPlan, out_path: Path,
                  resume: ResumeState | None = None,
                  callbacks: ScanCallbacks | None = None,
-                 burst_mode: bool = False):
+                 burst_mode: bool = False, strict_rows: bool = False):
         self._stage = stage
         self._scope = scope
         self._rotator = rotator
@@ -107,6 +107,9 @@ class ScanEngine:
         # Burst mode acquires as many whole rows per FastFrame acquisition as
         # the scope's frame memory holds, instead of one row per acquisition.
         self._burst_mode = burst_mode
+        # Strict row packing stops the scan on a frame-count mismatch
+        # instead of squaring the row up (see _check_frame_delta).
+        self._strict_rows = strict_rows
         self._max_frames = 0
         self._preflight_done = False
 
@@ -434,7 +437,7 @@ class ScanEngine:
             self._cb.on_status(f"Fetching CH{ch} data …")
             waveforms = scope_sras.transfer_channel(scope, ch)
             if ch == SCAN_CHANNELS[0]:
-                self._warn_frame_delta(row_idx, len(waveforms), n_frames)
+                self._check_frame_delta(row_idx, len(waveforms), n_frames)
             row = scope_burst.normalize_row(
                 b"".join(waveforms), 0, len(waveforms), n_frames, samples_per_frame)
             if ch == 4:
@@ -577,7 +580,7 @@ class ScanEngine:
         total_frames = sum(counts)
 
         for r, count in enumerate(counts):
-            self._warn_frame_delta(first_row + r, count, n_frames)
+            self._check_frame_delta(first_row + r, count, n_frames)
 
         for ch_idx, ch in enumerate(SCAN_CHANNELS):
             if ch == 3:
@@ -608,10 +611,26 @@ class ScanEngine:
 
         scan_file.seek(burst_start + len(counts) * row_bytes)
 
-    def _warn_frame_delta(self, row_idx: int, count: int, n_frames: int):
+    def _check_frame_delta(self, row_idx: int, count: int, n_frames: int):
+        """Decide what to do with a row that did not acquire n_frames frames.
+
+        v6 declares n_frames per row in the header and has no per-row length
+        field, so a mismatched row cannot just be written as-is — that would
+        shift every later row in the file.  The only two safe options are to
+        square it up or to stop, which is what strict_rows selects between.
+
+        Called before anything for the row is written (CH1 leads
+        SCAN_CHANNELS), so raising here leaves no partial row behind.
+        """
         if count == n_frames:
             return
         verb = "zero-padded" if count < n_frames else "truncated"
+        if self._strict_rows:
+            raise RuntimeError(
+                f"Row {row_idx + 1}: {count} frames acquired, {n_frames} "
+                f"expected. Strict row packing is on, so the scan stops here "
+                f"rather than writing a row that would be {verb}."
+            )
         msg = (f"Row {row_idx + 1}: {count} frames acquired, {n_frames} "
                f"expected — {verb} to keep the file layout intact.")
         logger.warning(msg)

gui/scan_bridge.py

@@ -32,7 +32,8 @@ class QtScanController(QObject):
     paused_changed = pyqtSignal(bool)                 # True while paused at a row boundary
 
     def __init__(self, stage, scope, rotator, plan, out_path,
-                 resume=None, on_scan_active=None, burst_mode=False):
+                 resume=None, on_scan_active=None, burst_mode=False,
+                 strict_rows=False):
         super().__init__()
         self._prompt_event = threading.Event()
         self._on_scan_active = on_scan_active
@@ -48,7 +49,8 @@ class QtScanController(QObject):
         )
         self._engine = ScanEngine(stage, scope, rotator, plan, out_path,
                                   resume=resume, callbacks=callbacks,
-                                  burst_mode=burst_mode)
+                                  burst_mode=burst_mode,
+                                  strict_rows=strict_rows)
 
     # ── Engine control (called from the GUI thread) ───────────────────────────
 

sc3-aui-main.ui

@@ -1007,6 +1007,16 @@
         </property>
        </widget>
       </item>
+      <item>
+       <widget class="QCheckBox" name="strict_rows_check">
+        <property name="toolTip">
+         <string>Stop the scan if a row does not acquire the expected number of frames, instead of zero-padding a short row or truncating a long one. Use for data runs where a silently squared-up row would be worse than a failed scan.</string>
+        </property>
+        <property name="text">
+         <string>Strict row packing (abort on frame-count mismatch)</string>
+        </property>
+       </widget>
+      </item>
       <item>
        <widget class="QPushButton" name="start_scan_btn">
         <property name="text">

sc3_aui_app.py

@@ -918,6 +918,7 @@ class MainWindow(QMainWindow):
         self.scan_prefix_edit.setText("scan")
         self.scan_save_dir_edit.setText(DEFAULTS.save_dir)
         self.burst_mode_check.setChecked(DEFAULTS.burst_mode)
+        self.strict_rows_check.setChecked(DEFAULTS.strict_rows)
 
         # Hide the old T3R manual controls; the connect toggle becomes the panel button.
         for w in (
@@ -979,6 +980,7 @@ class MainWindow(QMainWindow):
         self.bbd202_comport_edit.editingFinished.connect(self._persist_defaults)
         self.oscope_ip_edit.editingFinished.connect(self._persist_defaults)
         self.burst_mode_check.toggled.connect(self._persist_defaults)
+        self.strict_rows_check.toggled.connect(self._persist_defaults)
 
         # Camera
         self.show_camera_toggle.toggled.connect(self._on_camera_toggle)
@@ -1137,6 +1139,7 @@ class MainWindow(QMainWindow):
         DEFAULTS.oscope_ip = self.oscope_ip_edit.text().strip()
         DEFAULTS.save_dir = self.scan_save_dir_edit.text().strip()
         DEFAULTS.burst_mode = self.burst_mode_check.isChecked()
+        DEFAULTS.strict_rows = self.strict_rows_check.isChecked()
         DEFAULTS.save()
 
     # ── Camera toggle ─────────────────────────────────────────────────────────
@@ -1261,6 +1264,7 @@ class MainWindow(QMainWindow):
             on_scan_active=lambda active: setattr(
                 self._bbd_worker, "scanning_active", active),
             burst_mode=self.burst_mode_check.isChecked(),
+            strict_rows=self.strict_rows_check.isChecked(),
         )
         self._scan_worker.moveToThread(self._scan_thread)
         self._scan_thread.started.connect(self._scan_worker.run)

scan_format.md

@@ -222,9 +222,23 @@ re-armed for each acquiring pass.  Row boundaries inside the burst come from
 `ACQuire:NUMFRAMESACQuired?` sampled after each pass — the burst itself carries
 no row markers.  See `core/scope_burst.py`.
 
-Either path squares each row up to the declared `n_frames` (zero-padding a
-short row, dropping the tail of a long one), because the format has no per-row
-length field and a mismatch would shift every later row.
+### Row packing
+
+The format has no per-row length field, so a row that over- or under-triggers
+cannot be written as it arrived — that would shift every later row.  Two
+policies are selectable (`ScanEngine(strict_rows=…)`, a checkbox in the app),
+and the choice is not recorded in the file:
+
+| | Pad (default) | Strict |
+|---|---|---|
+| Short row | zero-padded to `n_frames`, warned | scan stops |
+| Long row | trailing frames dropped, warned | scan stops |
+
+Pad keeps a scan running through an occasional mis-trigger, at the cost that
+the affected row is indistinguishable from a good one afterwards — nothing in
+the file records that it was padded.  Strict is for data runs where that
+ambiguity is worse than a failed scan: it aborts before writing the row, so
+the file always ends on a whole-row boundary.
 
 ---
 

tests/test_scan_engine.py

@@ -27,7 +27,8 @@ def make_plan(num_angles=1, y_delta=0.005):
 
 
 def build(tmp_path, num_angles=1, callbacks=None, resume=None, plan=None,
-          burst_mode=False, max_frames=4096, out_name="out.sras", **kw):
+          burst_mode=False, max_frames=4096, out_name="out.sras",
+          strict_rows=False, **kw):
     trace = Trace()
     scope = FakeScope(trace, samples_per_frame=SPF, max_frames=max_frames)
     stage = FakeStage(trace, scope=scope)
@@ -37,7 +38,7 @@ def build(tmp_path, num_angles=1, callbacks=None, resume=None, plan=None,
     engine = ScanEngine(stage, scope, rotator, plan, tmp_path / out_name,
                         resume=resume,
                         callbacks=callbacks or ScanCallbacks(),
-                        burst_mode=burst_mode)
+                        burst_mode=burst_mode, strict_rows=strict_rows)
     return engine, trace, plan
 
 
@@ -356,6 +357,26 @@ def test_burst_preflight_rejects_a_dark_laser(tmp_path):
         engine.run()
 
 
+def _clip_one_row(engine, which_pass=2, lost=1):
+    """Make one acquiring pass come up `lost` frames short.
+
+    `which_pass` counts acquiring passes from 1, so the default clips the
+    second data row (row 2) — far enough in that a mishandled short row shows
+    up as a shift in the rows after it.
+    """
+    scope = engine._scope
+    real_acquire = scope.acquire_frames
+    passes = {"n": 0}
+
+    def clipped(n):
+        if scope._running:
+            passes["n"] += 1
+            if passes["n"] == which_pass:
+                n -= lost
+        real_acquire(n)
+    scope.acquire_frames = clipped
+
+
 @pytest.mark.parametrize("burst_mode", [False, True])
 def test_short_row_is_padded_to_declared_frame_count(tmp_path, burst_mode):
     """A clipped row must not shift every later row in the file.
@@ -374,17 +395,7 @@ def test_short_row_is_padded_to_declared_frame_count(tmp_path, burst_mode):
     # acquiring-pass count below identical in both modes.
     engine._preflight_done = True
 
-    scope = engine._scope
-    real_acquire = scope.acquire_frames
-    passes = {"n": 0}
-
-    def clipped(n):
-        if scope._running:
-            passes["n"] += 1
-            if passes["n"] == 2:        # second data row loses one frame
-                n -= 1
-        real_acquire(n)
-    scope.acquire_frames = clipped
+    _clip_one_row(engine)
 
     result = engine.run()
 
@@ -398,6 +409,62 @@ def test_short_row_is_padded_to_declared_frame_count(tmp_path, burst_mode):
     assert bytes(sras.load_row(0, 2, 0)[0]) != bytes(SPF)
 
 
+# ── Strict row packing ───────────────────────────────────────────────────────
+
+@pytest.mark.parametrize("burst_mode", [False, True])
+def test_strict_row_packing_aborts_on_a_short_row(tmp_path, burst_mode):
+    """Strict mode fails the scan instead of silently squaring a row up.
+
+    The default padding keeps the file readable but makes a mis-triggered row
+    indistinguishable from a good one after the fact, since v6 records no
+    per-row frame count. Strict mode trades the salvaged rows for knowing.
+    """
+    plan = make_plan(**BURST_PLAN)
+    engine, _, _ = build(tmp_path, plan=plan, burst_mode=burst_mode,
+                         max_frames=BURST_MAX_FRAMES, strict_rows=True)
+    engine._preflight_done = True
+    _clip_one_row(engine)
+
+    with pytest.raises(RuntimeError, match="Row 2: 3 frames acquired, 4 expected"):
+        engine.run()
+
+
+@pytest.mark.parametrize("burst_mode", [False, True])
+def test_strict_row_packing_does_not_disturb_a_clean_scan(tmp_path, burst_mode):
+    """Strict mode is inert when every row acquires what it declared."""
+    plan = make_plan(**BURST_PLAN)
+    engine, _, _ = build(tmp_path, plan=plan, burst_mode=burst_mode,
+                         max_frames=BURST_MAX_FRAMES, strict_rows=True)
+    engine._preflight_done = True
+
+    result = engine.run()
+
+    assert result.rows_written == plan.total_rows
+    sras = SrasFile(tmp_path / "out.sras")
+    assert [s.status for s in sras.angle_status()] == ["OK"]
+
+
+def test_strict_row_packing_writes_nothing_for_the_failed_row(tmp_path):
+    """The abort must not leave a half-written row behind.
+
+    CH1 leads SCAN_CHANNELS, so the frame count is known before any of the
+    row's channels are written — the file should end on a whole-row boundary.
+    """
+    plan = make_plan(**BURST_PLAN)
+    engine, _, _ = build(tmp_path, plan=plan, strict_rows=True)
+    engine._preflight_done = True
+    _clip_one_row(engine)
+
+    with pytest.raises(RuntimeError, match="Strict row packing"):
+        engine.run()
+
+    # Row 1 was written in full; row 2 aborted before writing anything, so
+    # the file ends exactly on a row boundary.
+    sras = SrasFile(tmp_path / "out.sras")
+    written = (tmp_path / "out.sras").stat().st_size - sras.data_start_offset
+    assert written == sras.row_bytes(0)
+
+
 def test_engine_imports_without_qt():
     """The engine must be usable from a non-Qt front end."""
     import subprocess