mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-15 00:12:26 -06:00
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:
@@ -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 });
|
||||
|
||||
Reference in New Issue
Block a user