From ffa58aa5ef8b512037e32e05c2574e134d1fae36 Mon Sep 17 00:00:00 2001 From: chengyongru <2755839590@qq.com> Date: Sun, 23 Aug 2026 04:42:25 +0800 Subject: [PATCH] fix(webui): keep answer text outside reasoning shell --- nanobot/webui/transcript.py | 27 ++- tests/utils/test_webui_transcript.py | 8 +- .../src/components/thread/ThreadMessages.tsx | 10 +- webui/src/lib/activity-timeline.ts | 119 +++++++------ .../live-replay-event-projection.json | 162 ++++++++++++++++++ webui/src/tests/thread-messages.test.tsx | 103 +++++++++-- 6 files changed, 340 insertions(+), 89 deletions(-) diff --git a/nanobot/webui/transcript.py b/nanobot/webui/transcript.py index 5bacf142c..80703c883 100644 --- a/nanobot/webui/transcript.py +++ b/nanobot/webui/transcript.py @@ -1870,7 +1870,14 @@ def replay_transcript_to_ui_messages( return None return str(last.get("id")) - def demote_interrupted_assistant(segment: str) -> None: + def close_interrupted_assistant() -> None: + """Close an answer segment before tool activity without changing its semantics. + + The wire protocol already marks answer, reasoning, and activity phases. + A later tool event does not turn previously emitted answer text into + reasoning; preserving ``content`` also keeps live and replay projections + equivalent. + """ nonlocal buffer_message_id, buffer_parts for i in range(len(messages) - 1, -1, -1): candidate = messages[i] @@ -1886,19 +1893,7 @@ def replay_transcript_to_ui_messages( or candidate.get("media") ): continue - reasoning_parts = [ - part - for part in (candidate.get("reasoning"), content) - if isinstance(part, str) and part.strip() - ] - messages[i] = { - **candidate, - "content": "", - "reasoning": "\n\n".join(reasoning_parts), - "reasoningStreaming": False, - "isStreaming": False, - "activitySegmentId": candidate.get("activitySegmentId") or segment, - } + messages[i] = {**candidate, "isStreaming": False} if buffer_message_id == candidate.get("id"): buffer_message_id = None buffer_parts = [] @@ -2069,7 +2064,7 @@ def replay_transcript_to_ui_messages( if not segment: segment = _new_activity_segment(activate=False) active_file_edit_segment_id = segment - demote_interrupted_assistant(segment) + close_interrupted_assistant() strip_covered_file_edit_tool_hints_from_recent_messages(edits, turn_fields) target_index = find_file_edit_trace_index(segment, edits) if target_index is not None: @@ -2363,7 +2358,7 @@ def replay_transcript_to_ui_messages( if not trace_lines: continue segment = _ensure_activity_segment() - demote_interrupted_assistant(segment) + close_interrupted_assistant() last = messages[-1] if messages else None if ( last diff --git a/tests/utils/test_webui_transcript.py b/tests/utils/test_webui_transcript.py index bd4893d35..738fd83cd 100644 --- a/tests/utils/test_webui_transcript.py +++ b/tests/utils/test_webui_transcript.py @@ -1485,7 +1485,7 @@ def test_replay_keeps_every_file_from_one_apply_patch_call() -> None: assert [edit["path"] for edit in msgs[0]["fileEdits"]] == ["USER.md", "MEMORY.md"] -def test_replay_keeps_interrupted_pre_tool_text_in_activity() -> None: +def test_replay_keeps_interrupted_pre_tool_text_as_answer() -> None: msgs = replay_transcript_to_ui_messages([ {"event": "delta", "chat_id": "t-stream", "text": "I will inspect first."}, {"event": "stream_end", "chat_id": "t-stream"}, @@ -1504,8 +1504,10 @@ def test_replay_keeps_interrupted_pre_tool_text_in_activity() -> None: assert len(msgs) == 3 assert msgs[0]["role"] == "assistant" - assert msgs[0]["content"] == "" - assert msgs[0]["reasoning"] == "I will inspect first." + assert msgs[0]["content"] == "I will inspect first." + assert msgs[0]["turnPhase"] == "answer" + assert "reasoning" not in msgs[0] + assert "activitySegmentId" not in msgs[0] assert "isStreaming" not in msgs[0] assert msgs[1]["kind"] == "trace" assert msgs[1]["traces"] == ['exec({"cmd":"ls"})'] diff --git a/webui/src/components/thread/ThreadMessages.tsx b/webui/src/components/thread/ThreadMessages.tsx index dd8601b4a..b4d0c410d 100644 --- a/webui/src/components/thread/ThreadMessages.tsx +++ b/webui/src/components/thread/ThreadMessages.tsx @@ -355,11 +355,15 @@ function activeTurnStartIndex(units: DisplayUnit[], activeTurnId: string | null) function displayUnitsEqual(previous: DisplayUnit, next: DisplayUnit): boolean { if (previous.type !== next.type) return false; if (previous.type === "message" && next.type === "message") { - return shallowMessageEqual(previous.message, next.message); + return ( + previous.sourceMessageCount === next.sourceMessageCount + && shallowMessageEqual(previous.message, next.message) + ); } if (previous.type !== "activity" || next.type !== "activity") return false; return ( - previous.turnLatencyMs === next.turnLatencyMs + previous.sourceMessageCount === next.sourceMessageCount + && previous.turnLatencyMs === next.turnLatencyMs && previous.startedAtMs === next.startedAtMs && previous.messages.length === next.messages.length && previous.messages.every((message, index) => @@ -383,7 +387,7 @@ function unitIndexAfterMessageCount( let seen = 0; for (let i = 0; i < units.length; i += 1) { const unit = units[i]; - seen += unit.type === "activity" ? unit.messages.length : 1; + seen += unit.sourceMessageCount; if (seen >= messageCount) return i; } return null; diff --git a/webui/src/lib/activity-timeline.ts b/webui/src/lib/activity-timeline.ts index a209d4463..a8e7778c3 100644 --- a/webui/src/lib/activity-timeline.ts +++ b/webui/src/lib/activity-timeline.ts @@ -1,20 +1,32 @@ import type { UIMessage } from "@/lib/types"; /** A completed turn has two surfaces: one activity container and one final - * answer. An active turn temporarily preserves arrival order so visible - * Markdown never moves when a later tool starts. */ + * answer. Answer text is never inferred to be activity merely because a later + * tool event arrived; explicit reasoning/activity fields own that distinction. */ export type TurnUnit = | { type: "activity"; messages: UIMessage[]; + /** Number of raw UI messages represented by this display unit. */ + sourceMessageCount: number; turnLatencyMs?: number; startedAtMs?: number; } - | { type: "message"; message: UIMessage }; + | { + type: "message"; + message: UIMessage; + /** Number of raw UI messages represented by this display unit. */ + sourceMessageCount: number; + }; export function isReasoningOnlyAssistant(message: UIMessage): boolean { if (message.role !== "assistant" || message.kind === "trace") return false; - if (message.activityKind === "model" || message.content.trim().length > 0) return false; + if ( + message.activityKind === "model" + || message.content.trim().length > 0 + || !!message.media?.length + || !!message.images?.length + ) return false; return !!(message.reasoning?.length || message.reasoningStreaming || message.isStreaming); } @@ -42,10 +54,9 @@ export function hasPendingAgentActivity(messages: UIMessage[]): boolean { * * user → [one live/completed activity surface] → [one final answer] * - * Assistant text that is followed by another activity is an intermediate model - * segment. It remains in the activity timeline, where the renderer keeps its - * normal Markdown surface, but it never creates a second answer bubble. This - * is the same causal model used by Codex-style transcripts. + * A provider may emit multiple answer segments around tool activity. They are + * merged into the one final answer instead of being reclassified as reasoning; + * only explicit reasoning, trace, and model-activity rows enter the fold. */ export function normalizeActivityTimeline( messages: UIMessage[], @@ -63,50 +74,41 @@ export function normalizeActivityTimeline( } const ordered = orderMessagesByTurnSeq(turnMessages); - const lastActivityIndex = ordered.reduce( - (index, message, current) => isRawActivity(message) ? current : index, - -1, - ); - const answerIndices = ordered - .map((message, index) => ({ message, index })) - .filter(({ message }) => isAssistantAnswer(message)); - const finalAnswerIndex = answerIndices.at(-1)?.index; - // A replay can deliver a completed answer before a late trace row. Keep - // that answer visible, but place the late activity in the single activity - // surface before it. An answer followed by more activity is an - // intermediate model segment and stays in that surface in turn order. - const hasFinalAnswer = finalAnswerIndex !== undefined - && (finalAnswerIndex > lastActivityIndex || ordered[finalAnswerIndex].isStreaming !== true); - const activity: UIMessage[] = []; const answers: UIMessage[] = []; - ordered.forEach((message, index) => { + let activitySourceMessageCount = 0; + for (const message of ordered) { if (isRawActivity(message)) { activity.push(message); + activitySourceMessageCount += 1; } else if (isAssistantAnswer(message)) { if (message.reasoning?.trim() || message.reasoningStreaming) { + // The synthetic reasoning row and answer both come from one raw + // message, so account for that source on the answer unit only. activity.push(reasoningOnlyMessageFromAnswer(message)); } - if (hasFinalAnswer && index === finalAnswerIndex) { - answers.push(stripInlineReasoning(message)); - } else { - activity.push(modelActivitySnippet(message)); - } + answers.push(stripInlineReasoning(message)); } else { activity.push(message); + activitySourceMessageCount += 1; } - }); + } if (activity.length) { units.push({ type: "activity", messages: activity, + sourceMessageCount: activitySourceMessageCount, turnLatencyMs: activityTurnLatencyMs(activity, ordered), startedAtMs: activeTurnStartedAtMs, }); } if (answers.length) { - units.push({ type: "message", message: mergeAssistantAnswers(answers) }); + units.push({ + type: "message", + message: mergeAssistantAnswers(answers), + sourceMessageCount: answers.length, + }); } turnMessages = []; @@ -117,7 +119,7 @@ export function normalizeActivityTimeline( for (const message of messages) { if (message.role === "user") { flushTurn(); - units.push({ type: "message", message }); + units.push({ type: "message", message, sourceMessageCount: 1 }); activeTurnId = message.turnId; activeTurnStartedAtMs = validCreatedAtMs(message.createdAt); continue; @@ -132,15 +134,16 @@ export function normalizeActivityTimeline( } /** - * Keep an in-flight turn in arrival order. Until ``turn_end`` there is no - * reliable way to know whether an assistant text segment is the final answer - * or commentary before another tool call. Reclassifying it when that tool - * arrives makes an already-visible Markdown tree jump between containers. + * Keep an in-flight turn in arrival order. Answer, reasoning, and activity + * semantics are explicit, but a later tool event can still arrive after + * already-visible answer Markdown. Reparenting on each event would make that + * Markdown tree jump between containers. * * Completed turns still use ``normalizeActivityTimeline`` and collapse into - * one audit surface plus the final answer. While the turn is active, text and - * contiguous activity phases stay in arrival order. A later tool therefore - * appends a Working surface after existing Markdown instead of reparenting it. + * one audit surface plus the merged answer. While the turn is active, answer + * text and contiguous activity phases stay in arrival order. A later tool + * therefore appends a Working surface after existing Markdown instead of + * reparenting it. */ export function projectActivityTimeline( messages: UIMessage[], @@ -176,23 +179,29 @@ function projectLiveTurn(messages: UIMessage[]): TurnUnit[] { const prompt = messages[0]; const startedAtMs = prompt?.role === "user" ? validCreatedAtMs(prompt.createdAt) : undefined; let activity: UIMessage[] = []; + let activitySourceMessageCount = 0; - if (prompt?.role === "user") units.push({ type: "message", message: prompt }); + if (prompt?.role === "user") { + units.push({ type: "message", message: prompt, sourceMessageCount: 1 }); + } const flushActivity = () => { if (!activity.length) return; units.push({ type: "activity", messages: activity, + sourceMessageCount: activitySourceMessageCount, turnLatencyMs: activityTurnLatencyMs(activity, activity), startedAtMs, }); activity = []; + activitySourceMessageCount = 0; }; for (const message of messages.slice(prompt?.role === "user" ? 1 : 0)) { if (isRawActivity(message)) { activity.push(message); + activitySourceMessageCount += 1; continue; } if (isAssistantAnswer(message)) { @@ -200,10 +209,15 @@ function projectLiveTurn(messages: UIMessage[]): TurnUnit[] { activity.push(reasoningOnlyMessageFromAnswer(message)); } flushActivity(); - units.push({ type: "message", message: stripInlineReasoning(message) }); + units.push({ + type: "message", + message: stripInlineReasoning(message), + sourceMessageCount: 1, + }); continue; } activity.push(message); + activitySourceMessageCount += 1; } flushActivity(); @@ -215,7 +229,16 @@ function isRawActivity(message: UIMessage): boolean { } function isAssistantAnswer(message: UIMessage): boolean { - return message.role === "assistant" && message.kind !== "trace" && message.content.trim().length > 0; + if (message.role !== "assistant" || message.kind === "trace" || message.activityKind === "model") { + return false; + } + if (message.turnPhase === "reasoning" || message.turnPhase === "activity") return false; + return ( + message.turnPhase === "answer" + || message.content.trim().length > 0 + || !!message.media?.length + || !!message.images?.length + ); } function orderMessagesByTurnSeq(messages: UIMessage[]): UIMessage[] { @@ -248,18 +271,6 @@ function mergeAssistantAnswers(answers: UIMessage[]): UIMessage { return merged; } -function modelActivitySnippet(message: UIMessage): UIMessage { - return { - ...stripInlineReasoning(message), - id: `${message.id}-activity`, - activityKind: "model", - turnPhase: "activity", - // Keep the source stream state so the activity surface can render this - // segment with the same Markdown streaming semantics as a normal answer. - isStreaming: message.isStreaming, - }; -} - function reasoningOnlyMessageFromAnswer(message: UIMessage): UIMessage { return { id: `${message.id}-reasoning`, diff --git a/webui/src/tests/fixtures/live-replay-event-projection.json b/webui/src/tests/fixtures/live-replay-event-projection.json index bc2f593c9..89866d5c3 100644 --- a/webui/src/tests/fixtures/live-replay-event-projection.json +++ b/webui/src/tests/fixtures/live-replay-event-projection.json @@ -595,6 +595,168 @@ "turnSeq": 4 } ] + }, + { + "name": "answer_tool_answer_preserves_answer_semantics", + "chat_id": "fixture-interleaved-answer", + "initial_messages": [ + { + "id": "fixture-user-interleaved-answer", + "role": "user", + "content": "Inspect and summarize.", + "turnId": "turn-interleaved-answer", + "turnPhase": "user", + "turnSeq": 1, + "createdAt": 1700000004000 + } + ], + "live_events": [ + { + "event": "delta", + "chat_id": "fixture-interleaved-answer", + "text": "First answer segment.", + "turn_id": "turn-interleaved-answer", + "turn_phase": "answer", + "turn_seq": 2 + }, + { + "event": "stream_end", + "chat_id": "fixture-interleaved-answer", + "text": "First answer segment.", + "turn_id": "turn-interleaved-answer", + "turn_phase": "answer", + "turn_seq": 3 + }, + { + "event": "message", + "chat_id": "fixture-interleaved-answer", + "kind": "tool_hint", + "text": "read_file()", + "turn_id": "turn-interleaved-answer", + "turn_phase": "activity", + "turn_seq": 4 + }, + { + "event": "delta", + "chat_id": "fixture-interleaved-answer", + "text": "Final answer segment.", + "turn_id": "turn-interleaved-answer", + "turn_phase": "answer", + "turn_seq": 5 + }, + { + "event": "stream_end", + "chat_id": "fixture-interleaved-answer", + "text": "Final answer segment.", + "turn_id": "turn-interleaved-answer", + "turn_phase": "answer", + "turn_seq": 6 + }, + { + "event": "turn_end", + "chat_id": "fixture-interleaved-answer", + "latency_ms": 12, + "turn_id": "turn-interleaved-answer", + "turn_phase": "complete", + "turn_seq": 7 + } + ], + "transcript": [ + { + "event": "user", + "chat_id": "fixture-interleaved-answer", + "text": "Inspect and summarize.", + "turn_id": "turn-interleaved-answer", + "turn_phase": "user", + "turn_seq": 1, + "created_at_ms": 1700000004000 + }, + { + "event": "delta", + "chat_id": "fixture-interleaved-answer", + "text": "First answer segment.", + "turn_id": "turn-interleaved-answer", + "turn_phase": "answer", + "turn_seq": 2 + }, + { + "event": "stream_end", + "chat_id": "fixture-interleaved-answer", + "text": "First answer segment.", + "turn_id": "turn-interleaved-answer", + "turn_phase": "answer", + "turn_seq": 3 + }, + { + "event": "message", + "chat_id": "fixture-interleaved-answer", + "kind": "tool_hint", + "text": "read_file()", + "turn_id": "turn-interleaved-answer", + "turn_phase": "activity", + "turn_seq": 4 + }, + { + "event": "delta", + "chat_id": "fixture-interleaved-answer", + "text": "Final answer segment.", + "turn_id": "turn-interleaved-answer", + "turn_phase": "answer", + "turn_seq": 5 + }, + { + "event": "stream_end", + "chat_id": "fixture-interleaved-answer", + "text": "Final answer segment.", + "turn_id": "turn-interleaved-answer", + "turn_phase": "answer", + "turn_seq": 6 + }, + { + "event": "turn_end", + "chat_id": "fixture-interleaved-answer", + "latency_ms": 12, + "turn_id": "turn-interleaved-answer", + "turn_phase": "complete", + "turn_seq": 7 + } + ], + "expected": [ + { + "role": "user", + "content": "Inspect and summarize.", + "turnId": "turn-interleaved-answer", + "turnPhase": "user", + "turnSeq": 1 + }, + { + "role": "assistant", + "content": "First answer segment.", + "turnId": "turn-interleaved-answer", + "turnPhase": "answer", + "turnSeq": 3 + }, + { + "role": "tool", + "content": "read_file()", + "kind": "trace", + "traces": [ + "read_file()" + ], + "activitySegmentId": "segment-1", + "turnId": "turn-interleaved-answer", + "turnPhase": "activity", + "turnSeq": 4 + }, + { + "role": "assistant", + "content": "Final answer segment.", + "latencyMs": 12, + "turnId": "turn-interleaved-answer", + "turnPhase": "answer", + "turnSeq": 6 + } + ] } ] } diff --git a/webui/src/tests/thread-messages.test.tsx b/webui/src/tests/thread-messages.test.tsx index 3ac60b028..f48dc8fbf 100644 --- a/webui/src/tests/thread-messages.test.tsx +++ b/webui/src/tests/thread-messages.test.tsx @@ -334,18 +334,31 @@ describe("ThreadMessages", () => { it("renders a fork boundary divider after the copied history", () => { const messages: UIMessage[] = [ { id: "u1", role: "user", content: "original", createdAt: 1 }, - { id: "a1", role: "assistant", content: "answer", createdAt: 2 }, - { id: "u2", role: "user", content: "branch prompt", createdAt: 3 }, + { id: "a1", role: "assistant", content: "first answer", createdAt: 2 }, + { + id: "t1", + role: "tool", + kind: "trace", + content: "search()", + traces: ["search()"], + createdAt: 3, + }, + { id: "a2", role: "assistant", content: "second answer", createdAt: 4 }, + { id: "u2", role: "user", content: "branch prompt", createdAt: 5 }, ]; - render( + const { container } = render( , ); - expect(screen.getByText("Forked from history")).toBeInTheDocument(); + const rows = Array.from(container.firstElementChild?.children ?? []); + const dividerIndex = rows.findIndex((row) => row.textContent?.includes("Forked from history")); + const branchPromptIndex = rows.findIndex((row) => row.textContent?.includes("branch prompt")); + expect(dividerIndex).toBeGreaterThan(0); + expect(dividerIndex).toBe(branchPromptIndex - 1); }); it("keeps turn unit keys stable across replayed ids and mutable turn sequence", () => { @@ -379,7 +392,6 @@ describe("ThreadMessages", () => { expect(unitKeysForDisplay(liveUnits)).toEqual(unitKeysForDisplay(replayUnits)); expect(unitKeysForDisplay(liveUnits)).toEqual([ "turn-turn-1-user", - "turn-turn-1-activity-1", "turn-turn-1-answer-1", ]); }); @@ -764,7 +776,7 @@ describe("ThreadMessages", () => { expect(screen.queryByText("Worked for 3s")).not.toBeInTheDocument(); }); - it("keeps late activity after the live assistant answer while streaming", () => { + it("keeps a streamed answer outside late activity when the prompt snapshot is absent", () => { const messages: UIMessage[] = [ { id: "t0", @@ -795,17 +807,21 @@ describe("ThreadMessages", () => { const units = buildDisplayUnits(messages, true); - expect(units).toHaveLength(1); + expect(units).toHaveLength(2); expect(units[0].type === "activity" ? units[0].messages.map((m) => m.id) : []).toEqual([ "t0", - "a1-activity", "t1", ]); + expect(units[1]).toMatchObject({ + type: "message", + message: { id: "a1", content: "partial answer" }, + }); render(); const answer = screen.getByText("partial answer"); const liveActivity = screen.getByRole("button", { name: /working/i }); + expect(answer.closest("[data-testid='activity-model-message']")).toBeNull(); expect(liveActivity.compareDocumentPosition(answer) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); }); @@ -1075,7 +1091,7 @@ describe("ThreadMessages", () => { expect(screen.queryByText("Worked for 0s")).not.toBeInTheDocument(); }); - it("projects assistant slices into one answer with one action set", () => { + it("keeps all assistant answer slices outside activity with one action set", () => { const messages: UIMessage[] = [ { id: "early", @@ -1099,6 +1115,18 @@ describe("ThreadMessages", () => { }, ]; + const units = buildDisplayUnits(messages); + expect(units).toHaveLength(2); + expect(units[0].type === "activity" ? units[0].messages.map((m) => m.id) : []).toEqual([ + "t1", + ]); + expect(units[0].sourceMessageCount).toBe(1); + expect(units[1]).toMatchObject({ + type: "message", + message: { id: "early", content: "starting…\n\nfinal reply" }, + sourceMessageCount: 2, + }); + render( { expect(screen.getAllByRole("button", { name: "Copy" })).toHaveLength(1); expect(screen.getAllByRole("button", { name: "Fork" })).toHaveLength(1); - expect(screen.getByText("starting…")).toBeInTheDocument(); - expect(screen.getByText("final reply")).toBeInTheDocument(); + expect(screen.getByText("starting…").closest("[data-testid='activity-model-message']")).toBeNull(); + expect(screen.getByText("final reply").closest("[data-testid='activity-model-message']")).toBeNull(); + }); + + it("keeps a media-only answer slice outside the activity surface", () => { + const messages: UIMessage[] = [ + { + id: "early", + role: "assistant", + content: "generated the file", + turnPhase: "answer", + createdAt: 1, + }, + { + id: "t1", + role: "tool", + kind: "trace", + content: "write_file()", + traces: ["write_file()"], + turnPhase: "activity", + createdAt: 2, + }, + { + id: "attachment", + role: "assistant", + content: "", + media: [{ kind: "file", url: "/api/media/result.csv", name: "result.csv" }], + turnPhase: "answer", + isStreaming: true, + createdAt: 3, + }, + ]; + + const units = buildDisplayUnits(messages); + + expect(units).toHaveLength(2); + expect(units[0].type === "activity" ? units[0].messages.map((m) => m.id) : []).toEqual([ + "t1", + ]); + expect(units[1]).toMatchObject({ + type: "message", + message: { + id: "early", + content: "generated the file", + media: [{ kind: "file", url: "/api/media/result.csv", name: "result.csv" }], + }, + sourceMessageCount: 2, + }); + + render(); + expect(screen.getByText("result.csv")).toBeInTheDocument(); }); it("hides current turn actions until turn_end", () => { @@ -1340,7 +1417,7 @@ describe("ThreadMessages", () => { .filter(Boolean); expect(assistantFlags).toEqual([ - ["a2", true], + ["a1", true], ["a3", true], ]); });