From 35fbf7616c434bcc6f0b2c6882beacd64b822b30 Mon Sep 17 00:00:00 2001 From: Ouroboros <311266734+ouroboros-agent@users.noreply.github.com> Date: Mon, 21 Sep 2026 12:21:07 +0300 Subject: [PATCH] Show since when a reviewer has been awaited, only where the host knows it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 2026-09-19 a task sat parked behind a reviewer for 7 h 16 min and the task card gave no hint for how long. PR #1159 made the wait itself visible — a plan wave or an acceptance panel lists an unanswered reviewer as `Awaiting answer: · ` — but never said WHEN the host sent that reviewer its request. The rule is that a time is never invented, inferred or back-filled. A row whose send moment the host did not record keeps exactly the text it has today. The moment is recorded once, at the source: `ActiveReviewAttempt` gains `started_at`, stamped with `utc_now_iso()` only where this process creates the entry for a NEW physical operation. The free replay of an already settled answer is not stamped, and neither is an `exact_recovery` rejoin: that operation was sent by an EARLIER process and its send moment is persisted nowhere, so this process honestly has nothing to show. `ReviewActorRecord` gains `awaiting_since`, set from the entry onto the two rows that wait — the `pending_dispatch` row released at the dispatch barrier and the `in_flight` row whose window expired. A settled or answered row never carries it. The plan row and the durable wave actor record carry it forward beside the other custody facts, so it reaches `wave.actors`; `_review_actor_projection` publishes it for `panel.actors` only when one of the two existing wait predicates still calls the row unanswered AND the stored value parses as an instant — otherwise the key is absent. The browser appends ` · since HH:MM` in the viewer's local 24-hour clock, with the short local date when the wait began on an earlier day so `since 23:50` cannot be misread as tonight, and renders nothing at all for an absent or unparseable value. --- docs/DESIGN.md | 4 + ouroboros/review_custody.py | 11 +- ouroboros/review_projection.py | 14 +- ouroboros/review_records.py | 5 + ouroboros/tools/plan_review_runtime.py | 3 + tests/test_review_awaiting_since.py | 281 +++++++++++++++++++++++++ web/modules/review_presentation.js | 8 +- web/modules/utils.js | 14 ++ web/tests/review_plan_custody.test.js | 114 ++++++++++ 9 files changed, 448 insertions(+), 6 deletions(-) create mode 100644 tests/test_review_awaiting_since.py diff --git a/docs/DESIGN.md b/docs/DESIGN.md index ee3bcd042..e2473dc14 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -543,6 +543,10 @@ not child-task cards and never prove execution by themselves. nor awaited (a settled failure, an expired window, lost custody, a refusal) adds `· m unavailable` and keeps the warning tone beside the awaited slots; a panel with no awaited slot keeps the warning tone and its `DEGRADED` verdict. +- An awaited or unresolved reviewer row adds `· since HH:MM` in the viewer's + local 24-hour clock, prefixed with the short date when the wait began on an + earlier day, only where the host recorded the moment it sent that reviewer's + request; a time is never inferred. - A panel that settled after its task ended stays one attempt row of its group, labelled as settled after the task ended; its note (which verdict, which revision, whether a reviewer's outcome is still unknown) is host-composed and diff --git a/ouroboros/review_custody.py b/ouroboros/review_custody.py index ed5c15af1..d8045777f 100644 --- a/ouroboros/review_custody.py +++ b/ouroboros/review_custody.py @@ -28,7 +28,7 @@ from ouroboros.review_dispatch import slot_id_for_row from ouroboros.usage_accounting import ( PHYSICAL_ATTEMPT_STATES, POSITIVE_PHYSICAL_ATTEMPT_STATES, ) -from ouroboros.utils import emit_cognitive_operation_event +from ouroboros.utils import emit_cognitive_operation_event, utc_now_iso log = logging.getLogger("review_custody") @@ -45,6 +45,9 @@ class ActiveReviewAttempt: retry_state: Dict[str, Any] = field(default_factory=dict) pending_invocation_checkpoint: Callable[[str], None] | None = None recovery_binding: Dict[str, Any] = field(default_factory=dict) + # Wall clock of the send THIS process performs; empty when it rejoins an + # operation an earlier process paid for, whose send moment it cannot know. + started_at: str = "" _ACTIVE_LOCK = threading.Lock() @@ -833,6 +836,7 @@ def _late_or_timeout_actor( entry.operation_id, "pending_dispatch", ) actor.late_result_pending = True + actor.awaiting_since = entry.started_at return actor actor = error_actor( slot, @@ -840,6 +844,8 @@ def _late_or_timeout_actor( entry.operation_id if entry is not None else "", "in_flight" if entry is not None else "settled", ) + if entry is not None: + actor.awaiting_since = entry.started_at if entry is not None and entry.retry_state: usage = dict(getattr(actor, "usage", None) or {}) usage.update({ @@ -1287,6 +1293,9 @@ def run_custodied_review_slots( operation_id=retry_operation_id or reserved_operation_id or new_call_id( f"review_{getattr(request, 'surface', 'review')}_{getattr(slot, 'slot_id', 'slot')}"), retry_state=retry_payload, + # A rejoin inherits an EARLIER process's send; only a new + # physical operation is sent from here, and only now. + started_at="" if exact_recovery else utc_now_iso(), ) entry.recovery_binding = review_operation_binding(request, slot, entry.operation_id) entry.wave_key = _wave_key(request) diff --git a/ouroboros/review_projection.py b/ouroboros/review_projection.py index 1f6559cd7..cb5e48010 100644 --- a/ouroboros/review_projection.py +++ b/ouroboros/review_projection.py @@ -16,9 +16,10 @@ import json import copy import logging from dataclasses import asdict +from datetime import datetime from typing import Any, Dict, List, TYPE_CHECKING -from ouroboros.review_records import review_slot_awaiting +from ouroboros.review_records import review_slot_awaiting, review_slot_unresolved # A slot released at the dispatch barrier has no transport or parse event: # its projection is a gap, never a transport failure or a malformed answer. @@ -186,6 +187,17 @@ def _review_actor_projection(actor: Any, surface: str) -> Dict[str, Any]: # Flat, redacted pointer to the private full response artifact. "response_ref": _response_ref_projection(row.get("response_ref")), } + # Since when this row has been waiting, published only where the host wrote + # a real instant on a row the two wait predicates still call unanswered. An + # absent key is a hole, never a back-filled or inferred moment. + if awaiting or review_slot_unresolved(row): + since = str(row.get("awaiting_since") or "").strip() + try: + datetime.fromisoformat(since) + except ValueError: + pass + else: + projection["awaiting_since"] = since # Structured rows ride only where a parsed response exists: an absent # `findings` key is a hole, never the claim "zero findings reported". if parsed is not None: diff --git a/ouroboros/review_records.py b/ouroboros/review_records.py index 4e4c0dbdb..78ffbd276 100644 --- a/ouroboros/review_records.py +++ b/ouroboros/review_records.py @@ -312,6 +312,11 @@ class ReviewActorRecord: operation_state: str = "settled" late_result_pending: bool = False recovery_binding: Dict[str, Any] = field(default_factory=dict) + # Wall clock at which THIS process sent this reviewer its request, for the + # rows that are still waiting for an answer. Empty whenever the host did not + # perform the send itself (a free replay, a rejoin of an earlier process's + # paid operation): the owner is never shown an inferred moment. + awaiting_since: str = "" @dataclass diff --git a/ouroboros/tools/plan_review_runtime.py b/ouroboros/tools/plan_review_runtime.py index cba6ecd4e..1f2abb7ab 100644 --- a/ouroboros/tools/plan_review_runtime.py +++ b/ouroboros/tools/plan_review_runtime.py @@ -543,6 +543,7 @@ def _plan_row_from_actor(actor: Dict[str, Any], slot: Any) -> dict: "operation_id": str(actor.get("operation_id") or ""), "operation_state": str(actor.get("operation_state") or "settled"), "late_result_pending": bool(actor.get("late_result_pending")), + "awaiting_since": str(actor.get("awaiting_since") or ""), "pending_invocation_id": str( usage.get("pending_invocation_id") or actor.get("pending_invocation_id") or "" ), @@ -580,6 +581,8 @@ def plan_row_typed_facts(row: Dict[str, Any]) -> Dict[str, Any]: "operation_id": str(row.get("operation_id") or ""), "operation_state": str(row.get("operation_state") or "settled"), "late_result_pending": bool(row.get("late_result_pending")), + # Since when the host has been waiting; '' whenever it never sent. + "awaiting_since": str(row.get("awaiting_since") or ""), "pending_invocation_id": pending_invocation_id, "delegated_run_id": delegated_run_id, }) diff --git a/tests/test_review_awaiting_since.py b/tests/test_review_awaiting_since.py new file mode 100644 index 000000000..5fee5c071 --- /dev/null +++ b/tests/test_review_awaiting_since.py @@ -0,0 +1,281 @@ +"""``since HH:MM`` is shown only where the host itself recorded the send moment. + +Two-sided, per row. A reviewer row the host is still waiting on carries the wall +clock of the physical operation THIS process started — the released-early +``pending_dispatch`` row and the expired-window ``in_flight`` row alike. Every +other row carries none: a free replay of an already settled answer, a rejoin of +an operation an EARLIER process paid for, an answered row, and a stored row whose +value is not a timestamp. The projection never invents, infers or back-fills the +moment, and a row recorded before the field existed still loads. +""" + +from __future__ import annotations + +import dataclasses +import datetime as _dt +import threading +import time +from types import SimpleNamespace + +import pytest + +from ouroboros.review_projection import _review_actor_projection +from ouroboros.review_records import ReviewActorRecord + + +def _instant(value: str) -> _dt.datetime: + """Parse exactly as the projection does; a lie here must raise, not pass.""" + return _dt.datetime.fromisoformat(value) + + +def _error_actor(slot, error, operation_id="", operation_state="settled"): + """The substrate's error-actor contract, as ``review_substrate`` supplies it.""" + return ReviewActorRecord( + slot_id=slot.slot_id, model=slot.model, status="error", error=error, + operation_id=operation_id, operation_state=operation_state, + late_result_pending=operation_state in {"in_flight", "custody_lost"}, + ) + + +def _run(tmp_path, request, slot, run_slot, ctx=None): + from ouroboros.review_custody import run_custodied_review_slots + from ouroboros.usage_accounting import UsageScope + + return run_custodied_review_slots( + request=request, slots=[slot], + usage_ctx=ctx if ctx is not None else SimpleNamespace(_review_paid_stamp=lambda: None), + task_id=request.task_id, usage_meta={}, + review_usage_scope=UsageScope(drive_root=tmp_path, task_id=request.task_id), + run_slot=run_slot, error_actor=_error_actor, + ) + + +def _blocking_worker(release, entered, workers): + def run_slot(slot, _operation_id, _retry_state, _deadline, _checkpoint): + workers.append(threading.current_thread()) + entered.set() + assert release.wait(10), "the test never released the review worker" + return ReviewActorRecord( + slot_id=slot.slot_id, model=slot.model, status="ok", raw_text="[]", + ) + + return run_slot + + +def test_a_released_slot_of_a_new_operation_records_when_the_host_sent_it(tmp_path): + """The owner's question — since when? — is answered by the host's own clock.""" + from ouroboros.review_substrate import ReviewRequest, ReviewSlot + + release, entered, workers = threading.Event(), threading.Event(), [] + request = ReviewRequest( + surface="task_acceptance", goal="review", task_id="wait-clock-new", + retry_key="acceptance:wait-clock-new", drain_deadline=time.monotonic(), + ) + slot = ReviewSlot(slot_id="slot-1", model="test/model", timeout_sec=30) + before = _dt.datetime.now(tz=_dt.timezone.utc) + try: + [actor] = _run(tmp_path, request, slot, _blocking_worker(release, entered, workers)) + after = _dt.datetime.now(tz=_dt.timezone.utc) + assert actor.operation_state == "pending_dispatch" + assert actor.late_result_pending is True + assert before <= _instant(actor.awaiting_since) <= after + projection = _review_actor_projection(dataclasses.asdict(actor), "task_acceptance") + assert projection["awaiting_since"] == actor.awaiting_since + finally: + release.set() + assert entered.wait(10), "the review worker never started" + for worker in workers: + worker.join(10) + assert not worker.is_alive(), "the review worker did not settle" + + +def test_an_expired_window_row_carries_the_same_recorded_moment(tmp_path): + """The unresolved twin of the awaited row: same operation, same recorded send.""" + from ouroboros.review_substrate import ReviewRequest, ReviewSlot + + release, entered, workers = threading.Event(), threading.Event(), [] + request = ReviewRequest( + surface="multi_model_review", goal="review", task_id="wait-clock-expired", + retry_key="commit_review:wait-clock-expired", + ) + slot = ReviewSlot(slot_id="slot-1", model="test/model", timeout_sec=0.05) + before = _dt.datetime.now(tz=_dt.timezone.utc) + try: + [actor] = _run(tmp_path, request, slot, _blocking_worker(release, entered, workers)) + after = _dt.datetime.now(tz=_dt.timezone.utc) + assert actor.operation_state == "in_flight" + assert before <= _instant(actor.awaiting_since) <= after + projection = _review_actor_projection(dataclasses.asdict(actor), "multi_model_review") + assert projection["awaiting_since"] == actor.awaiting_since + finally: + release.set() + assert entered.wait(10), "the review worker never started" + for worker in workers: + worker.join(10) + assert not worker.is_alive(), "the review worker did not settle" + + +def test_an_answered_row_and_its_free_replay_carry_no_moment(tmp_path): + """Nothing is awaited once the answer is here, and a $0 replay never waited.""" + from ouroboros.review_custody import prepare_frozen_review_reconciliation + from ouroboros.review_substrate import ReviewRequest, ReviewSlot + + request = ReviewRequest( + surface="multi_model_review", goal="review", task_id="wait-clock-answered", + retry_key="commit_review:wait-clock-answered", + ) + slot = ReviewSlot(slot_id="slot-1", model="cursor/test", timeout_sec=30) + + def answer(_slot, _operation_id, _retry_state, _deadline, _checkpoint): + return ReviewActorRecord( + slot_id=slot.slot_id, model=slot.model, status="ok", raw_text="[]", + ) + + [answered] = _run(tmp_path, request, slot, answer) + assert answered.status == "ok" + assert answered.awaiting_since == "" + assert "awaiting_since" not in _review_actor_projection( + dataclasses.asdict(answered), "multi_model_review") + + ctx = SimpleNamespace(_review_paid_stamp=lambda: pytest.fail("a replay paid")) + prepare_frozen_review_reconciliation(ctx, SimpleNamespace(triad_raw_results=[{ + "slot_id": "slot-1", "model_id": "cursor/test", "status": "ok", + "raw_text": "[]", "operation_id": "op-settled", "operation_state": "settled", + "late_result_pending": False, + }], scope_raw_result={})) + replay_request = ReviewRequest( + surface="multi_model_review", goal="review", task_id="wait-clock-replay", + retry_key="commit_review:wait-clock-replay", reconcile_only=True, + ) + [replayed] = _run( + tmp_path, replay_request, slot, + lambda *_args: pytest.fail("a free replay dispatched a reviewer"), ctx=ctx, + ) + assert replayed.operation_id == "op-settled" + assert replayed.awaiting_since == "" + assert "awaiting_since" not in _review_actor_projection( + dataclasses.asdict(replayed), "multi_model_review") + + +def test_a_rejoined_operation_an_earlier_process_paid_for_states_no_moment(tmp_path): + """This process did not send that request, so it does not know when it went.""" + import ouroboros.config as config + from ouroboros.review_custody import prepare_frozen_review_reconciliation + from ouroboros.review_execution import ReviewRouteKind + from ouroboros.review_substrate import ReviewRequest, ReviewSlot + + config_margin = config.NESTED_SETTLEMENT_MARGIN_SEC + release, entered, workers = threading.Event(), threading.Event(), [] + ctx = SimpleNamespace(_review_paid_stamp=lambda: pytest.fail("a rejoin paid")) + prepare_frozen_review_reconciliation(ctx, SimpleNamespace(triad_raw_results=[{ + "slot_id": "slot-1", "model_id": "cursor/test", "status": "error", + "operation_id": "op-existing", "operation_state": "in_flight", + "late_result_pending": True, "pending_invocation_id": "inv-existing", + }], scope_raw_result={})) + request = ReviewRequest( + surface="multi_model_review", goal="review", task_id="wait-clock-rejoin", + retry_key="commit_review:wait-clock-rejoin", reconcile_only=True, + drain_deadline=time.monotonic(), + ) + slot = ReviewSlot( + slot_id="slot-1", model="cursor/test", route=ReviewRouteKind.AGENT_SESSION, + ) + assert config_margin > 0, "the settlement margin must leave the rejoin a window" + try: + [actor] = _run(tmp_path, request, slot, + _blocking_worker(release, entered, workers), ctx=ctx) + assert actor.operation_id == "op-existing" + assert actor.operation_state == "pending_dispatch" + assert actor.awaiting_since == "" + assert "awaiting_since" not in _review_actor_projection( + dataclasses.asdict(actor), "multi_model_review") + finally: + release.set() + assert entered.wait(10), "the rejoined review worker never started" + for worker in workers: + worker.join(10) + assert not worker.is_alive(), "the rejoined review worker did not settle" + + +@pytest.mark.parametrize("stored", ["", " ", "soon", "since yesterday", "2026-13-45T99:99Z"]) +def test_a_stored_value_that_is_not_an_instant_projects_no_moment(stored): + """An unparseable record is a hole, never a rendered clock.""" + row = dataclasses.asdict(ReviewActorRecord( + slot_id="slot-1", model="test/model", status="error", + error="Pending dispatch; the physical review operation is in flight (window 60s)", + operation_id="op-1", operation_state="pending_dispatch", + late_result_pending=True, awaiting_since=stored, + )) + assert "awaiting_since" not in _review_actor_projection(row, "task_acceptance") + # The guard fires in the other direction on the identical row. + row["awaiting_since"] = "2026-09-19T08:41:00+00:00" + assert _review_actor_projection(row, "task_acceptance")["awaiting_since"] == ( + "2026-09-19T08:41:00+00:00") + + +def test_a_settled_row_never_carries_a_moment_even_when_one_was_stored(): + """Only the two wait predicates publish the field; a settled row is an answer.""" + for state in ("settled", "late_settled", "not_dispatched"): + row = dataclasses.asdict(ReviewActorRecord( + slot_id="slot-1", model="test/model", status="error", error="run failed", + operation_id="op-1", operation_state=state, + awaiting_since="2026-09-19T08:41:00+00:00", + )) + assert "awaiting_since" not in _review_actor_projection(row, "task_acceptance"), state + for state in ("pending_dispatch", "in_flight", "custody_lost"): + row = dataclasses.asdict(ReviewActorRecord( + slot_id="slot-1", model="test/model", status="error", error="waiting", + operation_id="op-1", operation_state=state, late_result_pending=True, + awaiting_since="2026-09-19T08:41:00+00:00", + )) + assert _review_actor_projection(row, "task_acceptance")["awaiting_since"] == ( + "2026-09-19T08:41:00+00:00"), state + + +def test_a_row_recorded_before_the_field_existed_still_loads(): + """Every rebuild path of a stored actor row survives the older shape.""" + from ouroboros.review_custody import _frozen_actor + from ouroboros.tools.plan_review_runtime import plan_wave_actor_record + + legacy = dataclasses.asdict(ReviewActorRecord( + slot_id="slot-1", model="test/model", status="error", error="waiting", + operation_id="op-1", operation_state="pending_dispatch", late_result_pending=True, + )) + legacy.pop("awaiting_since") + assert "awaiting_since" not in legacy + + # 1. the persisted producer outcome, rebuilt by recover_review_producer + assert ReviewActorRecord(**legacy).awaiting_since == "" + # 2. the frozen wave row, rebuilt for a free replay + frozen = _frozen_actor(legacy, SimpleNamespace(slot_id="slot-1", model="test/model")) + assert frozen.awaiting_since == "" + # 3. the durable plan-wave actor record + record = plan_wave_actor_record( + legacy, ok=False, error="waiting", disclosures=[], raw_text_preview_chars=100, + ) + assert record["awaiting_since"] == "" + # 4. the projection every surface reads + assert "awaiting_since" not in _review_actor_projection(legacy, "plan") + + +def test_the_plan_wave_row_carries_the_moment_the_host_recorded(tmp_path): + """The field reaches wave.actors, not only the acceptance panel.""" + from ouroboros.tools.plan_review_runtime import _plan_row_from_actor, plan_wave_actor_record + + actor = dataclasses.asdict(ReviewActorRecord( + slot_id="slot-1", model="test/model", status="error", error="waiting", + operation_id="op-1", operation_state="pending_dispatch", late_result_pending=True, + awaiting_since="2026-09-19T08:41:00+00:00", + )) + row = _plan_row_from_actor(actor, None) + assert row["awaiting_since"] == "2026-09-19T08:41:00+00:00" + record = plan_wave_actor_record( + row, ok=False, error="waiting", disclosures=[], raw_text_preview_chars=100, + ) + assert record["awaiting_since"] == "2026-09-19T08:41:00+00:00" + # A settled plan row keeps the honest empty value it was recorded with. + settled = {**actor, "operation_state": "settled", "awaiting_since": ""} + assert plan_wave_actor_record( + _plan_row_from_actor(settled, None), + ok=True, error="", disclosures=[], raw_text_preview_chars=100, + )["awaiting_since"] == "" diff --git a/web/modules/review_presentation.js b/web/modules/review_presentation.js index 6772d1e7a..c90ea8f2b 100644 --- a/web/modules/review_presentation.js +++ b/web/modules/review_presentation.js @@ -1,5 +1,5 @@ import { setInertCardPresentation } from './task_phase_chip.js'; -import { escapeHtmlAttr } from './utils.js'; +import { escapeHtmlAttr, sinceLocalTime } from './utils.js'; import { taskSourceDownloadUrl } from './api_client.js'; import { harnessIdentityMarkup } from './harness_presentation.js'; import { reconcileReviewMarkup } from './review_dom_patch.js'; @@ -535,8 +535,8 @@ function planActorAvailabilityLines(wave) { if (!actor || typeof actor !== 'object' || actor.ok !== false) continue; const identity = [text(actor.slot_id), text(actor.model)].filter(Boolean).join(' · ') || 'reviewer'; const gap = actorAwaiting(actor) - ? `Awaiting answer: ${identity}` - : (actorUnresolved(actor) ? `No answer: ${identity} — ${[text(actor.operation_state), text(actor.failure_code) || text(actor.error)].filter(Boolean).join(': ')}` : ''); + ? `Awaiting answer: ${identity}${sinceLocalTime(actor.awaiting_since)}` + : (actorUnresolved(actor) ? `No answer: ${identity} — ${[text(actor.operation_state), text(actor.failure_code) || text(actor.error)].filter(Boolean).join(': ')}${sinceLocalTime(actor.awaiting_since)}` : ''); if (gap) { lines.push(gap); continue; @@ -790,7 +790,7 @@ export function formatReviewProjection(projection) { if (!actor || typeof actor !== 'object') return; const slotId = String(actor.slot_id || '?'); lines.push( - `Reviewer ${slotId}: role=${String(actor.actor_role || 'reviewer')} · provider=${String(actor.provider || 'unknown')} · model=${String(actor.model || 'unknown')} · transport=${actorAwaiting(actor) ? 'awaiting' : String(actor.transport_status || 'unknown')} · parse=${actorAwaiting(actor) ? 'awaiting' : String(actor.parse_status || 'unknown')} · verdict=${String(actor.semantic_verdict || 'none')}${actor.outcome_tier ? ` · outcome_tier=${String(actor.outcome_tier)}` : ''}${actor.dialogue_status ? ` · dialogue=${String(actor.dialogue_status)}` : ''} · quorum=${actor.quorum_contribution ? 'contributes' : 'abstains'} · enforcement=${String(actor.enforcement_impact || 'unknown')}`, + `Reviewer ${slotId}: role=${String(actor.actor_role || 'reviewer')} · provider=${String(actor.provider || 'unknown')} · model=${String(actor.model || 'unknown')} · transport=${actorAwaiting(actor) ? 'awaiting' : String(actor.transport_status || 'unknown')} · parse=${actorAwaiting(actor) ? 'awaiting' : String(actor.parse_status || 'unknown')} · verdict=${String(actor.semantic_verdict || 'none')}${actor.outcome_tier ? ` · outcome_tier=${String(actor.outcome_tier)}` : ''}${actor.dialogue_status ? ` · dialogue=${String(actor.dialogue_status)}` : ''} · quorum=${actor.quorum_contribution ? 'contributes' : 'abstains'} · enforcement=${String(actor.enforcement_impact || 'unknown')}${actorAwaiting(actor) || actorUnresolved(actor) ? sinceLocalTime(actor.awaiting_since) : ''}`, ); const actorCoverage = compactCoverage(actor.coverage); if (actorCoverage) lines.push(`Reviewer ${slotId} coverage: ${actorCoverage}`); diff --git a/web/modules/utils.js b/web/modules/utils.js index b742cbace..aed1dfbe5 100644 --- a/web/modules/utils.js +++ b/web/modules/utils.js @@ -38,6 +38,20 @@ export function safeExternalHrefAttr(value) { return ''; } +/** + * ` · since HH:MM` in the viewer's own 24-hour clock, for an instant the host + * actually recorded. A wait that began on an earlier local day carries that day + * too, so `since 23:50` can never be misread as tonight. A missing or + * unparseable value yields '': a moment is never invented or inferred. + */ +export function sinceLocalTime(value, now = Date.now()) { + const at = new Date(Date.parse(String(value ?? '').trim())); + if (Number.isNaN(at.getTime())) return ''; + const clock = at.toLocaleTimeString([], { hour: '2-digit', minute: '2-digit', hour12: false }); + if (at.toDateString() === new Date(now).toDateString()) return ` · since ${clock}`; + return ` · since ${at.toLocaleDateString([], { month: 'short', day: 'numeric' })} ${clock}`; +} + /** Bound untrusted text with a visible marker before it reaches DOM surfaces. */ export function boundedText(value, maxLen = 1200) { const text = String(value ?? ''); diff --git a/web/tests/review_plan_custody.test.js b/web/tests/review_plan_custody.test.js index b40acf597..ff6b9d6f7 100644 --- a/web/tests/review_plan_custody.test.js +++ b/web/tests/review_plan_custody.test.js @@ -2,6 +2,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import { + formatReviewProjection, planReviewGroupFromTaskDetail, renderReviewsSection, } from '../modules/review_presentation.js'; @@ -204,3 +205,116 @@ test('a plan wave of a task that is not running is a recorded gap, never live wo assert.equal(mixed.progress, 'no verdict · 1 of 3 answered · 1 unavailable'); assert.equal(mixed.tone, 'warn'); }); + +// --- "since HH:MM": the host's own record of when it sent the request --------- +// The clock is the VIEWER's local time, so every expectation is computed with the +// same platform API the renderer uses; the suite must pass in any timezone. +const localClock = (iso) => new Date(iso) + .toLocaleTimeString([], { hour: '2-digit', minute: '2-digit', hour12: false }); +const localDay = (iso) => new Date(iso).toLocaleDateString([], { month: 'short', day: 'numeric' }); +const todayAt = (hour, minute) => { + const at = new Date(); + at.setHours(hour, minute, 0, 0); + return at.toISOString(); +}; +const daysAgoAt = (days, hour, minute) => { + const at = new Date(Date.now() - days * 86400000); + at.setHours(hour, minute, 0, 0); + return at.toISOString(); +}; + +test('an awaited reviewer says since when, in the viewer local clock', () => { + const sent = todayAt(9, 15); + const group = planGroup({ + custody_pending: true, + actors: [{ ...AWAITING, awaiting_since: sent }, ANSWERED], + }); + const [line] = availabilityLines(group.attempts[0], 'Awaiting answer:'); + assert.match(line, /^Awaiting answer: .* · since \d{2}:\d{2}$/); + assert.equal(line, `Awaiting answer: triad_286lhb · codex=gpt-6-astra · since ${localClock(sent)}`); +}); + +test('a reviewer row the host never timed keeps exactly its former line', () => { + // Both directions of the same guard: no field, an empty field and a value that + // is not an instant all render byte-identically to the line shipped before. + const before = 'Awaiting answer: triad_286lhb · codex=gpt-6-astra'; + for (const awaiting_since of [undefined, '', ' ', 'soon', 'since yesterday']) { + const group = planGroup({ + custody_pending: true, + actors: [{ ...AWAITING, ...(awaiting_since === undefined ? {} : { awaiting_since }) }], + }); + assert.deepEqual( + availabilityLines(group.attempts[0], 'Awaiting answer:'), [before], String(awaiting_since), + ); + } +}); + +test('a wait that began on an earlier day names that day too', () => { + // "since 23:50" must never be misread as tonight when the wait is 30 hours old. + const sent = daysAgoAt(1, 23, 50); + const group = planGroup({ custody_pending: true, actors: [{ ...AWAITING, awaiting_since: sent }] }); + const [line] = availabilityLines(group.attempts[0], 'Awaiting answer:'); + assert.equal(line, `Awaiting answer: triad_286lhb · codex=gpt-6-astra · since ${localDay(sent)} ${localClock(sent)}`); + assert.doesNotMatch(line, /^Awaiting answer: .* · since \d{2}:\d{2}$/); +}); + +test('an unresolved reviewer says since when it was sent, under the same rule', () => { + const sent = todayAt(7, 5); + const lost = { + ...AWAITING, operation_state: 'custody_lost', failure_code: 'review_custody_lost', + error: 'Review custody was lost before the slot settled', + }; + const timed = planGroup({ custody_pending: true, actors: [{ ...lost, awaiting_since: sent }] }); + assert.deepEqual(availabilityLines(timed.attempts[0], 'No answer:'), [ + `No answer: triad_286lhb · codex=gpt-6-astra — custody_lost: review_custody_lost · since ${localClock(sent)}`, + ]); + const untimed = planGroup({ custody_pending: true, actors: [lost] }); + assert.deepEqual(availabilityLines(untimed.attempts[0], 'No answer:'), [ + 'No answer: triad_286lhb · codex=gpt-6-astra — custody_lost: review_custody_lost', + ]); +}); + +test('a settled reviewer never grows a since suffix', () => { + // The stored moment belongs to the wait; an answer that arrived is judged by itself. + const group = planGroup({ + custody_pending: false, + actors: [{ ...FAILED, awaiting_since: todayAt(6, 30) }], + }); + assert.deepEqual(availabilityLines(group.attempts[0], 'Reviewer unavailable:'), [ + 'Reviewer unavailable: triad_bkydwq · codex=gpt-6-astra — run_failed', + ]); +}); + +test('the acceptance panel reviewer line says since when, under the same rule', () => { + const sent = todayAt(8, 41); + const row = (fields) => ({ + slot_id: 's1', model: 'codex=gpt-6-astra', provider: 'openrouter', + actor_role: 'task acceptance', transport_status: 'awaiting', parse_status: 'awaiting', + semantic_verdict: '', coverage: { criteria_total: 0, findings: 0 }, + quorum_contribution: false, enforcement_impact: 'abstains', operation_id: 'op-s1', + operation_state: 'pending_dispatch', late_result_pending: true, executions: [], + response_ref: {}, reason: 'no answer yet', ...fields, + }); + const panelText = (fields) => formatReviewProjection({ + panels: [{ + panel_id: 'panel_a72b23783ba34908', surface: 'task_acceptance', authority: 'host_root', + aggregate_signal: 'DEGRADED', transport_status: 'awaiting', parse_status: 'awaiting', + quorum: { required: 1, contributed: 0, configured: 1 }, + enforcement_impact: 'pending_feedback', actors: [row(fields)], + }], + }).split('\n').filter((line) => line.startsWith('Reviewer s1:')); + + assert.deepEqual(panelText({ awaiting_since: sent }), [ + `Reviewer s1: role=task acceptance · provider=openrouter · model=codex=gpt-6-astra · transport=awaiting · parse=awaiting · verdict=none · quorum=abstains · enforcement=abstains · since ${localClock(sent)}`, + ]); + assert.deepEqual(panelText({}), [ + 'Reviewer s1: role=task acceptance · provider=openrouter · model=codex=gpt-6-astra · transport=awaiting · parse=awaiting · verdict=none · quorum=abstains · enforcement=abstains', + ]); + assert.deepEqual(panelText({ awaiting_since: 'soon' }), panelText({})); + // A settled reviewer line is untouched even if a moment rode along. + assert.deepEqual( + panelText({ operation_state: 'settled', transport_status: 'success', parse_status: 'valid', + semantic_verdict: 'PASS', awaiting_since: sent }), + ['Reviewer s1: role=task acceptance · provider=openrouter · model=codex=gpt-6-astra · transport=success · parse=valid · verdict=PASS · quorum=abstains · enforcement=abstains'], + ); +});