From eaabc79eb3950c87e279f6951cdb3ecbc8469ef2 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Fri, 1 May 2026 14:05:07 -0700 Subject: [PATCH] fix(replay): repair saved-workstream tool result rendering + extend audit-trail decoration (#461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(replay): repair saved-workstream tool result rendering + extend audit-trail decoration Loading a saved workstream silently dropped tool results and missed verdict / output-guard / truncation signals on replay. Root cause was in `Pane.prototype.replayHistory`: an assistant message carrying both content and tool_calls cleared the `lastToolBlock` anchor before the following tool-result iteration could attach. The fix reorders content to render before the tool block (matching live SSE order) and restructures the tool-result branch to anchor by `data-call-id` so multi-tool batches render `[hdr A][out A][hdr B][out B]` rather than bunching outputs at the bottom. Beyond the bug, replay now reaches near-parity with the live UX: - Persisted intent verdicts and output_assessments flow through both the SSE replay (`_build_history`) and the `/history` REST endpoint used by coord. Single shared helper module owns the wire shape. - Memory/recall calls persist instead of being filtered at storage time — full audit trail; UI dims them by default with hover-reveal so heavy memory usage doesn't crowd the narrative. - Truncation indicator surfaces as a sibling pill (consistent across interactive + coord) when a tool result hit the 2000-char cap. - `replayHistory` wraps DOM work in `aria-busy` so screen readers don't get a chatty announce-flood on long replays. - `_build_history`'s storage I/O moves off the event loop via a new `events_replay_prepare` async hook for the SSE path; other async callers wrap in `asyncio.to_thread`. Coord parity: - `/history` REST endpoint decorates tool_calls with verdict + output_assessment + truncation flag (was previously raw `load_messages` output). - Coord JS stamps `judge_verdict` / `heuristic_verdict` from history-loaded `tc.verdict` so the existing batch render paints the persisted pill, seeds the verdict cache to dedupe later live SSE events, and emits an inline `.coord-tool-row-warning` chip per call instead of a generic chat line. - Memory/recall dim rule mirrored on `.coord-tool-row[data-tool-name=...]`. * fix(replay): address PR #461 review feedback + raise tool-result storage cap Copilot review feedback: - Sibling-chain dim rule (memory/recall) now adds :focus-within alongside :hover for .tool-output / .media-embed / .output-warning / .tool-output-truncated — keyboard users tabbing into a faded subtree now get full opacity. - ``cfg.open_post_load`` is now invoked via ``await asyncio.to_thread`` so its sync ``_build_history`` call (storage I/O for verdict indexes + message reconstruction) doesn't block the event loop on every workstream open. Mirrors the SSE replay path that's already protected via ``events_replay_prepare``. - Replaced the hardcoded ``2000`` literal in server.py and session.py with ``TOOL_RESULT_STORAGE_CAP`` from the shared decoration module so the UI truncation-pill detection can't silently desync from the storage write side. While here: - Raised ``TOOL_RESULT_STORAGE_CAP`` from 2000 → 10000. A 2000-char clip routinely cut grep / file-read bodies mid-line, leaving the audit trail useless for retrospective debugging. FTS5 + row size grow proportionally; the per-tool upper bound is still bounded upstream by ``_truncate_output``'s context-budget clamp. - Updated the user-visible truncation-pill tooltip on both interactive and coord to reflect the new cap. - ``test_decorates_tool_calls_and_marks_truncated`` now references the constant instead of a literal so it stays correct on future cap changes. --- tests/test_app_js.py | 68 +++++ tests/test_history_decoration.py | 232 +++++++++++++++++ .../static/coordinator/coordinator.css | 81 ++++++ .../console/static/coordinator/coordinator.js | 138 +++++++++- turnstone/core/history_decoration.py | 205 +++++++++++++++ turnstone/core/session.py | 52 ++-- turnstone/core/session_routes.py | 62 ++++- turnstone/server.py | 129 +++++++++- turnstone/ui/static/app.js | 236 +++++++++++++++--- turnstone/ui/static/style.css | 110 +++++--- 10 files changed, 1193 insertions(+), 120 deletions(-) create mode 100644 tests/test_history_decoration.py create mode 100644 turnstone/core/history_decoration.py diff --git a/tests/test_app_js.py b/tests/test_app_js.py index 37535fff..8ada08a5 100644 --- a/tests/test_app_js.py +++ b/tests/test_app_js.py @@ -92,3 +92,71 @@ def test_tool_error_does_not_overwrite_approval_badge() -> None: "badge instead so the approval verdict stays visible alongside " "the error." ) + + +def test_replay_history_renders_content_before_tool_block() -> None: + """In ``replayHistory``'s ``role === "assistant"`` branch, the + ``msg.content`` render must precede the ``msg.tool_calls`` render. + + Two reasons, both load-bearing: + + 1. **Structural** — the next loop iteration's ``role === "tool"`` + message anchors to ``lastToolBlock``. The tool-block branch sets + that anchor; the content branch clears it. If content runs after + the tool block, the clear silently drops the upcoming tool + result. Pre-fix, every interactive tool result was missing from + saved-workstream replays whenever the assistant turn carried + both narration and tool calls (very common output shape). + + 2. **Visual** — the live SSE path renders content first + (``stream_text`` streams before ``tool_info`` / + ``approve_request``), so replay should match. + + The test pins the order via the offsets of the ``msg.content`` and + ``msg.tool_calls`` branch headers inside the function body.""" + body = _APP_JS.read_text(encoding="utf-8") + start = body.index("Pane.prototype.replayHistory = function") + end = body.index("Pane.prototype._attachRetryToLastAssistant", start) + fn = body[start:end] + # Locate the assistant branch and bound the search to its body — + # the function also handles user / tool roles which would otherwise + # confuse the offset comparison. + asst_start = fn.index('msg.role === "assistant"') + asst_end = fn.index('msg.role === "tool"', asst_start) + asst = fn[asst_start:asst_end] + content_idx = asst.index("if (msg.content)") + tool_calls_idx = asst.index("if (msg.tool_calls && msg.tool_calls.length)") + assert content_idx < tool_calls_idx, ( + "replayHistory must render msg.content BEFORE msg.tool_calls " + "inside the assistant branch — otherwise the lastToolBlock " + "anchor is clobbered before the next iteration's tool result " + "can attach to it (and the visual order also drifts from the " + "live SSE flow)." + ) + + +def test_replay_history_renders_persisted_verdict_badge() -> None: + """Saved-workstream replays must paint the persisted intent verdict + next to each tool div, using the same ``renderVerdictBadge`` helper + the live ``showInlineToolBlock`` path uses. Pre-fix the audit trail + was complete in storage (``intent_verdicts`` table) but never + surfaced on replay — operators reviewing a saved workstream + couldn't see what the heuristic / LLM judge thought of any tool + call. This test pins the call site so a refactor that drops the + decoration regresses the audit surface.""" + body = _APP_JS.read_text(encoding="utf-8") + start = body.index("Pane.prototype.replayHistory = function") + end = body.index("Pane.prototype._attachRetryToLastAssistant", start) + fn = body[start:end] + # Match a `renderVerdictBadge(.verdict, ...)` call inside + # the replay loop. Loose on whitespace + identifier so a future + # rename of the iteration variable doesn't trip CI. + badge_call_re = re.compile( + r"renderVerdictBadge\(\s*\w+\.verdict\b", + ) + assert badge_call_re.search(fn), ( + "replayHistory must call renderVerdictBadge(tc.verdict, ...) " + "when a persisted verdict is attached to a tool_call entry — " + "otherwise the audit-trail data persisted to intent_verdicts " + "doesn't surface on saved-workstream replays." + ) diff --git a/tests/test_history_decoration.py b/tests/test_history_decoration.py new file mode 100644 index 00000000..c843e8f3 --- /dev/null +++ b/tests/test_history_decoration.py @@ -0,0 +1,232 @@ +"""Unit tests for ``turnstone.core.history_decoration``. + +The decoration helpers are shared between two surfaces — interactive's +SSE replay (``_build_history``) and the lifted ``/history`` REST +endpoint (``make_history_handler``, used by both interactive and +coord). Pinning the wire shape here lets a future schema/projection +change land in one file rather than spread across the two surfaces. +""" + +from __future__ import annotations + +from turnstone.core.history_decoration import ( + build_output_assessment_payload, + build_verdict_payload, + decorate_history_messages, + decorate_tool_call, +) + + +class TestBuildVerdictPayload: + """The wire-shape projection that's the single source of truth for + what intent_verdict fields ship to the client.""" + + def test_skips_unflagged_baseline(self) -> None: + """``risk_level`` "none" is the unflagged-tool baseline; the + client filters those anyway, so projecting None at the wire + layer keeps the payload tight on long workstreams.""" + row = {"risk_level": "none", "recommendation": "approve", "tier": "heuristic"} + assert build_verdict_payload(row) is None + + def test_drops_call_id_and_func_name(self) -> None: + """The client already has these on ``tc.id`` / ``tc.name``; + re-shipping them per-tool_call would balloon long replays.""" + row = { + "call_id": "call_abc", + "func_name": "bash", + "risk_level": "medium", + "recommendation": "review", + "confidence": 0.8, + "intent_summary": "summary", + "tier": "heuristic", + } + out = build_verdict_payload(row) + assert out is not None + assert "call_id" not in out + assert "func_name" not in out + # Sanity — the kept fields are the ones renderVerdictBadge reads. + assert out["risk_level"] == "medium" + assert out["recommendation"] == "review" + assert out["confidence"] == 0.8 + assert out["intent_summary"] == "summary" + assert out["tier"] == "heuristic" + + def test_includes_reasoning_for_either_tier_when_present(self) -> None: + """Heuristic verdicts in this project emit structured + rationales (one per matched pattern) — e.g. + ``policy.py`` writes a reasoning string per heuristic hit. + Ship the field for either tier when it has content; only + omit when the row didn't write one.""" + for tier in ("heuristic", "llm"): + row = { + "risk_level": "high", + "tier": tier, + "reasoning": "The command exfiltrates ~/.ssh/id_rsa over an external connection.", + } + out = build_verdict_payload(row) + assert out is not None + assert "id_rsa" in out["reasoning"] + + def test_omits_reasoning_when_empty(self) -> None: + """An absent / empty reasoning string shouldn't ship as + ``reasoning: ""`` — the rationale ``
`` block on the + client renders an empty disclosure when the field is present + but empty.""" + row = {"risk_level": "high", "tier": "heuristic", "reasoning": ""} + out = build_verdict_payload(row) + assert out is not None + assert "reasoning" not in out + + def test_includes_judge_model_when_present(self) -> None: + """``judge_model`` rides through so the batch tier badge can + render ``⚖ llm:claude-haiku-4`` on history-only replays + rather than the bare ``⚖ llm`` label.""" + row = {"risk_level": "high", "tier": "llm", "judge_model": "claude-haiku-4"} + out = build_verdict_payload(row) + assert out is not None + assert out["judge_model"] == "claude-haiku-4" + + def test_omits_judge_model_when_empty(self) -> None: + row = {"risk_level": "medium", "tier": "heuristic", "judge_model": ""} + out = build_verdict_payload(row) + assert out is not None + assert "judge_model" not in out + + +class TestBuildOutputAssessmentPayload: + """Output-guard wire shape — flags decoded from JSON string at + this layer so the client never has to parse twice.""" + + def test_skips_unflagged_baseline(self) -> None: + row = {"risk_level": "none", "flags": "[]"} + assert build_output_assessment_payload(row) is None + + def test_decodes_flags_from_json(self) -> None: + row = {"risk_level": "high", "flags": '["api_key","email"]', "redacted": 1} + out = build_output_assessment_payload(row) + assert out is not None + assert out["flags"] == ["api_key", "email"] + assert out["redacted"] is True + assert out["risk_level"] == "high" + + def test_handles_malformed_flags_json(self) -> None: + """Bad JSON in ``flags`` must not block the rest of the + assessment from rendering — degrade to empty list.""" + row = {"risk_level": "medium", "flags": "not-json", "redacted": 0} + out = build_output_assessment_payload(row) + assert out is not None + assert out["flags"] == [] + assert out["redacted"] is False + + +class TestDecorateToolCall: + """In-place mutation of either OpenAI-format or flattened tool_call + entries — both shapes carry ``id`` at the top level.""" + + def test_attaches_verdict_when_present(self) -> None: + tc: dict[str, object] = {"id": "call_1", "function": {"name": "bash", "arguments": "{}"}} + verdicts = { + "call_1": { + "risk_level": "medium", + "recommendation": "review", + "confidence": 0.7, + "intent_summary": "summary", + "tier": "heuristic", + } + } + decorate_tool_call(tc, verdicts, {}) + assert "verdict" in tc + assert tc["verdict"]["risk_level"] == "medium" # type: ignore[index] + + def test_skips_when_no_call_id_match(self) -> None: + tc: dict[str, object] = {"id": "call_other", "name": "bash"} + verdicts = { + "call_1": {"risk_level": "medium", "tier": "heuristic"}, + } + decorate_tool_call(tc, verdicts, {}) + assert "verdict" not in tc + + def test_skips_unflagged_verdict(self) -> None: + """``build_verdict_payload`` returns None for unflagged rows; + decorate_tool_call must not stamp ``verdict`` in that case.""" + tc: dict[str, object] = {"id": "call_1", "name": "bash"} + verdicts = {"call_1": {"risk_level": "none", "tier": "heuristic"}} + decorate_tool_call(tc, verdicts, {}) + assert "verdict" not in tc + + def test_handles_empty_id(self) -> None: + """A tool_call with no id can't be paired against the lookup + table — must not raise (or stamp the wrong row's verdict).""" + tc: dict[str, object] = {"id": "", "name": "bash"} + verdicts = {"call_1": {"risk_level": "high", "tier": "heuristic"}} + decorate_tool_call(tc, verdicts, {}) + assert "verdict" not in tc + + +class TestDecorateHistoryMessages: + """End-to-end mutation of a /history-shaped message list — covers + the full transform applied by ``make_history_handler``.""" + + def test_decorates_tool_calls_and_marks_truncated(self) -> None: + verdicts = { + "call_a": { + "risk_level": "high", + "recommendation": "deny", + "confidence": 0.95, + "intent_summary": "exfil", + "tier": "llm", + "reasoning": "ssh key access", + } + } + assessments = { + "call_a": {"risk_level": "high", "flags": '["secret"]', "redacted": 1}, + } + # Tool result content of exactly TOOL_RESULT_STORAGE_CAP chars + # hits the storage cap (longer is impossible — storage clamps + # at the cap). Reference the constant rather than a literal so + # this test stays correct if the cap moves again. + from turnstone.core.history_decoration import TOOL_RESULT_STORAGE_CAP + + truncated_content = "x" * TOOL_RESULT_STORAGE_CAP + messages: list[dict[str, object]] = [ + {"role": "user", "content": "hi"}, + { + "role": "assistant", + "content": "running", + "tool_calls": [ + { + "id": "call_a", + "function": {"name": "bash", "arguments": "{}"}, + } + ], + }, + {"role": "tool", "tool_call_id": "call_a", "content": truncated_content}, + {"role": "tool", "tool_call_id": "call_b", "content": "short"}, + ] + decorate_history_messages(messages, verdicts, assessments) + # Assistant tool_calls got both decorations. + tc = messages[1]["tool_calls"][0] # type: ignore[index] + assert tc["verdict"]["risk_level"] == "high" + assert tc["verdict"]["tier"] == "llm" + assert "reasoning" in tc["verdict"] + assert tc["output_assessment"]["flags"] == ["secret"] + assert tc["output_assessment"]["redacted"] is True + # Truncated tool message got the flag; the short one did not. + assert messages[2].get("truncated") is True + assert "truncated" not in messages[3] + + def test_no_op_on_empty_indexes(self) -> None: + """When neither table has rows for the workstream, the wire + shape passes through unchanged — replay must degrade + gracefully when verdict storage is empty / unavailable.""" + messages: list[dict[str, object]] = [ + { + "role": "assistant", + "content": "", + "tool_calls": [{"id": "call_a", "function": {"name": "bash", "arguments": "{}"}}], + }, + ] + decorate_history_messages(messages, {}, {}) + tc = messages[0]["tool_calls"][0] # type: ignore[index] + assert "verdict" not in tc + assert "output_assessment" not in tc diff --git a/turnstone/console/static/coordinator/coordinator.css b/turnstone/console/static/coordinator/coordinator.css index 8613e6c3..3ccc3589 100644 --- a/turnstone/console/static/coordinator/coordinator.css +++ b/turnstone/console/static/coordinator/coordinator.css @@ -396,6 +396,87 @@ background: color-mix(in srgb, var(--err) 12%, var(--panel-2)); } +/* Output-guard finding rendered under its specific .coord-tool-row. + Stays anchored to the call that tripped the guard rather than + floating into the chat log as a generic "[output guard]" line — + matches interactive's `.output-warning` placement convention. + Severity drives the hue (matches .verdict-badge.verdict-* palette + so an operator scanning a workstream reads risk consistently + across both surfaces). */ +.coord-tool-row-warning { + display: inline-flex; + align-items: center; + gap: 6px; + margin-top: 4px; + padding: 2px 8px; + font-size: 11px; + font-family: var(--font-mono); + border: 1px solid var(--hair); + border-radius: 3px; + border-left-width: 3px; + background: var(--panel-2); + color: var(--ink-2); + max-width: max-content; +} +.coord-tool-row-warning--low { + color: color-mix(in srgb, var(--ok) 70%, var(--ink-2)); + border-left-color: var(--ok); + background: color-mix(in srgb, var(--ok) 12%, var(--panel-2)); +} +.coord-tool-row-warning--medium { + color: color-mix(in srgb, var(--warn) 70%, var(--ink-2)); + border-left-color: var(--warn); + background: var(--warn-tint); +} +.coord-tool-row-warning--high, +.coord-tool-row-warning--critical { + color: color-mix(in srgb, var(--err) 70%, var(--ink-2)); + border-left-color: var(--err); + background: color-mix(in srgb, var(--err) 12%, var(--panel-2)); +} +.coord-tool-row-warning-redacted { + color: var(--ink-3); + font-size: 10px; +} + +/* Storage-truncation indicator — same convention as the interactive + UI's `.tool-output-truncated` pill (transparent bg, dim border, + small font) so the operator reads the affordance the same way on + both surfaces. Sibling node next to .coord-tool-row-result rather + than text-in-content so a future "best-effort JSON repair" pass + on the result body doesn't have to strip a marker string. */ +.coord-tool-truncated { + display: inline-block; + margin-top: 4px; + margin-left: 6px; + padding: 1px 6px; + font-size: 10px; + font-family: var(--font-mono); + color: var(--ink-3); + background: transparent; + border: 1px solid var(--ink-3); + border-radius: 3px; +} + +/* memory/recall calls are background metadata — the audit trail is + useful but they crowd the tree on workstreams with heavy memory + usage. Dim the row by default; full opacity on hover so they + stay inspectable. Mirrors the interactive UI's metacog dim rule + (style.css `.ts-approval-tool[data-func-name="memory"]`). The + row stamps `data-tool-name` from item.func_name in coordinator.js + so this selector has something to match. */ +.coord-tool-row[data-tool-name="memory"], +.coord-tool-row[data-tool-name="recall"] { + opacity: 0.55; + transition: opacity 120ms ease-out; +} +.coord-tool-row[data-tool-name="memory"]:hover, +.coord-tool-row[data-tool-name="memory"]:focus-within, +.coord-tool-row[data-tool-name="recall"]:hover, +.coord-tool-row[data-tool-name="recall"]:focus-within { + opacity: 1; +} + /* Tool result block — paired under its row, mono pre-block. Capped at 240px with internal scroll so a long tool output doesn't push the rest of the chat off-screen. The interactive UI uses a diff --git a/turnstone/console/static/coordinator/coordinator.js b/turnstone/console/static/coordinator/coordinator.js index 4d252979..7a9fe053 100644 --- a/turnstone/console/static/coordinator/coordinator.js +++ b/turnstone/console/static/coordinator/coordinator.js @@ -459,10 +459,10 @@ }; } - function appendToolResult(name, callId, output, isError) { + function appendToolResult(name, callId, output, isError, opts) { if (callId && toolRows.has(callId)) { const entry = toolRows.get(callId); - _appendResultToRow(entry.row, output, isError); + _appendResultToRow(entry.row, output, isError, opts); // Result blocks grow scrollHeight; without this the user pinned // at the bottom loses their pin when the row inflates. appendMsg // already routes through _scheduleScroll on the legacy path; this @@ -655,6 +655,11 @@ const row = document.createElement("div"); row.className = "coord-tool-row"; if (item.call_id) row.dataset.callId = item.call_id; + // Stamp the function name so the metacog dim rule for memory / + // recall (coordinator.css `.coord-tool-row[data-tool-name=...]`) + // has something to match. Cheap; lets long workstreams with + // heavy memory usage stay readable without per-call JS work. + if (item.func_name) row.dataset.toolName = item.func_name; const callLine = document.createElement("div"); callLine.className = "coord-tool-row-call"; @@ -683,6 +688,35 @@ return row; } + // Attach an output-guard finding chip to a specific .coord-tool-row. + // Idempotent — replacing an existing chip in place lets the live + // SSE handler upgrade severity without stacking duplicates when a + // late event arrives after replay seeded an initial chip. + function _attachOutputWarningChip(row, oa) { + if (!row || !oa) return; + const risk = String(oa.risk_level || "medium"); + const flags = oa.flags || []; + const existing = row.querySelector(".coord-tool-row-warning"); + const chip = existing || document.createElement("div"); + chip.className = "coord-tool-row-warning coord-tool-row-warning--" + risk; + chip.setAttribute("role", "status"); + chip.textContent = ""; + const label = document.createElement("span"); + label.className = "coord-tool-row-warning-label"; + label.textContent = "⚠ " + risk.toUpperCase(); + chip.appendChild(label); + if (flags.length) { + chip.appendChild(document.createTextNode(" " + flags.join(", "))); + } + if (oa.redacted) { + const redacted = document.createElement("span"); + redacted.className = "coord-tool-row-warning-redacted"; + redacted.textContent = " (credentials redacted)"; + chip.appendChild(redacted); + } + if (!existing) row.appendChild(chip); + } + // Stable signature for a verdict — used to skip the DOM rebuild when // an SSE replay (or duplicate intent_verdict event) carries the same // verdict body we already painted. Any field change (rec/risk/conf @@ -795,10 +829,14 @@ line.appendChild(chip); } - function _appendResultToRow(row, output, isError) { + function _appendResultToRow(row, output, isError, opts) { if (!row) return; const existing = row.querySelector(".coord-tool-row-result"); if (existing) existing.remove(); + // Re-fires (cancel + rerun, error + retry) clear any prior + // truncation pill so it doesn't stack on the new result. + const existingTrunc = row.querySelector(".coord-tool-truncated"); + if (existingTrunc) existingTrunc.remove(); if (isError) { row.classList.add("error"); // Lift the row's error onto the enclosing batch so the left @@ -854,6 +892,19 @@ body.textContent = pretty; block.appendChild(body); row.appendChild(block); + // Storage-truncation indicator — sibling pill (not text inside + // the result body) so renderers / parsers / copy-as-text paths + // see the unmodified output. Same convention as interactive's + // .tool-output-truncated; styled by .coord-tool-truncated in + // coordinator.css. + if (opts && opts.truncated) { + const pill = document.createElement("span"); + pill.className = "coord-tool-truncated"; + pill.textContent = "… truncated in storage"; + pill.title = + "Full tool output was sent to the model live; only the first 10000 characters are persisted to the conversation row."; + row.appendChild(pill); + } } function _makeActionButton(label, role, kbdHint, ariaLabel) { @@ -1891,14 +1942,27 @@ ); break; case "output_warning": - appendText( - "error", - "[output guard] " + - (ev.risk_level || "?") + - ": " + - (ev.flags || []).join(","), - { label: "warning" }, - ); + // Anchor the finding to the specific .coord-tool-row that + // tripped the guard so the operator reads call → finding + // adjacency on both live and replay surfaces. Falls back + // to a chat line only when the call_id no longer maps to a + // row (e.g. event arrived after the row was evicted). + if (ev.call_id && toolRows.has(ev.call_id)) { + _attachOutputWarningChip(toolRows.get(ev.call_id).row, { + risk_level: ev.risk_level, + flags: ev.flags, + redacted: ev.redacted, + }); + } else { + appendText( + "info", + "[output guard] " + + (ev.risk_level || "?") + + ": " + + (ev.flags || []).join(","), + { label: "warning" }, + ); + } break; case "error": appendText("error", ev.message || "(unknown error)", { @@ -3759,6 +3823,33 @@ parsedArgs, argsRaw, ); + // Server attaches the persisted intent_verdict to each + // tc on /history (newest-wins per call_id; LLM upgrade + // beats heuristic when both exist). Stamp on the item + // under the field name the render path already consumes + // (judge_verdict for LLM tier, heuristic_verdict + // otherwise) so the verdict pill paints on history rows + // without a render-path fork. Also seed the + // judgeVerdicts cache so a later live SSE event for the + // same call_id reads "already painted" and skips the + // rebuild. + if (tc && tc.verdict) { + if (tc.verdict.tier === "llm") { + item.judge_verdict = tc.verdict; + } else { + item.heuristic_verdict = tc.verdict; + } + if (callId) _cacheJudgeVerdict(callId, tc.verdict); + } + // Output-guard finding — surface as the same + // "[output guard] ..." chat line the live handler emits + // (case "output_warning" above). Stamp on the item so + // the post-batch loop below can read + emit; rendering + // anchored next to the call gives the operator the same + // adjacency they'd see live. + if (tc && tc.output_assessment) { + item.output_assessment = tc.output_assessment; + } // needs_approval is unknown at replay time (the // assistant.tool_calls history payload doesn't persist // the bit). Leave it unset; the upgrade-in-place path @@ -3786,6 +3877,22 @@ } else { appendToolBatch(items, { resolved: { approved: true } }); } + // Output-guard findings — render each one as a chip + // anchored to the .coord-tool-row that tripped the guard + // rather than a generic "[output guard]" chat line. + // Anchored placement preserves per-call adjacency on + // multi-tool batches (live + replay) and the chip's + // severity styling makes the visual weight match the + // verdict pill on the same row. + for (let oi = 0; oi < items.length; oi++) { + const oa = items[oi].output_assessment; + if (!oa || !oa.risk_level || oa.risk_level === "none") continue; + const cid = items[oi].call_id || ""; + if (!cid) continue; + const entry = toolRows.get(cid); + if (!entry || !entry.row) continue; + _attachOutputWarningChip(entry.row, oa); + } } // User messages with attachments arrive as multipart list @@ -3846,7 +3953,14 @@ const toolName = (callId && toolNameByCallId.get(callId)) || m.tool_name || "tool"; const isError = callOutcomes.get(callId) === "error"; - appendToolResult(toolName, callId, content || "", isError); + // Storage truncation surfaces as a sibling pill next to + // the result (see _appendResultToRow's opts.truncated + // branch) rather than as text inside the result body — a + // future "best-effort JSON repair" pass would otherwise + // need to strip a marker string before parsing. + appendToolResult(toolName, callId, content || "", isError, { + truncated: !!m.truncated, + }); // Tool-channel metacog reminders ride the same _reminders // side-channel as the user channel; surface as a themed // bubble below the .coord-tool-batch construct. diff --git a/turnstone/core/history_decoration.py b/turnstone/core/history_decoration.py new file mode 100644 index 00000000..c8fccccf --- /dev/null +++ b/turnstone/core/history_decoration.py @@ -0,0 +1,205 @@ +"""Shared history-replay decoration helpers. + +Both surfaces that build a history wire payload — interactive's SSE +``_build_history`` and the lifted ``make_history_handler`` REST +endpoint — need the same audit-trail data attached to each +``tool_calls`` entry: the persisted intent verdict (``intent_verdicts`` +table) and the output-guard assessment (``output_assessments`` table). + +Centralising the lookup + decoration here keeps the two surfaces from +drifting on which fields ship to the client and how they're shaped. +The shared helpers also let us project only the fields the UI actually +renders, dropping redundant ones (``call_id``/``func_name`` already +carried on ``tc.id``/``tc.name``) so the wire payload stays tight. + +All functions are pure I/O or pure transforms — safe to call from +either an async caller (via ``asyncio.to_thread``) or a sync hook. +""" + +from __future__ import annotations + +import json +from typing import Any + +from turnstone.core.log import get_logger + +log = get_logger(__name__) + +# Tool results are clamped at this length per row at storage time +# (see ``session.py``'s ``store_text = raw_output[:TOOL_RESULT_STORAGE_CAP]``). +# Keeping the constant here lets the truncation flag detection in +# ``decorate_history_messages`` stay in sync without a magic number +# duplicated across server.py / session.py. +# +# Raised from 2000 → 10000 because a 2000-char clip routinely cut +# the body of a single grep / file read mid-line, leaving the +# historical record useless for retrospective debugging. FTS5 +# index + row size grow proportionally; the per-tool upper bound is +# still bounded upstream by ``_truncate_output``'s context-budget +# clamp (so a single huge result can't blow past the live context +# window). +TOOL_RESULT_STORAGE_CAP = 10000 + + +def load_verdict_indexes( + ws_id: str, +) -> tuple[dict[str, dict[str, Any]], dict[str, dict[str, Any]]]: + """Bulk-load intent verdicts and output assessments for a workstream. + + Returns ``(verdicts_by_call_id, assessments_by_call_id)``. Both + tables are indexed by ws_id so the queries are O(rows-for-ws); the + DESC ordering plus first-seen-wins dedupe leaves the newest + verdict per call_id (LLM upgrade beats heuristic when both exist). + + Pure storage I/O — safe to run in ``asyncio.to_thread`` from an + async caller. Returns empty dicts when storage is unavailable or + the lookup raises (best-effort: replay must never block on + audit-trail decoration). + """ + verdicts_by_call_id: dict[str, dict[str, Any]] = {} + assessments_by_call_id: dict[str, dict[str, Any]] = {} + if not ws_id: + return verdicts_by_call_id, assessments_by_call_id + try: + from turnstone.core.storage._registry import get_storage + + storage = get_storage() + if storage is None: + return verdicts_by_call_id, assessments_by_call_id + for v in storage.list_intent_verdicts(ws_id=ws_id, limit=10000): + cid = v.get("call_id") or "" + if cid and cid not in verdicts_by_call_id: + verdicts_by_call_id[cid] = v + for a in storage.list_output_assessments(ws_id=ws_id, limit=10000): + cid = a.get("call_id") or "" + if cid and cid not in assessments_by_call_id: + assessments_by_call_id[cid] = a + except Exception: + # Missing storage / migration drift / driver error must not + # block replay — degrade to an unannotated history. + log.debug( + "verdict/assessment lookup failed; replay continues unannotated", + exc_info=True, + ) + return verdicts_by_call_id, assessments_by_call_id + + +def build_verdict_payload(vrow: dict[str, Any]) -> dict[str, Any] | None: + """Project a stored ``intent_verdicts`` row into the wire shape. + + Returns ``None`` when the verdict is the unflagged baseline + (``risk_level == "none"``) — the client's ``renderVerdictBadge`` + helper would suppress those anyway, so skipping at the wire layer + keeps the payload tight on long workstreams. + + Drops ``call_id`` and ``func_name`` from the wire payload — they're + already carried on the parent ``tc.id`` / ``tc.name`` fields. + Ships ``reasoning`` for either tier when the row has non-empty + prose (heuristic rules in this project DO write meaningful + rationales — e.g. ``policy.py`` emits structured reasoning per + matched pattern). ``judge_model`` rides through so the batch tier + badge can render ``⚖ llm:claude-haiku-4`` on history-only batches + rather than the bare ``⚖ llm`` label. + """ + if (vrow.get("risk_level") or "none") == "none": + return None + payload: dict[str, Any] = { + "risk_level": vrow.get("risk_level", "medium"), + "recommendation": vrow.get("recommendation", "review"), + "confidence": vrow.get("confidence", 0.0), + "intent_summary": vrow.get("intent_summary", ""), + "tier": vrow.get("tier", "heuristic"), + } + if vrow.get("reasoning"): + payload["reasoning"] = vrow.get("reasoning", "") + judge_model = vrow.get("judge_model") or "" + if judge_model: + payload["judge_model"] = judge_model + return payload + + +def build_output_assessment_payload(arow: dict[str, Any]) -> dict[str, Any] | None: + """Project a stored ``output_assessments`` row into the wire shape. + + Returns ``None`` when the assessment is the unflagged baseline + (``risk_level == "none"``) — same skip-on-clean pattern as + :func:`build_verdict_payload`. + + Decodes ``flags`` from its JSON string form here so the client + never has to parse twice. Falls back to an empty list on bad JSON + rather than raising — the rest of the assessment is still useful. + """ + if (arow.get("risk_level") or "none") == "none": + return None + flags_raw = arow.get("flags") or "[]" + try: + flags = json.loads(flags_raw) if isinstance(flags_raw, str) else flags_raw + except (ValueError, TypeError): + flags = [] + return { + "risk_level": arow.get("risk_level", "none"), + "flags": flags if isinstance(flags, list) else [], + "redacted": bool(arow.get("redacted", 0)), + } + + +def decorate_tool_call( + tc: dict[str, Any], + verdicts_by_call_id: dict[str, dict[str, Any]], + assessments_by_call_id: dict[str, dict[str, Any]], +) -> None: + """Mutate ``tc`` in place, attaching ``verdict`` / ``output_assessment``. + + Works on either tool_call shape: + - OpenAI format (``{id, function: {name, arguments}}``) — used by + ``/history`` REST. + - Flattened format (``{id, name, arguments}``) — used by SSE replay. + + Both carry ``id`` at the top level, which is the only field this + helper reads. No-ops cleanly when the call_id has no matching + row (unflagged tools stay clean). + """ + call_id = tc.get("id", "") or "" + if not call_id: + return + vrow = verdicts_by_call_id.get(call_id) + if vrow is not None: + verdict = build_verdict_payload(vrow) + if verdict is not None: + tc["verdict"] = verdict + arow = assessments_by_call_id.get(call_id) + if arow is not None: + assessment = build_output_assessment_payload(arow) + if assessment is not None: + tc["output_assessment"] = assessment + + +def decorate_history_messages( + messages: list[dict[str, Any]], + verdicts_by_call_id: dict[str, dict[str, Any]], + assessments_by_call_id: dict[str, dict[str, Any]], +) -> None: + """Mutate a list of OpenAI-format messages, decorating tool_calls. + + Used by the ``/history`` REST endpoint after ``load_messages`` + returns. For each assistant message with ``tool_calls``, runs + :func:`decorate_tool_call` on every entry. For each tool message + whose content hits the storage cap, sets ``truncated: True`` so + the client can render the "… truncated in storage" pill. + + Pure transform — no I/O. Async callers should pre-load the + indexes via :func:`load_verdict_indexes` (in ``to_thread``) and + pass them in. + """ + for msg in messages: + role = msg.get("role") + if role == "assistant": + tcs = msg.get("tool_calls") + if isinstance(tcs, list): + for tc in tcs: + if isinstance(tc, dict): + decorate_tool_call(tc, verdicts_by_call_id, assessments_by_call_id) + elif role == "tool": + content = msg.get("content") + if isinstance(content, str) and len(content) >= TOOL_RESULT_STORAGE_CAP: + msg["truncated"] = True diff --git a/turnstone/core/session.py b/turnstone/core/session.py index 047f4ed8..8073782c 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -44,6 +44,7 @@ from turnstone.core.attachments import ( ) from turnstone.core.config import get_tavily_key from turnstone.core.edit import find_occurrences, pick_nearest +from turnstone.core.history_decoration import TOOL_RESULT_STORAGE_CAP from turnstone.core.log import get_logger from turnstone.core.memory import ( count_structured_memories, @@ -2414,16 +2415,7 @@ class ChatSession: if assistant_msg.get("_provider_content"): provider_data = json.dumps(assistant_msg["_provider_content"]) - # Build tool_calls JSON (excluding memory tools) - tool_calls_json: str | None = None - if tc: - filtered_tc = [ - call - for call in tc - if call.get("function", {}).get("name", "") not in ("memory", "recall") - ] - if filtered_tc: - tool_calls_json = json.dumps(filtered_tc) + tool_calls_json: str | None = json.dumps(tc) if tc else None # Save assistant message atomically (content + tool_calls in one row) if content or provider_data is not None or tool_calls_json: @@ -2571,27 +2563,27 @@ class ChatSession: tok_est = max(1, int(len(output) / self._chars_per_token)) self._msg_tokens.append(tok_est) - # Log tool result (skip memory tools to avoid noise). - # Use raw_output (pre-advisory-wrap) so DB stores clean - # tool output without ephemeral advisory XML. + # Log tool result. Use raw_output (pre-advisory-wrap) + # so the DB stores clean tool output without ephemeral + # advisory XML. memory/recall persist alongside every + # other tool: replays show the full audit trail, and + # output already passes through _truncate_output above + # so size is bounded by the same budget every other + # tool uses. _tname = _tc_names.get(tc_id, "") - if _tname not in ( - "memory", - "recall", - ): - if isinstance(raw_output, list): - store_text = " ".join( - p.get("text", "") for p in raw_output if p.get("type") == "text" - )[:2000] - else: - store_text = raw_output[:2000] - save_message( - self._ws_id, - "tool", - store_text, - _tname, - tool_call_id=tc_id, - ) + if isinstance(raw_output, list): + store_text = " ".join( + p.get("text", "") for p in raw_output if p.get("type") == "text" + )[:TOOL_RESULT_STORAGE_CAP] + else: + store_text = raw_output[:TOOL_RESULT_STORAGE_CAP] + save_message( + self._ws_id, + "tool", + store_text, + _tname, + tool_call_id=tc_id, + ) # Inject user feedback from approval prompt (e.g. "y, use full path") if user_feedback: self.messages.append({"role": "user", "content": user_feedback}) diff --git a/turnstone/core/session_routes.py b/turnstone/core/session_routes.py index d5ec3b7d..45eb7aa6 100644 --- a/turnstone/core/session_routes.py +++ b/turnstone/core/session_routes.py @@ -394,6 +394,15 @@ class SessionEndpointConfig: # separate ``/history`` endpoint and doesn't render the per-tab # status bar). Kinds that don't need pre-replay wire ``None``. events_replay: EventsReplay | None = None + # async (ws, ui, request) -> None. Kind-specific async pre-step + # the lifted ``events`` body awaits BEFORE iterating + # ``events_replay``. Lets a kind move blocking storage I/O off + # the event loop (via ``asyncio.to_thread``) and stash results + # on ``request.state`` for the sync replay generator to read. + # Interactive uses it to pre-load intent_verdicts + + # output_assessments so ``_build_history``'s decoration stays + # off the hot path. Coord wires ``None``. + events_replay_prepare: Callable[..., Any] | None = None # (request) -> Executor for the SSE live-loop's blocking # ``queue.get`` wait. Interactive returns the dedicated # ``request.app.state.sse_executor`` (200-thread pool) so SSE @@ -1203,6 +1212,8 @@ def make_open_handler( """ async def open_ws(request: Request) -> Response: + import asyncio + if cfg.permission_gate is not None: err = cfg.permission_gate(request) if err is not None: @@ -1304,7 +1315,14 @@ def make_open_handler( # emit_rehydrated path). if cfg.open_post_load is not None: try: - cfg.open_post_load(request, ws) + # Off-loop: interactive's post_load runs the sync + # ``_build_history`` (storage I/O for verdict + # indexes + message reconstruction) — without the + # to_thread wrap this blocks the event loop on every + # workstream open, mirroring the SSE replay path + # that's already protected via + # ``events_replay_prepare``. + await asyncio.to_thread(cfg.open_post_load, request, ws) except Exception: # Post-load is observational — never let a hook bug # block the open. Log + continue. @@ -1454,6 +1472,20 @@ def make_events_handler(cfg: SessionEndpointConfig) -> Handler: # 500-slot cap on a chatty mid-generation workstream) # while replay was being built. if replay_cb is not None: + # Kind-specific async prep — runs before the sync + # replay generator iterates so blocking storage + # I/O lands in the executor pool rather than the + # event loop's hot path. Interactive uses this + # to pre-load verdict indexes; coord skips. + if cfg.events_replay_prepare is not None: + try: + await cfg.events_replay_prepare(ws, ui, request) + except Exception: + log.debug( + "ws.events.replay_prepare_failed ws=%s", + ws_id[:8], + exc_info=True, + ) try: for ev in replay_cb(ws, ui, request): yield {"data": json.dumps(ev)} @@ -2240,6 +2272,34 @@ def make_history_handler(cfg: SessionEndpointConfig) -> Handler: messages = await asyncio.to_thread(storage.load_messages, ws_id, limit=limit) except Exception: log.debug("ws.history.load_failed ws=%s", ws_id[:8], exc_info=True) + # Audit-trail decoration — attach persisted intent_verdict and + # output_assessment data to each assistant.tool_calls entry so + # the dashboard's history replay paints the same verdict pills + # / output-warning bubbles the live SSE path shows. Both + # storage queries are off-loop via ``to_thread``. Best-effort: + # any failure leaves messages undecorated — replay degrades to + # the pre-decoration shape rather than 500-ing. + if messages: + try: + from turnstone.core.history_decoration import ( + decorate_history_messages, + load_verdict_indexes, + ) + + indexes = await asyncio.to_thread(load_verdict_indexes, ws_id) + decorate_history_messages(messages, indexes[0], indexes[1]) + except Exception: + # Operationally interesting: a persistent decoration + # failure (missing migration, driver mismatch, schema + # drift) silently strips verdict pills + output + # warnings from every reload of every workstream. + # Log at warning so it surfaces in normal log review + # rather than only when DEBUG is on. + log.warning( + "ws.history.decoration_failed ws=%s", + ws_id[:8], + exc_info=True, + ) return JSONResponse({"ws_id": ws_id, "messages": messages}) return history diff --git a/turnstone/server.py b/turnstone/server.py index 9c71d2b9..7ea11678 100644 --- a/turnstone/server.py +++ b/turnstone/server.py @@ -53,6 +53,15 @@ from turnstone.core.auth import ( _DenyFilter, jwt_version_slot, ) +from turnstone.core.history_decoration import ( + TOOL_RESULT_STORAGE_CAP, +) +from turnstone.core.history_decoration import ( + decorate_tool_call as _decorate_tool_call, +) +from turnstone.core.history_decoration import ( + load_verdict_indexes as _load_verdict_indexes, +) from turnstone.core.log import get_logger from turnstone.core.metrics import metrics as _metrics from turnstone.core.ratelimit import resolve_client_ip @@ -408,8 +417,20 @@ class WebUI(SessionUIBase): # --------------------------------------------------------------------------- +# Verdict + output-assessment decoration helpers (``_decorate_tool_call``, +# ``_load_verdict_indexes``) are imported at module top alongside the +# rest of ``turnstone.core.*``. Both this builder and +# :func:`make_history_handler` (the /history REST endpoint coord uses +# as its primary history loader) share them so the two surfaces don't +# drift on the wire shape they emit. + + def _build_history( - session: ChatSession, has_pending_approval: bool = False + session: ChatSession, + has_pending_approval: bool = False, + *, + verdicts: dict[str, dict[str, Any]] | None = None, + assessments: dict[str, dict[str, Any]] | None = None, ) -> list[dict[str, Any]]: """Build a history replay list from ChatSession messages. @@ -421,6 +442,12 @@ def _build_history( ``"denied": True``, and the corresponding assistant entry that issued the tool calls is also marked ``"denied": True`` so the client can render the correct badge. + + ``verdicts`` and ``assessments`` are optional pre-loaded + ``{call_id → row}`` dicts (see :func:`_load_verdict_indexes`). + Async callers should pre-load via ``asyncio.to_thread`` and pass + them in to avoid blocking the event loop on storage I/O. When + omitted, the storage call runs inline (sync call sites). """ # Metacognitive nudges live on the message dict's ``_reminders`` # side-channel — user messages carry user-channel nudges @@ -432,6 +459,18 @@ def _build_history( # ``content`` never carries the ```` envelope — # that splice is transient, applied to a wire-bound copy in # ``ChatSession._apply_reminders_for_provider``. + # + # Verdict + output-assessment lookup tables — populated either + # inline (sync call sites) or pre-loaded by an async caller via + # asyncio.to_thread (see _load_verdict_indexes). Pre-loading is + # what keeps _build_history off the event loop's hot path on the + # SSE replay generator path. + if verdicts is not None and assessments is not None: + verdicts_by_call_id = verdicts + assessments_by_call_id = assessments + else: + ws_id = getattr(session, "_ws_id", "") or "" + verdicts_by_call_id, assessments_by_call_id = _load_verdict_indexes(ws_id) history = [] for msg in session.messages: content = msg.get("content") @@ -496,17 +535,45 @@ def _build_history( if clean_reminders: entry["reminders"] = clean_reminders if msg.get("tool_calls"): - entry["tool_calls"] = [ - { - "id": tc.get("id", ""), + tc_entries: list[dict[str, Any]] = [] + for tc in msg["tool_calls"]: + tc_entry: dict[str, Any] = { + "id": tc.get("id", "") or "", "name": tc["function"]["name"], "arguments": tc["function"].get("arguments", ""), } - for tc in msg["tool_calls"] - ] + # Decorate with persisted verdict + output_assessment + # via the shared helper (also used by + # ``make_history_handler``). Skips unflagged + # ("risk_level == 'none'") rows so the wire stays + # tight; ships only the fields the UI renders. + _decorate_tool_call( + tc_entry, + verdicts_by_call_id, + assessments_by_call_id, + ) + tc_entries.append(tc_entry) + entry["tool_calls"] = tc_entries # Detect denied/blocked/errored tool results by their content prefix. if msg.get("role") == "tool": content = msg.get("content", "") + # Propagate tool_call_id so replayHistory can anchor the + # rendered output to the specific .ts-approval-tool element + # by data-call-id (mirrors the live appendToolOutput path). + # Without this, multi-tool batches render every result at + # the bottom of the block rather than under each header. + result_call_id = msg.get("tool_call_id") + if result_call_id: + entry["tool_call_id"] = str(result_call_id) + # Tool results are clamped to TOOL_RESULT_STORAGE_CAP + # chars per row at storage time (session.py). Surface + # that on replay so the user knows the visible output is + # a clipped view of what the live session saw, rather + # than the full result. Reference the shared constant + # rather than a literal so the UI pill logic can't + # silently desync if the cap ever changes. + if isinstance(content, str) and len(content) >= TOOL_RESULT_STORAGE_CAP: + entry["truncated"] = True if isinstance(content, str): if content.startswith("Denied by user") or content.startswith("Blocked"): entry["denied"] = True @@ -747,6 +814,31 @@ def _audit_close_workstream( ) +async def _interactive_events_replay_prepare(ws: Workstream, ui: Any, request: Request) -> None: + """Async pre-step run before ``_interactive_events_replay`` iterates. + + Loads ``intent_verdicts`` + ``output_assessments`` for the + workstream off the event loop (via ``asyncio.to_thread``) and + stashes the result on ``request.state.verdict_indexes``. The sync + replay generator reads from there and passes the dicts into + ``_build_history`` so the storage I/O never blocks the event loop + on the SSE replay path. + + Best-effort: if the workstream has no session or no ws_id, leaves + ``request.state.verdict_indexes`` unset and ``_build_history`` + falls back to the inline storage call (sync path). + """ + del ui # not needed; lookup is keyed on ws.session._ws_id + session = ws.session + if session is None: + return + ws_id = getattr(session, "_ws_id", "") or "" + if not ws_id: + return + indexes = await asyncio.to_thread(_load_verdict_indexes, ws_id) + request.state.verdict_indexes = indexes + + def _interactive_events_replay( ws: Workstream, ui: Any, request: Request ) -> Iterable[dict[str, Any]]: @@ -764,7 +856,6 @@ def _interactive_events_replay( Pure read — never mutates ``ws`` / ``ui`` / ``session``. """ - del request # not needed; replay reads ws/ui/session state session = ws.session if session is None: # Defensive — the lifted body's UI presence check guarantees @@ -779,9 +870,22 @@ def _interactive_events_replay( # History replay — pending-approval flag rides on the last # assistant entry's tool_calls so the client renders them as - # awaiting approval rather than already approved. + # awaiting approval rather than already approved. Verdict / + # assessment indexes were pre-loaded off the event loop by + # _interactive_events_replay_prepare; passing them in here keeps + # _build_history's storage I/O out of the sync generator path. pending_approval = getattr(ui, "_pending_approval", None) - history = _build_history(session, has_pending_approval=pending_approval is not None) + cached_indexes = getattr(request.state, "verdict_indexes", None) + if isinstance(cached_indexes, tuple) and len(cached_indexes) == 2: + verdicts, assessments = cached_indexes + else: + verdicts, assessments = None, None + history = _build_history( + session, + has_pending_approval=pending_approval is not None, + verdicts=verdicts, + assessments=assessments, + ) if history: yield {"type": "history", "messages": history} @@ -1456,13 +1560,13 @@ async def command(request: Request) -> JSONResponse: ui._enqueue({"type": "clear_ui"}) elif cmd_word == "/resume": ui._enqueue({"type": "clear_ui"}) - history = _build_history(ws.session) + history = await asyncio.to_thread(_build_history, ws.session) if history: ui._enqueue({"type": "history", "messages": history}) elif cmd_word in ("/rewind", "/retry"): # Refresh frontend with truncated history ui._enqueue({"type": "clear_ui"}) - history = _build_history(ws.session) + history = await asyncio.to_thread(_build_history, ws.session) if history: ui._enqueue({"type": "history", "messages": history}) # Audit trail @@ -1937,7 +2041,7 @@ async def _interactive_create_post_install( ui = ws.ui if isinstance(ui, WebUI): ui._enqueue({"type": "clear_ui"}) - history = _build_history(ws.session) + history = await asyncio.to_thread(_build_history, ws.session) if history: ui._enqueue({"type": "history", "messages": history}) with contextlib.suppress(queue.Full): @@ -3433,6 +3537,7 @@ def create_app( open_resolve_alias=_resolve_workstream_alias, open_post_load=_interactive_open_post_load, events_replay=_interactive_events_replay, + events_replay_prepare=_interactive_events_replay_prepare, # Pre-lift ``events_sse`` used the dedicated 200-thread # ``sse_executor`` so SSE polling stayed isolated from # every other ``asyncio.to_thread`` caller in the process diff --git a/turnstone/ui/static/app.js b/turnstone/ui/static/app.js index c653edb6..a3a11597 100644 --- a/turnstone/ui/static/app.js +++ b/turnstone/ui/static/app.js @@ -1036,6 +1036,19 @@ Pane.prototype.replayHistory = function (messages) { this.showEmptyState(); return; } + // Suppress the polite live region while we batch-build the replay + // — messagesEl is aria-live="polite" so a fresh replay would otherwise + // queue an announcement for every approved/denied/verdict pill we + // insert. Restored after the loop so live SSE updates announce + // normally. WCAG 4.1.3 — historical content should not behave like + // real-time updates. + this.messagesEl.setAttribute("aria-busy", "true"); + // pendingAssessments[call_id] = output_assessment dict. Populated + // from the assistant branch, consumed by the role==="tool" branch + // (or after the loop, for legacy rows missing tool_call_id). + // Replaces a JSON.stringify→dataset→JSON.parse round-trip with an + // in-memory map keyed by call_id. + var pendingAssessments = {}; var lastToolBlock = null; for (var i = 0; i < messages.length; i++) { var msg = messages[i]; @@ -1052,6 +1065,25 @@ Pane.prototype.replayHistory = function (messages) { } lastToolBlock = null; } else if (msg.role === "assistant") { + // Render content BEFORE the tool block so the visual order + // matches the live SSE flow (stream_text streams content first, + // then tool_info / approve_request paints the tool block, then + // tool_result fills it in). Order also matters structurally: + // the tool-result message in the NEXT iteration anchors via + // lastToolBlock, which the tool-block branch sets last — so + // content must run first to avoid clobbering that anchor. + if (msg.content) { + var el = document.createElement("div"); + el.className = "msg assistant"; + var bodyEl = document.createElement("div"); + bodyEl.className = "msg-body"; + var rendered = renderMarkdown(msg.content); + bodyEl.innerHTML = rendered; + el.appendChild(bodyEl); + postRenderMarkdown(el); + self.messagesEl.appendChild(el); + lastToolBlock = null; + } if (msg.tool_calls && msg.tool_calls.length) { if (msg.pending) { lastToolBlock = null; @@ -1096,7 +1128,35 @@ Pane.prototype.replayHistory = function (messages) { cmd.textContent = tc.arguments.substring(0, 100); } div.appendChild(cmd); + // Verdict badge — anchor to THIS tool's row (div) rather + // than the whole block, so a multi-tool batch with one + // flagged call doesn't drift the badge above unrelated + // calls. Same renderVerdictBadge helper as live; pass + // judgePending=false because any verdict on replay is + // final — no spinner. + if (tc.verdict) { + div.insertAdjacentHTML( + "beforeend", + renderVerdictBadge(tc.verdict, false), + ); + } block.appendChild(div); + // Output-guard finding — defer insertion until the tool + // result lands so the warning anchors under the output + // (mirrors live showOutputWarning placement). Stash in + // a function-local map keyed by call_id so the + // role==="tool" branch below can pick it up; legacy rows + // missing tool_call_id are flushed at end-of-replay. + if ( + tc.output_assessment && + tc.output_assessment.risk_level && + tc.output_assessment.risk_level !== "none" + ) { + pendingAssessments[tc.id || ""] = { + assessment: tc.output_assessment, + toolDiv: div, + }; + } }); var badge = document.createElement("div"); badge.setAttribute("role", "status"); @@ -1112,18 +1172,6 @@ Pane.prototype.replayHistory = function (messages) { lastToolBlock = block; } } - if (msg.content) { - var el = document.createElement("div"); - el.className = "msg assistant"; - var bodyEl = document.createElement("div"); - bodyEl.className = "msg-body"; - var rendered = renderMarkdown(msg.content); - bodyEl.innerHTML = rendered; - el.appendChild(bodyEl); - postRenderMarkdown(el); - self.messagesEl.appendChild(el); - lastToolBlock = null; - } } else if (msg.role === "tool") { if (lastToolBlock) { var stripped = stripAnsi(msg.content || "").trim(); @@ -1132,27 +1180,76 @@ Pane.prototype.replayHistory = function (messages) { /^Denied by user/.test(stripped) || /^Blocked/.test(stripped); var isToolError = !!msg.is_error; + // Anchor the rendered output to the specific .ts-approval-tool + // element matching this result's tool_call_id — mirrors the + // live appendToolOutput path so multi-tool batches show + // [hdr A][out A][hdr B][out B] rather than [A][B][out A][out B]. + // Falls back to "before badge" when tool_call_id is absent + // (legacy rows pre-dating the wire-format addition). + var resultTarget = null; + if (msg.tool_call_id) { + resultTarget = lastToolBlock.querySelector( + '.ts-approval-tool[data-call-id="' + + CSS.escape(msg.tool_call_id) + + '"]', + ); + } + // Cursor-style append: cursor advances after each insert so + // the next sibling lands AFTER the previous one. Fixes the + // bug where calling resultTarget.after(node) twice put the + // second node BETWEEN resultTarget and the first (the second + // .after call was always relative to the same anchor). + // Resulting order with all three present: + // [tool div][output][truncation pill][output-warning] + var insertCursor = resultTarget; + var insertChained = function (node) { + if (insertCursor) { + insertCursor.after(node); + insertCursor = node; + } else { + var bdg = lastToolBlock.querySelector(".ts-approval-badge"); + if (bdg) lastToolBlock.insertBefore(node, bdg); + else lastToolBlock.appendChild(node); + } + }; if (stripped && !isDenied) { var media = !isToolError ? tryParseMedia(stripped) : null; if (media) { - var embed = buildMediaEmbed(media, stripped); - var bdg = lastToolBlock.querySelector(".ts-approval-badge"); - if (bdg) lastToolBlock.insertBefore(embed, bdg); - else lastToolBlock.appendChild(embed); + insertChained(buildMediaEmbed(media, stripped)); } else { var out = renderToolOutput(stripped, isToolError); if (out.textContent.split("\n").length > 10) { makeCollapsible(out); } - var bdg = lastToolBlock.querySelector(".ts-approval-badge"); - if (bdg) lastToolBlock.insertBefore(out, bdg); - else lastToolBlock.appendChild(out); + insertChained(out); + } + // Truncation pill — server marks this when the stored row + // hit the 2000-char cap. Live tool_result events carry full + // output so they don't need the indicator. + if (msg.truncated) { + var pill = document.createElement("span"); + pill.className = "tool-output-truncated"; + pill.textContent = "… truncated in storage"; + pill.title = + "The full tool output was sent to the model live; only the first 10000 characters are persisted to the conversation row."; + insertChained(pill); } } if (isToolError && !lastToolBlock.classList.contains("denied")) { lastToolBlock.classList.add("error"); appendToolErrorBadge(lastToolBlock); } + // Output-guard warning — pull the assessment out of the + // function-local pendingAssessments map (populated in the + // assistant branch). Skip when the tool result was denied — + // the ✗ denied badge already signals the deny path. + if (!isDenied && msg.tool_call_id) { + var pending = pendingAssessments[msg.tool_call_id]; + if (pending) { + insertChained(_buildOutputWarningEl(pending.assessment)); + delete pendingAssessments[msg.tool_call_id]; + } + } } // Tool-channel metacog reminders (tool_error / repeat) attach // to the LAST tool message in a batch; on replay we render the @@ -1165,10 +1262,74 @@ Pane.prototype.replayHistory = function (messages) { } } } + // Flush any output_assessments left in the map — these correspond + // to assistant tool_calls whose tool result row didn't carry a + // tool_call_id (legacy / migrated rows pre-dating the wire-format + // addition). Render the warning under the tool div itself rather + // than dropping the safety information silently. + var leftoverIds = Object.keys(pendingAssessments); + for (var p = 0; p < leftoverIds.length; p++) { + var leftover = pendingAssessments[leftoverIds[p]]; + if (!leftover) continue; + leftover.toolDiv.insertAdjacentElement( + "afterend", + _buildOutputWarningEl(leftover.assessment), + ); + } this._attachRetryToLastAssistant(); this.scrollToBottom(); + // Focus the input so keyboard users land on the next-action target + // after replay finishes — but only when this is the focused pane, + // there's no pending approval competing for focus, and an input + // element actually exists. Skipping when not the focused pane + // avoids stealing focus from another tab the user is interacting + // with while a background replay completes. + if ( + this.id === focusedPaneId && + !this.pendingApproval && + this.inputEl && + !this.busy + ) { + try { + this.inputEl.focus({ preventScroll: true }); + } catch (_) { + this.inputEl.focus(); + } + } + // Restore live-region semantics now that the batch build is done. + this.messagesEl.removeAttribute("aria-busy"); }; +// Shared output-warning DOM builder — used by both replayHistory +// (saved-workstream rendering) and the live appendToolOutput path +// via showOutputWarning. Single source of truth keeps the two +// surfaces from drifting on role / class / escape semantics. +function _buildOutputWarningEl(assessment) { + var risk = (assessment && assessment.risk_level) || "medium"; + var flags = (assessment && assessment.flags) || []; + var warning = document.createElement("div"); + warning.className = "output-warning output-warning-" + risk; + // role="status" (polite) rather than "alert" (assertive) — these + // are findings, not emergencies; the assertive announcement live + // would interrupt the user mid-typing on a high-risk match, which + // is more disruptive than informative. + warning.setAttribute("role", "status"); + var labelEl = document.createElement("span"); + labelEl.className = "output-warning-label"; + labelEl.textContent = "⚠ " + String(risk).toUpperCase(); + warning.appendChild(labelEl); + if (flags.length) { + warning.appendChild(document.createTextNode(" " + flags.join(", "))); + } + if (assessment && assessment.redacted) { + var redacted = document.createElement("span"); + redacted.className = "output-warning-redacted"; + redacted.textContent = " (credentials redacted)"; + warning.appendChild(redacted); + } + return warning; +} + Pane.prototype._attachRetryToLastAssistant = function () { // Remove any previous retry buttons var old = this.messagesEl.querySelectorAll(".msg.assistant .msg-actions"); @@ -1176,6 +1337,21 @@ Pane.prototype._attachRetryToLastAssistant = function () { // Find the last assistant message with content and add retry. // Reasoning blocks emit as .msg.reasoning (distinct modifier) so the // .msg.assistant selector already excludes them — no extra guard needed. + // + // Skip retry attachment when the most recent semantic turn is + // tool-only — last DOM child is a .ts-approval block. Walk back + // past .user-reminder bubbles (added via addToolReminder / + // addUserReminder AFTER the .ts-approval block they advise) so the + // guard fires correctly even when the tool turn carried a metacog + // reminder. Without this skip, retry lands on a stale prior + // assistant content bubble belonging to an earlier turn. + var lastChild = this.messagesEl.lastElementChild; + while (lastChild && lastChild.classList.contains("user-reminder")) { + lastChild = lastChild.previousElementSibling; + } + if (lastChild && lastChild.classList.contains("ts-approval")) { + return; + } var assistants = this.messagesEl.querySelectorAll(".msg.assistant"); if (assistants.length) { this._addRetryAction(assistants[assistants.length - 1]); @@ -1512,20 +1688,14 @@ Pane.prototype.showOutputWarning = function (evt) { '.ts-approval-tool[data-call-id="' + escapedId + '"]', ); if (!toolDiv) return; - var risk = evt.risk_level || "medium"; - var flags = evt.flags || []; - var warning = document.createElement("div"); - warning.className = "output-warning output-warning-" + risk; - warning.setAttribute("role", "alert"); - warning.innerHTML = - '\u26a0 ' + - escapeHtml(risk.toUpperCase()) + - " " + - flags.map(escapeHtml).join(", "); - if (evt.redacted) { - warning.innerHTML += - ' (credentials redacted)'; - } + // Shared DOM-builder with replayHistory \u2014 single source of truth for + // role / class / escape semantics. Argument shape mirrors the + // server-side output_assessment dict (risk_level / flags / redacted). + var warning = _buildOutputWarningEl({ + risk_level: evt.risk_level, + flags: evt.flags, + redacted: evt.redacted, + }); var nextEl = toolDiv.nextElementSibling; if (nextEl && nextEl.classList.contains("tool-output")) { nextEl.insertAdjacentElement("afterend", warning); diff --git a/turnstone/ui/static/style.css b/turnstone/ui/static/style.css index 46c6cd18..4d9849c0 100644 --- a/turnstone/ui/static/style.css +++ b/turnstone/ui/static/style.css @@ -1631,6 +1631,77 @@ body { .ts-approval-tool .tool-diff .diff-warn { color: var(--yellow); } +/* memory/recall calls are background metadata — the audit trail is + valuable but they crowd the narrative when a workstream contains + dozens of them. Dim by default; full opacity on hover/focus so + they remain inspectable without permanently competing for + attention. General-sibling combinator (~) extends the fade past + any verdict-badge or output-warning sitting between the tool row + and its output, so the whole sub-tree fades together rather than + leaving a full-opacity badge stranded next to a dim row. */ +.ts-approval-tool[data-func-name="memory"], +.ts-approval-tool[data-func-name="recall"] { + opacity: 0.55; + transition: opacity 120ms ease-out; +} +.ts-approval-tool[data-func-name="memory"]:hover, +.ts-approval-tool[data-func-name="memory"]:focus-within, +.ts-approval-tool[data-func-name="recall"]:hover, +.ts-approval-tool[data-func-name="recall"]:focus-within { + opacity: 1; +} +.ts-approval-tool[data-func-name="memory"] ~ .tool-output, +.ts-approval-tool[data-func-name="recall"] ~ .tool-output, +.ts-approval-tool[data-func-name="memory"] ~ .media-embed, +.ts-approval-tool[data-func-name="recall"] ~ .media-embed, +.ts-approval-tool[data-func-name="memory"] ~ .output-warning, +.ts-approval-tool[data-func-name="recall"] ~ .output-warning, +.ts-approval-tool[data-func-name="memory"] ~ .tool-output-truncated, +.ts-approval-tool[data-func-name="recall"] ~ .tool-output-truncated { + opacity: 0.55; + transition: opacity 120ms ease-out; +} +/* Reveal on hover OR focus-within across the entire dimmed + subtree. Without :focus-within on the siblings, a keyboard user + tabbing into a link or collapsible toggle inside .tool-output + sees the content remain dimmed — a11y regression. Cover the + warning + truncation pills too so they fully reveal alongside + the result they decorate. */ +.ts-approval-tool[data-func-name="memory"] ~ .tool-output:hover, +.ts-approval-tool[data-func-name="recall"] ~ .tool-output:hover, +.ts-approval-tool[data-func-name="memory"] ~ .tool-output:focus-within, +.ts-approval-tool[data-func-name="recall"] ~ .tool-output:focus-within, +.ts-approval-tool[data-func-name="memory"] ~ .media-embed:hover, +.ts-approval-tool[data-func-name="recall"] ~ .media-embed:hover, +.ts-approval-tool[data-func-name="memory"] ~ .media-embed:focus-within, +.ts-approval-tool[data-func-name="recall"] ~ .media-embed:focus-within, +.ts-approval-tool[data-func-name="memory"] ~ .output-warning:hover, +.ts-approval-tool[data-func-name="recall"] ~ .output-warning:hover, +.ts-approval-tool[data-func-name="memory"] ~ .output-warning:focus-within, +.ts-approval-tool[data-func-name="recall"] ~ .output-warning:focus-within, +.ts-approval-tool[data-func-name="memory"] ~ .tool-output-truncated:hover, +.ts-approval-tool[data-func-name="recall"] ~ .tool-output-truncated:hover, +.ts-approval-tool[data-func-name="memory"] ~ .tool-output-truncated:focus-within, +.ts-approval-tool[data-func-name="recall"] ~ .tool-output-truncated:focus-within { + opacity: 1; +} +/* Truncation indicator — the persisted tool result is clamped at + 2000 chars per row in storage; surface that on replay so users + know they're seeing a clipped view rather than the full output + the live session saw. Aligns with .output-warning's left gutter + (margin-left: 16px) and uses transparent background + dim border + so it reads as quiet metadata rather than a foreign element. */ +.tool-output-truncated { + display: inline-block; + margin-top: 4px; + margin-left: 16px; + padding: 1px 6px; + font-size: 10px; + color: var(--fg-dim); + background: transparent; + border-radius: 3px; + border: 1px solid var(--fg-dim); +} /* .ts-approval (chat.css) stacks its children with flex gap, so a border-top on the body would float above a strip of container background instead of sitting flush against the previous tool row. @@ -2475,38 +2546,13 @@ audio.media-player { } } -/* ========================================================================== - Verdict badges (intent judge) - ========================================================================== */ -.verdict-badge { - padding: 4px 10px; - font-size: 11px; - font-weight: 600; - display: flex; - align-items: center; - gap: 8px; - margin-top: 2px; - /* No top separator — .ts-verdict-badge (chat.css) shrinks to - max-content width, and a 1px border-top would extend only under - the badge text and read as a truncated line. */ -} -.verdict-low { - color: var(--green); - border-left: 3px solid var(--green); -} -.verdict-medium { - color: var(--yellow); - border-left: 3px solid var(--yellow); -} -.verdict-high { - color: var(--red); - border-left: 3px solid var(--red); -} -.verdict-critical { - color: var(--red); - border-left: 3px solid var(--red); - background: rgba(255, 80, 80, 0.05); -} +/* Verdict-badge styling lives in the color-mix block further down + in this file (.verdict-badge.verdict-{low,medium,high,critical}). + The earlier flat-palette duplicate that lived here was removed — + two competing .verdict-badge rule sets caused subtle cascade drift + (the color-mix block won for backgrounds, the flat one won for the + bare .verdict-low/medium/high/critical class names) which made + tweaks fragile. Single source of truth now. */ .verdict-detail { padding: 6px 12px;