From 43e5ef865da5f294c47a5512ec3755f2ca8aaffb Mon Sep 17 00:00:00 2001 From: Aari Date: Wed, 29 Jul 2026 14:01:28 +0800 Subject: [PATCH] fix(frontend): render one workspace-change card per run (#4559) * fix(frontend): render one workspace-change card per run The workspace-change card is resolved from (threadId, runId) alone, so every AI message in a run fetches the identical summary. It was rendered per AI message. getMessageGroups() opens a separate terminal assistant group for every AI message that has content and no tool calls, so a run ends in more than one bubble whenever the model emits answer text mid-run that never gains a tool call. Each bubble then painted a byte-identical "Edited N files" card. Fold the card onto a single position per run, matching how run duration already anchors its own run-scoped display after the run's last visible group. Anchoring keys on group index rather than message id because a terminal assistant group holds exactly one message whose id may be absent. Fixes #4555 * docs(frontend): explain the workspace-change anchor's group-type restriction The helper narrows anchor candidates to terminal assistant groups while getRunDurationDisplaysByGroupIndex accepts a run's last group of any type. That asymmetry is load-bearing: run duration is emitted by MessageList around every group, but the workspace-change card comes from MessageListItem, which MessageList invokes only for human/assistant groups. Anchoring a run that ends in an assistant:processing group would pick a position that never renders and silently drop the card. Record the rule in both places that could invite a future "unification": the helper's docstring, and the frontend AGENTS.md paragraph that tells maintainers where run-scoped displays belong. --- frontend/AGENTS.md | 2 + .../workspace/messages/message-list-item.tsx | 7 +- .../workspace/messages/message-list.tsx | 8 ++ .../core/messages/workspace-change-anchor.ts | 45 +++++++ frontend/tests/e2e/workspace-changes.spec.ts | 97 +++++++++++++++ .../messages/workspace-change-anchor.test.ts | 110 ++++++++++++++++++ 6 files changed, 268 insertions(+), 1 deletion(-) create mode 100644 frontend/src/core/messages/workspace-change-anchor.ts create mode 100644 frontend/tests/unit/core/messages/workspace-change-anchor.test.ts diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index af2c7266d..149ac5ea8 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -75,6 +75,8 @@ The frontend is a stateful chat application. Users create **threads** (conversat Run duration is run-scoped UI metadata even though the compatibility field `additional_kwargs.turn_duration` is repeated on historical AI messages. `core/messages/run-duration.ts` folds those copies into one display anchored after the run's last visible message group. `MessageList` owns the temporary client-side duration for a just-completed live turn until authoritative history arrives. The duration is total run wall-clock time, not per-message reasoning time; reasoning disclosure and run activity/duration are rendered separately. +The workspace-change card follows the same rule: it is resolved from `(threadId, runId)` alone, so every AI message of a run would render an identical copy. A run ends in more than one terminal assistant bubble whenever the model emits answer text that never gains a tool call, so `core/messages/workspace-change-anchor.ts` picks the run's last assistant bubble and `MessageListItem` renders the badge only for that anchor (#4555). Any future run-scoped display belongs in the same place — do not hang one off every message. The two anchor helpers deliberately differ in which group types they accept as a run's last position, because an anchor is only useful where the display is actually rendered: run duration is emitted by `MessageList` around every group, so it accepts any type, while the workspace-change card comes from `MessageListItem` and so restricts to `assistant`. Keep a new helper's candidate set matched to its own render site rather than unifying them. + Composer drafts are tab-scoped browser state. `core/threads/composer-draft.ts` stores only text plus the selected slash-skill name in `sessionStorage`, keyed by user, agent, and logical conversation scope. New-chat pages pass the stable scope `"new"` because their runtime `threadId` is a fresh UUID on every reload; established conversations use their real thread ID. `InputBox` waits for enabled skills before restoring a skill chip, degrades a missing/disabled skill back to editable slash text, and clears the stored draft through `SendMessageOptions.onSent` only after the send passes the in-flight guard. Attachments, sidecar quotes, voice state, and polish undo state are not persisted. Auth UI note: the login page's "keep me signed in" option submits only `remember_me` to the Gateway and may persist only the email address through `core/auth/remember-login.ts`. Passwords and tokens must never be stored in frontend storage; the `HttpOnly access_token` and readable `csrf_token` cookies remain Gateway-owned. diff --git a/frontend/src/components/workspace/messages/message-list-item.tsx b/frontend/src/components/workspace/messages/message-list-item.tsx index d69cab389..1a0c2f008 100644 --- a/frontend/src/components/workspace/messages/message-list-item.tsx +++ b/frontend/src/components/workspace/messages/message-list-item.tsx @@ -145,6 +145,7 @@ export function MessageListItem({ threadId, artifactPaths = [], showCopyButton = true, + showWorkspaceChanges = false, canEdit = false, isEditPending = false, onEditAndRegenerate, @@ -157,6 +158,7 @@ export function MessageListItem({ feedback?: FeedbackData | null; runId?: string; showCopyButton?: boolean; + showWorkspaceChanges?: boolean; canEdit?: boolean; isEditPending?: boolean; onEditAndRegenerate?: (replacementText: string) => void | Promise; @@ -213,6 +215,7 @@ export function MessageListItem({ threadId={threadId} artifactPaths={artifactPaths} runId={runId} + showWorkspaceChanges={showWorkspaceChanges} editState={ isHuman && isEditing ? { @@ -362,6 +365,7 @@ function MessageContent_({ threadId, artifactPaths, runId, + showWorkspaceChanges = false, editState, }: { className?: string; @@ -370,6 +374,7 @@ function MessageContent_({ threadId: string; artifactPaths: readonly string[]; runId?: string; + showWorkspaceChanges?: boolean; editState?: { draft: string; disabled: boolean; @@ -568,7 +573,7 @@ function MessageContent_({ components={components} /> - {message.type === "ai" && ( + {message.type === "ai" && showWorkspaceChanges && ( getRunDurationDisplaysByGroupIndex(groupedMessages), [groupedMessages], ); + const workspaceChangeAnchorGroupIndices = useMemo( + () => getWorkspaceChangeAnchorGroupIndices(groupedMessages), + [groupedMessages], + ); useEffect(() => { setClientDurationsByGroupId((current) => { let next: Map | undefined; @@ -1065,6 +1070,9 @@ export function MessageList({ : undefined } showCopyButton={group.type !== "assistant"} + showWorkspaceChanges={workspaceChangeAnchorGroupIndices.has( + groupIndex, + )} canEdit={ group.type === "human" && Boolean(msg.id) && diff --git a/frontend/src/core/messages/workspace-change-anchor.ts b/frontend/src/core/messages/workspace-change-anchor.ts new file mode 100644 index 000000000..35d9bc39f --- /dev/null +++ b/frontend/src/core/messages/workspace-change-anchor.ts @@ -0,0 +1,45 @@ +import { getMessageRunId } from "./run-duration"; +import type { MessageGroup } from "./utils"; + +/** + * Locate the single UI position that owns each run's workspace-change summary. + * The card is resolved from `(threadId, runId)` alone, so every AI message of a + * run renders an identical copy — and a run ends in more than one terminal + * assistant bubble whenever the model emits intermediate answer text that never + * gains a tool call. Anchor the card on the run's last assistant bubble instead + * of repeating it under each one (#4555). + * + * Returns group indices rather than message ids because terminal assistant + * groups hold exactly one message and that message's id may be absent. + * + * Candidates are restricted to `assistant` groups, unlike + * `getRunDurationDisplaysByGroupIndex`, which accepts a run's last group of any + * type. The asymmetry is load-bearing rather than an oversight: run duration is + * rendered by `MessageList` around every group, while this card is rendered by + * `MessageListItem`, which `MessageList` invokes only for `human`/`assistant` + * groups. Anchoring a run that ends in an `assistant:processing` group would + * pick a position that never renders and silently drop the card, so do not + * unify the two helpers. + */ +export function getWorkspaceChangeAnchorGroupIndices( + groups: MessageGroup[], +): Set { + const anchorByRunId = new Map(); + + groups.forEach((group, groupIndex) => { + if (group.type !== "assistant") { + return; + } + for (const message of group.messages) { + if (message.type !== "ai") { + continue; + } + const runId = getMessageRunId(message); + if (runId) { + anchorByRunId.set(runId, groupIndex); + } + } + }); + + return new Set(anchorByRunId.values()); +} diff --git a/frontend/tests/e2e/workspace-changes.spec.ts b/frontend/tests/e2e/workspace-changes.spec.ts index 8b94260bf..a6d1664f0 100644 --- a/frontend/tests/e2e/workspace-changes.spec.ts +++ b/frontend/tests/e2e/workspace-changes.spec.ts @@ -121,4 +121,101 @@ test.describe("Workspace changes", () => { await expect(page.getByText("+Ready")).toBeVisible(); await expect(page.getByText("-Draft")).toBeVisible(); }); + + test("renders one badge for a run that ends in two assistant bubbles", async ({ + page, + }) => { + // Answer text the model emitted mid-run that never gained a tool call + // settles into its own terminal assistant bubble, so this run owns two. + // The card is resolved from (threadId, runId) alone, so rendering it per + // message painted the identical summary twice (#4555). + mockLangGraphAPI(page, { + threads: [ + { + thread_id: THREAD_ID, + title: "Workspace changes", + updated_at: "2026-07-04T10:00:00Z", + messages: [ + { + type: "human", + id: "msg-human-multi-bubble", + content: [{ type: "text", text: "Create a report" }], + run_id: RUN_ID, + }, + { + type: "ai", + id: "msg-ai-multi-bubble-first", + content: "Let me check the workspace first.", + run_id: RUN_ID, + }, + { + type: "ai", + id: "msg-ai-multi-bubble-final", + content: "I updated the workspace report.", + run_id: RUN_ID, + }, + ], + }, + ], + }); + await page.route( + `**/api/threads/${THREAD_ID}/runs/${RUN_ID}/workspace-changes?*`, + async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + available: true, + version: 1, + summary: { + created: 0, + modified: 1, + deleted: 0, + symlink_created: 0, + additions: 8, + deletions: 2, + truncated: false, + }, + files: [ + { + path: "/mnt/user-data/outputs/report.md", + root: "outputs", + status: "modified", + binary: false, + sensitive: false, + size_before: 12, + size_after: 20, + sha256_before: "before", + sha256_after: "after", + diff: null, + diff_truncated: false, + diff_unavailable_reason: null, + additions: 8, + deletions: 2, + }, + ], + limits: {}, + }), + }); + }, + ); + + await page.goto(`/workspace/chats/${THREAD_ID}`); + + await expect(page.getByText("Edited 1 file")).toBeVisible({ + timeout: 15_000, + }); + await expect(page.getByText("Edited 1 file")).toHaveCount(1); + + // The surviving card belongs to the run's last bubble, matching how run + // duration anchors its own run-scoped display. + const assistantTurns = page.locator("[data-assistant-turn]"); + await expect(assistantTurns).toHaveCount(2); + await expect(assistantTurns.nth(0).getByText("Edited 1 file")).toHaveCount( + 0, + ); + await expect(assistantTurns.nth(1).getByText("Edited 1 file")).toHaveCount( + 1, + ); + }); }); diff --git a/frontend/tests/unit/core/messages/workspace-change-anchor.test.ts b/frontend/tests/unit/core/messages/workspace-change-anchor.test.ts new file mode 100644 index 000000000..2dc4353d1 --- /dev/null +++ b/frontend/tests/unit/core/messages/workspace-change-anchor.test.ts @@ -0,0 +1,110 @@ +import type { Message } from "@langchain/langgraph-sdk"; +import { describe, expect, test } from "@rstest/core"; + +import { getMessageGroups } from "@/core/messages/utils"; +import { getWorkspaceChangeAnchorGroupIndices } from "@/core/messages/workspace-change-anchor"; + +function message( + id: string, + type: Message["type"], + content: string, + runId?: string, +): Message { + return { + id, + type, + content, + ...(runId ? { run_id: runId } : {}), + } as Message; +} + +function toolCall(id: string, runId: string, callId: string): Message { + return { + ...message(id, "ai", "", runId), + tool_calls: [{ id: callId, name: "write_file", args: {} }], + } as Message; +} + +function toolResult(id: string, runId: string, callId: string): Message { + return { + ...message(id, "tool", "ok", runId), + tool_call_id: callId, + } as Message; +} + +describe("workspace change card placement", () => { + test("anchors one card on the run's last assistant bubble", () => { + // A settled run whose model emitted answer text mid-run that never gained a + // tool call: two terminal assistant bubbles share one run_id (#4555). + const groups = getMessageGroups([ + message("human-1", "human", "Update the config"), + message("ai-middle", "ai", "Let me check the repo first.", "run-1"), + toolCall("ai-tool-1", "run-1", "call-1"), + toolResult("tool-1", "run-1", "call-1"), + message("ai-final", "ai", "Done. I updated two files.", "run-1"), + ]); + + expect(groups.map((group) => group.type)).toEqual([ + "human", + "assistant", + "assistant:processing", + "assistant", + ]); + expect([...getWorkspaceChangeAnchorGroupIndices(groups)]).toEqual([3]); + }); + + test("anchors one card per run across consecutive turns", () => { + const groups = getMessageGroups([ + message("human-1", "human", "First task"), + message("ai-1-middle", "ai", "Working on it.", "run-1"), + message("ai-1-final", "ai", "First answer", "run-1"), + message("human-2", "human", "Second task"), + message("ai-2-final", "ai", "Second answer", "run-2"), + ]); + + expect([...getWorkspaceChangeAnchorGroupIndices(groups)]).toEqual([2, 4]); + }); + + test("ignores groups that never render the card", () => { + // Processing groups hold the tool steps and carry the same run_id, but the + // card is rendered only by terminal assistant bubbles. + const groups = getMessageGroups([ + message("human-1", "human", "Just run a tool"), + toolCall("ai-tool-1", "run-1", "call-1"), + toolResult("tool-1", "run-1", "call-1"), + ]); + + expect(getWorkspaceChangeAnchorGroupIndices(groups).size).toBe(0); + }); + + test("skips assistant bubbles without a run id", () => { + // The card resolves from (threadId, runId); a bubble with no run_id cannot + // fetch one and must not claim the anchor. + const groups = getMessageGroups([ + message("human-1", "human", "Hello"), + message("ai-final", "ai", "Hi there"), + ]); + + expect(getWorkspaceChangeAnchorGroupIndices(groups).size).toBe(0); + }); + + test("keeps the streaming bubble anchored while the turn is still loading", () => { + // Mid-stream a content-only message stays in the processing group, so the + // run has no terminal bubble yet and no card position. + const messages = [ + message("human-1", "human", "Update the config"), + message("ai-middle", "ai", "Let me check the repo first.", "run-1"), + ]; + + const streaming = getMessageGroups(messages, { + isCurrentTurnLoading: true, + }); + expect(getWorkspaceChangeAnchorGroupIndices(streaming).size).toBe(0); + + const settled = getMessageGroups([ + ...messages, + message("ai-final", "ai", "Done.", "run-1"), + ]); + expect([...getWorkspaceChangeAnchorGroupIndices(settled)]).toEqual([2]); + }); +});