From c56af60d1236079a9bff10561c09111f344a8002 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 26 May 2026 16:19:50 +0100 Subject: [PATCH] feat(site/src/pages/AgentsPage/components): collapse sequential read file events (#25075) --- .../ConversationTimeline.stories.tsx | 305 ++++++++++++++++-- .../ChatConversation/ConversationTimeline.tsx | 114 +++++-- .../StreamingOutput.stories.tsx | 38 +-- .../ChatConversation/blockUtils.test.ts | 75 ++++- .../components/ChatConversation/blockUtils.ts | 49 ++- .../ChatConversation/messageHelpers.test.ts | 291 ++++++++++++++--- .../ChatConversation/messageHelpers.ts | 91 +++++- .../ChatElements/tools/ReadFileTool.tsx | 85 +++-- .../ChatElements/tools/ReadFilesTool.tsx | 98 ++++++ .../components/ChatElements/tools/Tool.tsx | 24 +- 10 files changed, 1002 insertions(+), 168 deletions(-) create mode 100644 site/src/pages/AgentsPage/components/ChatElements/tools/ReadFilesTool.tsx diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx index 0e94afc57a..0d004362de 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx @@ -236,6 +236,104 @@ const buildStoryArgs = (...messages: TypesGen.ChatMessage[]) => ({ parsedMessages: buildMessages(messages), }); +const buildParsedReadFileEntry = ({ + messageId, + toolId, + path, + status, + content = "", + errorMessage, + isError = status === "error", +}: { + messageId: number; + toolId: string; + path: string; + status: "completed" | "error" | "running"; + content?: string; + errorMessage?: string; + isError?: boolean; +}): ParsedMessageEntry => { + const args = { path }; + const result = + content || errorMessage + ? { + ...(content ? { content } : {}), + ...(errorMessage ? { error: errorMessage } : {}), + } + : undefined; + + return { + message: { + ...baseMessage, + id: messageId, + role: "assistant", + content: [ + { + type: "tool-call", + tool_call_id: toolId, + tool_name: "read_file", + args, + }, + ], + }, + parsed: { + markdown: "", + reasoning: "", + toolCalls: [{ id: toolId, name: "read_file", args }], + toolResults: [], + tools: [ + { + id: toolId, + name: "read_file", + args, + result, + isError, + status, + }, + ], + blocks: [{ type: "tool", id: toolId }], + sources: [], + }, + }; +}; + +const buildReadFileExchange = ( + callMessageId: number, + toolId: string, + path: string, + content: string, +): TypesGen.ChatMessage[] => { + const args = { path }; + return [ + { + ...baseMessage, + id: callMessageId, + role: "assistant", + content: [ + { + type: "tool-call", + tool_call_id: toolId, + tool_name: "read_file", + args, + }, + ], + }, + { + ...baseMessage, + id: callMessageId + 1, + role: "tool", + content: [ + { + type: "tool-result", + tool_call_id: toolId, + tool_name: "read_file", + result: { content }, + }, + ], + }, + ]; +}; + const LONG_USER_MESSAGE = [ "This is a deliberately long user message that should stay pinned to the", "right edge while the bubble stops short of filling the entire timeline", @@ -2207,6 +2305,163 @@ export const ThinkingBlockAlwaysCollapsed: Story = { }, }; +export const SequentialReadFilesCollapsed: Story = { + args: { + ...defaultArgs, + parsedMessages: buildMessages([ + { + ...baseMessage, + id: 1, + role: "assistant", + content: [{ type: "text", text: "I'll inspect the relevant files." }], + }, + ...buildReadFileExchange( + 2, + "read-1", + "site/src/a.ts", + "export const a = 1;", + ), + ...buildReadFileExchange( + 4, + "read-2", + "site/src/b.ts", + "export const b = 2;", + ), + ...buildReadFileExchange( + 6, + "read-3", + "site/src/c.ts", + "export const c = 3;", + ), + ]), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const groupButton = canvas.getByRole("button", { name: /read 3 files/i }); + expect(groupButton).toBeInTheDocument(); + expect( + canvas.queryByRole("button", { name: /read a\.ts/i }), + ).not.toBeInTheDocument(); + await userEvent.click(groupButton); + await waitFor(() => { + expect(canvas.getByRole("button", { name: /read a\.ts/i })).toBeVisible(); + expect(canvas.getByRole("button", { name: /read b\.ts/i })).toBeVisible(); + expect(canvas.getByRole("button", { name: /read c\.ts/i })).toBeVisible(); + }); + const firstFileButton = canvas.getByRole("button", { name: /read a\.ts/i }); + expect(firstFileButton).toHaveAttribute("aria-expanded", "false"); + + await userEvent.click(firstFileButton); + await waitFor(() => { + expect(firstFileButton).toHaveAttribute("aria-expanded", "true"); + }); + }, +}; + +export const SequentialReadFilesEmptyAndErrorStates: Story = { + args: { + ...defaultArgs, + parsedMessages: [ + buildParsedReadFileEntry({ + messageId: 1, + toolId: "read-empty-1", + path: "site/src/empty-a.ts", + status: "completed", + }), + buildParsedReadFileEntry({ + messageId: 2, + toolId: "read-empty-2", + path: "site/src/empty-b.ts", + status: "completed", + }), + ...buildMessages([ + { + ...baseMessage, + id: 3, + role: "assistant", + content: [{ type: "text", text: "Trying a different file set." }], + }, + ]), + buildParsedReadFileEntry({ + messageId: 4, + toolId: "read-error-1", + path: "site/src/missing-a.ts", + status: "error", + errorMessage: "ENOENT: no such file or directory", + }), + buildParsedReadFileEntry({ + messageId: 5, + toolId: "read-error-2", + path: "site/src/missing-b.ts", + status: "error", + errorMessage: "permission denied", + }), + ] satisfies ParsedMessageEntry[], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const buttons = canvas.getAllByRole("button", { name: /read 2 files/i }); + expect(buttons).toHaveLength(2); + + await userEvent.click(buttons[0]); + await waitFor(() => { + expect(canvas.getByText("Read empty-a.ts")).toBeVisible(); + expect(canvas.getByText("Read empty-b.ts")).toBeVisible(); + }); + + await userEvent.click(buttons[1]); + await waitFor(() => { + expect( + canvas.getByRole("button", { name: /read missing-a\.ts/i }), + ).toBeVisible(); + expect( + canvas.getByRole("button", { name: /read missing-b\.ts/i }), + ).toBeVisible(); + }); + + await userEvent.click( + canvas.getByRole("button", { name: /read missing-a\.ts/i }), + ); + await waitFor(() => { + expect( + canvas.getByText("ENOENT: no such file or directory"), + ).toBeVisible(); + }); + }, +}; + +export const SequentialReadFilesRunningState: Story = { + args: { + ...defaultArgs, + parsedMessages: [ + buildParsedReadFileEntry({ + messageId: 1, + toolId: "read-running-1", + path: "site/src/one.ts", + status: "running", + }), + buildParsedReadFileEntry({ + messageId: 2, + toolId: "read-running-2", + path: "site/src/two.ts", + status: "running", + }), + buildParsedReadFileEntry({ + messageId: 3, + toolId: "read-running-3", + path: "site/src/three.ts", + status: "running", + }), + ] satisfies ParsedMessageEntry[], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect( + canvas.getByRole("button", { name: /reading 3 files/i }), + ).toBeInTheDocument(); + }, +}; + /** Collapsed thinking should visually align with adjacent tool calls. */ export const ThinkingBlockWithToolCall: Story = { parameters: { @@ -2268,14 +2523,16 @@ export const ThinkingBlockWithToolCall: Story = { const toolButton = canvas.getByRole("button", { name: /read package\.json/i, }); - const thinkingContainer = thinkingButton.closest("[data-transcript-row]"); - const toolContainer = toolButton.closest("[data-transcript-row]"); - expect(thinkingContainer).toBeInstanceOf(HTMLElement); - expect(toolContainer).toBeInstanceOf(HTMLElement); - expect(toolContainer?.firstElementChild).not.toHaveAttribute("data-state"); - expect(thinkingContainer?.firstElementChild).not.toHaveAttribute( - "data-state", - ); + const thinkingContainer = + thinkingButton.closest("[data-transcript-row]") ?? thinkingButton; + const toolContainer = + toolButton.closest("[data-transcript-row]") ?? toolButton; + expect( + toolContainer.firstElementChild ?? toolContainer, + ).not.toHaveAttribute("data-state"); + expect( + thinkingContainer.firstElementChild ?? thinkingContainer, + ).not.toHaveAttribute("data-state"); expect( canvas.queryByTestId("assistant-bottom-spacer"), ).not.toBeInTheDocument(); @@ -2362,30 +2619,20 @@ export const ThinkingBlockWithShellTools: Story = { name: /expand process output/i, }); - const thinkingRow = thinkingButton.closest( - "[data-transcript-row]", - )?.firstElementChild; - const executeRow = executeButton.closest( - "[data-transcript-row]", - )?.firstElementChild; - const processOutputRow = processOutputButton.closest( - "[data-transcript-row]", - )?.firstElementChild; + const wrappers = [ + thinkingButton.closest("[data-transcript-row]") ?? thinkingButton, + executeButton.closest("[data-transcript-row]") ?? executeButton, + processOutputButton.closest("[data-transcript-row]") ?? + processOutputButton, + ]; - expect(thinkingRow).toBeInstanceOf(HTMLElement); - expect(executeRow).toBeInstanceOf(HTMLElement); - expect(processOutputRow).toBeInstanceOf(HTMLElement); - - const rowHeights = [thinkingRow, executeRow, processOutputRow].map((row) => - Math.round((row as HTMLElement).getBoundingClientRect().height), + const rows = wrappers.map( + (wrapper) => wrapper.firstElementChild ?? wrapper, + ); + const rowHeights = rows.map((row) => + Math.round(row.getBoundingClientRect().height), ); expect(new Set(rowHeights)).toHaveLength(1); - - const wrappers = [ - thinkingButton.closest("[data-transcript-row]"), - executeButton.closest("[data-transcript-row]"), - processOutputButton.closest("[data-transcript-row]"), - ].map((row) => row as HTMLElement); const gaps = [ Math.round( wrappers[1].getBoundingClientRect().top - diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx index 5896627f1b..daf1b4c007 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx @@ -32,6 +32,11 @@ import { Tool, } from "../ChatElements"; import { WebSearchSources } from "../ChatElements/tools"; +import { ReadFilesTool } from "../ChatElements/tools/ReadFilesTool"; +import { + getReadFileToolData, + ReadFileTool, +} from "../ChatElements/tools/ReadFileTool"; import type { SubagentVariant } from "../ChatElements/tools/subagentDescriptor"; import { ToolCollapsible } from "../ChatElements/tools/ToolCollapsible"; import { ImageLightbox } from "../ImageLightbox"; @@ -40,8 +45,12 @@ import { AttachmentBlock, type PreviewTextAttachment, } from "./AttachmentBlocks"; +import { groupSequentialReadFileBlocks } from "./blockUtils"; import { FileProbeProvider } from "./FileProbeContext"; -import { deriveMessageDisplayState } from "./messageHelpers"; +import { + deriveMessageDisplayState, + groupSequentialReadFileMessages, +} from "./messageHelpers"; import { getEditableUserMessagePayload } from "./messageParsing"; import { useSmoothStreamingText } from "./SmoothText"; import { getThinkingDisclosureDisplay } from "./thinkingTitle"; @@ -204,6 +213,38 @@ const SmoothedResponse = memo<{ ); }); +const ReadFileTimelineBlock = memo<{ + tools: readonly MergedTool[]; +}>(({ tools }) => { + const [expanded, setExpanded] = useState(false); + const [firstTool] = tools; + if (!firstTool) { + return null; + } + + if (tools.length === 1) { + const readFile = getReadFileToolData(firstTool); + return ( +
+ +
+ ); + } + + return ( + + ); +}); + // Shared block renderer used by both ChatMessageItem (historical // messages) and StreamingOutput (live stream). Encapsulates the // response / thinking / tool / file / sources switch so both @@ -257,16 +298,20 @@ export const BlockList: FC<{ prefQuery.data?.code_diff_display_mode || "auto"; const toolByID = new Map(tools.map((tool) => [tool.id, tool])); + const displayBlocks = groupSequentialReadFileBlocks(blocks, tools); // Pre-compute which tool IDs have a corresponding block so // we can render "remaining" (block-less) tools afterwards. const blockToolIDs = new Set( - blocks - .filter( - (b): b is Extract => - b.type === "tool" && (toolByID.has(b.id) || isStreaming), - ) - .map((b) => b.id), + displayBlocks.flatMap((block) => { + if (block.type === "tool") { + return toolByID.has(block.id) || isStreaming ? [block.id] : []; + } + if (block.type === "tool-group") { + return block.ids; + } + return []; + }), ); const remainingTools = tools.filter((tool) => !blockToolIDs.has(tool.id)); @@ -274,12 +319,13 @@ export const BlockList: FC<{ // A thinking block is actively streaming only when it is the // very last block in the list. Once newer content arrives // (response, tool call, etc.) the thinking phase is over. - const lastBlockIsThinking = - blocks.length > 0 && blocks[blocks.length - 1].type === "thinking"; + const lastDisplayBlockIsThinking = + displayBlocks.length > 0 && + displayBlocks[displayBlocks.length - 1].type === "thinking"; return ( <> - {blocks.map((block, index) => { + {displayBlocks.map((block, index) => { switch (block.type) { case "response": { const responseEl = isStreaming ? ( @@ -311,8 +357,8 @@ export const BlockList: FC<{ text={block.text} isStreaming={ isStreaming && - lastBlockIsThinking && - index === blocks.length - 1 + lastDisplayBlockIsThinking && + index === displayBlocks.length - 1 } urlTransform={urlTransform} thinkingDisplayMode={thinkingDisplayMode} @@ -332,6 +378,21 @@ export const BlockList: FC<{ ); + case "tool-group": { + const groupTools = block.ids + .map((id) => toolByID.get(id)) + .filter((tool) => tool !== undefined); + const [firstGroupTool] = groupTools; + if (!firstGroupTool) { + return null; + } + return ( + + ); + } case "tool": { const tool = toolByID.get(block.id); if (!tool) { @@ -354,6 +415,9 @@ export const BlockList: FC<{ /> ); } + if (tool.name === "read_file") { + return ; + } return ( (parsedMessages.length).fill(false); - let nextVisibleIsUser = true; // no next visible => treat as chain end - for (let i = parsedMessages.length - 1; i >= 0; i--) { - const entry = parsedMessages[i]; - const { shouldHide } = deriveMessageDisplayState({ - message: entry.message, - parsed: entry.parsed, - hideActions: false, - hasActiveStream: false, - isAwaitingFirstStreamChunk: false, - }); + const flags = new Array(displayMessages.length).fill(false); + let nextVisibleIsUser = true; + for (let i = displayMessages.length - 1; i >= 0; i--) { + const entry = displayMessages[i]; if (entry.message.role !== "user") { flags[i] = nextVisibleIsUser; } - if (!shouldHide) { - nextVisibleIsUser = entry.message.role === "user"; - } + nextVisibleIsUser = entry.message.role === "user"; } return flags; } @@ -1086,7 +1141,8 @@ export const ConversationTimeline = memo( }); }; - const lastInChainFlags = computeLastInChainFlags(parsedMessages); + const displayMessages = groupSequentialReadFileMessages(parsedMessages); + const lastInChainFlags = computeLastInChainFlags(displayMessages); if (parsedMessages.length === 0) { return null; @@ -1178,7 +1234,7 @@ export const ConversationTimeline = memo( data-testid="conversation-timeline" className="flex flex-col gap-2" > - {parsedMessages.map(({ message, parsed }, msgIdx) => { + {displayMessages.map(({ message, parsed }, msgIdx) => { if (message.role === "user") { const { shouldHide } = deriveMessageDisplayState({ message, diff --git a/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.stories.tsx b/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.stories.tsx index 44922f69c2..e78762dd6c 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/StreamingOutput.stories.tsx @@ -281,32 +281,28 @@ export const ThinkingDuringStreamingWithToolCalls: Story = { // Tool-only stream chunks can otherwise clear the activity indicator before text arrives. expect(canvas.getAllByText("Thinking").length).toBeGreaterThanOrEqual(1); - const toolCallWrappers = Array.from( - canvasElement.querySelectorAll("[data-transcript-row]"), - ); - expect(toolCallWrappers).toHaveLength(3); + const executeButton = canvas.getByRole("button", { + name: /collapse command/i, + }); + const readFileLabel = canvas.getByText(/reading README\.md/i); + const thinkingText = canvas.getAllByText("Thinking").at(-1); + expect(thinkingText).toBeInstanceOf(HTMLElement); - const thinkingWrapper = toolCallWrappers.at(-1); - const previousToolWrapper = toolCallWrappers.at(-2); - expect(thinkingWrapper).toBeInstanceOf(HTMLElement); - expect(previousToolWrapper).toBeInstanceOf(HTMLElement); - expect(thinkingWrapper).toHaveTextContent("Thinking"); + const wrappers = [ + executeButton.closest("[data-transcript-row]") ?? executeButton, + readFileLabel.closest("[data-tool-call]") ?? readFileLabel, + (thinkingText as HTMLElement).closest("[data-transcript-row]") ?? + (thinkingText as HTMLElement), + ]; + expect(wrappers.at(-1)).toHaveTextContent("Thinking"); const gap = Math.round( - (thinkingWrapper as HTMLElement).getBoundingClientRect().top - - (previousToolWrapper as HTMLElement).getBoundingClientRect().bottom, + wrappers[2].getBoundingClientRect().top - + wrappers[1].getBoundingClientRect().bottom, ); expect(gap).toBe(8); - // The placeholder inner row must match the committed collapsed - // Thinking row height so the transition from streaming to settled - // does not jump. ToolCollapsible enforces min-h-6 (24px). - const placeholderRow = (thinkingWrapper as HTMLElement).firstElementChild; - expect(placeholderRow).toBeInstanceOf(HTMLElement); - expect( - Math.round( - (placeholderRow as HTMLElement).getBoundingClientRect().height, - ), - ).toBe(24); + const placeholderRow = wrappers[2].firstElementChild ?? wrappers[2]; + expect(Math.round(placeholderRow.getBoundingClientRect().height)).toBe(24); }, }; diff --git a/site/src/pages/AgentsPage/components/ChatConversation/blockUtils.test.ts b/site/src/pages/AgentsPage/components/ChatConversation/blockUtils.test.ts index 16955b4b45..b6e3c50fe1 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/blockUtils.test.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/blockUtils.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vitest"; -import { appendTextBlock, asNonEmptyString } from "./blockUtils"; -import type { RenderBlock } from "./types"; +import { + appendTextBlock, + asNonEmptyString, + groupSequentialReadFileBlocks, +} from "./blockUtils"; +import type { MergedTool, RenderBlock } from "./types"; // --------------------------------------------------------------------------- // asNonEmptyString @@ -96,3 +100,70 @@ describe("appendTextBlock", () => { expect(result).not.toBe(blocks); }); }); + +describe("groupSequentialReadFileBlocks", () => { + const tool = (id: string, name = "read_file"): MergedTool => ({ + id, + name, + isError: false, + status: "completed", + }); + const tools = [tool("read-1"), tool("read-2"), tool("execute-1", "execute")]; + + it("collapses consecutive read_file tool blocks", () => { + const result = groupSequentialReadFileBlocks( + [ + { type: "tool", id: "read-1" }, + { type: "tool", id: "read-2" }, + ], + tools, + ); + + expect(result).toEqual([ + { + type: "tool-group", + ids: ["read-1", "read-2"], + }, + ]); + }); + + it("leaves a single read_file tool block ungrouped", () => { + const result = groupSequentialReadFileBlocks( + [{ type: "tool", id: "read-1" }], + tools, + ); + + expect(result).toEqual([{ type: "tool", id: "read-1" }]); + }); + + it.each([ + [ + "response content", + [ + { type: "tool", id: "read-1" }, + { type: "response", text: "middle" }, + { type: "tool", id: "read-2" }, + ], + ], + [ + "another tool", + [ + { type: "tool", id: "read-1" }, + { type: "tool", id: "execute-1" }, + { type: "tool", id: "read-2" }, + ], + ], + [ + "an unresolved tool", + [ + { type: "tool", id: "read-1" }, + { type: "tool", id: "missing" }, + { type: "tool", id: "read-2" }, + ], + ], + ] satisfies Array< + [string, RenderBlock[]] + >)("does not collapse read_file blocks across %s", (_, blocks) => { + expect(groupSequentialReadFileBlocks(blocks, tools)).toEqual(blocks); + }); +}); diff --git a/site/src/pages/AgentsPage/components/ChatConversation/blockUtils.ts b/site/src/pages/AgentsPage/components/ChatConversation/blockUtils.ts index 3d13255139..aeb3442be1 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/blockUtils.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/blockUtils.ts @@ -1,5 +1,5 @@ import { asString } from "../ChatElements/runtimeTypeUtils"; -import type { RenderBlock } from "./types"; +import type { MergedTool, RenderBlock } from "./types"; export const asNonEmptyString = (value: unknown): string | undefined => { const next = asString(value).trim(); @@ -30,3 +30,50 @@ export const appendTextBlock = ( nextBlocks.push({ type, text }); return nextBlocks; }; + +type ToolGroupRenderBlock = { + type: "tool-group"; + ids: string[]; +}; + +type TimelineRenderBlock = RenderBlock | ToolGroupRenderBlock; + +export const groupSequentialReadFileBlocks = ( + blocks: readonly RenderBlock[], + tools: readonly MergedTool[], +): TimelineRenderBlock[] => { + const toolByID = new Map(tools.map((tool) => [tool.id, tool])); + const grouped: TimelineRenderBlock[] = []; + let currentReadFileIDs: string[] = []; + + const flushReadFileIDs = () => { + if (currentReadFileIDs.length === 0) { + return; + } + if (currentReadFileIDs.length === 1) { + grouped.push({ type: "tool", id: currentReadFileIDs[0] }); + } else { + grouped.push({ + type: "tool-group", + ids: currentReadFileIDs, + }); + } + currentReadFileIDs = []; + }; + + for (const block of blocks) { + if (block.type === "tool") { + const tool = toolByID.get(block.id); + if (tool?.name === "read_file") { + currentReadFileIDs.push(block.id); + continue; + } + } + + flushReadFileIDs(); + grouped.push(block); + } + + flushReadFileIDs(); + return grouped; +}; diff --git a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts index bd95a6f6ab..1503ae373d 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts @@ -1,13 +1,20 @@ import { describe, expect, it } from "vitest"; -import type { ChatMessage, ChatMessagePart } from "#/api/typesGenerated"; -import { deriveMessageDisplayState } from "./messageHelpers"; -import { parseMessagesWithMergedTools } from "./messageParsing"; -import type { ParsedMessageContent } from "./types"; +import type * as TypesGen from "#/api/typesGenerated"; +import { + deriveMessageDisplayState, + groupSequentialReadFileMessages, +} from "./messageHelpers"; +import { parseMessageContent } from "./messageParsing"; +import type { + MergedTool, + ParsedMessageContent, + ParsedMessageEntry, +} from "./types"; const buildMessage = ( - content: ChatMessagePart[], + content: TypesGen.ChatMessagePart[], role: "user" | "assistant" = "user", -): ChatMessage => ({ +): TypesGen.ChatMessage => ({ id: 1, chat_id: "chat-1", created_at: "2026-05-11T00:00:00.000Z", @@ -15,21 +22,135 @@ const buildMessage = ( content, }); -const getParsedMessage = (message: ChatMessage) => - parseMessagesWithMergedTools([message])[0].parsed; - const getDisplayState = ( - message: ChatMessage, + message: TypesGen.ChatMessage, overrides: Partial[0]> = {}, ) => deriveMessageDisplayState({ message, - parsed: getParsedMessage(message), + parsed: parseMessageContent(message.content), hideActions: false, hasActiveStream: false, ...overrides, }); +const baseMessage = { + chat_id: "chat", + created_at: "2026-03-10T00:00:00.000Z", +} as const; + +const parsed = ( + overrides: Partial = {}, +): ParsedMessageContent => ({ + markdown: "", + reasoning: "", + toolCalls: [], + toolResults: [], + tools: [], + blocks: [], + sources: [], + ...overrides, +}); + +const entry = ({ + messageID, + role = "assistant", + content = [], + parsedOverrides, +}: { + messageID: number; + role?: TypesGen.ChatMessageRole; + content?: TypesGen.ChatMessagePart[]; + parsedOverrides: Partial; +}): ParsedMessageEntry => ({ + message: { ...baseMessage, id: messageID, role, content }, + parsed: parsed(parsedOverrides), +}); + +const readFileArgs = (id: string) => ({ path: `${id}.ts` }); + +const readFileTool = (id: string): MergedTool => ({ + id, + name: "read_file", + args: readFileArgs(id), + result: { content: id }, + isError: false, + status: "completed", +}); + +const readFileToolResult = (id: string) => ({ + id, + name: "read_file" as const, + result: { content: id }, + isError: false, +}); + +const readFileMessage = ( + messageID: number, + toolID: string, + parsedOverrides: Partial = {}, +): ParsedMessageEntry => { + const args = readFileArgs(toolID); + return entry({ + messageID, + parsedOverrides: { + toolCalls: [{ id: toolID, name: "read_file", args }], + toolResults: [readFileToolResult(toolID)], + tools: [readFileTool(toolID)], + blocks: [{ type: "tool", id: toolID }], + ...parsedOverrides, + }, + }); +}; + +const hiddenToolResultMessage = ( + messageID: number, + toolID: string, +): ParsedMessageEntry => + entry({ + messageID, + role: "tool", + parsedOverrides: { + toolResults: [readFileToolResult(toolID)], + tools: [readFileTool(toolID)], + blocks: [{ type: "tool", id: toolID }], + }, + }); + +const textMessage = ( + messageID: number, + text: string, + role: TypesGen.ChatMessageRole = "assistant", +): ParsedMessageEntry => + entry({ + messageID, + role, + content: [{ type: "text", text }], + parsedOverrides: { + markdown: text, + blocks: [{ type: "response", text }], + }, + }); + +const executeMessage = (messageID: number): ParsedMessageEntry => { + const args = { command: "pnpm test" }; + const tool: MergedTool = { + id: "execute-1", + name: "execute", + args, + isError: false, + status: "completed", + }; + return entry({ + messageID, + parsedOverrides: { + toolCalls: [{ id: tool.id, name: tool.name, args }], + tools: [tool], + blocks: [{ type: "tool", id: tool.id }], + }, + }); +}; + describe("deriveMessageDisplayState", () => { it("marks text-only user messages as copyable", () => { const message = buildMessage([{ type: "text", text: "Copy this" }]); @@ -67,7 +188,7 @@ describe("deriveMessageDisplayState", () => { expect(getDisplayState(message).hasCopyableContent).toBe(false); }); - it("shows the assistant spacer for thinking-only messages when no suppressing flags apply", () => { + it("shows the assistant spacer for reasoning messages when no suppressing flags apply", () => { const message = buildMessage( [{ type: "reasoning", text: "I should think before answering." }], "assistant", @@ -76,40 +197,6 @@ describe("deriveMessageDisplayState", () => { expect(getDisplayState(message).needsAssistantBottomSpacer).toBe(true); }); - it("hides the assistant spacer when thinking is followed by a tool call", () => { - const message = buildMessage( - [ - { type: "reasoning", text: "I should think before acting." }, - { - type: "tool-call", - tool_call_id: "tool-1", - tool_name: "execute", - args: { command: "pnpm storybook --no-open" }, - }, - ], - "assistant", - ); - - expect(getDisplayState(message).needsAssistantBottomSpacer).toBe(false); - }); - - it("shows the assistant spacer when thinking is followed by a hidden execute tool", () => { - const message = buildMessage( - [ - { type: "reasoning", text: "I should think before acting." }, - { - type: "tool-call", - tool_call_id: "tool-1", - tool_name: "execute", - args: {}, - }, - ], - "assistant", - ); - - expect(getDisplayState(message).needsAssistantBottomSpacer).toBe(true); - }); - it("suppresses the assistant spacer while awaiting the first stream chunk", () => { const message = buildMessage( [{ type: "reasoning", text: "I should think before answering." }], @@ -187,13 +274,28 @@ describe("deriveMessageDisplayState", () => { "assistant", ); - expect(getDisplayState(message).shouldHide).toBe(false); + expect( + getDisplayState(message, { + parsed: parsed({ + tools: [ + { + id: "tool-1", + name: "execute", + args: { command: "pnpm test" }, + isError: false, + status: "completed", + }, + ], + blocks: [{ type: "tool", id: "tool-1" }], + }), + }).shouldHide, + ).toBe(false); }); it("hides running wait_agent messages until the chat id is available", () => { const message = buildMessage([], "assistant"); - const parsed: ParsedMessageContent = { - ...getParsedMessage(message), + const parsedContent: ParsedMessageContent = { + ...parseMessageContent(message.content), blocks: [{ type: "tool", id: "wait-1" }], tools: [ { @@ -209,10 +311,99 @@ describe("deriveMessageDisplayState", () => { expect( deriveMessageDisplayState({ message, - parsed, + parsed: parsedContent, hideActions: false, hasActiveStream: false, }).shouldHide, ).toBe(true); }); }); + +describe("groupSequentialReadFileMessages", () => { + it("returns a single read_file-only message unchanged", () => { + const readFile = readFileMessage(1, "read-1"); + + const result = groupSequentialReadFileMessages([readFile]); + + expect(result).toHaveLength(1); + expect(result[0]).toBe(readFile); + }); + + it("collapses read_file-only assistant messages across hidden tool results", () => { + const result = groupSequentialReadFileMessages([ + readFileMessage(1, "read-1"), + hiddenToolResultMessage(2, "read-1"), + readFileMessage(3, "read-2"), + hiddenToolResultMessage(4, "read-2"), + ]); + + expect(result).toHaveLength(1); + expect(result[0].message.id).toBe(1); + expect(result[0].parsed.toolCalls).toEqual([ + { id: "read-1", name: "read_file", args: { path: "read-1.ts" } }, + { id: "read-2", name: "read_file", args: { path: "read-2.ts" } }, + ]); + expect(result[0].parsed.toolResults).toEqual([ + readFileToolResult("read-1"), + readFileToolResult("read-2"), + ]); + expect(result[0].parsed.blocks).toEqual([ + { type: "tool", id: "read-1" }, + { type: "tool", id: "read-2" }, + ]); + expect(result[0].parsed.tools.map((tool) => tool.id)).toEqual([ + "read-1", + "read-2", + ]); + }); + + it.each([ + ["assistant", textMessage(2, "middle")], + ["user", textMessage(2, "middle", "user")], + ] satisfies Array< + [string, ParsedMessageEntry] + >)("does not collapse read_file messages across visible %s content", (_, message) => { + const result = groupSequentialReadFileMessages([ + readFileMessage(1, "read-1"), + message, + readFileMessage(3, "read-2"), + ]); + + expect(result.map((entry) => entry.message.id)).toEqual([1, 2, 3]); + expect(result[0].parsed.blocks).toEqual([{ type: "tool", id: "read-1" }]); + expect(result[2].parsed.blocks).toEqual([{ type: "tool", id: "read-2" }]); + }); + + it.each([ + ["markdown", { markdown: "Visible markdown" }], + ["reasoning", { reasoning: "Visible reasoning" }], + [ + "sources", + { + sources: [ + { url: "https://example.com/read-2", title: "Read 2 source" }, + ], + }, + ], + ] satisfies Array< + [string, Partial] + >)("does not collapse read_file messages with visible %s", (_, overrides) => { + const result = groupSequentialReadFileMessages([ + readFileMessage(1, "read-1"), + readFileMessage(2, "read-2", overrides), + readFileMessage(3, "read-3"), + ]); + + expect(result.map((entry) => entry.message.id)).toEqual([1, 2, 3]); + }); + + it("does not collapse read_file messages across another visible tool", () => { + const result = groupSequentialReadFileMessages([ + readFileMessage(1, "read-1"), + executeMessage(2), + readFileMessage(3, "read-2"), + ]); + + expect(result.map((entry) => entry.message.id)).toEqual([1, 2, 3]); + }); +}); diff --git a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts index c86624416e..6a9da7a589 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts @@ -1,6 +1,10 @@ import type * as TypesGen from "#/api/typesGenerated"; import { shouldRenderTool } from "../ChatElements/tools/toolVisibility"; -import type { ParsedMessageContent, RenderBlock } from "./types"; +import type { + ParsedMessageContent, + ParsedMessageEntry, + RenderBlock, +} from "./types"; export type UserInlineRenderBlock = | Extract @@ -119,3 +123,88 @@ export const deriveMessageDisplayState = ({ needsAssistantBottomSpacer, }; }; + +const isReadFileOnlyMessage = (entry: ParsedMessageEntry): boolean => { + if (entry.message.role !== "assistant") { + return false; + } + if ( + entry.parsed.blocks.length === 0 || + entry.parsed.markdown.trim() || + entry.parsed.reasoning.trim() || + entry.parsed.sources.length > 0 + ) { + return false; + } + + const toolByID = new Map(entry.parsed.tools.map((tool) => [tool.id, tool])); + return entry.parsed.blocks.every( + (block) => + block.type === "tool" && toolByID.get(block.id)?.name === "read_file", + ); +}; + +const mergeReadFileMessageGroup = ( + group: readonly ParsedMessageEntry[], +): ParsedMessageEntry => { + if (group.length === 1) { + return group[0]; + } + + const [first] = group; + return { + message: first.message, + parsed: { + markdown: "", + reasoning: "", + toolCalls: group.flatMap((entry) => entry.parsed.toolCalls), + toolResults: group.flatMap((entry) => entry.parsed.toolResults), + tools: group.flatMap((entry) => entry.parsed.tools), + blocks: group.flatMap((entry) => entry.parsed.blocks), + sources: [], + }, + }; +}; + +// Real transcripts place hidden tool-result-only messages between +// sequential read_file assistant messages. Those hidden entries stay +// transparent so the visible timeline reflects one file-reading run instead +// of one row per persisted message. Synthetic grouped entries deliberately +// render from merged parsed fields because their raw message payload still +// belongs to the first persisted message. +export const groupSequentialReadFileMessages = ( + entries: readonly ParsedMessageEntry[], +): ParsedMessageEntry[] => { + const grouped: ParsedMessageEntry[] = []; + let currentReadFileEntries: ParsedMessageEntry[] = []; + + const flushReadFileEntries = () => { + if (currentReadFileEntries.length === 0) { + return; + } + grouped.push(mergeReadFileMessageGroup(currentReadFileEntries)); + currentReadFileEntries = []; + }; + + for (const entry of entries) { + const displayState = deriveMessageDisplayState({ + message: entry.message, + parsed: entry.parsed, + hideActions: false, + hasActiveStream: false, + }); + if (displayState.shouldHide) { + continue; + } + if (isReadFileOnlyMessage(entry)) { + currentReadFileEntries.push(entry); + continue; + } + + flushReadFileEntries(); + grouped.push(entry); + } + + flushReadFileEntries(); + return grouped; +}; diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ReadFileTool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ReadFileTool.tsx index 8d7243fa91..2459a846b3 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/ReadFileTool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/ReadFileTool.tsx @@ -8,13 +8,60 @@ import { TooltipContent, TooltipTrigger, } from "#/components/Tooltip/Tooltip"; +import { asRecord, asString } from "../runtimeTypeUtils"; import { ToolCollapsible } from "./ToolCollapsible"; import { DIFFS_FONT_STYLE, getFileViewerOptionsMinimal, + parseArgs, type ToolStatus, } from "./utils"; +const ReadFileContent: React.FC<{ + path: string; + content: string; +}> = ({ path, content }) => { + const theme = useTheme(); + const isDark = theme.palette.mode === "dark"; + + return ( + + + + ); +}; + +export const getReadFileToolData = ({ + args, + result, + isError, +}: { + args?: unknown; + result?: unknown; + isError: boolean; +}) => { + const parsedArgs = parseArgs(args); + const path = parsedArgs ? asString(parsedArgs.path).trim() : ""; + const rec = asRecord(result); + return { + path: path || "file", + content: rec ? asString(rec.content).trim() : "", + isError, + errorMessage: rec ? asString(rec.error || rec.message) : undefined, + }; +}; + /** * Collapsed-by-default rendering for `read_file` tool calls. Shows * "Read " with a chevron; expanding reveals the file viewer. @@ -25,10 +72,18 @@ export const ReadFileTool: React.FC<{ status: ToolStatus; isError: boolean; errorMessage?: string; -}> = ({ path, content, status, isError, errorMessage }) => { - const theme = useTheme(); - const isDark = theme.palette.mode === "dark"; - const hasContent = content.length > 0; + expanded?: boolean; + onExpandedChange?: (expanded: boolean) => void; +}> = ({ + path, + content, + status, + isError, + errorMessage, + expanded, + onExpandedChange, +}) => { + const hasContent = content.length > 0 || isError; const isRunning = status === "running"; const filename = path.split("/").pop() || path; const label = isRunning ? `Reading ${filename}…` : `Read ${filename}`; @@ -37,6 +92,8 @@ export const ReadFileTool: React.FC<{ {label} @@ -56,20 +113,12 @@ export const ReadFileTool: React.FC<{ } > - - - + {isError && ( +
+ {errorMessage || "Failed to read file"} +
+ )} + {content.length > 0 && } ); }; diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/ReadFilesTool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/ReadFilesTool.tsx new file mode 100644 index 0000000000..93473bdb73 --- /dev/null +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/ReadFilesTool.tsx @@ -0,0 +1,98 @@ +import { LoaderIcon, TriangleAlertIcon } from "lucide-react"; +import { type FC, useState } from "react"; +import { + Tooltip, + TooltipContent, + TooltipTrigger, +} from "#/components/Tooltip/Tooltip"; +import type { MergedTool } from "../../ChatConversation/types"; +import { getReadFileToolData, ReadFileTool } from "./ReadFileTool"; +import { ToolCollapsible } from "./ToolCollapsible"; + +type ReadFileItem = { + id: string; + path: string; + content: string; + status: MergedTool["status"]; + isError: boolean; + errorMessage?: string; +}; + +const getReadFileItem = (tool: MergedTool): ReadFileItem => ({ + id: tool.id, + status: tool.status, + ...getReadFileToolData(tool), +}); + +export const ReadFilesTool: FC<{ + tools: readonly MergedTool[]; + expanded?: boolean; + onExpandedChange?: (expanded: boolean) => void; +}> = ({ tools, expanded, onExpandedChange }) => { + const [expandedFileIDs, setExpandedFileIDs] = useState>( + new Set(), + ); + const items = tools.map(getReadFileItem); + const isRunning = tools.some((tool) => tool.status === "running"); + const isError = tools.some((tool) => tool.isError); + const hasContent = items.length > 0; + const label = isRunning + ? `Reading ${tools.length} files…` + : `Read ${tools.length} files`; + const errorMessage = items.find((item) => item.errorMessage)?.errorMessage; + + return ( +
+ + {label} + {isError && ( + + + + + + {errorMessage || "Failed to read one or more files"} + + + )} + {isRunning && ( + + )} + + } + > +
+ {items.map((item) => ( +
+ { + setExpandedFileIDs((previous) => { + const next = new Set(previous); + if (nextExpanded) { + next.add(item.id); + } else { + next.delete(item.id); + } + return next; + }); + }} + /> +
+ ))} +
+ +
+ ); +}; diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx index e854718a7b..10ea071eda 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/Tool.tsx @@ -27,7 +27,7 @@ import { import { ListTemplatesTool } from "./ListTemplatesTool"; import { ProcessOutputTool } from "./ProcessOutputTool"; import { ProposePlanTool } from "./ProposePlanTool"; -import { ReadFileTool } from "./ReadFileTool"; +import { getReadFileToolData, ReadFileTool } from "./ReadFileTool"; import { ReadSkillTool } from "./ReadSkillTool"; import { ReadTemplateTool } from "./ReadTemplateTool"; import { StartWorkspaceTool } from "./StartWorkspaceTool"; @@ -315,22 +315,12 @@ const ReadFileRenderer: FC = ({ args, result, isError, -}) => { - const parsedArgs = parseArgs(args); - const path = parsedArgs ? asString(parsedArgs.path).trim() : ""; - const rec = asRecord(result); - const content = rec ? asString(rec.content).trim() : ""; - - return ( - - ); -}; +}) => ( + +); const ReadSkillRenderer: FC = ({ status,