From 245a1944eafc4929222dbb1f0e697494e81342da Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 5 Jun 2026 11:38:07 +0100 Subject: [PATCH] fix(site): dedupe agent tool timeline entries (#25970) --- .../ConversationTimeline.stories.tsx | 49 ++++++++++ .../ChatConversation/ConversationTimeline.tsx | 4 +- .../ChatConversation/messageHelpers.test.ts | 90 +++++++++++++++++-- .../ChatConversation/messageHelpers.ts | 74 ++++++++++----- 4 files changed, 185 insertions(+), 32 deletions(-) diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx index 0d004362de..550cac3d32 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx @@ -411,6 +411,55 @@ const meta: Meta = { export default meta; type Story = StoryObj; +export const DurableListTemplatesToolLifecycle: Story = { + args: { + ...defaultArgs, + parsedMessages: buildMessages([ + { + ...baseMessage, + id: 1, + role: "user", + content: [{ type: "text", text: "Show me available templates" }], + }, + { + ...baseMessage, + id: 2, + role: "assistant", + content: [ + { + type: "tool-call", + tool_call_id: "list-templates-1", + tool_name: "list_templates", + args: {}, + }, + ], + }, + { + ...baseMessage, + id: 3, + role: "tool", + content: [ + { + type: "tool-result", + tool_call_id: "list-templates-1", + tool_name: "list_templates", + result: { + count: "1", + templates: + '[{"id":"template-1","name":"docker","display_name":"Docker"}]', + }, + }, + ], + }, + ]), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getAllByText("Listed 1 template")).toHaveLength(1); + expect(canvas.queryByText("Listing templates…")).not.toBeInTheDocument(); + }, +}; + /** * User bubbles should stay right-aligned, shrink to fit short content, * and cap long content so the timeline keeps some breathing room. diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx index 83024550f3..7d4e0a6b70 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx @@ -49,8 +49,8 @@ import { import { groupSequentialReadFileBlocks } from "./blockUtils"; import { FileProbeProvider } from "./FileProbeContext"; import { + buildDisplayMessages, deriveMessageDisplayState, - groupSequentialReadFileMessages, } from "./messageHelpers"; import { getEditableUserMessagePayload } from "./messageParsing"; import { useSmoothStreamingText } from "./SmoothText"; @@ -1145,7 +1145,7 @@ export const ConversationTimeline = memo( }); }; - const displayMessages = groupSequentialReadFileMessages(parsedMessages); + const displayMessages = buildDisplayMessages(parsedMessages); const lastInChainFlags = computeLastInChainFlags(displayMessages); if (parsedMessages.length === 0) { diff --git a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts index 1503ae373d..883f72280e 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts @@ -1,10 +1,13 @@ import { describe, expect, it } from "vitest"; import type * as TypesGen from "#/api/typesGenerated"; import { + buildDisplayMessages, deriveMessageDisplayState, - groupSequentialReadFileMessages, } from "./messageHelpers"; -import { parseMessageContent } from "./messageParsing"; +import { + parseMessageContent, + parseMessagesWithMergedTools, +} from "./messageParsing"; import type { MergedTool, ParsedMessageContent, @@ -151,6 +154,21 @@ const executeMessage = (messageID: number): ParsedMessageEntry => { }); }; +const message = ({ + messageID, + role, + content, +}: { + messageID: number; + role: TypesGen.ChatMessageRole; + content: TypesGen.ChatMessagePart[]; +}): TypesGen.ChatMessage => ({ + ...baseMessage, + id: messageID, + role, + content, +}); + describe("deriveMessageDisplayState", () => { it("marks text-only user messages as copyable", () => { const message = buildMessage([{ type: "text", text: "Copy this" }]); @@ -319,18 +337,74 @@ describe("deriveMessageDisplayState", () => { }); }); -describe("groupSequentialReadFileMessages", () => { +describe("buildDisplayMessages", () => { + it("keeps durable tool calls visible after parser-level result merging", () => { + const result = buildDisplayMessages( + parseMessagesWithMergedTools([ + message({ + messageID: 1, + role: "assistant", + content: [ + { + type: "tool-call", + tool_call_id: "list-templates-1", + tool_name: "list_templates", + args: {}, + }, + ], + }), + message({ + messageID: 2, + role: "tool", + content: [ + { + type: "tool-result", + tool_call_id: "list-templates-1", + tool_name: "list_templates", + result: { + count: "1", + templates: '[{"name":"docker","display_name":"Docker"}]', + }, + }, + ], + }), + ]), + ); + + expect(result).toHaveLength(1); + expect(result[0].message.id).toBe(1); + expect(result[0].parsed.tools).toEqual([ + { + id: "list-templates-1", + name: "list_templates", + args: {}, + result: { + count: "1", + templates: '[{"name":"docker","display_name":"Docker"}]', + }, + isError: false, + status: "completed", + mcpServerConfigId: undefined, + modelIntent: undefined, + parsedCommands: undefined, + }, + ]); + expect(result[0].parsed.blocks).toEqual([ + { type: "tool", id: "list-templates-1" }, + ]); + }); + it("returns a single read_file-only message unchanged", () => { const readFile = readFileMessage(1, "read-1"); - const result = groupSequentialReadFileMessages([readFile]); + const result = buildDisplayMessages([readFile]); expect(result).toHaveLength(1); expect(result[0]).toBe(readFile); }); it("collapses read_file-only assistant messages across hidden tool results", () => { - const result = groupSequentialReadFileMessages([ + const result = buildDisplayMessages([ readFileMessage(1, "read-1"), hiddenToolResultMessage(2, "read-1"), readFileMessage(3, "read-2"), @@ -363,7 +437,7 @@ describe("groupSequentialReadFileMessages", () => { ] satisfies Array< [string, ParsedMessageEntry] >)("does not collapse read_file messages across visible %s content", (_, message) => { - const result = groupSequentialReadFileMessages([ + const result = buildDisplayMessages([ readFileMessage(1, "read-1"), message, readFileMessage(3, "read-2"), @@ -388,7 +462,7 @@ describe("groupSequentialReadFileMessages", () => { ] satisfies Array< [string, Partial] >)("does not collapse read_file messages with visible %s", (_, overrides) => { - const result = groupSequentialReadFileMessages([ + const result = buildDisplayMessages([ readFileMessage(1, "read-1"), readFileMessage(2, "read-2", overrides), readFileMessage(3, "read-3"), @@ -398,7 +472,7 @@ describe("groupSequentialReadFileMessages", () => { }); it("does not collapse read_file messages across another visible tool", () => { - const result = groupSequentialReadFileMessages([ + const result = buildDisplayMessages([ readFileMessage(1, "read-1"), executeMessage(2), readFileMessage(3, "read-2"), diff --git a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts index 6a9da7a589..c0cc884cd3 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts @@ -22,6 +22,16 @@ export type MessageDisplayState = { needsAssistantBottomSpacer: boolean; }; +type MessageEntryInput = { + message: TypesGen.ChatMessage; + parsed: ParsedMessageContent; +}; + +type HiddenTimelineEntryReason = + | "tool-result" + | "metadata-only" + | "empty-non-user"; + const isUserInlineRenderBlock = ( block: RenderBlock, ): block is UserInlineRenderBlock => @@ -69,6 +79,44 @@ const getRenderableContentState = (parsed: ParsedMessageContent) => { }; }; +const isToolResultOnlyEntry = ({ + message, + parsed, +}: MessageEntryInput): boolean => + message.role === "tool" && + parsed.toolResults.length > 0 && + parsed.toolCalls.length === 0 && + parsed.markdown === "" && + parsed.reasoning === ""; + +const getHiddenTimelineEntryReason = ({ + message, + parsed, +}: MessageEntryInput): HiddenTimelineEntryReason | undefined => { + const parts = message.content ?? []; + const { hasRenderableContent } = getRenderableContentState(parsed); + + if ( + isToolResultOnlyEntry({ message, parsed }) || + isProviderToolResultOnlyMessage(parts) + ) { + return "tool-result"; + } + + if (isMetadataOnlyMessage(parts)) { + return "metadata-only"; + } + + if (message.role !== "user" && !hasRenderableContent) { + return "empty-non-user"; + } + + return undefined; +}; + +const shouldHideTimelineEntry = (entry: MessageEntryInput): boolean => + getHiddenTimelineEntryReason(entry) !== undefined; + export const deriveMessageDisplayState = ({ message, parsed, @@ -93,8 +141,7 @@ export const deriveMessageDisplayState = ({ const hasFileBlocks = userFileBlocks.length > 0; const hasCopyableContent = Boolean(parsed.markdown.trim()) && !hasFileAttachments; - const { hasRenderableContent, hasThinkingOnlyContent } = - getRenderableContentState(parsed); + const { hasThinkingOnlyContent } = getRenderableContentState(parsed); const needsAssistantBottomSpacer = !hideActions && !hasActiveStream && @@ -102,19 +149,8 @@ export const deriveMessageDisplayState = ({ !isUser && !hasCopyableContent && (hasThinkingOnlyContent || parsed.sources.length > 0); - const hasToolResultsOnly = - parsed.toolResults.length > 0 && - parsed.toolCalls.length === 0 && - parsed.markdown === "" && - parsed.reasoning === ""; - const parts = message.content ?? []; - return { - shouldHide: - hasToolResultsOnly || - isProviderToolResultOnlyMessage(parts) || - isMetadataOnlyMessage(parts) || - (!isUser && !hasRenderableContent), + shouldHide: shouldHideTimelineEntry({ message, parsed }), userInlineContent, userFileBlocks, hasUserMessageBody, @@ -172,7 +208,7 @@ const mergeReadFileMessageGroup = ( // 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 = ( +export const buildDisplayMessages = ( entries: readonly ParsedMessageEntry[], ): ParsedMessageEntry[] => { const grouped: ParsedMessageEntry[] = []; @@ -187,13 +223,7 @@ export const groupSequentialReadFileMessages = ( }; for (const entry of entries) { - const displayState = deriveMessageDisplayState({ - message: entry.message, - parsed: entry.parsed, - hideActions: false, - hasActiveStream: false, - }); - if (displayState.shouldHide) { + if (shouldHideTimelineEntry(entry)) { continue; } if (isReadFileOnlyMessage(entry)) {