fix(coord): extend the system_turn /history+replay dedup to the coordinator pane

The interactive pane (app.js) got the event-id dedup that skips an
operator-context system turn already painted from /history when an SSE replay
redelivers it; the coordinator pane (coordinator.js) shares the identical
/history + live system_turn + last_event_id replay seam but was left without
the guard.  The backend row/event-id alignment already fixes the actual double
for both panes — this restores the defense-in-depth symmetry.

- Module-scoped renderedSystemEventIds (persists across reconnects like
  lastEventId), reset in refetchHistory.
- onmessage tags each event with its SSE id; the live system_turn handler skips
  an already-rendered id; the history loop records the ids it paints.
- Parallel static-shape regression test in test_coordinator_page.py.
This commit is contained in:
Patrick Buckley
2026-06-04 10:59:05 -07:00
parent 21af6c4970
commit 09e41d1502
2 changed files with 46 additions and 1 deletions
+26
View File
@@ -345,6 +345,32 @@ def test_coord_history_renders_system_turn_via_msg_variants():
)
def test_coord_dedups_system_turn_against_history_by_event_id():
"""The coord live ``system_turn`` handler skips an event already painted
from ``/history`` (matched by ``_event_id``) so an SSE replay redelivering
it past the resume cursor doesn't double-render the operator bubble.
Symmetric with ``test_app_js.py``'s interactive dedup and the row/event
id-alignment backend fix — both panes share the seam."""
from pathlib import Path
coord_js = Path(__file__).resolve().parent.parent / (
"turnstone/console/static/coordinator/coordinator.js"
)
body = coord_js.read_text(encoding="utf-8")
assert "renderedSystemEventIds.has(" in body, (
"the coord system_turn handler must skip an event whose id was already "
"rendered from /history."
)
assert "renderedSystemEventIds.add(" in body, (
"the coord history loop (and live handler) must record system-turn ids."
)
assert "renderedSystemEventIds.clear(" in body, (
"refetchHistory must reset the dedup set so a re-render doesn't "
"false-skip after clear_ui / replay_truncated."
)
def test_coord_retry_walk_skips_operator_context_cards():
"""Retry must NOT regenerate a stale assistant turn when the last DOM row is
a tool batch trailed by an operator-context row. ``_refreshRetryButton``
@@ -190,6 +190,11 @@
// close).
let lastEventId = null;
let reconnectTimer = null;
// Ids of operator-context system turns already painted from /history. A
// later SSE replay that redelivers one (resume-cursor overlap) is skipped
// by the system_turn handler — reset per refetchHistory. Mirrors
// ui/static/app.js's per-pane _renderedSystemEventIds.
const renderedSystemEventIds = new Set();
// Cache of judge verdicts keyed by call_id. intent_verdict and
// approve_request are async and may arrive in either order; the
@@ -2219,6 +2224,9 @@
} catch (_) {
return;
}
// Tag the event with its own SSE id so the system_turn handler can dedup
// a turn already painted from /history (mirrors ui/static/app.js).
if (event.lastEventId) data._event_id = event.lastEventId;
handleEvent(data);
};
}
@@ -2427,15 +2435,23 @@
// styling which mis-categorised them as tool calls.
appendText("info", ev.message || "", { label: "info" });
break;
case "system_turn":
case "system_turn": {
// First-class operator-context system turn (output-guard finding,
// user interjection, metacognitive nudge, watch result — see
// make_system_turn). Rendered in trajectory sequence (it FOLLOWS the
// turn it advises). ``renderSystemTurn`` routes by ``ev.source`` to the
// structured card (watch / guard / idle-children) or the operator bubble
// (carrying ``ev.meta`` so cards rebuild identically live and on replay).
// Dedup: skip a turn already painted from /history (matched by id) and
// redelivered by an SSE replay past the resume cursor. With the
// row/event id-alignment fix this shouldn't recur, but keeps the
// /history+replay seam idempotent. Mirrors ui/static/app.js.
const sysEid = ev._event_id != null ? String(ev._event_id) : null;
if (sysEid && renderedSystemEventIds.has(sysEid)) break;
renderSystemTurn(ev.source || "", ev.content || "", ev.meta);
if (sysEid) renderedSystemEventIds.add(sysEid);
break;
}
case "connected":
// First yield from _coord_events_replay — populates the
// status bar's model cell before any history arrives. Also
@@ -4533,6 +4549,7 @@
messagesEl.replaceChildren();
toolRows.clear();
activeBatch = null;
renderedSystemEventIds.clear();
if (!hist) return;
// Fresh-connect fast-forward: when the trailing turn is an executing
// in-flight tool batch the server can replay, /history returns a
@@ -4811,6 +4828,8 @@
// ``/history`` projection so replay matches the live render exactly.
if (!content) return;
renderSystemTurn(m.source || "", content, m.meta);
if (m.event_id != null)
renderedSystemEventIds.add(String(m.event_id));
} else {
if (!content) return;
appendText(role, content, { label: role });