From 62c97ce99326dc694e0765d85d5b0a8055f90859 Mon Sep 17 00:00:00 2001 From: xmap <16776958+xmap@users.noreply.github.com> Date: Fri, 14 Aug 2026 18:15:58 -0500 Subject: [PATCH 1/6] run: add observed_at to RunCompleted and RunAborted 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. --- .../record_export/_dispositions.py | 2 + .../api/src/cora/run/aggregates/run/events.py | 61 +++++++++++++--- .../cora/run/features/abort_run/decider.py | 2 + .../cora/run/features/complete_run/decider.py | 2 + ...hority_revocation_perf_against_postgres.py | 2 +- ...est_caution_drafter_subscriber_postgres.py | 1 + .../test_run_debriefer_subscriber_postgres.py | 2 +- ..._authority_revocation_holder_subscriber.py | 2 +- .../agent/test_caution_drafter_subscriber.py | 4 +- .../test_ratification_enforcer_subscriber.py | 2 +- .../agent/test_run_debriefer_subscriber.py | 4 +- .../tests/unit/run/test_abort_run_decider.py | 1 + .../run/test_abort_run_decider_properties.py | 1 + .../tests/unit/run/test_abort_run_handler.py | 2 +- .../run/test_append_observations_handler.py | 4 +- .../unit/run/test_complete_run_decider.py | 2 +- .../test_complete_run_decider_properties.py | 2 +- .../unit/run/test_complete_run_handler.py | 2 +- apps/api/tests/unit/run/test_run_events.py | 73 ++++++++++++++++--- apps/api/tests/unit/run/test_run_evolver.py | 48 +++++++----- 20 files changed, 169 insertions(+), 50 deletions(-) diff --git a/apps/api/src/cora/infrastructure/record_export/_dispositions.py b/apps/api/src/cora/infrastructure/record_export/_dispositions.py index a786e364763..fd764dbe47f 100644 --- a/apps/api/src/cora/infrastructure/record_export/_dispositions.py +++ b/apps/api/src/cora/infrastructure/record_export/_dispositions.py @@ -1555,6 +1555,7 @@ "RunAborted": { "actuation_kind": "drop:text", "decided_by_decision_id": "token:uuid", + "observed_at": "keep:time", "occurred_at": "keep:time", "producing_job_id": "drop:text", "reason": "drop:text", @@ -1577,6 +1578,7 @@ "RunCompleted": { "actuation_kind": "drop:text", "artifact_uri": "drop:text", + "observed_at": "keep:time", "occurred_at": "keep:time", "producing_job_id": "drop:text", "run_id": "token:uuid", diff --git a/apps/api/src/cora/run/aggregates/run/events.py b/apps/api/src/cora/run/aggregates/run/events.py index 0e268617d05..e221ccebf49 100644 --- a/apps/api/src/cora/run/aggregates/run/events.py +++ b/apps/api/src/cora/run/aggregates/run/events.py @@ -502,10 +502,20 @@ class RunCompleted: - `artifact_uri` is where the conducted job wrote its output, the handoff a later `register_dataset` uses as the Dataset uri (not folded onto state). + + `observed_at` is the SUBSTRATE's own time for the completion, + mirroring `EnclosurePermitObserved.observed_at`: it is `None` for a + driven completion (`complete_run` has no substrate reading to + report) and the TomoScan `ScanStatus` PV's own timestamp for a + witnessed one. Distinct from `occurred_at`, CORA's clock at + handler-append. NO default: every construction site must state + what the substrate said, including saying `None`, rather than let + a default silently drop the distinction. """ run_id: UUID occurred_at: datetime + observed_at: datetime | None actuation_kind: str | None = None producing_job_id: str | None = None artifact_uri: str | None = None @@ -542,11 +552,20 @@ class RunAborted: `producing_job_id` is the failed job's handle, an audit breadcrumb on the stream (not folded onto state). Both forward-compat additive via `payload.get(...)` -> None. + + `observed_at` is the SUBSTRATE's own time for the abort, mirroring + `EnclosurePermitObserved.observed_at`: `None` for an operator abort + (no substrate reading to report) and the TomoScan `ScanStatus` / + `AbortScan` PV's own timestamp for a witnessed one. Distinct from + `occurred_at`, CORA's clock at handler-append. NO default: every + construction site must state what the substrate said, including + saying `None`. """ run_id: UUID reason: str occurred_at: datetime + observed_at: datetime | None decided_by_decision_id: UUID | None = None actuation_kind: str | None = None producing_job_id: str | None = None @@ -882,6 +901,7 @@ def to_payload(event: RunEvent) -> dict[str, Any]: producing_job_id=producing_job_id, artifact_uri=artifact_uri, occurred_at=occurred_at, + observed_at=observed_at, ): return { "run_id": str(run_id), @@ -889,6 +909,10 @@ def to_payload(event: RunEvent) -> dict[str, Any]: "producing_job_id": producing_job_id, "artifact_uri": artifact_uri, "occurred_at": occurred_at.isoformat(), + # Present-as-null, not omit-when-None: an event written + # before this field existed must stay distinguishable + # from one that says "the substrate gave no time". + "observed_at": observed_at.isoformat() if observed_at is not None else None, } case RunAborted( run_id=run_id, @@ -897,6 +921,7 @@ def to_payload(event: RunEvent) -> dict[str, Any]: actuation_kind=actuation_kind, producing_job_id=producing_job_id, occurred_at=occurred_at, + observed_at=observed_at, ): return { "run_id": str(run_id), @@ -907,6 +932,7 @@ def to_payload(event: RunEvent) -> dict[str, Any]: "actuation_kind": actuation_kind, "producing_job_id": producing_job_id, "occurred_at": occurred_at.isoformat(), + "observed_at": observed_at.isoformat() if observed_at is not None else None, } case RunStopped( run_id=run_id, @@ -1147,28 +1173,38 @@ def _build_run_resumed() -> RunResumed: return deserialize_or_raise("RunResumed", _build_run_resumed) case "RunCompleted": - return deserialize_or_raise( - "RunCompleted", - lambda: RunCompleted( + + def _build_run_completed() -> RunCompleted: + # Compute-conduct provenance added additively; legacy + # (non-conducted) streams replay with these absent -> + # None via `.get(...)`. `observed_at` is the same + # additive shape: absent on every stream written before + # slice 9, `None` on a driven completion recorded since. + raw_observed_at = payload.get("observed_at") + return RunCompleted( run_id=UUID(payload["run_id"]), - # Compute-conduct provenance added additively; legacy - # (non-conducted) streams replay with these absent -> - # None via `.get(...)`. actuation_kind=payload.get("actuation_kind"), producing_job_id=payload.get("producing_job_id"), artifact_uri=payload.get("artifact_uri"), occurred_at=datetime.fromisoformat(payload["occurred_at"]), - ), - ) + observed_at=( + datetime.fromisoformat(raw_observed_at) + if raw_observed_at is not None + else None + ), + ) + + return deserialize_or_raise("RunCompleted", _build_run_completed) case "RunAborted": def _build_run_aborted() -> RunAborted: # `decided_by_decision_id` optional. Forward-compat # additive evolution: pre-existing streams replay without the # key via `.get(..., None)`. `actuation_kind` / - # `producing_job_id` are the same additive shape for - # conduct-failed aborts. + # `producing_job_id` / `observed_at` are the same + # additive shape for conduct-failed / witnessed aborts. raw_decided_by_abort = payload.get("decided_by_decision_id") + raw_observed_at = payload.get("observed_at") return RunAborted( run_id=UUID(payload["run_id"]), reason=payload["reason"], @@ -1178,6 +1214,11 @@ def _build_run_aborted() -> RunAborted: actuation_kind=payload.get("actuation_kind"), producing_job_id=payload.get("producing_job_id"), occurred_at=datetime.fromisoformat(payload["occurred_at"]), + observed_at=( + datetime.fromisoformat(raw_observed_at) + if raw_observed_at is not None + else None + ), ) return deserialize_or_raise("RunAborted", _build_run_aborted) diff --git a/apps/api/src/cora/run/features/abort_run/decider.py b/apps/api/src/cora/run/features/abort_run/decider.py index 25edf1349e2..bec3a322f07 100644 --- a/apps/api/src/cora/run/features/abort_run/decider.py +++ b/apps/api/src/cora/run/features/abort_run/decider.py @@ -71,5 +71,7 @@ def decide( actuation_kind=command.actuation_kind, producing_job_id=command.producing_job_id, occurred_at=now, + # An operator/agent abort has no substrate reading to report. + observed_at=None, ) ] diff --git a/apps/api/src/cora/run/features/complete_run/decider.py b/apps/api/src/cora/run/features/complete_run/decider.py index 59738236794..36a2d9d5e8f 100644 --- a/apps/api/src/cora/run/features/complete_run/decider.py +++ b/apps/api/src/cora/run/features/complete_run/decider.py @@ -46,5 +46,7 @@ def decide( producing_job_id=command.producing_job_id, artifact_uri=command.artifact_uri, occurred_at=now, + # A driven completion has no substrate reading to report. + observed_at=None, ) ] diff --git a/apps/api/tests/integration/test_authority_revocation_perf_against_postgres.py b/apps/api/tests/integration/test_authority_revocation_perf_against_postgres.py index f3ac60fe69b..4a4098a2149 100644 --- a/apps/api/tests/integration/test_authority_revocation_perf_against_postgres.py +++ b/apps/api/tests/integration/test_authority_revocation_perf_against_postgres.py @@ -151,7 +151,7 @@ def _replay_throughput() -> dict[str, float | int]: while len(events) < _REPLAY_STREAM - 1: events.append(RunHeld(run_id=rid, occurred_at=_NOW)) events.append(RunResumed(run_id=rid, occurred_at=_NOW)) - events.append(RunCompleted(run_id=rid, occurred_at=_NOW)) + events.append(RunCompleted(run_id=rid, occurred_at=_NOW, observed_at=None)) for _ in range(1000): # warm fold(events) # type: ignore[arg-type] diff --git a/apps/api/tests/integration/test_caution_drafter_subscriber_postgres.py b/apps/api/tests/integration/test_caution_drafter_subscriber_postgres.py index 32ca1aaea1b..204d81f9954 100644 --- a/apps/api/tests/integration/test_caution_drafter_subscriber_postgres.py +++ b/apps/api/tests/integration/test_caution_drafter_subscriber_postgres.py @@ -135,6 +135,7 @@ def _terminal_aborted_event(run_id: UUID) -> StoredEvent: run_id=run_id, reason="rotary stage encoder offline; interlock fired", occurred_at=_LATER, + observed_at=None, ) return StoredEvent( position=1, diff --git a/apps/api/tests/integration/test_run_debriefer_subscriber_postgres.py b/apps/api/tests/integration/test_run_debriefer_subscriber_postgres.py index 301d4da2d9d..e6a07f58508 100644 --- a/apps/api/tests/integration/test_run_debriefer_subscriber_postgres.py +++ b/apps/api/tests/integration/test_run_debriefer_subscriber_postgres.py @@ -81,7 +81,7 @@ async def _seed_run(deps, run_id: UUID, plan_id: UUID) -> None: def _terminal_event(run_id: UUID) -> StoredEvent: - domain = RunCompleted(run_id=run_id, occurred_at=_LATER) + domain = RunCompleted(run_id=run_id, occurred_at=_LATER, observed_at=None) return StoredEvent( position=1, event_id=UUID("01900000-0000-7000-8000-00000000fe01"), diff --git a/apps/api/tests/unit/agent/test_authority_revocation_holder_subscriber.py b/apps/api/tests/unit/agent/test_authority_revocation_holder_subscriber.py index 53e8a558077..8aac3e08b43 100644 --- a/apps/api/tests/unit/agent/test_authority_revocation_holder_subscriber.py +++ b/apps/api/tests/unit/agent/test_authority_revocation_holder_subscriber.py @@ -127,7 +127,7 @@ async def _seed_held_run(store: EventStore, *, starter: UUID) -> UUID: async def _seed_completed_run(store: EventStore, *, starter: UUID) -> UUID: """Append RunStarted + RunCompleted so the run folds to a terminal status.""" run_id = await _seed_running_run(store, starter=starter) - completed = RunCompleted(run_id=run_id, occurred_at=_NOW) + completed = RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=None) envelope = to_new_event( event_type=run_event_type_name(completed), payload=run_to_payload(completed), diff --git a/apps/api/tests/unit/agent/test_caution_drafter_subscriber.py b/apps/api/tests/unit/agent/test_caution_drafter_subscriber.py index ffccf32fe03..be91be2733b 100644 --- a/apps/api/tests/unit/agent/test_caution_drafter_subscriber.py +++ b/apps/api/tests/unit/agent/test_caution_drafter_subscriber.py @@ -220,10 +220,10 @@ def _terminal_event( """Build a StoredEvent for a terminal Run event.""" domain: Any if event_type == "RunCompleted": - domain = RunCompleted(run_id=run_id, occurred_at=_LATER) + domain = RunCompleted(run_id=run_id, occurred_at=_LATER, observed_at=None) elif event_type == "RunAborted": assert reason is not None - domain = RunAborted(run_id=run_id, reason=reason, occurred_at=_LATER) + domain = RunAborted(run_id=run_id, reason=reason, occurred_at=_LATER, observed_at=None) else: msg = f"unsupported event type for fixture: {event_type}" raise ValueError(msg) diff --git a/apps/api/tests/unit/agent/test_ratification_enforcer_subscriber.py b/apps/api/tests/unit/agent/test_ratification_enforcer_subscriber.py index a03778af0e0..a082afa0e92 100644 --- a/apps/api/tests/unit/agent/test_ratification_enforcer_subscriber.py +++ b/apps/api/tests/unit/agent/test_ratification_enforcer_subscriber.py @@ -274,7 +274,7 @@ async def test_hold_subscriber_noop_when_run_is_terminal() -> None: kernel = _kernel() await seed_ratification_enforcer_agent(kernel) run_id = await _seed_running_run(kernel.event_store) - completed = RunCompleted(run_id=run_id, occurred_at=_NOW) + completed = RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=None) await kernel.event_store.append( "Run", run_id, diff --git a/apps/api/tests/unit/agent/test_run_debriefer_subscriber.py b/apps/api/tests/unit/agent/test_run_debriefer_subscriber.py index 5e9e657bc47..4a9ffc069a4 100644 --- a/apps/api/tests/unit/agent/test_run_debriefer_subscriber.py +++ b/apps/api/tests/unit/agent/test_run_debriefer_subscriber.py @@ -195,10 +195,10 @@ def _terminal_event( """Build a StoredEvent for one of the four terminal Run events.""" domain: Any if event_type == "RunCompleted": - domain = RunCompleted(run_id=run_id, occurred_at=_LATER) + domain = RunCompleted(run_id=run_id, occurred_at=_LATER, observed_at=None) elif event_type == "RunAborted": assert reason is not None - domain = RunAborted(run_id=run_id, reason=reason, occurred_at=_LATER) + domain = RunAborted(run_id=run_id, reason=reason, occurred_at=_LATER, observed_at=None) elif event_type == "RunStopped": assert reason is not None domain = RunStopped(run_id=run_id, reason=reason, occurred_at=_LATER) diff --git a/apps/api/tests/unit/run/test_abort_run_decider.py b/apps/api/tests/unit/run/test_abort_run_decider.py index 06e026a279b..4e76c7a08c4 100644 --- a/apps/api/tests/unit/run/test_abort_run_decider.py +++ b/apps/api/tests/unit/run/test_abort_run_decider.py @@ -53,6 +53,7 @@ def test_decide_emits_run_aborted_for_running_state() -> None: run_id=state.id, reason="detector overheating", occurred_at=_NOW, + observed_at=None, ) ] diff --git a/apps/api/tests/unit/run/test_abort_run_decider_properties.py b/apps/api/tests/unit/run/test_abort_run_decider_properties.py index 79c469d67ff..750ba354b78 100644 --- a/apps/api/tests/unit/run/test_abort_run_decider_properties.py +++ b/apps/api/tests/unit/run/test_abort_run_decider_properties.py @@ -113,6 +113,7 @@ def test_abort_from_permitted_source_emits_single_event( reason=reason, decided_by_decision_id=decision_id, occurred_at=now, + observed_at=None, ) ] diff --git a/apps/api/tests/unit/run/test_abort_run_handler.py b/apps/api/tests/unit/run/test_abort_run_handler.py index bb79b5a132b..aae12e9790e 100644 --- a/apps/api/tests/unit/run/test_abort_run_handler.py +++ b/apps/api/tests/unit/run/test_abort_run_handler.py @@ -57,7 +57,7 @@ async def _seed_run_started(store: InMemoryEventStore, run_id: UUID) -> None: async def _seed_run_aborted(store: InMemoryEventStore, run_id: UUID) -> None: await _seed_run_started(store, run_id) - aborted = RunAborted(run_id=run_id, reason="prior abort", occurred_at=_NOW) + aborted = RunAborted(run_id=run_id, reason="prior abort", occurred_at=_NOW, observed_at=None) new_event = to_new_event( event_type=event_type_name(aborted), payload=to_payload(aborted), diff --git a/apps/api/tests/unit/run/test_append_observations_handler.py b/apps/api/tests/unit/run/test_append_observations_handler.py index 44b7811b79a..b783072cad0 100644 --- a/apps/api/tests/unit/run/test_append_observations_handler.py +++ b/apps/api/tests/unit/run/test_append_observations_handler.py @@ -290,11 +290,11 @@ async def test_handler_rejects_unknown_sampling_procedure() -> None: def _make_completed(rid: UUID) -> RunCompleted: - return RunCompleted(run_id=rid, occurred_at=_NOW) + return RunCompleted(run_id=rid, occurred_at=_NOW, observed_at=None) def _make_aborted(rid: UUID) -> RunAborted: - return RunAborted(run_id=rid, reason="emergency", occurred_at=_NOW) + return RunAborted(run_id=rid, reason="emergency", occurred_at=_NOW, observed_at=None) def _make_stopped(rid: UUID) -> RunStopped: diff --git a/apps/api/tests/unit/run/test_complete_run_decider.py b/apps/api/tests/unit/run/test_complete_run_decider.py index 0e9c9c7e992..78fb6bc41c8 100644 --- a/apps/api/tests/unit/run/test_complete_run_decider.py +++ b/apps/api/tests/unit/run/test_complete_run_decider.py @@ -42,7 +42,7 @@ def test_decide_emits_run_completed_for_running_state() -> None: command=CompleteRun(run_id=state.id), now=_NOW, ) - assert events == [RunCompleted(run_id=state.id, occurred_at=_NOW)] + assert events == [RunCompleted(run_id=state.id, occurred_at=_NOW, observed_at=None)] @pytest.mark.unit diff --git a/apps/api/tests/unit/run/test_complete_run_decider_properties.py b/apps/api/tests/unit/run/test_complete_run_decider_properties.py index 4ba89d8423c..30a5e2394fb 100644 --- a/apps/api/tests/unit/run/test_complete_run_decider_properties.py +++ b/apps/api/tests/unit/run/test_complete_run_decider_properties.py @@ -80,7 +80,7 @@ def test_complete_from_running_emits_single_event(run_id: UUID, now: datetime) - command=CompleteRun(run_id=run_id), now=now, ) - assert events == [RunCompleted(run_id=run_id, occurred_at=now)] + assert events == [RunCompleted(run_id=run_id, occurred_at=now, observed_at=None)] @pytest.mark.unit diff --git a/apps/api/tests/unit/run/test_complete_run_handler.py b/apps/api/tests/unit/run/test_complete_run_handler.py index a6690b81799..b5dca30656b 100644 --- a/apps/api/tests/unit/run/test_complete_run_handler.py +++ b/apps/api/tests/unit/run/test_complete_run_handler.py @@ -56,7 +56,7 @@ async def _seed_run_started(store: InMemoryEventStore, run_id: UUID) -> None: async def _seed_run_completed(store: InMemoryEventStore, run_id: UUID) -> None: await _seed_run_started(store, run_id) - completed = RunCompleted(run_id=run_id, occurred_at=_NOW) + completed = RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=None) new_event = to_new_event( event_type=event_type_name(completed), payload=to_payload(completed), diff --git a/apps/api/tests/unit/run/test_run_events.py b/apps/api/tests/unit/run/test_run_events.py index 8b1ea612ed4..36f1888d7f9 100644 --- a/apps/api/tests/unit/run/test_run_events.py +++ b/apps/api/tests/unit/run/test_run_events.py @@ -457,14 +457,14 @@ def test_acknowledged_cautions_round_trip_preserves_every_field() -> None: @pytest.mark.unit def test_event_type_name_for_run_completed() -> None: - event = RunCompleted(run_id=uuid4(), occurred_at=_NOW) + event = RunCompleted(run_id=uuid4(), occurred_at=_NOW, observed_at=None) assert event_type_name(event) == "RunCompleted" @pytest.mark.unit def test_to_payload_serializes_run_completed_to_primitives() -> None: run_id = uuid4() - event = RunCompleted(run_id=run_id, occurred_at=_NOW) + event = RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=None) assert to_payload(event) == { "run_id": str(run_id), # compute-conduct provenance: None for a non-conducted complete. @@ -472,6 +472,9 @@ def test_to_payload_serializes_run_completed_to_primitives() -> None: "producing_job_id": None, "artifact_uri": None, "occurred_at": _NOW.isoformat(), + # Present-as-null, not omitted: a driven completion has no + # substrate reading to report. + "observed_at": None, } @@ -484,6 +487,7 @@ def test_to_payload_serializes_run_completed_with_conduct_provenance() -> None: producing_job_id="inmem-job-1", artifact_uri="file:///data/recon.h5", occurred_at=_NOW, + observed_at=None, ) assert to_payload(event) == { "run_id": str(run_id), @@ -491,9 +495,17 @@ def test_to_payload_serializes_run_completed_with_conduct_provenance() -> None: "producing_job_id": "inmem-job-1", "artifact_uri": "file:///data/recon.h5", "occurred_at": _NOW.isoformat(), + "observed_at": None, } +@pytest.mark.unit +def test_to_payload_serializes_run_completed_with_observed_at() -> None: + run_id = uuid4() + event = RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=_NOW) + assert to_payload(event)["observed_at"] == _NOW.isoformat() + + @pytest.mark.unit def test_from_stored_rebuilds_run_completed() -> None: run_id = uuid4() @@ -505,12 +517,12 @@ def test_from_stored_rebuilds_run_completed() -> None: }, ) rebuilt = from_stored(stored) - assert rebuilt == RunCompleted(run_id=run_id, occurred_at=_NOW) + assert rebuilt == RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=None) @pytest.mark.unit def test_run_completed_round_trips() -> None: - original = RunCompleted(run_id=uuid4(), occurred_at=_NOW) + original = RunCompleted(run_id=uuid4(), occurred_at=_NOW, observed_at=None) stored = _stored("RunCompleted", to_payload(original)) assert from_stored(stored) == original @@ -523,26 +535,37 @@ def test_run_completed_round_trips_with_conduct_provenance() -> None: producing_job_id="inmem-job-1", artifact_uri="file:///data/recon.h5", occurred_at=_NOW, + observed_at=None, ) stored = _stored("RunCompleted", to_payload(original)) assert from_stored(stored) == original +@pytest.mark.unit +def test_run_completed_round_trips_with_observed_at() -> None: + original = RunCompleted(run_id=uuid4(), occurred_at=_NOW, observed_at=_NOW) + stored = _stored("RunCompleted", to_payload(original)) + assert from_stored(stored) == original + + @pytest.mark.unit def test_from_stored_rebuilds_run_completed_without_conduct_keys() -> None: """Pre-compute-conduct RunCompleted streams replay with the new keys - absent, folding to None via the `.get(...)` forward-compat path.""" + absent, folding to None via the `.get(...)` forward-compat path. + `observed_at` is the same additive shape: absent entirely on any + stream written before slice 9.""" run_id = uuid4() stored = _stored( "RunCompleted", {"run_id": str(run_id), "occurred_at": _NOW.isoformat()}, ) rebuilt = from_stored(stored) - assert rebuilt == RunCompleted(run_id=run_id, occurred_at=_NOW) + assert rebuilt == RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=None) assert isinstance(rebuilt, RunCompleted) assert rebuilt.actuation_kind is None assert rebuilt.producing_job_id is None assert rebuilt.artifact_uri is None + assert rebuilt.observed_at is None # ---------- RunAborted ---------- @@ -550,14 +573,16 @@ def test_from_stored_rebuilds_run_completed_without_conduct_keys() -> None: @pytest.mark.unit def test_event_type_name_for_run_aborted() -> None: - event = RunAborted(run_id=uuid4(), reason="X", occurred_at=_NOW) + event = RunAborted(run_id=uuid4(), reason="X", occurred_at=_NOW, observed_at=None) assert event_type_name(event) == "RunAborted" @pytest.mark.unit def test_to_payload_serializes_run_aborted_to_primitives() -> None: run_id = uuid4() - event = RunAborted(run_id=run_id, reason="detector overheating", occurred_at=_NOW) + event = RunAborted( + run_id=run_id, reason="detector overheating", occurred_at=_NOW, observed_at=None + ) assert to_payload(event) == { "run_id": str(run_id), "reason": "detector overheating", @@ -568,14 +593,25 @@ def test_to_payload_serializes_run_aborted_to_primitives() -> None: "actuation_kind": None, "producing_job_id": None, "occurred_at": _NOW.isoformat(), + # Present-as-null, not omitted: an operator abort has no + # substrate reading to report. + "observed_at": None, } +@pytest.mark.unit +def test_to_payload_serializes_run_aborted_with_observed_at() -> None: + run_id = uuid4() + event = RunAborted(run_id=run_id, reason="X", occurred_at=_NOW, observed_at=_NOW) + assert to_payload(event)["observed_at"] == _NOW.isoformat() + + @pytest.mark.unit def test_from_stored_rebuilds_run_aborted() -> None: """Pre-Phase-1 RunAborted streams replay without the decided_by_decision_id key via the `.get(..., None)` forward-compat - fold.""" + fold. `observed_at` is the same additive shape: absent entirely on + any stream written before slice 9.""" run_id = uuid4() stored = _stored( "RunAborted", @@ -590,12 +626,14 @@ def test_from_stored_rebuilds_run_aborted() -> None: run_id=run_id, reason="operator stop", occurred_at=_NOW, + observed_at=None, ) assert isinstance(rebuilt, RunAborted) assert rebuilt.decided_by_decision_id is None # Pre-compute-conduct streams fold the conduct-provenance keys to None. assert rebuilt.actuation_kind is None assert rebuilt.producing_job_id is None + assert rebuilt.observed_at is None @pytest.mark.unit @@ -606,6 +644,19 @@ def test_run_aborted_round_trips_with_conduct_provenance() -> None: actuation_kind="Simulated", producing_job_id="inmem-job-1", occurred_at=_NOW, + observed_at=None, + ) + stored = _stored("RunAborted", to_payload(original)) + assert from_stored(stored) == original + + +@pytest.mark.unit +def test_run_aborted_round_trips_with_observed_at() -> None: + original = RunAborted( + run_id=uuid4(), + reason="capture reported an abort", + occurred_at=_NOW, + observed_at=_NOW, ) stored = _stored("RunAborted", to_payload(original)) assert from_stored(stored) == original @@ -632,6 +683,7 @@ def test_from_stored_rebuilds_run_aborted_without_decision_id_key_as_none() -> N event = from_stored(stored) assert isinstance(event, RunAborted) assert event.decided_by_decision_id is None + assert event.observed_at is None @pytest.mark.unit @@ -640,6 +692,7 @@ def test_run_aborted_round_trips() -> None: run_id=uuid4(), reason="beam dump unscheduled", occurred_at=_NOW, + observed_at=None, ) stored = _stored("RunAborted", to_payload(original)) assert from_stored(stored) == original @@ -656,6 +709,7 @@ def test_to_payload_serializes_run_aborted_with_decision_id() -> None: reason="agent EquipmentAbortDecision triggered", decided_by_decision_id=decision_id, occurred_at=_NOW, + observed_at=None, ) assert to_payload(event)["decided_by_decision_id"] == str(decision_id) @@ -667,6 +721,7 @@ def test_run_aborted_with_decision_id_round_trips() -> None: reason="agent OperatorAbortDecision recorded", decided_by_decision_id=uuid4(), occurred_at=_NOW, + observed_at=None, ) stored = _stored("RunAborted", to_payload(original)) assert from_stored(stored) == original diff --git a/apps/api/tests/unit/run/test_run_evolver.py b/apps/api/tests/unit/run/test_run_evolver.py index d0b560ed644..dc957e33b58 100644 --- a/apps/api/tests/unit/run/test_run_evolver.py +++ b/apps/api/tests/unit/run/test_run_evolver.py @@ -107,7 +107,9 @@ def test_conduct_mode_survives_hold_resume_complete_round_trip() -> None: running = evolve(None, started) held = evolve(running, RunHeld(run_id=started.run_id, occurred_at=_NOW)) resumed = evolve(held, RunResumed(run_id=started.run_id, occurred_at=_NOW)) - completed = evolve(resumed, RunCompleted(run_id=started.run_id, occurred_at=_NOW)) + completed = evolve( + resumed, RunCompleted(run_id=started.run_id, occurred_at=_NOW, observed_at=None) + ) for state in (running, held, resumed, completed): assert state.conduct_mode is ConductMode.WITNESSED @@ -139,7 +141,9 @@ def test_fold_is_pure_same_input_same_output() -> None: def test_evolve_run_completed_transitions_to_completed_preserving_other_fields() -> None: started = _run_started() state = evolve(None, started) - completed = evolve(state, RunCompleted(run_id=started.run_id, occurred_at=_NOW)) + completed = evolve( + state, RunCompleted(run_id=started.run_id, occurred_at=_NOW, observed_at=None) + ) assert completed == replace(state, status=RunStatus.COMPLETED) assert completed.status is RunStatus.COMPLETED @@ -150,7 +154,9 @@ def test_evolve_run_aborted_transitions_to_aborted_preserving_other_fields() -> state = evolve(None, started) aborted = evolve( state, - RunAborted(run_id=started.run_id, reason="detector overheating", occurred_at=_NOW), + RunAborted( + run_id=started.run_id, reason="detector overheating", occurred_at=_NOW, observed_at=None + ), ) assert aborted == replace(state, status=RunStatus.ABORTED) assert aborted.status is RunStatus.ABORTED @@ -161,19 +167,19 @@ def test_evolve_run_completed_on_none_state_raises() -> None: """Defensive guard: a transition event before a genesis means the stream is contaminated. Fail loud rather than silently fold.""" with pytest.raises(ValueError, match="RunCompleted cannot be applied to empty state"): - evolve(None, RunCompleted(run_id=uuid4(), occurred_at=_NOW)) + evolve(None, RunCompleted(run_id=uuid4(), occurred_at=_NOW, observed_at=None)) @pytest.mark.unit def test_evolve_run_aborted_on_none_state_raises() -> None: with pytest.raises(ValueError, match="RunAborted cannot be applied to empty state"): - evolve(None, RunAborted(run_id=uuid4(), reason="X", occurred_at=_NOW)) + evolve(None, RunAborted(run_id=uuid4(), reason="X", occurred_at=_NOW, observed_at=None)) @pytest.mark.unit def test_fold_started_then_completed_yields_completed() -> None: started = _run_started() - state = fold([started, RunCompleted(run_id=started.run_id, occurred_at=_NOW)]) + state = fold([started, RunCompleted(run_id=started.run_id, occurred_at=_NOW, observed_at=None)]) assert state is not None assert state.status is RunStatus.COMPLETED @@ -184,7 +190,9 @@ def test_fold_started_then_aborted_yields_aborted_and_preserves_run_fields() -> plan_id = uuid4() subject_id = uuid4() started = _run_started(run_id=run_id, plan_id=plan_id, subject_id=subject_id) - state = fold([started, RunAborted(run_id=run_id, reason="beam dump", occurred_at=_NOW)]) + state = fold( + [started, RunAborted(run_id=run_id, reason="beam dump", occurred_at=_NOW, observed_at=None)] + ) assert state is not None assert state.id == run_id assert state.plan_id == plan_id @@ -266,7 +274,7 @@ def test_fold_multi_cycle_hold_resume_then_complete_yields_completed() -> None: RunResumed(run_id=run_id, occurred_at=_NOW), RunHeld(run_id=run_id, occurred_at=_NOW), RunResumed(run_id=run_id, occurred_at=_NOW), - RunCompleted(run_id=run_id, occurred_at=_NOW), + RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=None), ] ) assert state is not None @@ -282,7 +290,9 @@ def test_fold_started_then_held_then_aborted_yields_aborted() -> None: [ started, RunHeld(run_id=run_id, occurred_at=_NOW), - RunAborted(run_id=run_id, reason="emergency during hold", occurred_at=_NOW), + RunAborted( + run_id=run_id, reason="emergency during hold", occurred_at=_NOW, observed_at=None + ), ] ) assert state is not None @@ -422,7 +432,9 @@ def test_fold_preserves_raid_across_every_transition_path( raid_value = "https://raid.org/10.7935/cora-fold-test" events: list[object] = [_run_started_with_raid(run_id=run_id, raid=raid_value)] for cls in transitions: - if cls is RunAborted or cls is RunStopped: + if cls is RunAborted: + events.append(RunAborted(run_id=run_id, reason="X", occurred_at=_NOW, observed_at=None)) + elif cls is RunStopped: events.append(cls(run_id=run_id, reason="X", occurred_at=_NOW)) elif cls is RunTruncated: events.append( @@ -433,6 +445,8 @@ def test_fold_preserves_raid_across_every_transition_path( occurred_at=_NOW, ) ) + elif cls is RunCompleted: + events.append(RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=None)) else: events.append(cls(run_id=run_id, occurred_at=_NOW)) state = fold(events) # type: ignore[arg-type] @@ -540,7 +554,7 @@ def test_evolve_terminal_after_logbook_opened_preserves_logbook_id() -> None: schema=OBSERVATION_LOGBOOK_SCHEMA, occurred_at=_NOW, ), - RunCompleted(run_id=run_id, occurred_at=_NOW), + RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=None), ] ) assert state is not None @@ -549,11 +563,11 @@ def test_evolve_terminal_after_logbook_opened_preserves_logbook_id() -> None: def _make_completed(rid: UUID) -> RunCompleted: - return RunCompleted(run_id=rid, occurred_at=_NOW) + return RunCompleted(run_id=rid, occurred_at=_NOW, observed_at=None) def _make_aborted(rid: UUID) -> RunAborted: - return RunAborted(run_id=rid, reason="emergency", occurred_at=_NOW) + return RunAborted(run_id=rid, reason="emergency", occurred_at=_NOW, observed_at=None) def _make_stopped(rid: UUID) -> RunStopped: @@ -652,7 +666,7 @@ def test_legacy_stream_without_reading_logbook_folds_with_none_reading_logbook_i subject_id=None, occurred_at=_NOW, ), - RunCompleted(run_id=run_id, occurred_at=_NOW), + RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=None), ] ) assert state is not None @@ -797,7 +811,7 @@ def test_campaign_id_survives_lifecycle_transitions() -> None: ), RunHeld(run_id=run_id, occurred_at=_NOW), RunResumed(run_id=run_id, occurred_at=_NOW), - RunCompleted(run_id=run_id, occurred_at=_NOW), + RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=None), ] ) assert state is not None @@ -820,7 +834,7 @@ def test_legacy_run_stream_without_campaign_id_folds_to_none() -> None: subject_id=None, occurred_at=_NOW, ), - RunCompleted(run_id=run_id, occurred_at=_NOW), + RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=None), ] ) assert state is not None @@ -1030,7 +1044,7 @@ def test_legacy_run_stream_without_run_adjusted_folds_to_zero_count() -> None: subject_id=None, occurred_at=_NOW, ), - RunCompleted(run_id=run_id, occurred_at=_NOW), + RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=None), ] ) assert state is not None From ae11d290101031f87d4abb479743efb32681e18c Mon Sep 17 00:00:00 2001 From: xmap <16776958+xmap@users.noreply.github.com> Date: Fri, 14 Aug 2026 19:05:03 -0500 Subject: [PATCH 2/6] run: add record_witnessed_run_outcome, the witnessed-terminal slice 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. --- .../src/cora/run/aggregates/run/__init__.py | 6 + apps/api/src/cora/run/aggregates/run/state.py | 98 +++- .../record_witnessed_run_outcome/__init__.py | 29 ++ .../record_witnessed_run_outcome/command.py | 52 ++ .../record_witnessed_run_outcome/decider.py | 99 ++++ .../record_witnessed_run_outcome/handler.py | 47 ++ .../record_witnessed_run_outcome/route.py | 19 + .../record_witnessed_run_outcome/tool.py | 30 ++ apps/api/src/cora/run/routes.py | 24 +- apps/api/src/cora/run/tools.py | 9 + apps/api/src/cora/run/wire.py | 12 + .../architecture/test_slice_test_coverage.py | 8 + .../test_witnessed_genesis_laundering_wall.py | 9 +- ..._witnessed_run_outcome_handler_postgres.py | 194 +++++++ ...st_record_witnessed_run_outcome_decider.py | 298 +++++++++++ ...itnessed_run_outcome_decider_properties.py | 483 ++++++++++++++++++ ...st_record_witnessed_run_outcome_handler.py | 241 +++++++++ 17 files changed, 1645 insertions(+), 13 deletions(-) create mode 100644 apps/api/src/cora/run/features/record_witnessed_run_outcome/__init__.py create mode 100644 apps/api/src/cora/run/features/record_witnessed_run_outcome/command.py create mode 100644 apps/api/src/cora/run/features/record_witnessed_run_outcome/decider.py create mode 100644 apps/api/src/cora/run/features/record_witnessed_run_outcome/handler.py create mode 100644 apps/api/src/cora/run/features/record_witnessed_run_outcome/route.py create mode 100644 apps/api/src/cora/run/features/record_witnessed_run_outcome/tool.py create mode 100644 apps/api/tests/integration/test_record_witnessed_run_outcome_handler_postgres.py create mode 100644 apps/api/tests/unit/run/test_record_witnessed_run_outcome_decider.py create mode 100644 apps/api/tests/unit/run/test_record_witnessed_run_outcome_decider_properties.py create mode 100644 apps/api/tests/unit/run/test_record_witnessed_run_outcome_handler.py diff --git a/apps/api/src/cora/run/aggregates/run/__init__.py b/apps/api/src/cora/run/aggregates/run/__init__.py index b8d7426b3da..7fd0f787d30 100644 --- a/apps/api/src/cora/run/aggregates/run/__init__.py +++ b/apps/api/src/cora/run/aggregates/run/__init__.py @@ -92,6 +92,7 @@ InvalidRunExternalRefError, InvalidRunInterruptedAtError, InvalidRunNameError, + InvalidRunObservedAtError, InvalidRunParametersError, InvalidRunStopReasonError, InvalidRunTruncateReasonError, @@ -111,6 +112,7 @@ RunCannotStopError, RunCannotTruncateError, RunCapabilitiesNotSatisfiedError, + RunCapturePhaseNotTerminalError, RunClearanceCoverageMismatchError, RunComputeResourceUnknownError, RunEnclosureCoverageMismatchError, @@ -120,6 +122,7 @@ RunMonitorTriggerNotPermittedError, RunName, RunNotFoundError, + RunNotWitnessedError, RunObservationLogbookClosedError, RunPlanAssetDecommissionedError, RunRequiresActiveClearanceError, @@ -176,6 +179,7 @@ "InvalidRunExternalRefError", "InvalidRunInterruptedAtError", "InvalidRunNameError", + "InvalidRunObservedAtError", "InvalidRunParametersError", "InvalidRunStopReasonError", "InvalidRunTruncateReasonError", @@ -202,6 +206,7 @@ "RunCannotStopError", "RunCannotTruncateError", "RunCapabilitiesNotSatisfiedError", + "RunCapturePhaseNotTerminalError", "RunClearanceCoverageMismatchError", "RunCompleted", "RunComputeResourceUnknownError", @@ -214,6 +219,7 @@ "RunMonitorTriggerNotPermittedError", "RunName", "RunNotFoundError", + "RunNotWitnessedError", "RunObservationLogbookClosedError", "RunObservationLogbookOpened", "RunPlanAssetDecommissionedError", diff --git a/apps/api/src/cora/run/aggregates/run/state.py b/apps/api/src/cora/run/aggregates/run/state.py index 324bd153d59..d87b3f0fe2b 100644 --- a/apps/api/src/cora/run/aggregates/run/state.py +++ b/apps/api/src/cora/run/aggregates/run/state.py @@ -376,18 +376,23 @@ def __init__(self, run_id: UUID) -> None: class RunMonitorTriggerNotPermittedError(Exception): - """`record_witnessed_run` carried a non-Monitor trigger. + """A witnessed-path command carried a non-Monitor trigger. + + Shared by both witnessed-path commands, `record_witnessed_run` (the + genesis) and `record_witnessed_run_outcome` (the terminal): the same + invariant governs both ends of a witnessed Run's lifecycle, so one + error class enforces it rather than a near-duplicate per slice. Mirrors the Enclosure BC's `MonitorTriggerNotPermittedError` (`observe_enclosure_status`, D6.L2 observation-axis-only anti-lock): - a witnessed genesis is reachable only via Monitor-driven inbound - observation from the substrate; there is no operator path to it. The + a witnessed act is reachable only via Monitor-driven inbound + observation from the substrate; there is no operator path to it. Each command surface types `monitor_source_id` as `MonitorSourceId` so an operator cannot supply non-Monitor attribution at the type level; this error fences the same invariant defensively at the decider so a programmer mistake in a custom handler, test fixture, or future - adapter cannot smuggle an operator-asserted Run genesis onto the - spine through the witnessed path. + adapter cannot smuggle an operator-asserted act onto the spine + through the witnessed path. HTTP 400 (semantically a request the caller cannot issue, not a state-transition conflict). @@ -395,14 +400,40 @@ class RunMonitorTriggerNotPermittedError(Exception): def __init__(self, run_id: UUID, trigger: str) -> None: super().__init__( - f"Run {run_id}: trigger {trigger!r} is not permitted on " - f"record_witnessed_run; only 'Monitor' is accepted per the " + f"Run {run_id}: trigger {trigger!r} is not permitted on a " + f"witnessed-path command; only 'Monitor' is accepted per the " f"observation-axis-only anti-lock." ) self.run_id = run_id self.trigger = trigger +class RunCapturePhaseNotTerminalError(Exception): + """`record_witnessed_run_outcome` carried a non-terminal observed phase. + + The command exists to record ONE of the two facts a witnessed + capture's lifecycle can end on, `Ended` or `Aborted`; every other + `CapturePhase` value (`Begun`, `Progressing`, `Unrecognized`) is + something the RunWitness runtime already handles without touching + the Run aggregate at all (dedup, or a no-op). A caller reaching this + decider with a non-terminal phase is a programmer mistake in the + runtime's own dispatch, not a fact about the world, so this is a + request-shape rejection like `RunMonitorTriggerNotPermittedError`, + checked before touching any state. + + HTTP 400. + """ + + def __init__(self, run_id: UUID, observed_phase: str) -> None: + super().__init__( + f"Run {run_id}: observed_phase '{observed_phase}' is not a terminal " + f"capture phase; record_witnessed_run_outcome accepts only Ended or " + f"Aborted." + ) + self.run_id = run_id + self.observed_phase = observed_phase + + class RunNotFoundError(Exception): """Attempted an operation on a run whose stream has no events.""" @@ -411,6 +442,35 @@ def __init__(self, run_id: UUID) -> None: self.run_id = run_id +class RunNotWitnessedError(Exception): + """`record_witnessed_run_outcome` targeted a Conducted Run. + + The RunWitness runtime is granted this command so it can terminate + the Runs it itself created; a Conducted Run's terminal belongs + exclusively to `complete_run` / `abort_run` / `stop_run` / + `truncate_run`, reached by an operator or the RunSupervisor, never by + the monitor. `conduct_mode` is immutable after genesis + (see `ConductMode`'s own docstring), so this can never become true + later for a Run where it is false now; there is no retry that fixes + it. Checked defensively at the decider, mirroring + `RunMonitorTriggerNotPermittedError`'s posture, rather than trusting + the runtime's own bookkeeping never to misdirect a call: without + this guard, a granted RunWitness principal could terminate any + operator-driven Run. + + HTTP 400 (the command itself does not apply to this Run, not a + timing conflict with its current status). + """ + + def __init__(self, run_id: UUID, conduct_mode: str) -> None: + super().__init__( + f"Run {run_id} cannot be recorded via record_witnessed_run_outcome: " + f"conduct_mode is '{conduct_mode}', not 'Witnessed'." + ) + self.run_id = run_id + self.conduct_mode = conduct_mode + + class RunBoundPlanDeprecatedError(Exception): """Attempted to start a Run against a Deprecated Plan. @@ -1220,6 +1280,30 @@ def __init__(self, interrupted_at: datetime, now: datetime) -> None: self.now = now +class InvalidRunObservedAtError(ValueError): + """The supplied witnessed-outcome `observed_at` is in the future relative to `now`. + + Same shape and same rationale as `InvalidRunInterruptedAtError`: + `observed_at` is the substrate's own claim about when a capture + ended or aborted, separate from `occurred_at` (when + `record_witnessed_run_outcome` was processed). A substrate cannot + report a time later than CORA's own clock at receipt, so this + catches a malformed or clock-skewed adapter reading before it + reaches the record. + + Mapped to HTTP 400. + """ + + def __init__(self, observed_at: datetime, now: datetime) -> None: + super().__init__( + f"Run witnessed-outcome observed_at {observed_at.isoformat()} is in the " + f"future (now is {now.isoformat()}); the substrate cannot have reported " + f"this later than CORA learned of it" + ) + self.observed_at = observed_at + self.now = now + + @dataclass(frozen=True) class RunTruncateReason: """Free-form truncate reason. Trimmed; 1-500 chars. diff --git a/apps/api/src/cora/run/features/record_witnessed_run_outcome/__init__.py b/apps/api/src/cora/run/features/record_witnessed_run_outcome/__init__.py new file mode 100644 index 00000000000..8836eed4c7a --- /dev/null +++ b/apps/api/src/cora/run/features/record_witnessed_run_outcome/__init__.py @@ -0,0 +1,29 @@ +"""Vertical slice for the `RecordWitnessedRunOutcome` command: the witnessed terminal. + +In-process-only by design: no REST route, no MCP tool, mirroring +`record_witnessed_run`. Module-as-namespace surface: + + from cora.run.features import record_witnessed_run_outcome + + cmd = record_witnessed_run_outcome.RecordWitnessedRunOutcome( + run_id=..., capture_code=..., observed_phase=CapturePhase.ENDED, + observed_at=..., monitor_source_id=..., trigger="Monitor", + ) + handler = record_witnessed_run_outcome.bind(deps) + await handler(cmd, principal_id=..., correlation_id=...) +""" + +from cora.run.features.record_witnessed_run_outcome import tool +from cora.run.features.record_witnessed_run_outcome.command import RecordWitnessedRunOutcome +from cora.run.features.record_witnessed_run_outcome.decider import decide +from cora.run.features.record_witnessed_run_outcome.handler import Handler, bind +from cora.run.features.record_witnessed_run_outcome.route import router + +__all__ = [ + "Handler", + "RecordWitnessedRunOutcome", + "bind", + "decide", + "router", + "tool", +] diff --git a/apps/api/src/cora/run/features/record_witnessed_run_outcome/command.py b/apps/api/src/cora/run/features/record_witnessed_run_outcome/command.py new file mode 100644 index 00000000000..4c491870033 --- /dev/null +++ b/apps/api/src/cora/run/features/record_witnessed_run_outcome/command.py @@ -0,0 +1,52 @@ +"""The `RecordWitnessedRunOutcome` command -- intent dataclass for this slice. + +A witnessed terminal: CORA records that an external tool reported a +capture's lifecycle ended, closing a Run the witnessed-genesis slice +opened. Carries only the substrate-observed facts: + + - `run_id` -- the Run to terminate. Resolved by the RunWitness runtime + from its own dedup map (capture_code -> open Run id), never + operator-supplied. + - `capture_code` -- carried for logging/error-message attribution only; + not used to look up the Run (the runtime already resolved `run_id`). + - `observed_phase` -- which terminal the substrate reported, `Ended` or + `Aborted`. The decider refuses every other `CapturePhase` value: the + runtime never calls this command for `Begun` / `Progressing` / + `Unrecognized`, so reaching here with one of those is a caller + mistake, not a fact about the world. + - `observed_at` -- the substrate's own time for the terminal reading, + carried straight onto `RunCompleted.observed_at` / + `RunAborted.observed_at`. `None` when the substrate reported no time + at all (same shape as `CaptureObservation.observed_at`). + - `monitor_source_id` -- the stable `MonitorSourceId` of the in-process + RunWitness runtime, mirroring `RecordWitnessedRun.monitor_source_id`. + - `trigger` -- command-tier guard string. The decider rejects any value + other than the literal `"Monitor"` with + `RunMonitorTriggerNotPermittedError`, the same anti-lock + `RecordWitnessedRun` carries: there is no operator path to a + witnessed terminal. + +No `reason` field: for an `Aborted` outcome the decider composes the +`RunAborted.reason` text itself from `capture_code`, so no +operator-injectable string reaches the event through this command. No +`decided_by_decision_id`: RunWitness has no Decision-BC input to link. +""" + +from dataclasses import dataclass +from datetime import datetime +from uuid import UUID + +from cora.shared.capture_phase import CapturePhase +from cora.shared.identity import MonitorSourceId + + +@dataclass(frozen=True) +class RecordWitnessedRunOutcome: + """Record that an external tool reported a witnessed capture's terminal.""" + + run_id: UUID + capture_code: str + observed_phase: CapturePhase + observed_at: datetime | None + monitor_source_id: MonitorSourceId + trigger: str diff --git a/apps/api/src/cora/run/features/record_witnessed_run_outcome/decider.py b/apps/api/src/cora/run/features/record_witnessed_run_outcome/decider.py new file mode 100644 index 00000000000..dd4b91147ac --- /dev/null +++ b/apps/api/src/cora/run/features/record_witnessed_run_outcome/decider.py @@ -0,0 +1,99 @@ +"""Pure decider for the `RecordWitnessedRunOutcome` command. + +Closes the witnessed genesis: `Running -> Completed` for an observed +`Ended`, `Running -> Aborted` for an observed `Aborted`. Emits the +EXISTING `RunCompleted` / `RunAborted` events (no new event type, no new +evolver arm, no new projection or export surface); only the command, +this decider, and the handler are new. + +The two request-shape guards (`trigger`, `observed_phase`) run first, +mirroring `record_witnessed_run.decider`'s own ordering rationale: they +reject a malformed call before touching any state. `RunNotWitnessedError` +runs before the timestamp and status checks because it is the more +fundamental refusal -- an operator-driven Run's terminal never belongs to +this command regardless of what `observed_at` says or what status the +Run is in. + +For an `Aborted` outcome, the `RunAborted.reason` text is composed here +from `capture_code`, never taken from the command: RunWitness has no +operator-injectable reason field to launder through this path. + +Invariants: + - command.trigger must be "Monitor" + -> RunMonitorTriggerNotPermittedError + - command.observed_phase must be Ended or Aborted + -> RunCapturePhaseNotTerminalError + - State must not be None -> RunNotFoundError + - State.conduct_mode must be Witnessed -> RunNotWitnessedError + - command.observed_at, when set, must not be in the future + -> InvalidRunObservedAtError + - State.status must be in {Running} for an Ended outcome + -> RunCannotCompleteError(current_status=...) + - State.status must be in {Running} for an Aborted outcome + -> RunCannotAbortError(current_status=...) +""" + +from datetime import datetime + +from cora.run.aggregates.run import ( + InvalidRunObservedAtError, + Run, + RunAborted, + RunAbortReason, + RunCannotAbortError, + RunCannotCompleteError, + RunCapturePhaseNotTerminalError, + RunCompleted, + RunMonitorTriggerNotPermittedError, + RunNotFoundError, + RunNotWitnessedError, + RunStatus, +) +from cora.run.features.record_witnessed_run_outcome.command import RecordWitnessedRunOutcome +from cora.shared.capture_phase import CapturePhase + +_TERMINABLE_STATUSES: tuple[RunStatus, ...] = (RunStatus.RUNNING,) +_TERMINAL_PHASES: tuple[CapturePhase, ...] = (CapturePhase.ENDED, CapturePhase.ABORTED) +_REQUIRED_TRIGGER = "Monitor" + + +def decide( + state: Run | None, + command: RecordWitnessedRunOutcome, + *, + now: datetime, +) -> list[RunCompleted] | list[RunAborted]: + """Decide the events produced by closing a witnessed Run.""" + if command.trigger != _REQUIRED_TRIGGER: + raise RunMonitorTriggerNotPermittedError(command.run_id, command.trigger) + if command.observed_phase not in _TERMINAL_PHASES: + raise RunCapturePhaseNotTerminalError(command.run_id, command.observed_phase) + if state is None: + raise RunNotFoundError(command.run_id) + if state.conduct_mode != "Witnessed": + raise RunNotWitnessedError(state.id, state.conduct_mode) + if command.observed_at is not None and command.observed_at > now: + raise InvalidRunObservedAtError(command.observed_at, now) + + if command.observed_phase is CapturePhase.ENDED: + if state.status not in _TERMINABLE_STATUSES: + raise RunCannotCompleteError(state.id, current_status=state.status) + return [ + RunCompleted( + run_id=state.id, + occurred_at=now, + observed_at=command.observed_at, + ) + ] + + if state.status not in _TERMINABLE_STATUSES: + raise RunCannotAbortError(state.id, current_status=state.status) + reason = RunAbortReason(f"RunWitness observed capture {command.capture_code} as Aborted") + return [ + RunAborted( + run_id=state.id, + reason=reason.value, + occurred_at=now, + observed_at=command.observed_at, + ) + ] diff --git a/apps/api/src/cora/run/features/record_witnessed_run_outcome/handler.py b/apps/api/src/cora/run/features/record_witnessed_run_outcome/handler.py new file mode 100644 index 00000000000..2093b2f62a1 --- /dev/null +++ b/apps/api/src/cora/run/features/record_witnessed_run_outcome/handler.py @@ -0,0 +1,47 @@ +"""Application handler for the `record_witnessed_run_outcome` slice. + +Update-style handler. Canonical body lives in +`cora.run._run_update_handler.make_run_update_handler`; this module is a +thin slice-specific bind, same shape as `complete_run/handler.py` and +`abort_run/handler.py`. + +Per the roadmap's anti-scope: no REST route, no MCP tool reach this +handler (see `route.py` / `tool.py`, both stubs, mirroring +`record_witnessed_run`'s own in-process-only lock). The authorized path +in is the bound handler on `RunHandlers.record_witnessed_run_outcome`, +called only by the in-process RunWitness runtime as a seeded Agent +principal. +""" + +from typing import Protocol +from uuid import UUID + +from cora.infrastructure.kernel import Kernel +from cora.infrastructure.routing import NIL_SENTINEL_ID +from cora.run._run_update_handler import make_run_update_handler +from cora.run.features.record_witnessed_run_outcome.command import RecordWitnessedRunOutcome +from cora.run.features.record_witnessed_run_outcome.decider import decide + + +class Handler(Protocol): + """Callable interface every record_witnessed_run_outcome handler implements.""" + + async def __call__( + self, + command: RecordWitnessedRunOutcome, + *, + principal_id: UUID, + correlation_id: UUID, + causation_id: UUID | None = None, + surface_id: UUID = NIL_SENTINEL_ID, + ) -> None: ... + + +def bind(deps: Kernel) -> Handler: + """Build a record_witnessed_run_outcome handler closed over the shared deps.""" + return make_run_update_handler( + deps, + command_name="RecordWitnessedRunOutcome", + log_prefix="record_witnessed_run_outcome", + decide_fn=decide, + ) diff --git a/apps/api/src/cora/run/features/record_witnessed_run_outcome/route.py b/apps/api/src/cora/run/features/record_witnessed_run_outcome/route.py new file mode 100644 index 00000000000..68835a69037 --- /dev/null +++ b/apps/api/src/cora/run/features/record_witnessed_run_outcome/route.py @@ -0,0 +1,19 @@ +"""Stub route module for `record_witnessed_run_outcome` (in-process-only slice). + +Per the roadmap's anti-scope: no operator path to a witnessed terminal, +mirroring `record_witnessed_run`'s own lock. No REST route, no MCP tool. +In-process adapters (the RunWitness runtime) call +`RunHandlers.record_witnessed_run_outcome(...)` directly. + +The empty `router` exists only to satisfy the slice-file-shape + +routes-completeness architecture fitness functions; no routes are +registered on it. +""" + +from fastapi import APIRouter, Depends + +from cora.infrastructure.routing import get_surface_id + +router = APIRouter() + +_STUB_DEPENDS = Depends(get_surface_id) diff --git a/apps/api/src/cora/run/features/record_witnessed_run_outcome/tool.py b/apps/api/src/cora/run/features/record_witnessed_run_outcome/tool.py new file mode 100644 index 00000000000..b0d18165a01 --- /dev/null +++ b/apps/api/src/cora/run/features/record_witnessed_run_outcome/tool.py @@ -0,0 +1,30 @@ +"""Stub MCP tool module for `record_witnessed_run_outcome` (in-process-only slice). + +Per the roadmap's anti-scope: this slice is NOT exposed as an MCP tool. +In-process adapters call `RunHandlers.record_witnessed_run_outcome(...)` +directly. + +The no-op `register` exists only to satisfy the slice-file-shape + +tools-completeness architecture fitness functions; no MCP tool is +registered. The `get_mcp_surface_id` import satisfies the +mcp-surface-id-injection fitness; the resolver is not actually called +because no tool consumes it. +""" + +from collections.abc import Callable + +from mcp.server.fastmcp import FastMCP + +from cora.infrastructure.routing import get_mcp_surface_id +from cora.run.features.record_witnessed_run_outcome.handler import Handler + +_STUB_RESOLVER = get_mcp_surface_id + + +def register(mcp: FastMCP, *, get_handler: Callable[[], Handler]) -> None: + """No-op MCP registration: record_witnessed_run_outcome is in-process-only.""" + _ = mcp + _ = get_handler + _ = _STUB_RESOLVER + if False: # pragma: no cover -- AST satisfaction for fitness scan + _ = get_mcp_surface_id(None) # type: ignore[arg-type] diff --git a/apps/api/src/cora/run/routes.py b/apps/api/src/cora/run/routes.py index ce5aeecbf14..e8bc9577aa9 100644 --- a/apps/api/src/cora/run/routes.py +++ b/apps/api/src/cora/run/routes.py @@ -50,7 +50,10 @@ - 409 (Run adjust transition guard, 6j): RunCannotAdjustError - 400 (validation, 12b-5 adds): InvalidPinnedCalibrationsError - 400 (validation): InvalidInputDatasetsError - - 400 (witnessed-genesis trigger guard): RunMonitorTriggerNotPermittedError + - 400 (witnessed-path trigger guard, shared by record_witnessed_run and + record_witnessed_run_outcome): RunMonitorTriggerNotPermittedError + - 400 (witnessed-terminal guards): RunCapturePhaseNotTerminalError, + InvalidRunObservedAtError, RunNotWitnessedError """ from fastapi import FastAPI, Request, status @@ -68,6 +71,7 @@ InvalidRunExternalRefError, InvalidRunInterruptedAtError, InvalidRunNameError, + InvalidRunObservedAtError, InvalidRunParametersError, InvalidRunStopReasonError, InvalidRunTruncateReasonError, @@ -85,6 +89,7 @@ RunCannotStopError, RunCannotTruncateError, RunCapabilitiesNotSatisfiedError, + RunCapturePhaseNotTerminalError, RunClearanceCoverageMismatchError, RunComputeResourceUnknownError, RunEnclosureCoverageMismatchError, @@ -93,6 +98,7 @@ RunInputNotVerifiedError, RunMonitorTriggerNotPermittedError, RunNotFoundError, + RunNotWitnessedError, RunObservationLogbookClosedError, RunPlanAssetDecommissionedError, RunRequiresActiveClearanceError, @@ -113,6 +119,7 @@ hold_run, list_runs, record_witnessed_run, + record_witnessed_run_outcome, resume_run, start_run, stop_run, @@ -206,6 +213,9 @@ def register_run_routes(app: FastAPI) -> None: # routes-completeness architecture fitness without exposing a public # HTTP surface. app.include_router(record_witnessed_run.router) + # Stub router inclusion for the in-process-only witnessed-terminal slice; + # same rationale as record_witnessed_run above. + app.include_router(record_witnessed_run_outcome.router) app.include_router(complete_run.router) app.include_router(abort_run.router) app.include_router(hold_run.router) @@ -241,8 +251,18 @@ def register_run_routes(app: FastAPI) -> None: InvalidInputDatasetsError, # Watched-genesis trigger guard: mirrors the Enclosure BC's # MonitorTriggerNotPermittedError registration for an - # in-process-only slice even with no route mounted. + # in-process-only slice even with no route mounted. Shared by + # record_witnessed_run and record_witnessed_run_outcome. RunMonitorTriggerNotPermittedError, + # Witnessed-terminal request-shape guards (record_witnessed_run_outcome): + # a non-terminal observed phase, or a substrate timestamp in the future. + RunCapturePhaseNotTerminalError, + InvalidRunObservedAtError, + # Witnessed-terminal applicability guard: the targeted Run is + # Conducted, not Witnessed. Not a state-transition conflict (the + # command never applies to this Run, regardless of status), so + # 400 rather than 409. + RunNotWitnessedError, ): app.add_exception_handler(validation_cls, _handle_validation_error) # Obligation gate (Gate III): missing/invalid justification -> 422. diff --git a/apps/api/src/cora/run/tools.py b/apps/api/src/cora/run/tools.py index f1e527c1f0d..f818bfdbeae 100644 --- a/apps/api/src/cora/run/tools.py +++ b/apps/api/src/cora/run/tools.py @@ -18,6 +18,9 @@ from cora.run.features.hold_run import tool as hold_run_tool from cora.run.features.list_runs import tool as list_runs_tool from cora.run.features.record_witnessed_run import tool as record_witnessed_run_tool +from cora.run.features.record_witnessed_run_outcome import ( + tool as record_witnessed_run_outcome_tool, +) from cora.run.features.resume_run import tool as resume_run_tool from cora.run.features.start_run import tool as start_run_tool from cora.run.features.stop_run import tool as stop_run_tool @@ -43,6 +46,12 @@ def register_run_tools( mcp, get_handler=lambda: get_handlers().record_witnessed_run, ) + # Stub registration for the in-process-only witnessed-terminal slice; + # same rationale as record_witnessed_run above. + record_witnessed_run_outcome_tool.register( + mcp, + get_handler=lambda: get_handlers().record_witnessed_run_outcome, + ) complete_run_tool.register( mcp, get_handler=lambda: get_handlers().complete_run, diff --git a/apps/api/src/cora/run/wire.py b/apps/api/src/cora/run/wire.py index 51230a58341..579ad54d8b1 100644 --- a/apps/api/src/cora/run/wire.py +++ b/apps/api/src/cora/run/wire.py @@ -30,6 +30,11 @@ collapse a retry against. In-process-only; no route, no MCP tool ever call it. +`record_witnessed_run_outcome` closes the witnessed path: update-style, +bare Handler protocol, same posture as the four driven terminals (strict- +not-idempotent, ConcurrencyError handles the double-submit case). Also +in-process-only; no route, no MCP tool ever call it. + `append_observations` writes the polymorphic sensor / motor observation logbook (SOSA `sampling_procedure` discriminator; lazy open-on-first- write). Not idempotency-wrapped: natural idempotence via the @@ -76,6 +81,7 @@ hold_run, list_runs, record_witnessed_run, + record_witnessed_run_outcome, resume_run, start_run, stop_run, @@ -91,6 +97,7 @@ class RunHandlers: start_run: start_run.IdempotentHandler record_witnessed_run: record_witnessed_run.Handler + record_witnessed_run_outcome: record_witnessed_run_outcome.Handler complete_run: complete_run.Handler abort_run: abort_run.Handler hold_run: hold_run.Handler @@ -128,6 +135,11 @@ def wire_run(deps: Kernel) -> RunHandlers: command_name="RecordWitnessedRun", bc=_BC, ), + record_witnessed_run_outcome=with_tracing( + record_witnessed_run_outcome.bind(deps), + command_name="RecordWitnessedRunOutcome", + bc=_BC, + ), complete_run=with_tracing( complete_run.bind(deps), command_name="CompleteRun", diff --git a/apps/api/tests/architecture/test_slice_test_coverage.py b/apps/api/tests/architecture/test_slice_test_coverage.py index 30bb5faa0f3..af0de3a45a2 100644 --- a/apps/api/tests/architecture/test_slice_test_coverage.py +++ b/apps/api/tests/architecture/test_slice_test_coverage.py @@ -113,6 +113,10 @@ # Enclosure / Supply monitor-trigger precedent. In-process adapters # call via RunHandlers.record_witnessed_run. "cora.run.features.record_witnessed_run", + # Witnessed-terminal slice: same anti-scope as record_witnessed_run + # above, no operator path to close a witnessed Run either. In-process + # adapters call via RunHandlers.record_witnessed_run_outcome. + "cora.run.features.record_witnessed_run_outcome", # Frame slices: contract tests deferred to a follow-up commit # so the Frame + Mount REST + MCP suite can be authored together # against the shared PlacementBody surface. Decider tests + @@ -199,6 +203,9 @@ # Watched-genesis slice: in-process-only by design, no MCP tool. # Mirrors the Enclosure / Supply monitor-trigger precedent. "cora.run.features.record_witnessed_run", + # Witnessed-terminal slice: same in-process-only design as + # record_witnessed_run above. + "cora.run.features.record_witnessed_run_outcome", # --- TODO: real gaps to fill ----------------------------------- # The slice is MCP-registered in `cora..tools.py` but no # contract test exercises the tool schema or call surface. Each @@ -537,6 +544,7 @@ def test_exempt_entries_actually_exist(allowlist_name: str) -> None: "cora.supply.features.observe_supply_status", "cora.enclosure.features.observe_enclosure_status", "cora.run.features.record_witnessed_run", + "cora.run.features.record_witnessed_run_outcome", } ) diff --git a/apps/api/tests/architecture/test_witnessed_genesis_laundering_wall.py b/apps/api/tests/architecture/test_witnessed_genesis_laundering_wall.py index 51e5faf61f8..a18f448fbc0 100644 --- a/apps/api/tests/architecture/test_witnessed_genesis_laundering_wall.py +++ b/apps/api/tests/architecture/test_witnessed_genesis_laundering_wall.py @@ -9,10 +9,10 @@ way to claim WITNESSED, which is precisely the laundering hole the axis exists to close. 2. The in-process-only slices (`observe_enclosure_status`, - `record_witnessed_run`) expose zero REST routes and zero MCP tools. - Their `route.py` / `tool.py` modules are stubs by design; this - confirms the stub actually stays empty rather than trusting the - docstring that says so. + `record_witnessed_run`, `record_witnessed_run_outcome`) expose zero + REST routes and zero MCP tools. Their `route.py` / `tool.py` + modules are stubs by design; this confirms the stub actually stays + empty rather than trusting the docstring that says so. """ import ast @@ -25,6 +25,7 @@ _IN_PROCESS_ONLY_SLICES: tuple[str, ...] = ( "enclosure/features/observe_enclosure_status", "run/features/record_witnessed_run", + "run/features/record_witnessed_run_outcome", ) diff --git a/apps/api/tests/integration/test_record_witnessed_run_outcome_handler_postgres.py b/apps/api/tests/integration/test_record_witnessed_run_outcome_handler_postgres.py new file mode 100644 index 00000000000..be9969eef67 --- /dev/null +++ b/apps/api/tests/integration/test_record_witnessed_run_outcome_handler_postgres.py @@ -0,0 +1,194 @@ +"""Postgres integration test for the `record_witnessed_run_outcome` handler. + +Seeds a real Witnessed Run via `record_witnessed_run` (the genesis, same +fixture as `test_record_witnessed_run_handler_postgres.py`), then closes +it through the new handler and confirms the terminal event round-trips +through a real event store, `observed_at` included. +""" + +from datetime import UTC, datetime +from uuid import UUID, uuid4 + +import asyncpg +import pytest + +from cora.equipment.aggregates.family import FamilyName, family_stream_id +from cora.run.features.record_witnessed_run import RecordWitnessedRun +from cora.run.features.record_witnessed_run import bind as bind_genesis +from cora.run.features.record_witnessed_run_outcome import RecordWitnessedRunOutcome +from cora.run.features.record_witnessed_run_outcome import bind as bind_outcome +from cora.shared.capture_phase import CapturePhase +from cora.shared.identity import MonitorSourceId +from tests.integration._helpers import build_postgres_deps, make_pg_profile_store +from tests.integration.scenarios._facility_fixture import operator_for +from tests.integration.scenarios._tomography_fixture import ( + RecipeSpec, + TomographyAssetIds, + define_recipe_ladder, + install_and_activate_tomography_assets, + recipe_ladder_id_prefix, + tomography_install_id_prefix, +) + +_NOW = datetime(2026, 8, 15, 3, 0, 0, tzinfo=UTC) +_OBSERVED_AT = datetime(2026, 8, 15, 2, 58, 0, tzinfo=UTC) +_PRINCIPAL_ID = operator_for(__file__) +_CORRELATION_ID = UUID("01900000-0000-7000-8000-0000004ca601") +_MONITOR_SOURCE_ID = MonitorSourceId(UUID("01900000-0000-7000-8000-000072756e01")) + +# Scenario tag: 4ca6 (record_witnessed_run_outcome handler round-trip). +_2BM_UNIT_ID = UUID("01900000-0000-7000-8000-00000004caa1") + +_CAP_ROTARY_STAGE_ID = family_stream_id(FamilyName("RotaryStage")) +_CAP_LINEAR_STAGE_ID = family_stream_id(FamilyName("LinearStage")) +_CAP_CAMERA_ID = family_stream_id(FamilyName("Camera")) +_CAP_SCINTILLATOR_ID = family_stream_id(FamilyName("Scintillator")) + +_ASSET_ROTARY_ID = UUID("01900000-0000-7000-8000-00000004cab1") +_ASSET_LINEAR_X_ID = UUID("01900000-0000-7000-8000-00000004cab2") +_ASSET_CAMERA_ID = UUID("01900000-0000-7000-8000-00000004cab3") +_ASSET_SCINTILLATOR_ID = UUID("01900000-0000-7000-8000-00000004cab4") + +_METHOD_ID = UUID("01900000-0000-7000-8000-00000004cac1") +_CAPABILITY_ID = UUID("01900000-0000-7000-8000-00000004cac2") +_PRACTICE_ID = UUID("01900000-0000-7000-8000-00000004cac3") +_PLAN_ID = UUID("01900000-0000-7000-8000-00000004cac4") + +_TOMO_ASSETS = TomographyAssetIds( + unit_id=_2BM_UNIT_ID, + rotary_cap_id=_CAP_ROTARY_STAGE_ID, + linear_x_cap_id=_CAP_LINEAR_STAGE_ID, + camera_cap_id=_CAP_CAMERA_ID, + scintillator_cap_id=_CAP_SCINTILLATOR_ID, + rotary_id=_ASSET_ROTARY_ID, + linear_x_id=_ASSET_LINEAR_X_ID, + camera_id=_ASSET_CAMERA_ID, + scintillator_id=_ASSET_SCINTILLATOR_ID, +) + +_RECIPE = RecipeSpec( + capability_id=_CAPABILITY_ID, + capability_code="cora.capability.tomography", + capability_name="Tomography", + method_id=_METHOD_ID, + method_name="tomography", + needed_family_ids=frozenset( + {_CAP_ROTARY_STAGE_ID, _CAP_LINEAR_STAGE_ID, _CAP_CAMERA_ID, _CAP_SCINTILLATOR_ID} + ), + practice_id=_PRACTICE_ID, + practice_name="2BM_tomography_practice", + site_id=_2BM_UNIT_ID, + plan_id=_PLAN_ID, + plan_name="2BM_witnessed_outcome_tomography_plan", + plan_asset_ids=frozenset( + {_ASSET_ROTARY_ID, _ASSET_LINEAR_X_ID, _ASSET_CAMERA_ID, _ASSET_SCINTILLATOR_ID} + ), +) + + +def _id_queue() -> list[UUID]: + e = uuid4 + return [ + *tomography_install_id_prefix(asset_ids=_TOMO_ASSETS), + *recipe_ladder_id_prefix(spec=_RECIPE), + *[e() for _ in range(20)], # headroom: genesis + outcome event ids + ] + + +async def _seed_witnessed_run(deps: object, *, capture_code: str) -> UUID: + handler = bind_genesis(deps) # type: ignore[arg-type] + return await handler( + RecordWitnessedRun( + name="2BM witnessed capture", + plan_id=_PLAN_ID, + capture_code=capture_code, + monitor_source_id=_MONITOR_SOURCE_ID, + trigger="Monitor", + ), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + + +@pytest.mark.integration +async def test_ended_outcome_persists_run_completed_with_observed_at( + db_pool: asyncpg.Pool, +) -> None: + deps = build_postgres_deps(db_pool, now=_NOW, ids=_id_queue()) + await install_and_activate_tomography_assets( + deps, + profile_store=make_pg_profile_store(db_pool), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + asset_ids=_TOMO_ASSETS, + ) + await define_recipe_ladder( + deps, + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + spec=_RECIPE, + ) + run_id = await _seed_witnessed_run(deps, capture_code="2bmb-tomoscan") + + outcome_handler = bind_outcome(deps) # type: ignore[arg-type] + result = await outcome_handler( + RecordWitnessedRunOutcome( + run_id=run_id, + capture_code="2bmb-tomoscan", + observed_phase=CapturePhase.ENDED, + observed_at=_OBSERVED_AT, + monitor_source_id=_MONITOR_SOURCE_ID, + trigger="Monitor", + ), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + assert result is None + + events, stream_version = await deps.event_store.load("Run", run_id) # type: ignore[attr-defined] + assert stream_version == 2 + assert [e.event_type for e in events] == ["RunStarted", "RunCompleted"] + outcome_event = events[1] + assert outcome_event.payload["observed_at"] == _OBSERVED_AT.isoformat() + assert outcome_event.metadata == {"command": "RecordWitnessedRunOutcome"} + + +@pytest.mark.integration +async def test_aborted_outcome_persists_run_aborted_with_capture_code_in_reason( + db_pool: asyncpg.Pool, +) -> None: + deps = build_postgres_deps(db_pool, now=_NOW, ids=_id_queue()) + await install_and_activate_tomography_assets( + deps, + profile_store=make_pg_profile_store(db_pool), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + asset_ids=_TOMO_ASSETS, + ) + await define_recipe_ladder( + deps, + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + spec=_RECIPE, + ) + run_id = await _seed_witnessed_run(deps, capture_code="2bmb-tomoscan") + + outcome_handler = bind_outcome(deps) # type: ignore[arg-type] + await outcome_handler( + RecordWitnessedRunOutcome( + run_id=run_id, + capture_code="2bmb-tomoscan", + observed_phase=CapturePhase.ABORTED, + observed_at=None, + monitor_source_id=_MONITOR_SOURCE_ID, + trigger="Monitor", + ), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + + events, _ = await deps.event_store.load("Run", run_id) # type: ignore[attr-defined] + assert [e.event_type for e in events] == ["RunStarted", "RunAborted"] + outcome_event = events[1] + assert outcome_event.payload["observed_at"] is None + assert "2bmb-tomoscan" in outcome_event.payload["reason"] diff --git a/apps/api/tests/unit/run/test_record_witnessed_run_outcome_decider.py b/apps/api/tests/unit/run/test_record_witnessed_run_outcome_decider.py new file mode 100644 index 00000000000..69c77cde153 --- /dev/null +++ b/apps/api/tests/unit/run/test_record_witnessed_run_outcome_decider.py @@ -0,0 +1,298 @@ +"""Unit tests for the `record_witnessed_run_outcome` slice's pure decider. + +Closes a witnessed Run: `Ended` -> `RunCompleted`, `Aborted` -> +`RunAborted`. Mirrors `test_complete_run_decider.py` + +`test_abort_run_decider.py` combined, since one decider produces either +event depending on `command.observed_phase`. +""" + +from datetime import UTC, datetime, timedelta +from uuid import UUID, uuid4 + +import pytest + +from cora.run.aggregates.run import ( + ConductMode, + InvalidRunObservedAtError, + Run, + RunAborted, + RunCannotAbortError, + RunCannotCompleteError, + RunCapturePhaseNotTerminalError, + RunCompleted, + RunMonitorTriggerNotPermittedError, + RunName, + RunNotFoundError, + RunNotWitnessedError, + RunStatus, +) +from cora.run.features import record_witnessed_run_outcome +from cora.run.features.record_witnessed_run_outcome import RecordWitnessedRunOutcome +from cora.shared.capture_phase import CapturePhase + +_NOW = datetime(2026, 8, 15, 12, 0, 0, tzinfo=UTC) +_TRIGGER = "Monitor" +_MONITOR_SOURCE_ID = UUID("01900000-0000-7000-8000-000072756e01") + + +def _run( + *, + status: RunStatus = RunStatus.RUNNING, + conduct_mode: ConductMode = ConductMode.WITNESSED, +) -> Run: + return Run( + id=uuid4(), + name=RunName("2BM fly scan"), + plan_id=uuid4(), + subject_id=None, + status=status, + conduct_mode=conduct_mode, + ) + + +def _command(**overrides: object) -> RecordWitnessedRunOutcome: + defaults: dict[str, object] = { + "run_id": uuid4(), + "capture_code": "2bmb-tomoscan", + "observed_phase": CapturePhase.ENDED, + "observed_at": _NOW, + "monitor_source_id": _MONITOR_SOURCE_ID, + "trigger": _TRIGGER, + } + defaults.update(overrides) + return RecordWitnessedRunOutcome(**defaults) # type: ignore[arg-type] + + +# ---------- Ended -> RunCompleted ---------- + + +@pytest.mark.unit +def test_decide_emits_run_completed_for_ended_phase() -> None: + state = _run() + events = record_witnessed_run_outcome.decide( + state=state, + command=_command(run_id=state.id, observed_phase=CapturePhase.ENDED, observed_at=_NOW), + now=_NOW, + ) + assert events == [RunCompleted(run_id=state.id, occurred_at=_NOW, observed_at=_NOW)] + + +@pytest.mark.unit +def test_decide_carries_none_observed_at_onto_completed_event() -> None: + state = _run() + events = record_witnessed_run_outcome.decide( + state=state, + command=_command(run_id=state.id, observed_phase=CapturePhase.ENDED, observed_at=None), + now=_NOW, + ) + assert events[0].observed_at is None + + +@pytest.mark.unit +def test_decide_raises_cannot_complete_when_not_running() -> None: + state = _run(status=RunStatus.COMPLETED) + with pytest.raises(RunCannotCompleteError) as exc_info: + record_witnessed_run_outcome.decide( + state=state, + command=_command(run_id=state.id, observed_phase=CapturePhase.ENDED), + now=_NOW, + ) + assert exc_info.value.current_status is RunStatus.COMPLETED + + +# ---------- Aborted -> RunAborted ---------- + + +@pytest.mark.unit +def test_decide_emits_run_aborted_for_aborted_phase() -> None: + state = _run() + events = record_witnessed_run_outcome.decide( + state=state, + command=_command( + run_id=state.id, + capture_code="2bmb-tomoscan", + observed_phase=CapturePhase.ABORTED, + observed_at=_NOW, + ), + now=_NOW, + ) + assert events == [ + RunAborted( + run_id=state.id, + reason="RunWitness observed capture 2bmb-tomoscan as Aborted", + occurred_at=_NOW, + observed_at=_NOW, + ) + ] + + +@pytest.mark.unit +def test_decide_composes_abort_reason_from_capture_code_not_operator_input() -> None: + """No operator-injectable text reaches RunAborted.reason through this + command: the command carries no reason field at all.""" + state = _run() + events = record_witnessed_run_outcome.decide( + state=state, + command=_command( + run_id=state.id, capture_code="32id-fastccd", observed_phase=CapturePhase.ABORTED + ), + now=_NOW, + ) + event = events[0] + assert isinstance(event, RunAborted) + assert "32id-fastccd" in event.reason + + +@pytest.mark.unit +def test_decide_carries_none_observed_at_onto_aborted_event() -> None: + state = _run() + events = record_witnessed_run_outcome.decide( + state=state, + command=_command(run_id=state.id, observed_phase=CapturePhase.ABORTED, observed_at=None), + now=_NOW, + ) + assert events[0].observed_at is None + + +@pytest.mark.unit +def test_decide_raises_cannot_abort_when_not_running() -> None: + state = _run(status=RunStatus.ABORTED) + with pytest.raises(RunCannotAbortError) as exc_info: + record_witnessed_run_outcome.decide( + state=state, + command=_command(run_id=state.id, observed_phase=CapturePhase.ABORTED), + now=_NOW, + ) + assert exc_info.value.current_status is RunStatus.ABORTED + + +# ---------- The trigger guard ---------- + + +@pytest.mark.unit +@pytest.mark.parametrize("bad_trigger", ["Operator", "API", "", "monitor"]) +def test_decide_rejects_any_non_monitor_trigger(bad_trigger: str) -> None: + state = _run() + with pytest.raises(RunMonitorTriggerNotPermittedError): + record_witnessed_run_outcome.decide( + state=state, + command=_command(run_id=state.id, trigger=bad_trigger), + now=_NOW, + ) + + +@pytest.mark.unit +def test_decide_rejects_bad_trigger_before_checking_state() -> None: + """Request-shape rejection: fires even against a nonexistent Run.""" + with pytest.raises(RunMonitorTriggerNotPermittedError): + record_witnessed_run_outcome.decide( + state=None, + command=_command(trigger="Operator"), + now=_NOW, + ) + + +# ---------- The phase guard ---------- + + +@pytest.mark.unit +@pytest.mark.parametrize( + "non_terminal_phase", [CapturePhase.BEGUN, CapturePhase.PROGRESSING, CapturePhase.UNRECOGNIZED] +) +def test_decide_rejects_any_non_terminal_phase(non_terminal_phase: CapturePhase) -> None: + state = _run() + with pytest.raises(RunCapturePhaseNotTerminalError): + record_witnessed_run_outcome.decide( + state=state, + command=_command(run_id=state.id, observed_phase=non_terminal_phase), + now=_NOW, + ) + + +@pytest.mark.unit +def test_decide_rejects_non_terminal_phase_before_checking_state() -> None: + with pytest.raises(RunCapturePhaseNotTerminalError): + record_witnessed_run_outcome.decide( + state=None, + command=_command(observed_phase=CapturePhase.BEGUN), + now=_NOW, + ) + + +# ---------- Existence + conduct-mode guards ---------- + + +@pytest.mark.unit +def test_decide_raises_run_not_found_when_state_is_none() -> None: + target_id = uuid4() + with pytest.raises(RunNotFoundError) as exc_info: + record_witnessed_run_outcome.decide( + state=None, + command=_command(run_id=target_id), + now=_NOW, + ) + assert exc_info.value.run_id == target_id + + +@pytest.mark.unit +def test_decide_raises_not_witnessed_for_a_conducted_run() -> None: + """The applicability guard: this command must never terminate an + operator-driven Run, regardless of its status or the observed phase.""" + state = _run(conduct_mode=ConductMode.CONDUCTED) + with pytest.raises(RunNotWitnessedError) as exc_info: + record_witnessed_run_outcome.decide( + state=state, + command=_command(run_id=state.id), + now=_NOW, + ) + assert exc_info.value.run_id == state.id + assert exc_info.value.conduct_mode == "Conducted" + + +# ---------- The observed_at guard ---------- + + +@pytest.mark.unit +def test_decide_raises_invalid_observed_at_when_in_the_future() -> None: + state = _run() + future = _NOW + timedelta(seconds=1) + with pytest.raises(InvalidRunObservedAtError): + record_witnessed_run_outcome.decide( + state=state, + command=_command(run_id=state.id, observed_at=future), + now=_NOW, + ) + + +@pytest.mark.unit +def test_decide_allows_observed_at_equal_to_now() -> None: + state = _run() + events = record_witnessed_run_outcome.decide( + state=state, + command=_command(run_id=state.id, observed_at=_NOW), + now=_NOW, + ) + assert events[0].observed_at == _NOW + + +@pytest.mark.unit +def test_decide_allows_observed_at_none_regardless_of_now() -> None: + state = _run() + events = record_witnessed_run_outcome.decide( + state=state, + command=_command(run_id=state.id, observed_at=None), + now=_NOW, + ) + assert events[0].observed_at is None + + +# ---------- Purity ---------- + + +@pytest.mark.unit +def test_decide_is_pure_same_inputs_same_outputs() -> None: + state = _run() + command = _command(run_id=state.id) + first = record_witnessed_run_outcome.decide(state=state, command=command, now=_NOW) + second = record_witnessed_run_outcome.decide(state=state, command=command, now=_NOW) + assert first == second diff --git a/apps/api/tests/unit/run/test_record_witnessed_run_outcome_decider_properties.py b/apps/api/tests/unit/run/test_record_witnessed_run_outcome_decider_properties.py new file mode 100644 index 00000000000..9e5a01b5072 --- /dev/null +++ b/apps/api/tests/unit/run/test_record_witnessed_run_outcome_decider_properties.py @@ -0,0 +1,483 @@ +"""Property-based tests for `record_witnessed_run_outcome.decide` (Run BC). + +Complements the example-based `test_record_witnessed_run_outcome_decider.py` +with universal claims across generated inputs. The decider closes a +witnessed Run: + + (state, command, now) -> list[RunCompleted] | list[RunAborted] + +Load-bearing properties: + + - A non-Monitor trigger always raises `RunMonitorTriggerNotPermittedError`, + regardless of every other input (including state=None) -- request-shape + rejection, checked first, mirrors `record_witnessed_run`'s own PBT. + - A non-terminal `observed_phase` always raises `RunCapturePhaseNotTerminalError`, + regardless of state. + - state=None always raises `RunNotFoundError` carrying command.run_id. + - A Conducted Run always raises `RunNotWitnessedError` carrying its + conduct_mode, regardless of status or observed_phase. + - A future `observed_at` always raises `InvalidRunObservedAtError`. + - The source-state partition is total over `RunStatus` for a Witnessed + Run: `Running` emits exactly one event (RunCompleted for Ended, + RunAborted for Aborted) carrying `observed_at` verbatim; every other + status raises the matching Cannot*Error carrying the current status. + - The emitted event's run_id is `state.id`, never `command.run_id`. + - Pure: same (state, command, now) returns equal events. +""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from typing import TYPE_CHECKING +from uuid import UUID + +import pytest +from hypothesis import assume, given +from hypothesis import strategies as st + +from cora.run.aggregates.run import ( + ConductMode, + InvalidRunObservedAtError, + Run, + RunAborted, + RunCannotAbortError, + RunCannotCompleteError, + RunCapturePhaseNotTerminalError, + RunCompleted, + RunMonitorTriggerNotPermittedError, + RunName, + RunNotFoundError, + RunNotWitnessedError, + RunStatus, +) +from cora.run.features import record_witnessed_run_outcome +from cora.run.features.record_witnessed_run_outcome import RecordWitnessedRunOutcome +from cora.shared.capture_phase import CapturePhase +from tests._strategies import printable_ascii_text + +if TYPE_CHECKING: + from datetime import datetime as _datetime + +_PLAN_ID = UUID(int=1) +_MONITOR_SOURCE_ID = UUID(int=3) +_CAPTURE_CODE = printable_ascii_text(min_size=1, max_size=64) + +_TERMINABLE_SOURCES = (RunStatus.RUNNING,) +_NONTERMINABLE_SOURCES = tuple(s for s in RunStatus if s not in frozenset(_TERMINABLE_SOURCES)) +_TERMINAL_PHASES = (CapturePhase.ENDED, CapturePhase.ABORTED) +_NONTERMINAL_PHASES = tuple(p for p in CapturePhase if p not in frozenset(_TERMINAL_PHASES)) + +_ANY_STATUS = st.sampled_from(list(RunStatus)) +_ANY_CONDUCT_MODE = st.sampled_from(list(ConductMode)) +_ANY_PHASE = st.sampled_from(list(CapturePhase)) +_SOME_NOW = st.datetimes( + min_value=datetime(2000, 1, 1), + max_value=datetime(2200, 1, 1), + timezones=st.just(UTC), +) + + +def _run(*, run_id: UUID, status: RunStatus, conduct_mode: ConductMode) -> Run: + return Run( + id=run_id, + name=RunName("2BM fly scan"), + plan_id=_PLAN_ID, + subject_id=None, + status=status, + conduct_mode=conduct_mode, + ) + + +def _command( + *, + run_id: UUID, + capture_code: str, + observed_phase: CapturePhase, + observed_at: _datetime | None, + trigger: str, +) -> RecordWitnessedRunOutcome: + return RecordWitnessedRunOutcome( + run_id=run_id, + capture_code=capture_code, + observed_phase=observed_phase, + observed_at=observed_at, + monitor_source_id=_MONITOR_SOURCE_ID, # type: ignore[arg-type] + trigger=trigger, + ) + + +# ---------- Trigger guard: request-shape, unconditional ---------- + + +@pytest.mark.unit +@given( + run_id=st.uuids(), + capture_code=_CAPTURE_CODE, + phase=_ANY_PHASE, + status=_ANY_STATUS, + conduct_mode=_ANY_CONDUCT_MODE, + trigger=st.text().filter(lambda t: t != "Monitor"), + now=_SOME_NOW, +) +def test_non_monitor_trigger_always_raises_regardless_of_everything_else( + run_id: UUID, + capture_code: str, + phase: CapturePhase, + status: RunStatus, + conduct_mode: ConductMode, + trigger: str, + now: _datetime, +) -> None: + state = _run(run_id=run_id, status=status, conduct_mode=conduct_mode) + with pytest.raises(RunMonitorTriggerNotPermittedError): + record_witnessed_run_outcome.decide( + state=state, + command=_command( + run_id=run_id, + capture_code=capture_code, + observed_phase=phase, + observed_at=None, + trigger=trigger, + ), + now=now, + ) + + +@pytest.mark.unit +@given(run_id=st.uuids(), capture_code=_CAPTURE_CODE, phase=_ANY_PHASE, now=_SOME_NOW) +def test_non_monitor_trigger_raises_even_against_a_nonexistent_run( + run_id: UUID, + capture_code: str, + phase: CapturePhase, + now: _datetime, +) -> None: + with pytest.raises(RunMonitorTriggerNotPermittedError): + record_witnessed_run_outcome.decide( + state=None, + command=_command( + run_id=run_id, + capture_code=capture_code, + observed_phase=phase, + observed_at=None, + trigger="Operator", + ), + now=now, + ) + + +# ---------- Phase guard: request-shape, checked before state ---------- + + +@pytest.mark.unit +@given( + run_id=st.uuids(), + capture_code=_CAPTURE_CODE, + phase=st.sampled_from(_NONTERMINAL_PHASES), + now=_SOME_NOW, +) +def test_non_terminal_phase_always_raises_even_against_a_nonexistent_run( + run_id: UUID, + capture_code: str, + phase: CapturePhase, + now: _datetime, +) -> None: + with pytest.raises(RunCapturePhaseNotTerminalError): + record_witnessed_run_outcome.decide( + state=None, + command=_command( + run_id=run_id, + capture_code=capture_code, + observed_phase=phase, + observed_at=None, + trigger="Monitor", + ), + now=now, + ) + + +# ---------- Existence guard ---------- + + +@pytest.mark.unit +@given( + run_id=st.uuids(), + capture_code=_CAPTURE_CODE, + phase=st.sampled_from(_TERMINAL_PHASES), + now=_SOME_NOW, +) +def test_none_state_always_raises_not_found( + run_id: UUID, + capture_code: str, + phase: CapturePhase, + now: _datetime, +) -> None: + with pytest.raises(RunNotFoundError) as exc: + record_witnessed_run_outcome.decide( + state=None, + command=_command( + run_id=run_id, + capture_code=capture_code, + observed_phase=phase, + observed_at=None, + trigger="Monitor", + ), + now=now, + ) + assert exc.value.run_id == run_id + + +# ---------- Conduct-mode applicability guard ---------- + + +@pytest.mark.unit +@given( + run_id=st.uuids(), + capture_code=_CAPTURE_CODE, + status=_ANY_STATUS, + phase=st.sampled_from(_TERMINAL_PHASES), + now=_SOME_NOW, +) +def test_conducted_run_always_raises_not_witnessed_regardless_of_status_or_phase( + run_id: UUID, + capture_code: str, + status: RunStatus, + phase: CapturePhase, + now: _datetime, +) -> None: + state = _run(run_id=run_id, status=status, conduct_mode=ConductMode.CONDUCTED) + with pytest.raises(RunNotWitnessedError) as exc: + record_witnessed_run_outcome.decide( + state=state, + command=_command( + run_id=run_id, + capture_code=capture_code, + observed_phase=phase, + observed_at=None, + trigger="Monitor", + ), + now=now, + ) + assert exc.value.run_id == run_id + assert exc.value.conduct_mode == "Conducted" + + +# ---------- observed_at guard ---------- + + +@pytest.mark.unit +@given( + run_id=st.uuids(), + capture_code=_CAPTURE_CODE, + phase=st.sampled_from(_TERMINAL_PHASES), + now=_SOME_NOW, + gap=st.integers(min_value=1, max_value=86_400), +) +def test_future_observed_at_always_raises_invalid( + run_id: UUID, + capture_code: str, + phase: CapturePhase, + now: _datetime, + gap: int, +) -> None: + observed_at = now + timedelta(seconds=gap) + state = _run(run_id=run_id, status=RunStatus.RUNNING, conduct_mode=ConductMode.WITNESSED) + with pytest.raises(InvalidRunObservedAtError): + record_witnessed_run_outcome.decide( + state=state, + command=_command( + run_id=run_id, + capture_code=capture_code, + observed_phase=phase, + observed_at=observed_at, + trigger="Monitor", + ), + now=now, + ) + + +# ---------- Source-state partition, for a Witnessed Run ---------- + + +@pytest.mark.unit +@given( + run_id=st.uuids(), + capture_code=_CAPTURE_CODE, + observed_at=st.none() | _SOME_NOW, + now=_SOME_NOW, +) +def test_running_witnessed_ended_emits_single_completed_with_observed_at_threaded( + run_id: UUID, + capture_code: str, + observed_at: _datetime | None, + now: _datetime, +) -> None: + assume(observed_at is None or observed_at <= now) + state = _run(run_id=run_id, status=RunStatus.RUNNING, conduct_mode=ConductMode.WITNESSED) + events = record_witnessed_run_outcome.decide( + state=state, + command=_command( + run_id=run_id, + capture_code=capture_code, + observed_phase=CapturePhase.ENDED, + observed_at=observed_at, + trigger="Monitor", + ), + now=now, + ) + assert events == [RunCompleted(run_id=run_id, occurred_at=now, observed_at=observed_at)] + + +@pytest.mark.unit +@given( + run_id=st.uuids(), + capture_code=_CAPTURE_CODE, + observed_at=st.none() | _SOME_NOW, + now=_SOME_NOW, +) +def test_running_witnessed_aborted_emits_single_aborted_with_observed_at_threaded( + run_id: UUID, + capture_code: str, + observed_at: _datetime | None, + now: _datetime, +) -> None: + assume(observed_at is None or observed_at <= now) + state = _run(run_id=run_id, status=RunStatus.RUNNING, conduct_mode=ConductMode.WITNESSED) + events = record_witnessed_run_outcome.decide( + state=state, + command=_command( + run_id=run_id, + capture_code=capture_code, + observed_phase=CapturePhase.ABORTED, + observed_at=observed_at, + trigger="Monitor", + ), + now=now, + ) + assert len(events) == 1 + event = events[0] + assert isinstance(event, RunAborted) + assert event.run_id == run_id + assert event.occurred_at == now + assert event.observed_at == observed_at + assert capture_code in event.reason + + +@pytest.mark.unit +@given( + run_id=st.uuids(), + capture_code=_CAPTURE_CODE, + source=st.sampled_from(_NONTERMINABLE_SOURCES), + now=_SOME_NOW, +) +def test_nonrunning_witnessed_ended_always_raises_cannot_complete( + run_id: UUID, + capture_code: str, + source: RunStatus, + now: _datetime, +) -> None: + state = _run(run_id=run_id, status=source, conduct_mode=ConductMode.WITNESSED) + with pytest.raises(RunCannotCompleteError) as exc: + record_witnessed_run_outcome.decide( + state=state, + command=_command( + run_id=run_id, + capture_code=capture_code, + observed_phase=CapturePhase.ENDED, + observed_at=None, + trigger="Monitor", + ), + now=now, + ) + assert exc.value.current_status is source + + +@pytest.mark.unit +@given( + run_id=st.uuids(), + capture_code=_CAPTURE_CODE, + source=st.sampled_from(_NONTERMINABLE_SOURCES), + now=_SOME_NOW, +) +def test_nonrunning_witnessed_aborted_always_raises_cannot_abort( + run_id: UUID, + capture_code: str, + source: RunStatus, + now: _datetime, +) -> None: + state = _run(run_id=run_id, status=source, conduct_mode=ConductMode.WITNESSED) + with pytest.raises(RunCannotAbortError) as exc: + record_witnessed_run_outcome.decide( + state=state, + command=_command( + run_id=run_id, + capture_code=capture_code, + observed_phase=CapturePhase.ABORTED, + observed_at=None, + trigger="Monitor", + ), + now=now, + ) + assert exc.value.current_status is source + + +# ---------- run_id provenance ---------- + + +@pytest.mark.unit +@given( + state_run_id=st.uuids(), + command_run_id=st.uuids(), + capture_code=_CAPTURE_CODE, + phase=st.sampled_from(_TERMINAL_PHASES), + now=_SOME_NOW, +) +def test_emitted_event_uses_state_id_not_command_run_id( + state_run_id: UUID, + command_run_id: UUID, + capture_code: str, + phase: CapturePhase, + now: _datetime, +) -> None: + assume(state_run_id != command_run_id) + state = _run(run_id=state_run_id, status=RunStatus.RUNNING, conduct_mode=ConductMode.WITNESSED) + events = record_witnessed_run_outcome.decide( + state=state, + command=_command( + run_id=command_run_id, + capture_code=capture_code, + observed_phase=phase, + observed_at=None, + trigger="Monitor", + ), + now=now, + ) + assert events[0].run_id == state_run_id + + +# ---------- Purity ---------- + + +@pytest.mark.unit +@given( + run_id=st.uuids(), + capture_code=_CAPTURE_CODE, + phase=st.sampled_from(_TERMINAL_PHASES), + now=_SOME_NOW, +) +def test_decide_is_pure_same_inputs_same_outputs( + run_id: UUID, + capture_code: str, + phase: CapturePhase, + now: _datetime, +) -> None: + state = _run(run_id=run_id, status=RunStatus.RUNNING, conduct_mode=ConductMode.WITNESSED) + command = _command( + run_id=run_id, + capture_code=capture_code, + observed_phase=phase, + observed_at=None, + trigger="Monitor", + ) + first = record_witnessed_run_outcome.decide(state=state, command=command, now=now) + second = record_witnessed_run_outcome.decide(state=state, command=command, now=now) + assert first == second diff --git a/apps/api/tests/unit/run/test_record_witnessed_run_outcome_handler.py b/apps/api/tests/unit/run/test_record_witnessed_run_outcome_handler.py new file mode 100644 index 00000000000..8a41860ba7a --- /dev/null +++ b/apps/api/tests/unit/run/test_record_witnessed_run_outcome_handler.py @@ -0,0 +1,241 @@ +"""Unit tests for the `record_witnessed_run_outcome` application handler. + +Mirror of `test_complete_run_handler.py` shape: update-style, strict-not- +idempotent, append-once-on-success. Every seeded Run is Witnessed, since +the applicability guard (`RunNotWitnessedError`) refuses a Conducted one +regardless of status. +""" + +from datetime import UTC, datetime +from uuid import UUID, uuid4 + +import pytest + +from cora.infrastructure.adapters.in_memory_event_store import InMemoryEventStore +from cora.infrastructure.event_envelope import to_new_event +from cora.run import RunHandlers, UnauthorizedError, wire_run +from cora.run.aggregates.run import ( + ConductMode, + RunCannotAbortError, + RunCannotCompleteError, + RunNotFoundError, + RunNotWitnessedError, +) +from cora.run.aggregates.run.events import ( + RunCompleted, + RunStarted, + event_type_name, + to_payload, +) +from cora.run.features import record_witnessed_run_outcome +from cora.run.features.record_witnessed_run_outcome import RecordWitnessedRunOutcome +from cora.shared.capture_phase import CapturePhase +from tests.unit._helpers import build_deps + +_NOW = datetime(2026, 8, 15, 12, 0, 0, tzinfo=UTC) +_RUN_ID = UUID("01900000-0000-7000-8000-00000000fd01") +_OUTCOME_EVENT_ID = UUID("01900000-0000-7000-8000-00000000fd02") +_PRINCIPAL_ID = UUID("01900000-0000-7000-8000-000000000099") +_CORRELATION_ID = UUID("01900000-0000-7000-8000-0000000000aa") +_MONITOR_SOURCE_ID = UUID("01900000-0000-7000-8000-000072756e01") + + +async def _seed_witnessed_run_started( + store: InMemoryEventStore, + run_id: UUID, + *, + conduct_mode: ConductMode = ConductMode.WITNESSED, +) -> None: + event = RunStarted( + run_id=run_id, + name="Witnessed capture 2bmb-tomoscan", + plan_id=uuid4(), + subject_id=None, + conduct_mode=conduct_mode, + occurred_at=_NOW, + ) + new_event = to_new_event( + event_type=event_type_name(event), + payload=to_payload(event), + occurred_at=_NOW, + event_id=uuid4(), + command_name="RecordWitnessedRun", + correlation_id=_CORRELATION_ID, + principal_id=uuid4(), + ) + await store.append(stream_type="Run", stream_id=run_id, expected_version=0, events=[new_event]) + + +async def _seed_witnessed_run_completed(store: InMemoryEventStore, run_id: UUID) -> None: + await _seed_witnessed_run_started(store, run_id) + completed = RunCompleted(run_id=run_id, occurred_at=_NOW, observed_at=None) + new_event = to_new_event( + event_type=event_type_name(completed), + payload=to_payload(completed), + occurred_at=_NOW, + event_id=uuid4(), + command_name="RecordWitnessedRunOutcome", + correlation_id=_CORRELATION_ID, + principal_id=uuid4(), + ) + await store.append(stream_type="Run", stream_id=run_id, expected_version=1, events=[new_event]) + + +def _command(**overrides: object) -> RecordWitnessedRunOutcome: + defaults: dict[str, object] = { + "run_id": _RUN_ID, + "capture_code": "2bmb-tomoscan", + "observed_phase": CapturePhase.ENDED, + "observed_at": _NOW, + "monitor_source_id": _MONITOR_SOURCE_ID, + "trigger": "Monitor", + } + defaults.update(overrides) + return RecordWitnessedRunOutcome(**defaults) # type: ignore[arg-type] + + +@pytest.mark.unit +async def test_handler_returns_none_on_success() -> None: + store = InMemoryEventStore() + await _seed_witnessed_run_started(store, _RUN_ID) + deps = build_deps(ids=[_OUTCOME_EVENT_ID], now=_NOW, event_store=store) + + result = await record_witnessed_run_outcome.bind(deps)( + _command(), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + assert result is None + + +@pytest.mark.unit +async def test_handler_appends_run_completed_for_ended_phase() -> None: + store = InMemoryEventStore() + await _seed_witnessed_run_started(store, _RUN_ID) + deps = build_deps(ids=[_OUTCOME_EVENT_ID], now=_NOW, event_store=store) + + await record_witnessed_run_outcome.bind(deps)( + _command(observed_phase=CapturePhase.ENDED), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + + events, version = await store.load("Run", _RUN_ID) + assert version == 2 + assert [e.event_type for e in events] == ["RunStarted", "RunCompleted"] + outcome = events[1] + assert outcome.event_id == _OUTCOME_EVENT_ID + assert outcome.metadata == {"command": "RecordWitnessedRunOutcome"} + + +@pytest.mark.unit +async def test_handler_appends_run_aborted_for_aborted_phase() -> None: + store = InMemoryEventStore() + await _seed_witnessed_run_started(store, _RUN_ID) + deps = build_deps(ids=[_OUTCOME_EVENT_ID], now=_NOW, event_store=store) + + await record_witnessed_run_outcome.bind(deps)( + _command(observed_phase=CapturePhase.ABORTED), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + + events, _ = await store.load("Run", _RUN_ID) + assert [e.event_type for e in events] == ["RunStarted", "RunAborted"] + + +@pytest.mark.unit +async def test_handler_raises_run_not_found_when_run_does_not_exist() -> None: + deps = build_deps(ids=[_OUTCOME_EVENT_ID], now=_NOW) + handler = record_witnessed_run_outcome.bind(deps) + + with pytest.raises(RunNotFoundError): + await handler( + _command(), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + + +@pytest.mark.unit +async def test_handler_raises_not_witnessed_for_a_conducted_run() -> None: + store = InMemoryEventStore() + await _seed_witnessed_run_started(store, _RUN_ID, conduct_mode=ConductMode.CONDUCTED) + deps = build_deps(ids=[_OUTCOME_EVENT_ID], now=_NOW, event_store=store) + + with pytest.raises(RunNotWitnessedError): + await record_witnessed_run_outcome.bind(deps)( + _command(), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + + +@pytest.mark.unit +async def test_handler_raises_cannot_complete_when_already_completed() -> None: + """Strict-not-idempotent: re-recording the outcome raises.""" + store = InMemoryEventStore() + await _seed_witnessed_run_completed(store, _RUN_ID) + deps = build_deps(ids=[_OUTCOME_EVENT_ID], now=_NOW, event_store=store) + + with pytest.raises(RunCannotCompleteError): + await record_witnessed_run_outcome.bind(deps)( + _command(observed_phase=CapturePhase.ENDED), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + + +@pytest.mark.unit +async def test_handler_raises_cannot_abort_when_already_completed() -> None: + store = InMemoryEventStore() + await _seed_witnessed_run_completed(store, _RUN_ID) + deps = build_deps(ids=[_OUTCOME_EVENT_ID], now=_NOW, event_store=store) + + with pytest.raises(RunCannotAbortError): + await record_witnessed_run_outcome.bind(deps)( + _command(observed_phase=CapturePhase.ABORTED), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + + +@pytest.mark.unit +async def test_handler_raises_unauthorized_on_deny() -> None: + store = InMemoryEventStore() + await _seed_witnessed_run_started(store, _RUN_ID) + deny_deps = build_deps(ids=[_OUTCOME_EVENT_ID], now=_NOW, event_store=store, deny=True) + + with pytest.raises(UnauthorizedError) as exc_info: + await record_witnessed_run_outcome.bind(deny_deps)( + _command(), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + assert exc_info.value.reason == "denied for test" + + +@pytest.mark.unit +async def test_handler_propagates_causation_id_to_appended_event() -> None: + causation = UUID("01900000-0000-7000-8000-0000000000bb") + store = InMemoryEventStore() + await _seed_witnessed_run_started(store, _RUN_ID) + deps = build_deps(ids=[_OUTCOME_EVENT_ID], now=_NOW, event_store=store) + + await record_witnessed_run_outcome.bind(deps)( + _command(), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + causation_id=causation, + ) + + events, _ = await store.load("Run", _RUN_ID) + assert events[1].causation_id == causation + + +@pytest.mark.unit +def test_wire_run_includes_record_witnessed_run_outcome() -> None: + deps = build_deps(ids=[_OUTCOME_EVENT_ID], now=_NOW) + handlers = wire_run(deps) + assert isinstance(handlers, RunHandlers) + assert callable(handlers.record_witnessed_run_outcome) From b9be9dd7dee91cd36fb2f23f65bef7acac9ce38a Mon Sep 17 00:00:00 2001 From: xmap <16776958+xmap@users.noreply.github.com> Date: Fri, 14 Aug 2026 19:59:45 -0500 Subject: [PATCH 3/6] api: wire RunWitness terminals and missed-terminal recovery 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. --- apps/api/src/cora/agent/seed_run_witness.py | 32 ++- apps/api/src/cora/api/_run_witness.py | 216 ++++++++++++++++-- apps/api/src/cora/api/main.py | 2 + .../cora/run/features/truncate_run/command.py | 11 +- .../test_run_transitions_handler_postgres.py | 4 + apps/api/tests/unit/api/test_run_witness.py | 206 +++++++++++++++-- 6 files changed, 421 insertions(+), 50 deletions(-) diff --git a/apps/api/src/cora/agent/seed_run_witness.py b/apps/api/src/cora/agent/seed_run_witness.py index f215ce5ece6..cca4b2be85e 100644 --- a/apps/api/src/cora/agent/seed_run_witness.py +++ b/apps/api/src/cora/agent/seed_run_witness.py @@ -15,15 +15,31 @@ (`prompt_template_id=None`) and a sentinel `ModelRef` (`provider="deterministic"`). Never used to build an LLM: the runtime is a substrate-observation loop, not an LLM subscriber. - - Authorization: the runtime issues `RecordWitnessedRun` through the + - Authorization: the runtime issues four distinct commands through the Authorize port like any principal. Under the default AllowAllAuthorize - it is permitted; under TrustAuthorize the operator's single configured - Policy must include this principal + {RecordWitnessedRun, ListRuns}. - ListRuns is the restart-rebuild read: without it a restart cannot - rediscover which captures are already open, and would re-promote - them. Without the RecordWitnessedRun grant, a real BEGUN observation - logs `run_witness.promotion_unauthorized` and stays IDLE (retried on - the next BEGUN, same posture as RunInitiator's StartRun grant). + all four are permitted; under TrustAuthorize the operator's single + configured Policy must include this principal + {RecordWitnessedRun, + RecordWitnessedRunOutcome, TruncateRun, ListRuns}. ListRuns is the + restart-rebuild read: without it a restart cannot rediscover which + captures are already open, and would re-promote them. Without the + RecordWitnessedRun grant, a real BEGUN observation logs + `run_witness.promotion_unauthorized` and stays IDLE (retried on the + next BEGUN). Without RecordWitnessedRunOutcome, a real terminal logs + `run_witness.outcome_unauthorized` and leaves the Run open (retried + on the next BEGUN via truncation). Without TruncateRun, a missed + terminal cannot be recovered and logs `run_witness.truncate_unauthorized`, + but the new capture still promotes regardless. + + UNLIKE the RecordWitnessedRunOutcome grant, TruncateRun's decider + carries no `conduct_mode` gate (it accepts any Running-or-Held Run, + same as every other operator-facing terminal). This principal's + safety therefore rests on `_run_witness.py`'s own bookkeeping + discipline: `_truncate_stale` only ever supplies a `run_id` it + popped from its own `_open_captures` dict, which is populated + exclusively by this same runtime's own promotions, so it can only + ever name a Run it created. A future change to `_run_witness.py` + that sources a `run_id` for this call from anywhere else would lose + that guarantee with no decider-level backstop to catch it. """ from __future__ import annotations diff --git a/apps/api/src/cora/api/_run_witness.py b/apps/api/src/cora/api/_run_witness.py index 8ebcba84e67..e5ea7dec612 100644 --- a/apps/api/src/cora/api/_run_witness.py +++ b/apps/api/src/cora/api/_run_witness.py @@ -35,7 +35,7 @@ time for an absent one). These log lines are unconditional: they fire identically whether or not recording is enabled. -## Promotion (when run_witness_recording_enabled is True) +## Promotion and termination (when run_witness_recording_enabled is True) Per capture_code, a small dedup state machine: @@ -43,11 +43,20 @@ and, on success, remember the returned run_id as OPEN. On failure (any raised error, including an authorization misconfiguration), log and stay unopened so the next `BEGUN` retries. - - `BEGUN` while a Run is already open for this code: no-op (already - promoted this capture cycle). - - `ENDED` / `ABORTED` while a Run is open: clear the local dedup entry - (log only, no Run action; terminal recording is a separate future - slice). + - `BEGUN` while a Run is already open for this code: the previous + terminal was missed (dropped CA transition, or the substrate + restarted mid-capture). `TruncateRun` the stale Run first + (`interrupted_at=None`: CORA does not know when it actually ended, + only that it did not see the terminal), then promote the new + capture as if idle. A truncate failure does not block the + promotion: the new capture is a real fact regardless of whether the + stale Run could be closed. + - `ENDED` / `ABORTED` while a Run is open: call + `record_witnessed_run_outcome` (`Ended` -> `RunCompleted`, + `Aborted` -> `RunAborted`), carrying the observation's own + `observed_at`. On success, clear the local dedup entry. On failure, + leave the entry open: the next `BEGUN` for this code truncates it + and promotes fresh, so the truncation path doubles as retry. - `ENDED` / `ABORTED` while nothing is open, or `PROGRESSING` / `UNRECOGNIZED` / a `None` phase in any state: no-op. @@ -56,6 +65,42 @@ `external_refs`, so a still-open capture at process restart is never re-promoted. +## Known gap as of this commit: `ENDED` does not yet distinguish abort from success + +`CaptureObservation.phase` classifies purely off the substrate's status +literal (see `ControlPortCaptureObserver`), and as of this commit that +adapter reads only the `status` PV role, not the deployment's `abort` +role. At 2-BM, `fly_scan()`'s exception handlers for `ScanAbortError` / +`CameraTimeoutError` / `FileOverwriteError` still run +`finally: self.end_scan()`, which writes the identical +`'Scan complete'` literal a genuine success writes. So until the abort +PV is read (tracked as the very next commit in this same effort), +`ENDED` unconditionally maps to `RunCompleted` here, which would +misrecord a real 2-BM abort as a success the moment +`run_witness_recording_enabled` is turned on. Nothing in this file or +in `Settings` gates recording on the abort role being configured; the +locked deployment decision is to keep `run_witness_recording_enabled` +off at 2-BM until both this slice and the abort-PV wiring are live. + +## Accepted residual: a reconnect can misread a still-open capture as new + +The `BEGUN`-while-open heuristic assumes a second `BEGUN` for the same +code always means the prior terminal was missed. That is not quite +total: `camonitor`-style subscriptions deliver the PV's CURRENT value +immediately on a fresh subscribe (see `EpicsCaControlPort`), and the +observer resubscribes after every disconnect (see "Retry + resilience" +below). If a reconnect happens to land in the narrow window where the +substrate's status PV still genuinely reads the `BEGUN` literal for a +capture that has not actually restarted, this recorder cannot tell that +apart from a real new capture: it truncates the still-live Run +(spurious `RunTruncated`) and promotes a duplicate. The window is the +duration of one `BEGUN`-classified literal (milliseconds, per the +measured arcturus phase durations), and the outcome is a data-quality +degradation (an extra Run pair for one physical scan), not a control or +interlock concern. No narrower signal (comparing against the +last-observed reading, or `reach_tier`) is implemented; revisit if this +is ever observed in practice. + ## Retry + resilience Mirrors `run_enclosure_permit_monitor`: `observe()` ending (stream @@ -84,6 +129,8 @@ from cora.run.errors import UnauthorizedError from cora.run.features.list_runs.query import ListRuns from cora.run.features.record_witnessed_run.command import RecordWitnessedRun +from cora.run.features.record_witnessed_run_outcome.command import RecordWitnessedRunOutcome +from cora.run.features.truncate_run.command import TruncateRun from cora.run.ports.capture_observer import CaptureObserverScope, CapturePhase from cora.shared.identity import MonitorSourceId @@ -95,6 +142,10 @@ from cora.run.aggregates.run.state import Run from cora.run.features.list_runs.handler import Handler as ListRunsHandler from cora.run.features.record_witnessed_run.handler import Handler as RecordWitnessedRunHandler + from cora.run.features.record_witnessed_run_outcome.handler import ( + Handler as RecordWitnessedRunOutcomeHandler, + ) + from cora.run.features.truncate_run.handler import Handler as TruncateRunHandler from cora.run.ports.capture_observer import CaptureObservation, CaptureObserver from cora.shared.identifier import Identifier @@ -152,11 +203,15 @@ def __init__( *, deps: Kernel, record_witnessed_run: RecordWitnessedRunHandler, + record_witnessed_run_outcome: RecordWitnessedRunOutcomeHandler, + truncate_run: TruncateRunHandler, settings: Settings, open_captures: dict[str, UUID] | None = None, ) -> None: self._deps = deps self._record_witnessed_run = record_witnessed_run + self._record_witnessed_run_outcome = record_witnessed_run_outcome + self._truncate_run = truncate_run self._settings = settings self._open_captures: dict[str, UUID] = dict(open_captures or {}) @@ -165,17 +220,14 @@ async def observe_capture(self, observation: CaptureObservation) -> None: if not self._settings.run_witness_recording_enabled: return - code = observation.capture_code phase = observation.phase if phase is CapturePhase.BEGUN: - if code in self._open_captures: - return + if observation.capture_code in self._open_captures: + await self._truncate_stale(observation) await self._promote(observation) elif phase in _TERMINAL_PHASES: - run_id = self._open_captures.pop(code, None) - if run_id is not None: - _log.info("run_witness.open_capture_cleared", capture_code=code, run_id=str(run_id)) + await self._record_outcome(observation) # PROGRESSING, UNRECOGNIZED, and a None phase make no status # claim this state machine acts on: no-op regardless of state. @@ -231,6 +283,114 @@ async def _promote(self, observation: CaptureObservation) -> None: run_id=str(run_id), ) + async def _truncate_stale(self, observation: CaptureObservation) -> None: + code = observation.capture_code + # Pop unconditionally, before attempting the truncate: the new + # capture promotes regardless of whether the stale Run could be + # closed, so the dedup state must already read IDLE by the time + # `_promote` runs next in `observe_capture`. + # + # SECURITY NOTE (see seed_run_witness.py): TruncateRun's decider + # has no conduct_mode gate, unlike RecordWitnessedRunOutcome's. + # This principal's safety depends entirely on `stale_run_id` + # coming from `_open_captures`, which this runtime populates + # exclusively from its own promotions. Never source a run_id + # for this call from anywhere else (substrate input, a + # capture_code-derived guess, etc.). + stale_run_id = self._open_captures.pop(code, None) + if stale_run_id is None: + return + + try: + await self._truncate_run( + TruncateRun( + run_id=stale_run_id, + reason=( + f"RunWitness observed a new Begun for capture {code} " + f"while the previous Run was still open: the terminal " + f"for that capture was never observed." + ), + interrupted_at=None, + ), + principal_id=RUN_WITNESS_AGENT_ID, + correlation_id=self._deps.id_generator.new_id(), + ) + except asyncio.CancelledError: + raise + except UnauthorizedError: + _log.warning( + "run_witness.truncate_unauthorized", + capture_code=code, + run_id=str(stale_run_id), + ) + except Exception: + _log.exception( + "run_witness.truncate_failed", + capture_code=code, + run_id=str(stale_run_id), + ) + else: + _log.info( + "run_witness.truncated_stale_run", + capture_code=code, + run_id=str(stale_run_id), + ) + + async def _record_outcome(self, observation: CaptureObservation) -> None: + phase = observation.phase + if phase is not CapturePhase.ENDED and phase is not CapturePhase.ABORTED: + # Defensive: observe_capture only calls this method for a + # terminal phase, but re-checking here (rather than trusting + # the caller) also narrows `phase` from `CapturePhase | None` + # for the RecordWitnessedRunOutcome construction below. + return + code = observation.capture_code + run_id = self._open_captures.get(code) + if run_id is None: + return + + command = RecordWitnessedRunOutcome( + run_id=run_id, + capture_code=code, + observed_phase=phase, + observed_at=observation.observed_at, + monitor_source_id=RUN_WITNESS_MONITOR_SOURCE_ID, + trigger="Monitor", + ) + try: + await self._record_witnessed_run_outcome( + command, + principal_id=RUN_WITNESS_AGENT_ID, + correlation_id=self._deps.id_generator.new_id(), + ) + except asyncio.CancelledError: + raise + except UnauthorizedError: + # Configuration fault: the RunWitness principal is not + # granted RecordWitnessedRunOutcome. Log loudly; leave the + # entry open so the next BEGUN truncates it and promotes + # fresh once the grant is fixed. + _log.warning( + "run_witness.outcome_unauthorized", + capture_code=code, + run_id=str(run_id), + ) + return + except Exception: + _log.exception( + "run_witness.outcome_failed", + capture_code=code, + run_id=str(run_id), + ) + return + self._open_captures.pop(code, None) + _log.info( + "run_witness.outcome_recorded", + capture_code=code, + run_id=str(run_id), + observed_phase=str(observation.phase), + ) + def _extract_capture_code(external_refs: frozenset[Identifier]) -> str | None: """Find the `Identifier(scheme="capture-code", ...)` entry's value. @@ -322,6 +482,8 @@ async def run_witness_lifespan( capture_codes: frozenset[str], deps: Kernel | None = None, record_witnessed_run: RecordWitnessedRunHandler | None = None, + record_witnessed_run_outcome: RecordWitnessedRunOutcomeHandler | None = None, + truncate_run: TruncateRunHandler | None = None, open_captures: dict[str, UUID] | None = None, ) -> AsyncGenerator[None]: """Run the watcher as a background task for the app's lifetime. @@ -333,22 +495,40 @@ async def run_witness_lifespan( `deps` stays optional (unlike the sibling `run_supervisor_lifespan` / `run_initiator_lifespan`, which require it) so every existing shadow-only caller needs no change: recording is the only thing that - needs a Kernel (for id generation), so it is only required when - `record_witnessed_run` is also supplied. + needs a Kernel (for id generation), so `deps`, `record_witnessed_run_outcome`, + and `truncate_run` are only required when `record_witnessed_run` is + also supplied. All three: a recorder that could promote but not + terminate would reintroduce the exact wedge (a witnessed Run stuck + in `Running` forever) this slice exists to close. """ if not capture_codes: yield return - if record_witnessed_run is not None and deps is None: - msg = "run_witness_lifespan: record_witnessed_run requires deps" - raise ValueError(msg) + if record_witnessed_run is not None: + missing = [ + name + for name, value in ( + ("deps", deps), + ("record_witnessed_run_outcome", record_witnessed_run_outcome), + ("truncate_run", truncate_run), + ) + if value is None + ] + if missing: + msg = f"run_witness_lifespan: record_witnessed_run requires {', '.join(missing)}" + raise ValueError(msg) recorder: RunWitnessRecorder | None = None if record_witnessed_run is not None: - assert deps is not None # narrowed by the check above + # Narrowed by the check above. + assert deps is not None + assert record_witnessed_run_outcome is not None + assert truncate_run is not None recorder = RunWitnessRecorder( deps=deps, record_witnessed_run=record_witnessed_run, + record_witnessed_run_outcome=record_witnessed_run_outcome, + truncate_run=truncate_run, settings=deps.settings, open_captures=open_captures, ) diff --git a/apps/api/src/cora/api/main.py b/apps/api/src/cora/api/main.py index f67fa08b04a..babbd39539e 100644 --- a/apps/api/src/cora/api/main.py +++ b/apps/api/src/cora/api/main.py @@ -1169,6 +1169,8 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None]: capture_codes=capture_watch_codes, deps=deps, record_witnessed_run=app.state.run.record_witnessed_run, + record_witnessed_run_outcome=app.state.run.record_witnessed_run_outcome, + truncate_run=app.state.run.truncate_run, open_captures=open_captures, ), ): diff --git a/apps/api/src/cora/run/features/truncate_run/command.py b/apps/api/src/cora/run/features/truncate_run/command.py index bc28842b5cf..9ff9dd7460f 100644 --- a/apps/api/src/cora/run/features/truncate_run/command.py +++ b/apps/api/src/cora/run/features/truncate_run/command.py @@ -10,8 +10,15 @@ Distinct from stop: stop = controlled exit while the system is responsive; truncate = retroactive cleanup for a Run that became de-facto dead through interruption (power loss, process crash, -hardware fault). The system itself does not detect de-facto-dead -Runs (separate liveness concern); truncate is operator-driven. +hardware fault). Two callers today: an operator (REST / MCP), and +the RunSupervisor's run-liveness act rung, both threading +`decided_by_decision_id` when autonomous. The RunWitness runtime +(`cora.api._run_witness`) is a third, in-process-only caller: it +truncates a witnessed Run whose terminal observation was missed, +recovering the dedup state so a fresh capture on the same code can +promote. None of the three detect de-facto-dead Runs on their own +initiative from nothing; each has its own trigger (an operator's +judgment, a liveness ceiling, a new Begun for an already-open code). """ from dataclasses import dataclass diff --git a/apps/api/tests/integration/test_run_transitions_handler_postgres.py b/apps/api/tests/integration/test_run_transitions_handler_postgres.py index caac5961643..fbb63215651 100644 --- a/apps/api/tests/integration/test_run_transitions_handler_postgres.py +++ b/apps/api/tests/integration/test_run_transitions_handler_postgres.py @@ -215,6 +215,8 @@ async def test_complete_run_persists_and_round_trips_to_completed_state( "producing_job_id": None, "artifact_uri": None, "occurred_at": _NOW.isoformat(), + # A driven completion has no substrate reading to report. + "observed_at": None, } state = await load_run(deps.event_store, run_id) @@ -308,6 +310,8 @@ async def test_abort_run_persists_with_trimmed_reason_and_round_trips_to_aborted "actuation_kind": None, "producing_job_id": None, "occurred_at": _NOW.isoformat(), + # An operator abort has no substrate reading to report. + "observed_at": None, } state = await load_run(deps.event_store, run_id) diff --git a/apps/api/tests/unit/api/test_run_witness.py b/apps/api/tests/unit/api/test_run_witness.py index 2d1fbac112d..54b451d7ed3 100644 --- a/apps/api/tests/unit/api/test_run_witness.py +++ b/apps/api/tests/unit/api/test_run_witness.py @@ -17,6 +17,7 @@ import asyncio import contextlib +import dataclasses from collections.abc import AsyncGenerator from datetime import UTC, datetime from typing import Any @@ -40,6 +41,8 @@ from cora.run.errors import UnauthorizedError from cora.run.features.list_runs import RunListPage, RunSummaryItem from cora.run.features.record_witnessed_run.command import RecordWitnessedRun +from cora.run.features.record_witnessed_run_outcome.command import RecordWitnessedRunOutcome +from cora.run.features.truncate_run.command import TruncateRun from cora.run.ports.capture_observer import CaptureObservation, CaptureObserverScope, CapturePhase from cora.shared.reach import ReachTier from tests.unit._helpers import build_deps @@ -239,9 +242,55 @@ async def __call__( return self.run_id +class _FakeRecordWitnessedRunOutcome: + """Fake `record_witnessed_run_outcome` handler: records every call, + returns None, or raises a configured exception instead.""" + + def __init__(self, *, raises: Exception | None = None) -> None: + self.raises = raises + self.calls: list[RecordWitnessedRunOutcome] = [] + + async def __call__( + self, + command: RecordWitnessedRunOutcome, + *, + principal_id: UUID, + correlation_id: UUID, + causation_id: UUID | None = None, + surface_id: UUID = NIL_SENTINEL_ID, + ) -> None: + self.calls.append(command) + if self.raises is not None: + raise self.raises + + +class _FakeTruncateRun: + """Fake `truncate_run` handler: records every call, returns None, or + raises a configured exception instead.""" + + def __init__(self, *, raises: Exception | None = None) -> None: + self.raises = raises + self.calls: list[TruncateRun] = [] + + async def __call__( + self, + command: TruncateRun, + *, + principal_id: UUID, + correlation_id: UUID, + causation_id: UUID | None = None, + surface_id: UUID = NIL_SENTINEL_ID, + ) -> None: + self.calls.append(command) + if self.raises is not None: + raise self.raises + + def _recorder( *, record_witnessed_run: _FakeRecordWitnessedRun, + record_witnessed_run_outcome: _FakeRecordWitnessedRunOutcome | None = None, + truncate_run: _FakeTruncateRun | None = None, run_witness_recording_enabled: bool = True, capture_watch_plan_id: UUID | None = _PLAN_ID, open_captures: dict[str, UUID] | None = None, @@ -250,9 +299,13 @@ def _recorder( run_witness_recording_enabled=run_witness_recording_enabled, capture_watch_plan_id=capture_watch_plan_id, ) + outcome = record_witnessed_run_outcome or _FakeRecordWitnessedRunOutcome() + truncate = truncate_run or _FakeTruncateRun() return RunWitnessRecorder( deps=build_deps(ids=[uuid4() for _ in range(10)]), record_witnessed_run=record_witnessed_run, + record_witnessed_run_outcome=outcome, + truncate_run=truncate, settings=settings, open_captures=open_captures, ) @@ -274,15 +327,43 @@ async def test_run_witness_recorder_promotes_a_begun_capture_while_idle() -> Non @pytest.mark.unit -async def test_run_witness_recorder_does_not_repromote_a_begun_capture_while_open() -> None: - fake = _FakeRecordWitnessedRun() - recorder = _recorder(record_witnessed_run=fake) +async def test_run_witness_recorder_truncates_stale_run_and_repromotes_on_a_second_begun() -> None: + """A second BEGUN for a code that is already open means the previous + terminal was missed: truncate the stale Run (interrupted_at=None, + the moment it actually ended is unknown), then promote a new one.""" + stale_run_id = uuid4() + fresh_run_id = uuid4() + genesis = _FakeRecordWitnessedRun(run_id=stale_run_id) + truncate = _FakeTruncateRun() + recorder = _recorder(record_witnessed_run=genesis, truncate_run=truncate) begun = _obs(reported_status="Beginning scan", phase=CapturePhase.BEGUN) await recorder.observe_capture(begun) + + genesis.run_id = fresh_run_id await recorder.observe_capture(begun) - assert len(fake.calls) == 1 + assert len(genesis.calls) == 2 + assert len(truncate.calls) == 1 + truncate_command = truncate.calls[0] + assert truncate_command.run_id == stale_run_id + assert truncate_command.interrupted_at is None + + +@pytest.mark.unit +async def test_run_witness_recorder_promotes_even_when_the_stale_truncate_fails() -> None: + """The new capture is a real fact regardless of whether the stale Run + could be closed: a truncate failure must not block the promotion.""" + genesis = _FakeRecordWitnessedRun() + truncate = _FakeTruncateRun(raises=RuntimeError("Run already terminal")) + recorder = _recorder(record_witnessed_run=genesis, truncate_run=truncate) + + begun = _obs(reported_status="Beginning scan", phase=CapturePhase.BEGUN) + await recorder.observe_capture(begun) + await recorder.observe_capture(begun) + + assert len(genesis.calls) == 2 + assert len(truncate.calls) == 1 @pytest.mark.unit @@ -313,49 +394,116 @@ async def test_run_witness_recorder_logs_a_distinct_event_on_unauthorized() -> N @pytest.mark.unit -async def test_run_witness_recorder_clears_on_ended_while_open() -> None: +async def test_run_witness_recorder_records_ended_outcome_while_open() -> None: run_id = uuid4() - fake = _FakeRecordWitnessedRun(run_id=run_id) - recorder = _recorder(record_witnessed_run=fake, open_captures={_CODE: run_id}) + genesis = _FakeRecordWitnessedRun(run_id=run_id) + outcome = _FakeRecordWitnessedRunOutcome() + recorder = _recorder( + record_witnessed_run=genesis, + record_witnessed_run_outcome=outcome, + open_captures={_CODE: run_id}, + ) await recorder.observe_capture(_obs(reported_status="Scan complete", phase=CapturePhase.ENDED)) + assert len(outcome.calls) == 1 + command = outcome.calls[0] + assert command.run_id == run_id + assert command.capture_code == _CODE + assert command.observed_phase is CapturePhase.ENDED + assert command.observed_at == _NOW + assert command.trigger == "Monitor" + assert command.monitor_source_id == RUN_WITNESS_MONITOR_SOURCE_ID + # Reopening after the close promotes again: proves the entry was # actually cleared, not merely left stale. await recorder.observe_capture(_obs(reported_status="Beginning scan", phase=CapturePhase.BEGUN)) - assert len(fake.calls) == 1 + assert len(genesis.calls) == 1 @pytest.mark.unit -async def test_run_witness_recorder_clears_on_aborted_while_open() -> None: +async def test_run_witness_recorder_records_aborted_outcome_while_open() -> None: run_id = uuid4() - fake = _FakeRecordWitnessedRun(run_id=run_id) - recorder = _recorder(record_witnessed_run=fake, open_captures={_CODE: run_id}) + genesis = _FakeRecordWitnessedRun(run_id=run_id) + outcome = _FakeRecordWitnessedRunOutcome() + recorder = _recorder( + record_witnessed_run=genesis, + record_witnessed_run_outcome=outcome, + open_captures={_CODE: run_id}, + ) await recorder.observe_capture(_obs(reported_status="Scan aborted", phase=CapturePhase.ABORTED)) + assert outcome.calls[0].observed_phase is CapturePhase.ABORTED + await recorder.observe_capture(_obs(reported_status="Beginning scan", phase=CapturePhase.BEGUN)) + assert len(genesis.calls) == 1 - assert len(fake.calls) == 1 + +@pytest.mark.unit +async def test_run_witness_recorder_leaves_entry_open_after_a_failed_outcome() -> None: + """A failed outcome write leaves the entry open; the next BEGUN + truncates it (recovering via the same path as a missed terminal) + rather than the failure being silently swallowed.""" + run_id = uuid4() + genesis = _FakeRecordWitnessedRun() + outcome = _FakeRecordWitnessedRunOutcome(raises=RuntimeError("append failed")) + truncate = _FakeTruncateRun() + recorder = _recorder( + record_witnessed_run=genesis, + record_witnessed_run_outcome=outcome, + truncate_run=truncate, + open_captures={_CODE: run_id}, + ) + + await recorder.observe_capture(_obs(reported_status="Scan complete", phase=CapturePhase.ENDED)) + await recorder.observe_capture(_obs(reported_status="Beginning scan", phase=CapturePhase.BEGUN)) + + assert len(truncate.calls) == 1 + assert truncate.calls[0].run_id == run_id + assert len(genesis.calls) == 1 + + +@pytest.mark.unit +async def test_run_witness_recorder_logs_a_distinct_event_on_outcome_unauthorized() -> None: + run_id = uuid4() + outcome = _FakeRecordWitnessedRunOutcome(raises=UnauthorizedError("not granted")) + recorder = _recorder( + record_witnessed_run=_FakeRecordWitnessedRun(), + record_witnessed_run_outcome=outcome, + open_captures={_CODE: run_id}, + ) + + with structlog.testing.capture_logs() as logs: + await recorder.observe_capture( + _obs(reported_status="Scan complete", phase=CapturePhase.ENDED) + ) + + events = [entry["event"] for entry in logs] + assert "run_witness.outcome_unauthorized" in events @pytest.mark.unit async def test_run_witness_recorder_noop_on_ended_while_idle() -> None: - fake = _FakeRecordWitnessedRun() - recorder = _recorder(record_witnessed_run=fake) + genesis = _FakeRecordWitnessedRun() + outcome = _FakeRecordWitnessedRunOutcome() + recorder = _recorder(record_witnessed_run=genesis, record_witnessed_run_outcome=outcome) await recorder.observe_capture(_obs(reported_status="Scan complete", phase=CapturePhase.ENDED)) - assert fake.calls == [] + assert genesis.calls == [] + assert outcome.calls == [] @pytest.mark.unit async def test_run_witness_recorder_noop_on_aborted_while_idle() -> None: - fake = _FakeRecordWitnessedRun() - recorder = _recorder(record_witnessed_run=fake) + genesis = _FakeRecordWitnessedRun() + outcome = _FakeRecordWitnessedRunOutcome() + recorder = _recorder(record_witnessed_run=genesis, record_witnessed_run_outcome=outcome) await recorder.observe_capture(_obs(reported_status="Scan aborted", phase=CapturePhase.ABORTED)) - assert fake.calls == [] + assert genesis.calls == [] + assert outcome.calls == [] @pytest.mark.unit @@ -424,21 +572,35 @@ async def test_run_witness_recorder_is_a_pass_through_when_recording_disabled() @pytest.mark.unit async def test_run_witness_lifespan_seeds_open_captures_from_the_supplied_map() -> None: + """A code seeded as open at construction reads OPEN: a BEGUN for it + goes through the truncate-then-promote recovery path rather than a + blind idle-promote, proving the supplied map was actually consulted.""" run_id = uuid4() - fake = _FakeRecordWitnessedRun() + genesis = _FakeRecordWitnessedRun() + truncate = _FakeTruncateRun() observer = _FakeObserver([_obs(reported_status="Beginning scan", phase=CapturePhase.BEGUN)]) - deps = build_deps() + deps = dataclasses.replace( + build_deps(ids=[uuid4() for _ in range(10)]), + settings=Settings( # type: ignore[call-arg] + run_witness_recording_enabled=True, + capture_watch_plan_id=_PLAN_ID, + ), + ) async with run_witness_lifespan( observer=observer, capture_codes=frozenset({_CODE}), deps=deps, - record_witnessed_run=fake, + record_witnessed_run=genesis, + record_witnessed_run_outcome=_FakeRecordWitnessedRunOutcome(), + truncate_run=truncate, open_captures={_CODE: run_id}, ): await asyncio.sleep(0.02) - assert fake.calls == [] + assert len(truncate.calls) == 1 + assert truncate.calls[0].run_id == run_id + assert len(genesis.calls) == 1 @pytest.mark.unit From 7ea2e58fe979abe66ed48c940165badca2d0ae94 Mon Sep 17 00:00:00 2001 From: xmap <16776958+xmap@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:23:10 -0500 Subject: [PATCH 4/6] api: read the abort PV so Aborted is reachable and Completed is honest 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. --- apps/api/src/cora/api/_capture_observer.py | 123 ++++++++++++++-- .../tests/unit/api/test_capture_observer.py | 136 ++++++++++++++++++ 2 files changed, 249 insertions(+), 10 deletions(-) diff --git a/apps/api/src/cora/api/_capture_observer.py b/apps/api/src/cora/api/_capture_observer.py index 8e87451390b..82521c89586 100644 --- a/apps/api/src/cora/api/_capture_observer.py +++ b/apps/api/src/cora/api/_capture_observer.py @@ -52,6 +52,40 @@ _SOURCE_KIND = "EpicsPv" _STATUS_ROLE = "status" +_ABORT_ROLE = "abort" + +# Conventional EPICS binary state labels, mirroring +# `_enclosure_permit_observer._PERMITTED_LABELS` / `_NOT_PERMITTED_LABELS` +# exactly: a DBR_ENUM reading through `EpicsCaControlPort` reaches this +# module as its resolved label, never as its index (2-BM's `AbortScan` +# is confirmed live as one such ENUM, resolving to `'No'`), so the label +# is the only thing left to compare against. `CaprotoControlPort`, by +# contrast, leaves the raw integer unresolved; the `int(value)` fallback +# below covers that shape too. +_ASSERTED_LABELS = frozenset({"1", "ON", "TRUE", "YES"}) +_CLEAR_LABELS = frozenset({"0", "OFF", "FALSE", "NO"}) + + +def _binary_code(value: object) -> int | None: + """Resolve a binary-role reading to 1 / 0, or None when it is neither. + + Unrecognized resolves to `None`, never a guess: same fail-toward- + silence posture as `_enclosure_permit_observer._binary_code`, whose + docstring documents the production incident (`int('ON')` raising) + that made string-label matching necessary in the first place. + """ + if isinstance(value, str): + token = value.strip().upper() + if token in _ASSERTED_LABELS: + return 1 + if token in _CLEAR_LABELS: + return 0 + return None + try: + code = int(value) # type: ignore[call-overload] + except (TypeError, ValueError): + return None + return code if code in (0, 1) else None def classify_capture_status(reported_status: str, status_phases: Mapping[str, str]) -> CapturePhase: @@ -81,17 +115,23 @@ class _PumpDone: class ControlPortCaptureObserver: - """`CaptureObserver` over a `ControlPort` (one `status` PV per capture code). + """`CaptureObserver` over a `ControlPort` (`status` + optional `abort` PV + per capture code). `capture_pvs` is code -> role -> PV, matching - `Settings.capture_watch_pvs`. Only the `status` role is subscribed - in this slice; a code whose PV set has no `status` entry cannot be - watched and is silently excluded from scope, mirroring the - Enclosure adapter's `if code in self._permit_pvs` filter. The other - declared roles (`server_running`, `abort`, `images_saved`, - `images_collected`) are read by a later slice; declaring them now - costs nothing and lets a deployment's config stabilize ahead of the - code that consumes it. + `Settings.capture_watch_pvs`. The `status` role is required; a code + whose PV set has no `status` entry cannot be watched and is + silently excluded from scope, mirroring the Enclosure adapter's + `if code in self._permit_pvs` filter. The `abort` role is optional + per code: when declared, a truthy reading on it is a direct + `ABORTED` phase claim, letting a real abort be distinguished from a + successful end even where the `status` PV alone cannot (2-BM's + `fly_scan` writes the identical `'Scan complete'` literal on both). + A code with no `abort` entry watches `status` only, exactly as + before this role existed. The remaining declared roles + (`server_running`, `images_saved`, `images_collected`) are read by + a later slice; declaring them now costs nothing and lets a + deployment's config stabilize ahead of the code that consumes it. """ def __init__( @@ -108,6 +148,9 @@ def __init__( for code, roles in capture_pvs.items() if _STATUS_ROLE in roles } + self._abort_pvs = { + code: roles[_ABORT_ROLE] for code, roles in capture_pvs.items() if _ABORT_ROLE in roles + } self._status_phases = dict(status_phases) self._tick_seconds = tick_seconds @@ -122,8 +165,15 @@ async def _drain(self, scope: CaptureObserverScope) -> AsyncGenerator[CaptureObs ] if not pvs: return + abort_pvs = [ + (code, self._abort_pvs[code]) + for code in sorted(scope.capture_codes) + if code in self._abort_pvs + ] queue: asyncio.Queue[CaptureObservation | _PumpDone] = asyncio.Queue() - pump_tasks = [asyncio.create_task(self._pump(code, pv, queue)) for code, pv in pvs] + pump_tasks = [asyncio.create_task(self._pump(code, pv, queue)) for code, pv in pvs] + [ + asyncio.create_task(self._pump_abort(code, pv, queue)) for code, pv in abort_pvs + ] poll_tasks = ( [asyncio.create_task(self._poll(code, pv, queue)) for code, pv in pvs] if self._tick_seconds is not None @@ -171,6 +221,29 @@ async def _pump( finally: queue.put_nowait(_PUMP_DONE) + async def _pump_abort( + self, + code: str, + pv: str, + queue: asyncio.Queue[CaptureObservation | _PumpDone], + ) -> None: + """Sibling pump for the optional `abort` role. + + Unlike `_pump`, not every reading is enqueued: a falsy value (the + busy record's idle/reset state) makes no phase claim at all and + must not be pushed as a no-op observation, per `_from_abort_reading`. + """ + try: + async for reading in self._control_port.subscribe(pv): + observation = self._from_abort_reading(code, pv, reading) + if observation is not None: + queue.put_nowait(observation) + queue.put_nowait(self._unreached(code, pv)) + except ControlNotConnectedError: + queue.put_nowait(self._unreached(code, pv)) + finally: + queue.put_nowait(_PUMP_DONE) + async def _poll( self, code: str, @@ -205,6 +278,36 @@ def _from_reading(self, code: str, pv: str, reading: Measurement) -> CaptureObse source_id=pv, ) + def _from_abort_reading( + self, code: str, pv: str, reading: Measurement + ) -> CaptureObservation | None: + """An asserted abort-role reading is a direct `ABORTED` claim. + + NOT Python truthiness: 2-BM's `AbortScan` is a DBR_ENUM that + resolves to the label `'No'` when idle, and `bool('No')` is + `True`. `_binary_code` decodes the conventional EPICS binary + labels (or a raw 0/1 index) instead, so "No" correctly resolves + to clear, not asserted. + + A clear or unresolvable reading makes no phase claim: it is not + "the capture is not aborted" so much as "nothing happened on + this PV" (or a label this cannot decode), and reporting it as a + phase would let a stale idle read silently arrive between a + real `Begun` and its `status` transitions. `None` return means + the caller enqueues nothing. + """ + if _binary_code(reading.value) != 1: + return None + return CaptureObservation( + capture_code=code, + reported_status=str(reading.value), + phase=CapturePhase.ABORTED, + reach_tier=ReachTier.RELAYED, + observed_at=reading.produced_at, + source_kind=_SOURCE_KIND, + source_id=pv, + ) + def _probe_only(self, code: str, pv: str, reach_tier: ReachTier) -> CaptureObservation: """A poll tick's result: reach evidence with no status claim.""" return CaptureObservation( diff --git a/apps/api/tests/unit/api/test_capture_observer.py b/apps/api/tests/unit/api/test_capture_observer.py index ea48cef7c1a..726457da85f 100644 --- a/apps/api/tests/unit/api/test_capture_observer.py +++ b/apps/api/tests/unit/api/test_capture_observer.py @@ -234,6 +234,142 @@ async def test_observe_merges_multiple_codes() -> None: } +# ---------- Abort role ---------- + + +@pytest.mark.unit +async def test_observe_a_code_with_no_abort_role_watches_status_only() -> None: + """A code missing the `abort` entry behaves exactly as before this + role existed: no abort pump is spawned, no extra observation.""" + port = _ScriptedControlPort(readings={"pvA": [_reading("Beginning scan")]}) + observer = _observer(port, {"tomoscan": {"status": "pvA"}}) + + observations = await _collect(observer, {"tomoscan"}) + + assert [(o.capture_code, o.phase) for o in observations] == [ + ("tomoscan", CapturePhase.BEGUN), + ("tomoscan", None), # status pump's clean stream end + ] + + +@pytest.mark.unit +async def test_observe_a_truthy_abort_reading_is_a_direct_aborted_claim() -> None: + port = _ScriptedControlPort( + readings={"pvA": [_reading("Collecting projections")], "pvAbort": [_reading(1)]} + ) + observer = _observer(port, {"tomoscan": {"status": "pvA", "abort": "pvAbort"}}) + + observations = await _collect(observer, {"tomoscan"}) + + aborted = [o for o in observations if o.phase is CapturePhase.ABORTED] + assert len(aborted) == 1 + assert aborted[0].capture_code == "tomoscan" + assert aborted[0].reported_status == "1" + assert aborted[0].reach_tier is ReachTier.RELAYED + assert aborted[0].source_id == "pvAbort" + assert aborted[0].observed_at == _T + + +@pytest.mark.unit +async def test_observe_a_falsy_abort_reading_emits_nothing() -> None: + """The busy record's idle/reset value between scans makes no phase + claim at all: it must not be enqueued as a no-op observation. The + stream then ends cleanly, which still yields its own no-status + observation (same shape as `_pump`'s clean end) -- what must NOT + appear is an `ABORTED` claim from the falsy reading itself.""" + port = _ScriptedControlPort( + readings={"pvA": [_reading("Beginning scan")], "pvAbort": [_reading(0)]} + ) + observer = _observer(port, {"tomoscan": {"status": "pvA", "abort": "pvAbort"}}) + + observations = await _collect(observer, {"tomoscan"}) + + assert not any(o.phase is CapturePhase.ABORTED for o in observations) + + +@pytest.mark.unit +@pytest.mark.parametrize("clear_label", ["No", "no", "OFF", "False", "0"]) +async def test_observe_a_clear_enum_label_reading_is_not_a_python_truthiness_trap( + clear_label: str, +) -> None: + """Regression: 2-BM's real `AbortScan` is a DBR_ENUM that resolves + through the aioca adapter to the label `'No'` when idle, and + `bool('No')` is `True` in Python. A naive truthiness check on + `reading.value` would misclassify every idle reading as an abort; + `_binary_code` must decode the label instead.""" + port = _ScriptedControlPort( + readings={"pvA": [_reading("Beginning scan")], "pvAbort": [_reading(clear_label)]} + ) + observer = _observer(port, {"tomoscan": {"status": "pvA", "abort": "pvAbort"}}) + + observations = await _collect(observer, {"tomoscan"}) + + assert not any(o.phase is CapturePhase.ABORTED for o in observations) + + +@pytest.mark.unit +@pytest.mark.parametrize("asserted_label", ["Yes", "yes", "ON", "True", "1"]) +async def test_observe_an_asserted_enum_label_reading_is_aborted(asserted_label: str) -> None: + port = _ScriptedControlPort( + readings={ + "pvA": [_reading("Collecting projections")], + "pvAbort": [_reading(asserted_label)], + } + ) + observer = _observer(port, {"tomoscan": {"status": "pvA", "abort": "pvAbort"}}) + + observations = await _collect(observer, {"tomoscan"}) + + assert any(o.phase is CapturePhase.ABORTED for o in observations) + + +@pytest.mark.unit +async def test_observe_an_unrecognized_abort_label_makes_no_claim() -> None: + """A label this cannot decode fails toward silence, not toward a + guessed ABORTED claim, mirroring `_enclosure_permit_observer`'s + fail-closed posture for its own binary-label decode.""" + port = _ScriptedControlPort( + readings={"pvA": [_reading("Beginning scan")], "pvAbort": [_reading("MAYBE")]} + ) + observer = _observer(port, {"tomoscan": {"status": "pvA", "abort": "pvAbort"}}) + + observations = await _collect(observer, {"tomoscan"}) + + assert not any(o.phase is CapturePhase.ABORTED for o in observations) + + +@pytest.mark.unit +async def test_observe_abort_pump_disconnect_yields_a_no_status_observation() -> None: + port = _ScriptedControlPort( + readings={"pvA": [_reading("Beginning scan")], "pvAbort": []}, + disconnect=frozenset({"pvAbort"}), + ) + observer = _observer(port, {"tomoscan": {"status": "pvA", "abort": "pvAbort"}}) + + observations = await _collect(observer, {"tomoscan"}) + + abort_source_obs = [o for o in observations if o.source_id == "pvAbort"] + assert len(abort_source_obs) == 1 + assert abort_source_obs[0].phase is None + assert abort_source_obs[0].reach_tier is ReachTier.UNREACHED + + +@pytest.mark.unit +async def test_observe_merges_status_and_abort_readings_for_one_code() -> None: + port = _ScriptedControlPort( + readings={ + "pvA": [_reading("Beginning scan")], + "pvAbort": [_reading(1)], + } + ) + observer = _observer(port, {"tomoscan": {"status": "pvA", "abort": "pvAbort"}}) + + observations = await _collect(observer, {"tomoscan"}) + + phases = {o.phase for o in observations if o.phase is not None} + assert phases == {CapturePhase.BEGUN, CapturePhase.ABORTED} + + async def _collect_until( gen: AsyncGenerator[CaptureObservation], predicate: Callable[[list[CaptureObservation]], bool], From a665e6bb38d59a0ef2ca32ba76349925c3ca77ad Mon Sep 17 00:00:00 2001 From: xmap <16776958+xmap@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:29:10 -0500 Subject: [PATCH 5/6] api: update the abort/success gap docstring now that the pump reads it 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. --- apps/api/src/cora/api/_run_witness.py | 34 ++++++++++++++++----------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/apps/api/src/cora/api/_run_witness.py b/apps/api/src/cora/api/_run_witness.py index e5ea7dec612..b776c41f9b0 100644 --- a/apps/api/src/cora/api/_run_witness.py +++ b/apps/api/src/cora/api/_run_witness.py @@ -65,22 +65,28 @@ `external_refs`, so a still-open capture at process restart is never re-promoted. -## Known gap as of this commit: `ENDED` does not yet distinguish abort from success - -`CaptureObservation.phase` classifies purely off the substrate's status -literal (see `ControlPortCaptureObserver`), and as of this commit that -adapter reads only the `status` PV role, not the deployment's `abort` -role. At 2-BM, `fly_scan()`'s exception handlers for `ScanAbortError` / -`CameraTimeoutError` / `FileOverwriteError` still run -`finally: self.end_scan()`, which writes the identical -`'Scan complete'` literal a genuine success writes. So until the abort -PV is read (tracked as the very next commit in this same effort), -`ENDED` unconditionally maps to `RunCompleted` here, which would -misrecord a real 2-BM abort as a success the moment -`run_witness_recording_enabled` is turned on. Nothing in this file or +## Closing the abort/success gap needs a deployment change too + +`CaptureObservation.phase` classifies the `status` role's literal off +the deployment's declared table, and separately, `ControlPortCaptureObserver` +now also reads an optional `abort` role: a decoded-asserted reading on +it is a direct `ABORTED` claim (see that module's docstring), landing +here as a terminal `_record_outcome` call ahead of whatever the +`status` PV says next. At 2-BM, `fly_scan()`'s exception handlers for +`ScanAbortError` / `CameraTimeoutError` / `FileOverwriteError` still +run `finally: self.end_scan()`, which writes the identical +`'Scan complete'` literal a genuine success writes, so the `abort` role +is the only thing that can tell the two apart there. + +The code capability exists as of this commit; the gap only closes once +a deployment's `capture_watch_pvs` also declares the `abort` role for +each code (2-BM: `"abort": "2bmb:TomoScan:AbortScan"`). A code with no +`abort` entry watches `status` only, unchanged, so `ENDED` still +unconditionally maps to `RunCompleted` for it. Nothing in this file or in `Settings` gates recording on the abort role being configured; the locked deployment decision is to keep `run_witness_recording_enabled` -off at 2-BM until both this slice and the abort-PV wiring are live. +off at 2-BM until both this effort's code and its own deployment +config change (adding the `abort` role) are live. ## Accepted residual: a reconnect can misread a still-open capture as new From 609b8c6bceeabc84be7e835c13d9e39fe4c29850 Mon Sep 17 00:00:00 2001 From: xmap <16776958+xmap@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:42:08 -0500 Subject: [PATCH 6/6] api: add end-to-end replay tests against the real arcturus sequence 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. --- apps/api/src/cora/api/_run_witness.py | 21 ++ .../api/test_run_witness_capture_replay.py | 254 ++++++++++++++++++ 2 files changed, 275 insertions(+) create mode 100644 apps/api/tests/unit/api/test_run_witness_capture_replay.py diff --git a/apps/api/src/cora/api/_run_witness.py b/apps/api/src/cora/api/_run_witness.py index b776c41f9b0..2b438cf2a80 100644 --- a/apps/api/src/cora/api/_run_witness.py +++ b/apps/api/src/cora/api/_run_witness.py @@ -107,6 +107,27 @@ last-observed reading, or `reach_tier`) is implemented; revisit if this is ever observed in practice. +## Accepted residual: the `status` and `abort` pumps have no enforced ordering + +`_capture_observer.py` runs the `status` and `abort` roles as two +independent pumps feeding one merged queue; nothing in this file or +that one enforces that an `ABORTED` reading is processed before a +later, causally-dependent `ENDED` reading from the other pump, only +that it usually will be, because 2-BM's own `abort_scan()` writes +`AbortScan` before its caller's `finally: end_scan()` writes +`ScanStatus`. This is a real ordering dependency on realistic, +network-driven CA delivery interleaving the two subscriptions fairly, +not a structural guarantee; a deliberately adversarial or bursty +delivery pattern (confirmed by constructing exactly this case against +a fake `ControlPort` that does not yield between readings, in +`test_run_witness_capture_replay.py`) could let the trailing `ENDED` +arrive first, in which case that capture records as `Completed` and +the correct `ABORTED` observation lands on the now-idle no-open-Run +path, a no-op. Same outcome, same severity, as the coalesced-abort +residual in `_capture_observer.py`'s own docstring: a real 2-BM abort +degrading to a `Completed` record, never a corrupted attribution to a +different Run. + ## Retry + resilience Mirrors `run_enclosure_permit_monitor`: `observe()` ending (stream diff --git a/apps/api/tests/unit/api/test_run_witness_capture_replay.py b/apps/api/tests/unit/api/test_run_witness_capture_replay.py new file mode 100644 index 00000000000..b4955fdbd9b --- /dev/null +++ b/apps/api/tests/unit/api/test_run_witness_capture_replay.py @@ -0,0 +1,254 @@ +"""End-to-end replay tests: real `ControlPortCaptureObserver` classification +feeding a real `RunWitnessRecorder`, driven by the actual literal sequence +measured on arcturus (2026-08-14, 431 real captures) rather than +pre-classified phases. + +Unlike `test_capture_observer.py` (adapter alone) and `test_run_witness.py` +(recorder alone, fed pre-built `CaptureObservation`s), this file wires both +together so the deployment's real `CAPTURE_STATUS_PHASES` table and the +recorder's dedup/terminal/truncate state machine are exercised as one +pipeline, the way `run_witness_loop` actually runs them. +""" + +import asyncio +import contextlib +from uuid import UUID, uuid4 + +import pytest + +from cora.api._capture_observer import ControlPortCaptureObserver +from cora.api._run_witness import RunWitnessRecorder, run_witness_loop +from cora.infrastructure.config import Settings +from cora.infrastructure.routing import NIL_SENTINEL_ID +from cora.operation.ports.control_port import Measurement +from cora.run.features.record_witnessed_run.command import RecordWitnessedRun +from cora.run.features.record_witnessed_run_outcome.command import RecordWitnessedRunOutcome +from cora.run.features.truncate_run.command import TruncateRun +from tests.unit._helpers import build_deps + +_CODE = "2bmb-tomoscan" +_STATUS_PV = "2bmb:TomoScan:ScanStatus" +_ABORT_PV = "2bmb:TomoScan:AbortScan" +_PLAN_ID = UUID("01900000-0000-7000-8000-000000007107") + +# arcturus's live CAPTURE_STATUS_PHASES, including the "Programming PSO" +# fix shipped alongside this effort's deploy. +_PHASES = { + "Beginning scan": "Begun", + "Waiting for overwrite confirmation": "Progressing", + "Moving rotation axis to start": "Progressing", + "Programming PSO": "Progressing", + "Collecting dark fields": "Progressing", + "Collecting flat fields": "Progressing", + "Collecting projections": "Progressing", + "fdt file transfer complete": "Progressing", + "scp file transfer complete": "Progressing", + "Scan complete": "Ended", +} + +# One real fly-scan cycle's literal sequence, in order, per the arcturus +# log (2026-08-14T22:30:34Z onward): Beginning scan -> Programming PSO -> +# Moving rotation axis to start -> dark -> flat -> projections -> fdt +# transfer -> Scan complete. +_HAPPY_CYCLE = ( + "Beginning scan", + "Programming PSO", + "Moving rotation axis to start", + "Collecting dark fields", + "Collecting flat fields", + "Collecting projections", + "fdt file transfer complete", + "Scan complete", +) + + +class _ScriptedPort: + """Minimal fake `ControlPort`: replays a fixed reading list per address. + + Yields control back to the event loop between readings (real EPICS + CA delivery is network-driven and naturally interleaves independent + subscriptions this way); without it, one address's whole script + would race ahead of a sibling address's pump before the loop ever + gets a chance to schedule it, which is a fake-port artifact, not a + real-deployment ordering guarantee this test should rely on. + """ + + def __init__(self, readings: dict[str, list[Measurement]]) -> None: + self._readings = readings + + async def subscribe(self, address: str): + for reading in self._readings.get(address, []): + await asyncio.sleep(0) + yield reading + + +def _reading(value: str) -> Measurement: + return Measurement(value=value, kind="Categorical", quality="Good", produced_at=None) # type: ignore[arg-type] + + +class _FakeGenesis: + """Fake `record_witnessed_run` handler: records every call, returns a + fresh run_id each time.""" + + def __init__(self) -> None: + self.calls: list[RecordWitnessedRun] = [] + + async def __call__( + self, + command: RecordWitnessedRun, + *, + principal_id: UUID, + correlation_id: UUID, + causation_id: UUID | None = None, + surface_id: UUID = NIL_SENTINEL_ID, + ) -> UUID: + self.calls.append(command) + return uuid4() + + +class _FakeOutcome: + """Fake `record_witnessed_run_outcome` handler: records every call.""" + + def __init__(self) -> None: + self.calls: list[RecordWitnessedRunOutcome] = [] + + async def __call__( + self, + command: RecordWitnessedRunOutcome, + *, + principal_id: UUID, + correlation_id: UUID, + causation_id: UUID | None = None, + surface_id: UUID = NIL_SENTINEL_ID, + ) -> None: + self.calls.append(command) + + +class _FakeTruncate: + """Fake `truncate_run` handler: records every call.""" + + def __init__(self) -> None: + self.calls: list[TruncateRun] = [] + + async def __call__( + self, + command: TruncateRun, + *, + principal_id: UUID, + correlation_id: UUID, + causation_id: UUID | None = None, + surface_id: UUID = NIL_SENTINEL_ID, + ) -> None: + self.calls.append(command) + + +def _recorder( + *, genesis: _FakeGenesis, outcome: _FakeOutcome, truncate: _FakeTruncate +) -> RunWitnessRecorder: + settings = Settings( # type: ignore[call-arg] + run_witness_recording_enabled=True, + capture_watch_plan_id=_PLAN_ID, + ) + return RunWitnessRecorder( + deps=build_deps(ids=[uuid4() for _ in range(200)]), + record_witnessed_run=genesis, + record_witnessed_run_outcome=outcome, + truncate_run=truncate, + settings=settings, + ) + + +async def _run_loop_over( + port: _ScriptedPort, recorder: RunWitnessRecorder, *, settle_seconds: float = 0.05 +) -> None: + observer = ControlPortCaptureObserver( + control_port=port, # type: ignore[arg-type] + capture_pvs={_CODE: {"status": _STATUS_PV, "abort": _ABORT_PV}}, + status_phases=_PHASES, + ) + task = asyncio.create_task( + run_witness_loop(observer=observer, capture_codes=frozenset({_CODE}), recorder=recorder) + ) + await asyncio.sleep(settle_seconds) + task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await task + + +@pytest.mark.unit +async def test_replay_three_clean_cycles_promotes_and_completes_each_exactly_once() -> None: + """Three back-to-back real fly-scan cycles: exactly 3 Runs promoted, + all 3 closed as Completed via the real Ended classification, zero + truncates (no missed terminal in a clean sequence), and the `fdt` + transfer-start literal never terminates anything along the way.""" + literals = _HAPPY_CYCLE * 3 + port = _ScriptedPort({_STATUS_PV: [_reading(v) for v in literals]}) + genesis = _FakeGenesis() + outcome = _FakeOutcome() + truncate = _FakeTruncate() + recorder = _recorder(genesis=genesis, outcome=outcome, truncate=truncate) + + await _run_loop_over(port, recorder) + + assert len(genesis.calls) == 3 + assert len(outcome.calls) == 3 + assert all(call.observed_phase.value == "Ended" for call in outcome.calls) + assert truncate.calls == [] + + +@pytest.mark.unit +async def test_replay_a_real_abort_edge_closes_as_aborted_not_completed() -> None: + """A real AbortScan assertion closes the capture as Aborted before + the trailing 'Scan complete' the exception handler's `finally` + block still writes; that trailing literal must land as a no-op on + the now-idle capture, not a second outcome call.""" + port = _ScriptedPort( + { + _STATUS_PV: [ + _reading("Beginning scan"), + _reading("Programming PSO"), + _reading("Collecting projections"), + _reading("fdt file transfer complete"), + _reading("Scan complete"), + ], + _ABORT_PV: [_reading("Yes")], + } + ) + genesis = _FakeGenesis() + outcome = _FakeOutcome() + truncate = _FakeTruncate() + recorder = _recorder(genesis=genesis, outcome=outcome, truncate=truncate) + + await _run_loop_over(port, recorder) + + assert len(genesis.calls) == 1 + assert len(outcome.calls) == 1 + assert outcome.calls[0].observed_phase.value == "Aborted" + + +@pytest.mark.unit +async def test_replay_a_missed_terminal_is_recovered_by_the_next_begun() -> None: + """The status pump's terminal literal is dropped (models a CA + transition loss); the next cycle's Beginning scan truncates the + stale Run before promoting a fresh one.""" + literals = [ + "Beginning scan", + "Programming PSO", + "Collecting projections", + # 'Scan complete' dropped here. + "Beginning scan", + "Programming PSO", + "Collecting projections", + "Scan complete", + ] + port = _ScriptedPort({_STATUS_PV: [_reading(v) for v in literals]}) + genesis = _FakeGenesis() + outcome = _FakeOutcome() + truncate = _FakeTruncate() + recorder = _recorder(genesis=genesis, outcome=outcome, truncate=truncate) + + await _run_loop_over(port, recorder) + + assert len(genesis.calls) == 2 + assert len(truncate.calls) == 1 + assert len(outcome.calls) == 1