From 09e41d1502de657b3f8fab4163e465167ab292d8 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Thu, 4 Jun 2026 10:59:05 -0700 Subject: [PATCH] fix(coord): extend the system_turn /history+replay dedup to the coordinator pane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tests/test_coordinator_page.py | 26 +++++++++++++++++++ .../console/static/coordinator/coordinator.js | 21 ++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/tests/test_coordinator_page.py b/tests/test_coordinator_page.py index 8c0ee7f6..5fbbf90e 100644 --- a/tests/test_coordinator_page.py +++ b/tests/test_coordinator_page.py @@ -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`` diff --git a/turnstone/console/static/coordinator/coordinator.js b/turnstone/console/static/coordinator/coordinator.js index 67ab8979..487d3953 100644 --- a/turnstone/console/static/coordinator/coordinator.js +++ b/turnstone/console/static/coordinator/coordinator.js @@ -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 });