fix(site): dedupe agent tool timeline entries (#25970)

This commit is contained in:
Danielle Maywood
2026-06-05 11:38:07 +01:00
committed by GitHub
parent c4792cf104
commit 245a1944ea
4 changed files with 185 additions and 32 deletions
@@ -411,6 +411,55 @@ const meta: Meta<typeof ConversationTimeline> = {
export default meta;
type Story = StoryObj<typeof ConversationTimeline>;
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.
@@ -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<ConversationTimelineProps>(
});
};
const displayMessages = groupSequentialReadFileMessages(parsedMessages);
const displayMessages = buildDisplayMessages(parsedMessages);
const lastInChainFlags = computeLastInChainFlags(displayMessages);
if (parsedMessages.length === 0) {
@@ -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<ParsedMessageContent>]
>)("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"),
@@ -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)) {