Slice 9: terminals and boundaries for the witnessed path - #659
Merged
Conversation
Slice 9 (witnessed-Run terminals) needs to carry the substrate's own time for an observed capture ending or aborting, and RunCompleted / RunAborted currently have nowhere to put it -- record_witnessed_run's own command docstring explicitly deferred this field to "the terminal-recording slice that actually needs it". Mirrors EnclosurePermitObserved.observed_at: no default, so every construction site (both driven deciders pass None today) states what the substrate said rather than letting a default silently drop the distinction, and present-as-null on the wire so a pre-slice-9 event stays distinguishable from one that says "the substrate gave no time" forever.
Closes the witnessed genesis: record_witnessed_run can start a witnessed Run, but nothing could ever end one. This new in-process-only slice terminates one, emitting the EXISTING RunCompleted / RunAborted events (no new event, evolver arm, projection, or export surface) with observed_at threaded from the substrate. Three new guard errors on the Run aggregate: RunCapturePhaseNotTerminalError (the command only accepts an Ended or Aborted observed_phase), RunNotWitnessedError (the applicability guard stopping a granted RunWitness principal from ever terminating an operator-driven Run -- conduct_mode is immutable, so there is no retry that fixes this), and InvalidRunObservedAtError (mirrors InvalidRunInterruptedAtError). RunMonitorTriggerNotPermittedError is generalized in place rather than duplicated per naming-r3-reviewer: one invariant (no operator path through either witnessed-path command), one class. Not yet wired to any caller -- the RunWitness recorder still only clears its dedup entry on a terminal observation, per the existing module docstring's "terminal recording is a separate future slice". That wiring, plus the caller-composed abort reason and truncation recovery for a missed terminal, is the next commit.
The runtime can now actually close a witnessed Run instead of only opening one: an Ended/Aborted observation calls the (already-shipped) record_witnessed_run_outcome, and a Begun for an already-open capture means the previous terminal was missed, so the stale Run is truncated (interrupted_at=None) before the new one promotes. A failed outcome write leaves the entry open, so the next Begun's truncation doubles as the retry path -- the recorder never needs its own separate retry timer. Gate-reviewed across five lenses before landing (safety/interlock, security/authz, record/publishing, cross-BC, beamline domain). Fixed along the way: a CapturePhase|None narrowing gap resolved with explicit narrowing instead of a type: ignore; documented (rather than silently carried) two residuals -- a CA reconnect can in principle misread a still-open capture as new, and until the next commit reads the abort PV, a real 2-BM abort still persists as RunCompleted, so recording stays off at 2-BM until both land; and noted that TruncateRun's grant to this principal has no decider-level conduct_mode backstop, unlike RecordWitnessedRunOutcome's, so it depends on this file only ever sourcing a run_id from its own bookkeeping. Also fixed: two integration-test payload assertions for RunCompleted/ RunAborted missed when observed_at was added two commits ago.
At 2-BM, fly_scan()'s exception handlers for ScanAbortError /
CameraTimeoutError / FileOverwriteError still run finally: end_scan(),
which writes the identical 'Scan complete' literal a real success
writes. Until now, RunWitness had no way to tell them apart -- Ended
always meant Completed, which was the gap the previous commit's module
docstring documented as still open.
ControlPortCaptureObserver now grows an optional second pump per
capture code for the deployment's `abort` role (a code with no `abort`
entry watches `status` only, unchanged). A decoded-asserted reading is
a direct ABORTED claim; a clear or unresolvable one makes no phase
claim at all, so a stale idle read can never overwrite a real status
transition.
Caught before it shipped: 2-BM's live AbortScan PV is a DBR_ENUM that
resolves through the aioca adapter to the label 'No' when idle, not a
plain 0/1. `bool('No')` is `True` in Python, so a naive truthiness
check on the reading would have misclassified every idle read as an
abort. Decodes through the same binary-label idiom
`_enclosure_permit_observer.py` already established for exactly this
class of problem (`_binary_code`), rather than reinventing it --
including its int() fallback for CaprotoControlPort, which leaves the
raw index unresolved.
Deploying `capture_watch_pvs`'s abort role for 2-BM (adding
"abort": "2bmb:TomoScan:AbortScan" to arcturus's config) is a separate,
explicit deployment step, not part of this commit.
Amends the previous commit's own note: the code capability exists now, so the remaining gap is deployment config (arcturus's capture_watch_pvs still needs the abort role added), not missing code.
Drives the real ControlPortCaptureObserver classification table through the real RunWitnessRecorder, using the actual literal sequences measured on arcturus (2026-08-14, 431 captures), rather than pre-classified phases: three clean cycles promote and complete exactly once each with zero truncates, a real abort edge closes as Aborted before the trailing 'Scan complete' lands as a no-op, and a dropped terminal is recovered by the next Begun's truncation. Building the abort-edge case exposed a real gap worth documenting: the status and abort pumps are two independent tasks with no enforced ordering between them. The correct outcome depends on realistic, network-driven CA delivery interleaving the two fairly, matching 2-BM's actual write order (AbortScan before the trailing ScanStatus), not a structural guarantee -- confirmed by constructing the adversarial non-interleaved case directly against a fake ControlPort that doesn't yield between readings. Documented as an accepted residual alongside the coalesced-abort one already noted: same outcome (a real abort degrades to a Completed record), never a corrupted attribution to a different Run.
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the witnessed-Run lifecycle:
record_witnessed_run(already on main) can start a witnessed Run, but nothing could end one until now. Four commits:observed_atadded toRunCompleted/RunAborted, mirroringEnclosurePermitObserved.observed_at— the substrate's own time for a completion/abort,Nonefor the two driven deciders.record_witnessed_run_outcome, a new in-process-only slice that closes a witnessed Run (Ended -> RunCompleted,Aborted -> RunAborted), guarded by aRunNotWitnessedErrorapplicability check so a granted RunWitness principal can never terminate an operator-driven Run.RunWitnessRecordernow actually calls the outcome command on a terminal observation, and recovers a missed terminal by truncating the stale Run (interrupted_at=None) before promoting a new one when aBegunarrives for an already-open capture. Gate-reviewed across 5 lenses (safety/interlock, security/authz, record/publishing, cross-BC, beamline domain) before landing.'Scan complete'literal to the status PV, soControlPortCaptureObservergrows an optionalabortrole that gives a directABORTEDclaim independent of that ambiguity.Plus a docstring amendment and end-to-end replay tests driving the real arcturus-measured literal sequence through the real classification + recorder pipeline together.
A bug caught before it shipped
2-BM's live
AbortScanPV is a DBR_ENUM resolving to the label'No'when idle (confirmed viacagetagainst the real host), not a plain0/1. The first cut used Python truthiness (if not reading.value) —bool('No')isTrue, so every idle reading would have misclassified as an abort. Fixed by decoding through the same binary-label idiom_enclosure_permit_observer.pyalready established for exactly this problem.Accepted residuals (documented in
_run_witness.py's module docstring, not silently assumed away)Begunvalue for a capture that never restarted, which the recorder can't distinguish from a real new capture.statusandabortpumps are two independent tasks with no structurally-enforced ordering; correctness depends on realistic CA delivery preserving 2-BM's actual write order. Confirmed reachable by constructing the adversarial case against a non-interleaving fake port while writing the replay tests. Same outcome either way: a real abort degrades toCompleted, never a corrupted attribution to a different Run.TruncateRun's grant to the RunWitness principal has no decider-levelconduct_modegate (unlike the new outcome command's); its safety rests on this file only ever sourcing arun_idfrom its own dedup bookkeeping.Deliberately out of scope
capture_watch_pvsneeds theabortrole added) and turning onrun_witness_recording_enabled— separate, explicit deploy steps.Beguntruncation only, per locked decision).Test plan