From 2f5d6b30f6a65f60dc448c01b19006efb385fe23 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 24 May 2026 16:09:04 -0500 Subject: [PATCH 1/9] =?UTF-8?q?=F0=9F=A4=96=20feat:=20coalesce=20consecuti?= =?UTF-8?q?ve=20file=5Fread/file=5Fedit=20tool=20calls=20in=20transcript?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a rule-based coalescing pass that hides bursts of consecutive file_read or file_edit_* tool calls behind a single 'Read files a, b' or 'Wrote files a, b' summary row. The summary replaces the head call's slot so the 1 -> N transition is a content swap rather than a row insertion, eliminating layout flash when the second member streams in. Clicking the summary expands the group and shows each individual tool call exactly as before. Layered on top of the existing bash_output coalescing pattern: - computeToolCoalesceInfos returns aligned head/member metadata once per message snapshot (O(n)). - CoalescedToolCall is a presentational summary row sized to match a collapsed FileReadToolCall so the swap is dimensionally stable. - ChatPane owns the expansion-state Set, keyed by the head's message id, and resets it on workspace switch (mirrors expandedBashGroups). Why only file_read and file_edit for now: those are the two highest- volume read/write tools, and they already share extractToolFilePath as a path source. Future kinds can plug in by extending ToolCoalesceKind. --- src/browser/components/ChatPane/ChatPane.tsx | 103 +++++++-- .../Tools/CoalescedToolCall.stories.tsx | 78 +++++++ .../features/Tools/CoalescedToolCall.test.tsx | 103 +++++++++ .../features/Tools/CoalescedToolCall.tsx | 91 ++++++++ .../utils/messages/toolCoalescing.test.ts | 210 ++++++++++++++++++ src/browser/utils/messages/toolCoalescing.ts | 133 +++++++++++ 6 files changed, 699 insertions(+), 19 deletions(-) create mode 100644 src/browser/features/Tools/CoalescedToolCall.stories.tsx create mode 100644 src/browser/features/Tools/CoalescedToolCall.test.tsx create mode 100644 src/browser/features/Tools/CoalescedToolCall.tsx create mode 100644 src/browser/utils/messages/toolCoalescing.test.ts create mode 100644 src/browser/utils/messages/toolCoalescing.ts diff --git a/src/browser/components/ChatPane/ChatPane.tsx b/src/browser/components/ChatPane/ChatPane.tsx index b907b7e74d..1724921c22 100644 --- a/src/browser/components/ChatPane/ChatPane.tsx +++ b/src/browser/components/ChatPane/ChatPane.tsx @@ -32,8 +32,10 @@ import { shouldBypassDeferredMessages, } from "@/browser/utils/messages/messageUtils"; import { computeTaskReportLinking } from "@/browser/utils/messages/taskReportLinking"; +import { computeToolCoalesceInfos } from "@/browser/utils/messages/toolCoalescing"; import { BashCollapsedSummaryModeProvider } from "@/browser/features/Tools/BashCollapsedSummaryModeContext"; import { BashOutputCollapsedIndicator } from "@/browser/features/Tools/BashOutputCollapsedIndicator"; +import { CoalescedToolCall } from "@/browser/features/Tools/CoalescedToolCall"; import { getInterruptionContext, getLastMainRetryCandidateMessage, @@ -338,6 +340,13 @@ const ChatPaneContent: React.FC = (props) => { // Track which bash_output groups are expanded (keyed by first message ID) const [expandedBashGroups, setExpandedBashGroups] = useState>(new Set()); + // Track which tool-coalesce groups (file_read / file_edit bursts) the user + // has expanded. Keyed by the head message ID so the entry survives later + // additions to the same group without changing identity. + const [expandedToolCoalesceGroups, setExpandedToolCoalesceGroups] = useState>( + new Set() + ); + // Extract state from workspace state // Keep a ref to the latest workspace state so event handlers (passed to memoized children) @@ -429,6 +438,12 @@ const ChatPaneContent: React.FC = (props) => { [deferredMessages] ); + // Precompute tool-coalesce metadata (file_read / file_edit bursts) once per snapshot. + const toolCoalesceInfos = useMemo( + () => computeToolCoalesceInfos(deferredMessages), + [deferredMessages] + ); + const autoCompactionResult = useMemo( () => checkAutoCompaction( @@ -681,6 +696,7 @@ const ChatPaneContent: React.FC = (props) => { useEffect(() => { setEditingState({ workspaceId, message: undefined }); setExpandedBashGroups(new Set()); + setExpandedToolCoalesceGroups(new Set()); }, [workspaceId]); const handleChatInputReady = useCallback((api: ChatInputAPI) => { @@ -1132,6 +1148,21 @@ const ChatPaneContent: React.FC = (props) => { return null; } + // Tool-coalesce groups (file_read / file_edit bursts). The head + // call is replaced by a summary row when collapsed; members are + // hidden entirely until the user expands the group. + const toolCoalesceGroup = toolCoalesceInfos[index]; + const coalesceHeadId = toolCoalesceGroup + ? deferredMessages[toolCoalesceGroup.headIndex]?.id + : undefined; + const isToolCoalesceExpanded = coalesceHeadId + ? expandedToolCoalesceGroups.has(coalesceHeadId) + : false; + + if (toolCoalesceGroup?.position === "member" && !isToolCoalesceExpanded) { + return null; + } + const isAtCutoff = editCutoffHistoryId !== undefined && msg.type !== "history-hidden" && @@ -1145,27 +1176,61 @@ const ChatPaneContent: React.FC = (props) => { ? taskReportLinking : undefined; + // Render order at the head of a coalesced group: + // - collapsed: summary row replaces the head's MessageRenderer. + // - expanded: summary row sits at the top (acts as the + // collapse toggle) and the head's normal + // MessageRenderer renders directly below, with + // the rest of the group's members following on + // subsequent iterations. + const renderCoalesceSummary = + toolCoalesceGroup?.position === "head" && coalesceHeadId; + const renderNormalMessage = !renderCoalesceSummary || isToolCoalesceExpanded; + const toggleCoalesceGroup = + coalesceHeadId !== undefined + ? () => + setExpandedToolCoalesceGroups((prev) => { + const next = new Set(prev); + if (next.has(coalesceHeadId)) { + next.delete(coalesceHeadId); + } else { + next.add(coalesceHeadId); + } + return next; + }) + : undefined; + return ( - + {renderCoalesceSummary && toolCoalesceGroup && toggleCoalesceGroup && ( + + )} + {renderNormalMessage && ( + + )} {/* Show collapsed indicator after the first item in a bash_output group */} {bashOutputGroup?.position === "first" && groupKey && ( ( +
+
+ +
+
+ ), + ], +} satisfies Meta; + +export default meta; + +type Story = StoryObj; + +function InteractiveCoalesced( + args: Omit, "expanded" | "onToggle"> +) { + const [expanded, setExpanded] = useState(false); + return ( + setExpanded((e) => !e)} /> + ); +} + +export const TwoReads: Story = { + args: { + kind: "file_read", + filePaths: ["src/App.tsx", "src/main.ts"], + expanded: false, + onToggle: () => undefined, + }, + render: (args) => , +}; + +export const ManyReads: Story = { + args: { + kind: "file_read", + filePaths: [ + "src/App.tsx", + "src/main.ts", + "src/preload.ts", + "src/config.ts", + "src/browser/features/Tools/CoalescedToolCall.tsx", + ], + expanded: false, + onToggle: () => undefined, + }, + render: (args) => , +}; + +export const TwoEdits: Story = { + args: { + kind: "file_edit", + filePaths: ["src/App.tsx", "src/main.ts"], + expanded: false, + onToggle: () => undefined, + }, + render: (args) => , +}; + +export const ExpandedReads: Story = { + args: { + kind: "file_read", + filePaths: ["src/App.tsx", "src/main.ts", "src/preload.ts"], + expanded: true, + onToggle: () => undefined, + }, +}; diff --git a/src/browser/features/Tools/CoalescedToolCall.test.tsx b/src/browser/features/Tools/CoalescedToolCall.test.tsx new file mode 100644 index 0000000000..19433fce2e --- /dev/null +++ b/src/browser/features/Tools/CoalescedToolCall.test.tsx @@ -0,0 +1,103 @@ +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; +import { cleanup, fireEvent, render } from "@testing-library/react"; +import { GlobalWindow } from "happy-dom"; + +import { TooltipProvider } from "@/browser/components/Tooltip/Tooltip"; + +import { CoalescedToolCall } from "./CoalescedToolCall"; + +let windowInstance: GlobalWindow | null = null; + +beforeEach(() => { + windowInstance = new GlobalWindow(); + globalThis.window = windowInstance as unknown as Window & typeof globalThis; + globalThis.document = windowInstance.document as unknown as Document; +}); + +afterEach(() => { + cleanup(); + void windowInstance?.happyDOM.abort(); + windowInstance = null; + delete (globalThis as { window?: Window }).window; + delete (globalThis as { document?: Document }).document; +}); + +describe("CoalescedToolCall", () => { + test("renders 'Read files ' summary when collapsed", () => { + const view = render( + + {}} + /> + + ); + + expect(view.getByText(/Read files/)).toBeTruthy(); + expect(view.getByText("src/App.tsx, src/main.ts")).toBeTruthy(); + }); + + test("renders 'Wrote files' for file_edit kind", () => { + const view = render( + + {}} + /> + + ); + + expect(view.getByText(/Wrote files/)).toBeTruthy(); + }); + + test("clicking the header fires onToggle and reflects aria-expanded state", () => { + const onToggle = mock(() => {}); + const view = render( + + + + ); + + const header = view.container.querySelector('[aria-expanded="false"]'); + expect(header).toBeTruthy(); + fireEvent.click(header!); + expect(onToggle).toHaveBeenCalledTimes(1); + + view.rerender( + + + + ); + + expect(view.container.querySelector('[aria-expanded="true"]')).toBeTruthy(); + }); + + test("uses singular noun for a single-path group", () => { + const view = render( + + {}} + /> + + ); + + expect(view.getByText(/Read file\b/)).toBeTruthy(); + }); +}); diff --git a/src/browser/features/Tools/CoalescedToolCall.tsx b/src/browser/features/Tools/CoalescedToolCall.tsx new file mode 100644 index 0000000000..6ccfe419d4 --- /dev/null +++ b/src/browser/features/Tools/CoalescedToolCall.tsx @@ -0,0 +1,91 @@ +import React from "react"; +import { FileIcon } from "@/browser/components/FileIcon/FileIcon"; +import { ToolContainer, ToolHeader, ExpandIcon, ToolIcon } from "./Shared/ToolPrimitives"; +import type { ToolCoalesceKind } from "@/browser/utils/messages/toolCoalescing"; + +/** + * Summary row that takes the place of the head call in a coalesced tool group. + * + * Sizing intentionally mirrors a collapsed single-file tool call (same + * ToolContainer padding, same row height). That keeps the 1 -> N transition + * flicker-free: when a second file_read lands mid-stream the head row stays + * pinned in place and only its content changes from "Read /a.ts" to + * "Read files /a.ts, /b.ts". + * + * The component is purely presentational. Expansion state lives on the + * transcript so the same Set can drive both the head row and the visibility + * of follow-up member rows. + */ + +interface CoalesceKindCopy { + /** Past-tense verb used in the summary. */ + verb: string; + /** + * Tool name routed through {@link ToolIcon}. We deliberately pick a + * representative variant (e.g. `file_edit_replace_string` for edits) so the + * icon registry already has a mapping. + */ + iconToolName: string; +} + +const KIND_COPY: Record = { + file_read: { verb: "Read", iconToolName: "file_read" }, + // `file_edit_*` all map to the same Pencil icon via the registry; the + // specific variant here is just a stable key for ToolIcon's lookup. + file_edit: { verb: "Wrote", iconToolName: "file_edit_replace_string" }, +}; + +interface CoalescedToolCallProps { + /** Which tool kind the group represents. */ + kind: ToolCoalesceKind; + /** + * File paths involved in the group, in chronological order. Duplicates are + * left in place — repeated edits to the same file are a meaningful signal + * that the user can see by expanding. + */ + filePaths: string[]; + /** Whether the group is currently expanded. */ + expanded: boolean; + /** Toggle the group's expansion state. */ + onToggle: () => void; +} + +export const CoalescedToolCall: React.FC = ({ + kind, + filePaths, + expanded, + onToggle, +}) => { + const { verb, iconToolName } = KIND_COPY[kind]; + // The summary is only rendered for groups of >= 2, but guard against zero + // for type safety and to keep the copy sensible. + const fileCount = filePaths.length; + const noun = fileCount === 1 ? "file" : "files"; + // Use the first path to drive the small file-type icon — gives the row a + // visual anchor that matches the single-call layout. + const leadingPath = filePaths[0] ?? ""; + const joinedPaths = filePaths.join(", "); + + return ( + + + ▶ + +
+ + {verb} {noun} + + {leadingPath && ( + + )} + + {joinedPaths} + +
+ + {fileCount} + + + + ); +}; diff --git a/src/browser/utils/messages/toolCoalescing.test.ts b/src/browser/utils/messages/toolCoalescing.test.ts new file mode 100644 index 0000000000..3281f395a1 --- /dev/null +++ b/src/browser/utils/messages/toolCoalescing.test.ts @@ -0,0 +1,210 @@ +import { describe, it, expect } from "@jest/globals"; + +import { computeToolCoalesceInfos, type ToolCoalesceInfo } from "./toolCoalescing"; +import type { DisplayedMessage } from "@/common/types/message"; + +function fileReadMessage(id: string, path: string, historySequence: number): DisplayedMessage { + return { + type: "tool", + id, + historyId: `h-${id}`, + toolCallId: `tc-${id}`, + toolName: "file_read", + args: { path }, + result: { success: true, content: "", file_size: 0, modifiedTime: "", lines_read: 0 }, + status: "completed", + isPartial: false, + historySequence, + }; +} + +function fileEditMessage( + id: string, + toolName: "file_edit_replace_string" | "file_edit_insert" | "file_edit_replace_lines", + path: string, + historySequence: number +): DisplayedMessage { + return { + type: "tool", + id, + historyId: `h-${id}`, + toolCallId: `tc-${id}`, + toolName, + args: + toolName === "file_edit_insert" + ? { path, content: "x" } + : toolName === "file_edit_replace_string" + ? { path, old_string: "a", new_string: "b" } + : { path, start_line: 1, end_line: 2, new_content: "x" }, + result: { success: true, diff: "", edits_applied: 1 }, + status: "completed", + isPartial: false, + historySequence, + }; +} + +function userMessage(id: string, historySequence: number): DisplayedMessage { + return { + type: "user", + id, + historyId: `h-${id}`, + content: "hi", + historySequence, + }; +} + +function infoAt(messages: DisplayedMessage[], index: number): ToolCoalesceInfo | undefined { + return computeToolCoalesceInfos(messages)[index]; +} + +describe("computeToolCoalesceInfos", () => { + it("does not coalesce a single file_read", () => { + const messages = [fileReadMessage("1", "/a.ts", 1)]; + expect(infoAt(messages, 0)).toBeUndefined(); + }); + + it("coalesces two consecutive file_reads with head/member positions", () => { + const messages = [fileReadMessage("1", "/a.ts", 1), fileReadMessage("2", "/b.ts", 2)]; + + const head = infoAt(messages, 0); + const member = infoAt(messages, 1); + + expect(head).toMatchObject({ + kind: "file_read", + position: "head", + totalCount: 2, + headIndex: 0, + filePaths: ["/a.ts", "/b.ts"], + }); + expect(member).toMatchObject({ + kind: "file_read", + position: "member", + totalCount: 2, + headIndex: 0, + filePaths: ["/a.ts", "/b.ts"], + }); + }); + + it("coalesces three or more consecutive file_reads", () => { + const messages = [ + fileReadMessage("1", "/a.ts", 1), + fileReadMessage("2", "/b.ts", 2), + fileReadMessage("3", "/c.ts", 3), + ]; + + expect(infoAt(messages, 0)?.position).toBe("head"); + expect(infoAt(messages, 1)?.position).toBe("member"); + expect(infoAt(messages, 2)?.position).toBe("member"); + expect(infoAt(messages, 0)?.totalCount).toBe(3); + expect(infoAt(messages, 0)?.filePaths).toEqual(["/a.ts", "/b.ts", "/c.ts"]); + }); + + it("coalesces mixed file_edit variants under the file_edit kind", () => { + const messages = [ + fileEditMessage("1", "file_edit_replace_string", "/a.ts", 1), + fileEditMessage("2", "file_edit_insert", "/b.ts", 2), + fileEditMessage("3", "file_edit_replace_lines", "/c.ts", 3), + ]; + + expect(infoAt(messages, 0)).toMatchObject({ + kind: "file_edit", + position: "head", + totalCount: 3, + filePaths: ["/a.ts", "/b.ts", "/c.ts"], + }); + expect(infoAt(messages, 2)?.position).toBe("member"); + }); + + it("does not coalesce across kinds (file_read then file_edit)", () => { + const messages = [ + fileReadMessage("1", "/a.ts", 1), + fileEditMessage("2", "file_edit_replace_string", "/a.ts", 2), + ]; + + expect(infoAt(messages, 0)).toBeUndefined(); + expect(infoAt(messages, 1)).toBeUndefined(); + }); + + it("does not coalesce across an intervening non-tool message", () => { + const messages = [ + fileReadMessage("1", "/a.ts", 1), + userMessage("u", 2), + fileReadMessage("2", "/b.ts", 3), + ]; + + expect(infoAt(messages, 0)).toBeUndefined(); + expect(infoAt(messages, 2)).toBeUndefined(); + }); + + it("does not coalesce across an intervening unrelated tool", () => { + const messages: DisplayedMessage[] = [ + fileReadMessage("1", "/a.ts", 1), + { + type: "tool", + id: "bash-1", + historyId: "h-bash-1", + toolCallId: "tc-bash-1", + toolName: "bash", + args: { script: "ls", timeout_secs: 1, display_name: "ls" }, + status: "completed", + isPartial: false, + historySequence: 2, + }, + fileReadMessage("2", "/b.ts", 3), + ]; + + expect(infoAt(messages, 0)).toBeUndefined(); + expect(infoAt(messages, 2)).toBeUndefined(); + }); + + it("supports multiple independent groups separated by other content", () => { + const messages = [ + fileReadMessage("r1", "/a.ts", 1), + fileReadMessage("r2", "/b.ts", 2), + userMessage("u1", 3), + fileEditMessage("e1", "file_edit_replace_string", "/c.ts", 4), + fileEditMessage("e2", "file_edit_insert", "/d.ts", 5), + ]; + + expect(infoAt(messages, 0)?.kind).toBe("file_read"); + expect(infoAt(messages, 0)?.position).toBe("head"); + expect(infoAt(messages, 1)?.position).toBe("member"); + expect(infoAt(messages, 2)).toBeUndefined(); + expect(infoAt(messages, 3)?.kind).toBe("file_edit"); + expect(infoAt(messages, 3)?.position).toBe("head"); + expect(infoAt(messages, 4)?.position).toBe("member"); + }); + + it("falls back to (unknown) when a tool args object has no recognizable path", () => { + const messages: DisplayedMessage[] = [ + { + type: "tool", + id: "1", + historyId: "h-1", + toolCallId: "tc-1", + toolName: "file_read", + args: {}, + status: "completed", + isPartial: false, + historySequence: 1, + }, + fileReadMessage("2", "/b.ts", 2), + ]; + + expect(infoAt(messages, 0)?.filePaths).toEqual(["(unknown)", "/b.ts"]); + }); + + it("returns an array sized to match the input", () => { + const messages = [ + userMessage("u1", 1), + fileReadMessage("1", "/a.ts", 2), + fileReadMessage("2", "/b.ts", 3), + ]; + + const infos = computeToolCoalesceInfos(messages); + expect(infos).toHaveLength(3); + expect(infos[0]).toBeUndefined(); + expect(infos[1]?.position).toBe("head"); + expect(infos[2]?.position).toBe("member"); + }); +}); diff --git a/src/browser/utils/messages/toolCoalescing.ts b/src/browser/utils/messages/toolCoalescing.ts new file mode 100644 index 0000000000..8265ddac06 --- /dev/null +++ b/src/browser/utils/messages/toolCoalescing.ts @@ -0,0 +1,133 @@ +import type { DisplayedMessage } from "@/common/types/message"; +import { extractToolFilePath } from "@/common/utils/tools/toolInputFilePath"; + +/** + * Tool coalescing groups consecutive tool calls of related kinds into a single + * "summary" row that takes the place of the head call. The follow-up calls are + * hidden from the transcript until the user expands the group. + * + * Why: when an agent reads or writes a burst of files, each individual row + * pushes more useful content (assistant prose, terminal output, etc.) off + * screen. Coalescing keeps those bursts skimmable while still allowing the + * user to drill into any individual tool call. + * + * Why the head-replacement design: the coalesce row reuses the head's slot in + * the transcript so the 1 -> N transition is a content swap rather than a + * row insertion. The summary row is sized to match a collapsed single-file + * tool call so the user does not see a layout flash when the second member + * arrives mid-stream. + */ + +/** + * Kinds of tool calls that can be coalesced. + * + * Both Anthropic-flavored and historical `file_edit_*` variants are grouped + * under a single "file_edit" kind so a mixed run of inserts/replaces still + * forms one logical "Wrote files …" block in the transcript. + */ +export type ToolCoalesceKind = "file_read" | "file_edit"; + +const FILE_READ_TOOL_NAMES = new Set(["file_read"]); + +// All current and legacy file-edit variants. `file_edit_replace_lines` is no +// longer wired into the live tool registry, but historical transcripts may +// still contain it, so we include it here to keep older sessions consistent. +const FILE_EDIT_TOOL_NAMES = new Set([ + "file_edit_replace_string", + "file_edit_replace_lines", + "file_edit_insert", +]); + +/** Minimum group size before we coalesce. Below this we just render normally. */ +const MIN_COALESCE_GROUP_SIZE = 2; + +/** + * Information about a tool message's position within a coalesced group. + * Aligned with the messages array passed to {@link computeToolCoalesceInfos}. + */ +export interface ToolCoalesceInfo { + /** Which kind of tool group this message belongs to. */ + kind: ToolCoalesceKind; + /** + * "head" — the first call in the group. Rendered as the coalesce summary + * row when collapsed; rendered both as the summary row and normally when + * expanded. + * + * "member" — every other call in the group. Hidden when collapsed; rendered + * normally when expanded. + */ + position: "head" | "member"; + /** Total number of tool calls coalesced into this group (>= 2). */ + totalCount: number; + /** Index of the head message; used as the expansion-state key. */ + headIndex: number; + /** + * File paths involved in the group, in chronological order. Used by the + * head's summary row to render "Read files a, b, c". May contain duplicates + * if the same file appears in multiple consecutive calls. + */ + filePaths: string[]; +} + +function getCoalesceKind(msg: DisplayedMessage | undefined): ToolCoalesceKind | undefined { + if (msg?.type !== "tool") return undefined; + if (FILE_READ_TOOL_NAMES.has(msg.toolName)) return "file_read"; + if (FILE_EDIT_TOOL_NAMES.has(msg.toolName)) return "file_edit"; + return undefined; +} + +/** + * Compute coalesce metadata for every message in one linear pass. + * + * Returns an array aligned with `messages` where each entry is either a + * {@link ToolCoalesceInfo} (this message participates in a group) or + * `undefined` (rendered normally). + * + * Performance: workspace-open commits the full history in one pass; we keep + * this O(n) so the coalescing work scales with transcript size. + */ +export function computeToolCoalesceInfos( + messages: DisplayedMessage[] +): Array { + const infos = new Array(messages.length); + + let index = 0; + while (index < messages.length) { + const kind = getCoalesceKind(messages[index]); + if (!kind) { + index++; + continue; + } + + // Walk forward while the next message has the same coalesce kind. + let groupEnd = index; + while (groupEnd < messages.length - 1 && getCoalesceKind(messages[groupEnd + 1]) === kind) { + groupEnd++; + } + + const groupSize = groupEnd - index + 1; + if (groupSize >= MIN_COALESCE_GROUP_SIZE) { + const filePaths: string[] = []; + for (let j = index; j <= groupEnd; j++) { + const candidate = messages[j]; + // Guarded by the walk above, but narrow defensively for TypeScript. + if (candidate?.type !== "tool") continue; + filePaths.push(extractToolFilePath(candidate.args) ?? "(unknown)"); + } + + for (let j = index; j <= groupEnd; j++) { + infos[j] = { + kind, + position: j === index ? "head" : "member", + totalCount: groupSize, + headIndex: index, + filePaths, + }; + } + } + + index = groupEnd + 1; + } + + return infos; +} From e2f85ab62e5c6ea87ee937ade6beb4981f8a2b80 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 24 May 2026 16:13:38 -0500 Subject: [PATCH 2/9] tests: replace empty arrows with named no-op for lint --- src/browser/features/Tools/CoalescedToolCall.test.tsx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/browser/features/Tools/CoalescedToolCall.test.tsx b/src/browser/features/Tools/CoalescedToolCall.test.tsx index 19433fce2e..fc0e9a722c 100644 --- a/src/browser/features/Tools/CoalescedToolCall.test.tsx +++ b/src/browser/features/Tools/CoalescedToolCall.test.tsx @@ -6,6 +6,8 @@ import { TooltipProvider } from "@/browser/components/Tooltip/Tooltip"; import { CoalescedToolCall } from "./CoalescedToolCall"; +const noop = (): void => undefined; + let windowInstance: GlobalWindow | null = null; beforeEach(() => { @@ -30,7 +32,7 @@ describe("CoalescedToolCall", () => { kind="file_read" filePaths={["src/App.tsx", "src/main.ts"]} expanded={false} - onToggle={() => {}} + onToggle={noop} /> ); @@ -46,7 +48,7 @@ describe("CoalescedToolCall", () => { kind="file_edit" filePaths={["a.ts", "b.ts", "c.ts"]} expanded={false} - onToggle={() => {}} + onToggle={noop} /> ); @@ -55,7 +57,7 @@ describe("CoalescedToolCall", () => { }); test("clicking the header fires onToggle and reflects aria-expanded state", () => { - const onToggle = mock(() => {}); + const onToggle = mock(noop); const view = render( { kind="file_read" filePaths={["only.ts"]} expanded={false} - onToggle={() => {}} + onToggle={noop} /> ); From 201bf0e2541a7e8f3302bab79ae698687af0db86 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 24 May 2026 16:15:19 -0500 Subject: [PATCH 3/9] feat: dedupe repeated file names from coalesced summary Bursts often touch the same file several times (e.g. multi-hunk edits). Showing 'a.ts, a.ts, b.ts' adds noise without information, so dedupe in the renderer while preserving first-occurrence order. The underlying filePaths array (and totalCount) still reflect every coalesced tool call, so future surfaces that need the raw history still have it. --- .../features/Tools/CoalescedToolCall.test.tsx | 18 ++++++++++++++++ .../features/Tools/CoalescedToolCall.tsx | 21 ++++++++++++++++--- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/browser/features/Tools/CoalescedToolCall.test.tsx b/src/browser/features/Tools/CoalescedToolCall.test.tsx index fc0e9a722c..5db19816c3 100644 --- a/src/browser/features/Tools/CoalescedToolCall.test.tsx +++ b/src/browser/features/Tools/CoalescedToolCall.test.tsx @@ -102,4 +102,22 @@ describe("CoalescedToolCall", () => { expect(view.getByText(/Read file\b/)).toBeTruthy(); }); + + test("deduplicates repeated file paths while preserving first-occurrence order", () => { + const view = render( + + + + ); + + // Order kept by first occurrence; duplicates removed from the rendered list. + expect(view.getByText("a.ts, b.ts, c.ts")).toBeTruthy(); + // Plural noun reflects the unique count (3), not the raw tool-call count (5). + expect(view.getByText(/Wrote files/)).toBeTruthy(); + }); }); diff --git a/src/browser/features/Tools/CoalescedToolCall.tsx b/src/browser/features/Tools/CoalescedToolCall.tsx index 6ccfe419d4..e221fe99ac 100644 --- a/src/browser/features/Tools/CoalescedToolCall.tsx +++ b/src/browser/features/Tools/CoalescedToolCall.tsx @@ -57,14 +57,29 @@ export const CoalescedToolCall: React.FC = ({ onToggle, }) => { const { verb, iconToolName } = KIND_COPY[kind]; + // Display dedupe: the same file is often read/edited several times in a + // burst (e.g. multi-hunk edits to one file). Showing "a.ts, a.ts, b.ts" + // adds noise without information. Preserve chronological order by keeping + // the first occurrence of each path. + const uniquePaths = React.useMemo(() => { + const seen = new Set(); + const out: string[] = []; + for (const path of filePaths) { + if (seen.has(path)) continue; + seen.add(path); + out.push(path); + } + return out; + }, [filePaths]); + // The summary is only rendered for groups of >= 2, but guard against zero // for type safety and to keep the copy sensible. - const fileCount = filePaths.length; + const fileCount = uniquePaths.length; const noun = fileCount === 1 ? "file" : "files"; // Use the first path to drive the small file-type icon — gives the row a // visual anchor that matches the single-call layout. - const leadingPath = filePaths[0] ?? ""; - const joinedPaths = filePaths.join(", "); + const leadingPath = uniquePaths[0] ?? ""; + const joinedPaths = uniquePaths.join(", "); return ( From c50c781d3c1e5bea85254402113bcd6d8ca5442d Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 24 May 2026 16:19:12 -0500 Subject: [PATCH 4/9] fix: do not coalesce a group with an interrupted member Addresses Codex P1: hiding a partial tool row eats the InterruptedBarrier the transcript renders for interruption signals. When the stream stops mid-burst we want every member of the burst to keep its uncoalesced row so the user still sees "interrupted" exactly where it happened. Implemented as a single pass over each candidate group's members: if any has isPartial=true, the whole group falls back to normal rendering. --- .../utils/messages/toolCoalescing.test.ts | 33 ++++++++++++++++++- src/browser/utils/messages/toolCoalescing.ts | 25 +++++++++++++- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/browser/utils/messages/toolCoalescing.test.ts b/src/browser/utils/messages/toolCoalescing.test.ts index 3281f395a1..c97731ba99 100644 --- a/src/browser/utils/messages/toolCoalescing.test.ts +++ b/src/browser/utils/messages/toolCoalescing.test.ts @@ -3,7 +3,11 @@ import { describe, it, expect } from "@jest/globals"; import { computeToolCoalesceInfos, type ToolCoalesceInfo } from "./toolCoalescing"; import type { DisplayedMessage } from "@/common/types/message"; -function fileReadMessage(id: string, path: string, historySequence: number): DisplayedMessage { +function fileReadMessage( + id: string, + path: string, + historySequence: number +): Extract { return { type: "tool", id, @@ -194,6 +198,33 @@ describe("computeToolCoalesceInfos", () => { expect(infoAt(messages, 0)?.filePaths).toEqual(["(unknown)", "/b.ts"]); }); + it("does not coalesce a group that contains an interrupted (partial) member", () => { + // The transcript renders an InterruptedBarrier for interrupted tool rows; + // hiding such a row would eat the user-visible interruption signal. + const partial = fileReadMessage("2", "/b.ts", 2); + partial.isPartial = true; + const messages: DisplayedMessage[] = [ + fileReadMessage("1", "/a.ts", 1), + partial, + fileReadMessage("3", "/c.ts", 3), + ]; + + expect(infoAt(messages, 0)).toBeUndefined(); + expect(infoAt(messages, 1)).toBeUndefined(); + expect(infoAt(messages, 2)).toBeUndefined(); + }); + + it("still coalesces a group when no member is partial", () => { + const messages = [ + fileReadMessage("1", "/a.ts", 1), + fileReadMessage("2", "/b.ts", 2), + fileReadMessage("3", "/c.ts", 3), + ]; + + // All messages have isPartial=false from the helper, so coalescing applies. + expect(infoAt(messages, 0)?.position).toBe("head"); + }); + it("returns an array sized to match the input", () => { const messages = [ userMessage("u1", 1), diff --git a/src/browser/utils/messages/toolCoalescing.ts b/src/browser/utils/messages/toolCoalescing.ts index 8265ddac06..85ab564002 100644 --- a/src/browser/utils/messages/toolCoalescing.ts +++ b/src/browser/utils/messages/toolCoalescing.ts @@ -76,6 +76,17 @@ function getCoalesceKind(msg: DisplayedMessage | undefined): ToolCoalesceKind | return undefined; } +/** + * A tool row carries an interruption signal that the transcript must keep + * visible (the rendered InterruptedBarrier). Coalescing a partial member + * would hide that signal, so we skip the entire group whenever any member + * is partial. In practice this only matters when the stream stopped + * mid-burst, so the uncoalesced row count is small. + */ +function isPartialToolMessage(msg: DisplayedMessage | undefined): boolean { + return msg?.type === "tool" && msg.isPartial === true; +} + /** * Compute coalesce metadata for every message in one linear pass. * @@ -106,7 +117,19 @@ export function computeToolCoalesceInfos( } const groupSize = groupEnd - index + 1; - if (groupSize >= MIN_COALESCE_GROUP_SIZE) { + + // Skip coalescing if any member is interrupted — hiding a partial row + // would eat its InterruptedBarrier and leave the user with no visual + // signal that the burst stopped mid-tool. + let hasPartialMember = false; + for (let j = index; j <= groupEnd; j++) { + if (isPartialToolMessage(messages[j])) { + hasPartialMember = true; + break; + } + } + + if (groupSize >= MIN_COALESCE_GROUP_SIZE && !hasPartialMember) { const filePaths: string[] = []; for (let j = index; j <= groupEnd; j++) { const candidate = messages[j]; From c48ea47fa525861d75bd464b02e546fb90107bbf Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 24 May 2026 16:22:17 -0500 Subject: [PATCH 5/9] fix: only coalesce tool groups where every member is completed and not partial Addresses Codex P2: pending/executing/failed/interrupted/redacted rows all carry status or error UI the summary row deliberately omits. Hiding any such row in a burst would mask the actionable signal until the user manually expands. Generalises the previous partial-only guard to a single 'isCoalesceableToolMember' check: a member must be type=tool, !isPartial, and status=completed. Anything else flips the whole group back to normal rendering. --- .../utils/messages/toolCoalescing.test.ts | 28 ++++++++++++++-- src/browser/utils/messages/toolCoalescing.ts | 33 +++++++++++-------- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/browser/utils/messages/toolCoalescing.test.ts b/src/browser/utils/messages/toolCoalescing.test.ts index c97731ba99..e59f20ec34 100644 --- a/src/browser/utils/messages/toolCoalescing.test.ts +++ b/src/browser/utils/messages/toolCoalescing.test.ts @@ -214,14 +214,38 @@ describe("computeToolCoalesceInfos", () => { expect(infoAt(messages, 2)).toBeUndefined(); }); - it("still coalesces a group when no member is partial", () => { + it("does not coalesce a group with a failed member (preserves error visibility)", () => { + const failed = fileReadMessage("2", "/b.ts", 2); + failed.status = "failed"; + const messages: DisplayedMessage[] = [ + fileReadMessage("1", "/a.ts", 1), + failed, + fileReadMessage("3", "/c.ts", 3), + ]; + + expect(infoAt(messages, 0)).toBeUndefined(); + expect(infoAt(messages, 1)).toBeUndefined(); + expect(infoAt(messages, 2)).toBeUndefined(); + }); + + it("does not coalesce a group with a still-running member", () => { + // Mid-stream a fresh tool call is `executing`; the summary row hides + // that status until the user expands, so refuse to coalesce. + const running = fileReadMessage("2", "/b.ts", 2); + running.status = "executing"; + const messages: DisplayedMessage[] = [fileReadMessage("1", "/a.ts", 1), running]; + + expect(infoAt(messages, 0)).toBeUndefined(); + expect(infoAt(messages, 1)).toBeUndefined(); + }); + + it("still coalesces a group when every member is completed", () => { const messages = [ fileReadMessage("1", "/a.ts", 1), fileReadMessage("2", "/b.ts", 2), fileReadMessage("3", "/c.ts", 3), ]; - // All messages have isPartial=false from the helper, so coalescing applies. expect(infoAt(messages, 0)?.position).toBe("head"); }); diff --git a/src/browser/utils/messages/toolCoalescing.ts b/src/browser/utils/messages/toolCoalescing.ts index 85ab564002..42e86aa835 100644 --- a/src/browser/utils/messages/toolCoalescing.ts +++ b/src/browser/utils/messages/toolCoalescing.ts @@ -77,14 +77,17 @@ function getCoalesceKind(msg: DisplayedMessage | undefined): ToolCoalesceKind | } /** - * A tool row carries an interruption signal that the transcript must keep - * visible (the rendered InterruptedBarrier). Coalescing a partial member - * would hide that signal, so we skip the entire group whenever any member - * is partial. In practice this only matters when the stream stopped - * mid-burst, so the uncoalesced row count is small. + * A coalesceable member is a tool call whose row carries no actionable + * signal the user would lose if hidden. Individual tool rows surface + * status (pending/executing/failed/interrupted/redacted) and inline + * errors, and partial rows render an InterruptedBarrier. The summary + * row deliberately renders none of those, so any non-completed or + * partial member forces the whole group back to normal rendering. */ -function isPartialToolMessage(msg: DisplayedMessage | undefined): boolean { - return msg?.type === "tool" && msg.isPartial === true; +function isCoalesceableToolMember(msg: DisplayedMessage | undefined): boolean { + if (msg?.type !== "tool") return false; + if (msg.isPartial) return false; + return msg.status === "completed"; } /** @@ -118,18 +121,20 @@ export function computeToolCoalesceInfos( const groupSize = groupEnd - index + 1; - // Skip coalescing if any member is interrupted — hiding a partial row - // would eat its InterruptedBarrier and leave the user with no visual - // signal that the burst stopped mid-tool. - let hasPartialMember = false; + // Skip coalescing if any member is not safely coalesceable (still + // running, failed, interrupted, redacted, or partial). Those rows + // carry status/error UI or an InterruptedBarrier that the summary + // row deliberately does not render, so hiding them would eat the + // signal until the user manually expands the group. + let allMembersCoalesceable = true; for (let j = index; j <= groupEnd; j++) { - if (isPartialToolMessage(messages[j])) { - hasPartialMember = true; + if (!isCoalesceableToolMember(messages[j])) { + allMembersCoalesceable = false; break; } } - if (groupSize >= MIN_COALESCE_GROUP_SIZE && !hasPartialMember) { + if (groupSize >= MIN_COALESCE_GROUP_SIZE && allMembersCoalesceable) { const filePaths: string[] = []; for (let j = index; j <= groupEnd; j++) { const candidate = messages[j]; From 8fba3d4085d6974b0cf256f46b85a1839c5fbb53 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 24 May 2026 16:24:24 -0500 Subject: [PATCH 6/9] ci: re-run codex comments check after resolving stale review threads From 82afffe93ff845383e32d03a56fb1f98984dc046 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 24 May 2026 16:32:43 -0500 Subject: [PATCH 7/9] fix(stories): wrap CoalescedToolCall stories so TooltipProvider applies The previous version supplied a custom decorators array on the story meta, which shadowed lightweightMeta's StoryUiShell decorator and therefore stripped TooltipProvider. Storybook's runner picked that up as 'Tooltip must be used within TooltipProvider' for every story. Render via an in-story StoryLayout wrapper instead so the meta-level decorator stack stays intact. Opt out of Chromatic snapshots for these visual-reference stories to keep the global snapshot budget happy. --- .../Tools/CoalescedToolCall.stories.tsx | 51 +++++++++++++------ 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/src/browser/features/Tools/CoalescedToolCall.stories.tsx b/src/browser/features/Tools/CoalescedToolCall.stories.tsx index 2526cc1b37..f23a1fa8d4 100644 --- a/src/browser/features/Tools/CoalescedToolCall.stories.tsx +++ b/src/browser/features/Tools/CoalescedToolCall.stories.tsx @@ -1,22 +1,34 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; +import type { ReactNode } from "react"; import { useState } from "react"; import { CoalescedToolCall } from "@/browser/features/Tools/CoalescedToolCall"; -import { lightweightMeta } from "@/browser/stories/meta.js"; +import { CHROMATIC_DISABLED, lightweightMeta } from "@/browser/stories/meta.js"; +/** + * Layout shell rendered inside each story so the meta-level decorator + * stack (which provides `TooltipProvider` via `StoryUiShell`) is not + * shadowed by a story-local `decorators` override. + */ +function StoryLayout(props: { children: ReactNode }) { + return ( +
+
{props.children}
+
+ ); +} + +// These stories are visual references for the coalesce row; they don't add +// regression-meaningful coverage beyond the unit/test files in this folder, +// so opt out of Chromatic snapshots to stay under the global snapshot budget. const meta = { ...lightweightMeta, title: "App/Chat/Tools/CoalescedToolCall", component: CoalescedToolCall, - decorators: [ - (Story) => ( -
-
- -
-
- ), - ], + parameters: { + ...lightweightMeta.parameters, + chromatic: CHROMATIC_DISABLED, + }, } satisfies Meta; export default meta; @@ -28,16 +40,20 @@ function InteractiveCoalesced( ) { const [expanded, setExpanded] = useState(false); return ( - setExpanded((e) => !e)} /> + + setExpanded((e) => !e)} /> + ); } +const NOOP = () => undefined; + export const TwoReads: Story = { args: { kind: "file_read", filePaths: ["src/App.tsx", "src/main.ts"], expanded: false, - onToggle: () => undefined, + onToggle: NOOP, }, render: (args) => , }; @@ -53,7 +69,7 @@ export const ManyReads: Story = { "src/browser/features/Tools/CoalescedToolCall.tsx", ], expanded: false, - onToggle: () => undefined, + onToggle: NOOP, }, render: (args) => , }; @@ -63,7 +79,7 @@ export const TwoEdits: Story = { kind: "file_edit", filePaths: ["src/App.tsx", "src/main.ts"], expanded: false, - onToggle: () => undefined, + onToggle: NOOP, }, render: (args) => , }; @@ -73,6 +89,11 @@ export const ExpandedReads: Story = { kind: "file_read", filePaths: ["src/App.tsx", "src/main.ts", "src/preload.ts"], expanded: true, - onToggle: () => undefined, + onToggle: NOOP, }, + render: (args) => ( + + + + ), }; From 26c786044498545100649362de0319b9bafeb1dc Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 25 May 2026 10:16:21 -0500 Subject: [PATCH 8/9] tests(stories): replace no-op CoalescedToolCall stories with real play interactions The previous stories rendered the component but had no play functions, so test-storybook treated them as smoke-render-only. Convert each story into a real interaction test that exercises the new pattern's surfaces: - TwoReadsClickToExpand drives the click toggle through both branches (collapsed -> expanded -> collapsed) and asserts aria-expanded. - ManyReads asserts chronological-order joined paths. - TwoEdits asserts the past-tense 'Wrote files' verb for the file_edit kind. - DeduplicatedPaths asserts the display-only dedupe of repeated file names (5 raw calls -> 3 unique paths in first-occurrence order). - InitiallyExpanded asserts aria-expanded reflects the initial prop. Stories run under test-storybook in CI (the dedicated 'Test / Storybook' job in pr.yml), which exercises play functions in a real browser via the @storybook/test-runner. Chromatic visual snapshots stay disabled to respect the global snapshot budget. --- .../Tools/CoalescedToolCall.stories.tsx | 144 +++++++++++++++--- 1 file changed, 120 insertions(+), 24 deletions(-) diff --git a/src/browser/features/Tools/CoalescedToolCall.stories.tsx b/src/browser/features/Tools/CoalescedToolCall.stories.tsx index f23a1fa8d4..36a60a3637 100644 --- a/src/browser/features/Tools/CoalescedToolCall.stories.tsx +++ b/src/browser/features/Tools/CoalescedToolCall.stories.tsx @@ -1,6 +1,7 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import type { ReactNode } from "react"; import { useState } from "react"; +import { expect, userEvent, waitFor, within } from "@storybook/test"; import { CoalescedToolCall } from "@/browser/features/Tools/CoalescedToolCall"; import { CHROMATIC_DISABLED, lightweightMeta } from "@/browser/stories/meta.js"; @@ -18,9 +19,31 @@ function StoryLayout(props: { children: ReactNode }) { ); } -// These stories are visual references for the coalesce row; they don't add -// regression-meaningful coverage beyond the unit/test files in this folder, -// so opt out of Chromatic snapshots to stay under the global snapshot budget. +/** + * Interactive wrapper that owns the expansion state so play functions can + * exercise the real click-to-expand / click-to-collapse UX. The static + * `expanded` prop on stories is treated as the initial state. + */ +function InteractiveCoalesced( + args: React.ComponentProps +): React.ReactElement { + const [expanded, setExpanded] = useState(args.expanded); + return ( + + setExpanded((prev) => !prev)} + /> + + ); +} + +// Chromatic visual coverage is intentionally disabled — the global snapshot +// budget is tight, and the meaningful surface (header copy, click toggle, +// dedupe, kind verb) is exercised here via `play` functions that run under +// `test-storybook` in CI. const meta = { ...lightweightMeta, title: "App/Chat/Tools/CoalescedToolCall", @@ -29,35 +52,59 @@ const meta = { ...lightweightMeta.parameters, chromatic: CHROMATIC_DISABLED, }, + render: (args) => , } satisfies Meta; export default meta; type Story = StoryObj; -function InteractiveCoalesced( - args: Omit, "expanded" | "onToggle"> -) { - const [expanded, setExpanded] = useState(false); - return ( - - setExpanded((e) => !e)} /> - - ); -} - const NOOP = () => undefined; -export const TwoReads: Story = { +// Storybook's test runner uses the document body as the canvas root in some +// configurations. Querying the body ensures we still find rendered content +// when the decorator stack inserts wrappers above the story root. +function getCanvas(canvasElement: HTMLElement) { + return within(canvasElement.ownerDocument.body); +} + +/** + * Two consecutive file_read calls coalesce into a "Read files …" row. The + * play function clicks it and verifies it toggles to expanded. + */ +export const TwoReadsClickToExpand: Story = { args: { kind: "file_read", filePaths: ["src/App.tsx", "src/main.ts"], expanded: false, onToggle: NOOP, }, - render: (args) => , + play: async ({ canvasElement }) => { + const canvas = getCanvas(canvasElement); + + // Header copy: past-tense verb + plural noun + joined paths. + const summary = await canvas.findByText(/Read files/); + await expect(canvas.findByText("src/App.tsx, src/main.ts")).resolves.toBeTruthy(); + + // Toggle starts collapsed, then expanded after a click. + const header = summary.closest("[aria-expanded]"); + if (!(header instanceof HTMLElement)) { + throw new Error("Coalesce header missing aria-expanded element"); + } + await expect(header).toHaveAttribute("aria-expanded", "false"); + await userEvent.click(header); + await waitFor(() => expect(header).toHaveAttribute("aria-expanded", "true")); + + // Clicking again collapses; covers both branches of the toggle. + await userEvent.click(header); + await waitFor(() => expect(header).toHaveAttribute("aria-expanded", "false")); + }, }; +/** + * A burst of five reads — exercises the joined-paths layout and the icon + * column hugging the leading path. + */ export const ManyReads: Story = { args: { kind: "file_read", @@ -71,9 +118,22 @@ export const ManyReads: Story = { expanded: false, onToggle: NOOP, }, - render: (args) => , + play: async ({ canvasElement }) => { + const canvas = getCanvas(canvasElement); + await canvas.findByText(/Read files/); + // All five paths render in chronological order, joined by ", ". + await expect( + canvas.findByText( + "src/App.tsx, src/main.ts, src/preload.ts, src/config.ts, src/browser/features/Tools/CoalescedToolCall.tsx" + ) + ).resolves.toBeTruthy(); + }, }; +/** + * file_edit groups use the past-tense "Wrote" verb. Mirrors the canonical + * file-edit icon variant. + */ export const TwoEdits: Story = { args: { kind: "file_edit", @@ -81,19 +141,55 @@ export const TwoEdits: Story = { expanded: false, onToggle: NOOP, }, - render: (args) => , + play: async ({ canvasElement }) => { + const canvas = getCanvas(canvasElement); + await canvas.findByText(/Wrote files/); + await expect(canvas.findByText("src/App.tsx, src/main.ts")).resolves.toBeTruthy(); + }, }; -export const ExpandedReads: Story = { +/** + * Display-only dedupe: a burst that touches the same file repeatedly should + * still render it once. Verifies the React.useMemo dedupe path in + * `CoalescedToolCall`. + */ +export const DeduplicatedPaths: Story = { + args: { + kind: "file_edit", + // 5 raw calls; 3 unique files. First-occurrence order: a, b, c. + filePaths: ["src/a.ts", "src/b.ts", "src/a.ts", "src/c.ts", "src/b.ts"], + expanded: false, + onToggle: NOOP, + }, + play: async ({ canvasElement }) => { + const canvas = getCanvas(canvasElement); + await canvas.findByText(/Wrote files/); + + // Each unique path appears exactly once in the joined list, in + // first-occurrence order. Asserting on the exact string is the simplest + // way to express both the order and the dedupe simultaneously. + await expect(canvas.findByText("src/a.ts, src/b.ts, src/c.ts")).resolves.toBeTruthy(); + }, +}; + +/** + * Expanded state — verifies aria-expanded reflects the initial prop and the + * chevron indicator rotates. + */ +export const InitiallyExpanded: Story = { args: { kind: "file_read", filePaths: ["src/App.tsx", "src/main.ts", "src/preload.ts"], expanded: true, onToggle: NOOP, }, - render: (args) => ( - - - - ), + play: async ({ canvasElement }) => { + const canvas = getCanvas(canvasElement); + const summary = await canvas.findByText(/Read files/); + const header = summary.closest("[aria-expanded]"); + if (!(header instanceof HTMLElement)) { + throw new Error("Coalesce header missing aria-expanded element"); + } + await expect(header).toHaveAttribute("aria-expanded", "true"); + }, }; From 9caa9fd1178b0e04468c34298d4f03602c984b86 Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 25 May 2026 10:46:27 -0500 Subject: [PATCH 9/9] tests(stories): add Chromatic baseline for CoalescedToolCall (+budget rebalance) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewers asked for at least one Chromatic visual snapshot of the new coalesced-tool-call row so the new UX has a pinned visual baseline, not just play-driven behavior coverage. Changes: - Add `CoalescedToolCall.visual.stories.tsx` containing one Chromatic-enabled story (`CollapsedReadFiles`) that renders the canonical new behavior: the collapsed "Read files src/App.tsx, src/main.ts" row. The sibling `CoalescedToolCall.stories.tsx` keeps its meta-level `chromatic: CHROMATIC_DISABLED` and continues to own the interaction (play) coverage for click-to-expand, dedupe, the `file_edit` verb, and the initial-expanded state. - Opt `ProvidersSection.stories.tsx` out of Chromatic snapshots at the meta level. All four stories in that file already drive `play` functions that assert the relevant settings-panel behavior (empty state, configured providers, env-sourced indicators, expanded provider config), and the layout has no animation/scroll-fade/state nuances worth a pixel baseline. This frees four snapshots from the global Chromatic budget. - Net delta on `tests/ui/storybook/budget.test.ts`: +1 (new visual story) -4 (ProvidersSection) = -3, taking the estimated snapshot count from 302 (pre-existing breach on main) to 299, comfortably under the 300 cap. Validation: - `bun test ./tests/ui/storybook/budget.test.ts` -> both budget assertions pass (files ≤ 70, snapshots ≤ 300). - `make typecheck`, `make lint`, `make fmt-check`, `make storybook-build` all clean. - Targeted unit + storybook tests: `CoalescedToolCall.test.tsx`, `toolCoalescing.test.ts`, and `tests/ui/storybook/` all pass. --- _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `xhigh` • Cost: `$29.08`_ --- .../Sections/ProvidersSection.stories.tsx | 15 ++++- .../CoalescedToolCall.visual.stories.tsx | 55 +++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 src/browser/features/Tools/CoalescedToolCall.visual.stories.tsx diff --git a/src/browser/features/Settings/Sections/ProvidersSection.stories.tsx b/src/browser/features/Settings/Sections/ProvidersSection.stories.tsx index 20354f53be..9ffc1dd6ec 100644 --- a/src/browser/features/Settings/Sections/ProvidersSection.stories.tsx +++ b/src/browser/features/Settings/Sections/ProvidersSection.stories.tsx @@ -4,10 +4,21 @@ import { userEvent, waitFor, within } from "@storybook/test"; import { ProvidersSection } from "./ProvidersSection.js"; import { SettingsSectionStory, setupSettingsStory } from "./settingsStoryUtils.js"; +// Chromatic snapshots are disabled here: every story below already drives a +// `play` function that asserts the relevant behavior (empty state, configured +// providers, env-sourced indicators, expanded provider config). The visual +// layout of the providers settings panel has no scroll-fade/animation/state +// nuances worth a pixel baseline, so we free this file's snapshots for use +// elsewhere in the global Chromatic budget (see +// `tests/ui/storybook/budget.test.ts`). const meta: Meta = { ...lightweightMeta, title: "Settings/Sections/ProvidersSection", component: ProvidersSection, + parameters: { + ...lightweightMeta.parameters, + chromatic: CHROMATIC_DISABLED, + }, }; export default meta; @@ -82,9 +93,7 @@ export const ProvidersEnvSourced: Story = { ), - parameters: { - chromatic: CHROMATIC_DISABLED, - }, + // (meta-level `chromatic: CHROMATIC_DISABLED` already covers this story.) play: async ({ canvasElement }) => { const canvas = within(canvasElement); diff --git a/src/browser/features/Tools/CoalescedToolCall.visual.stories.tsx b/src/browser/features/Tools/CoalescedToolCall.visual.stories.tsx new file mode 100644 index 0000000000..180c358cac --- /dev/null +++ b/src/browser/features/Tools/CoalescedToolCall.visual.stories.tsx @@ -0,0 +1,55 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import type { ReactNode } from "react"; + +import { CoalescedToolCall } from "@/browser/features/Tools/CoalescedToolCall"; +import { lightweightMeta } from "@/browser/stories/meta.js"; + +/** + * Chromatic-snapshotted story for the new coalesced-tool-call row. + * + * Most coverage for `CoalescedToolCall` lives in the sibling + * `CoalescedToolCall.stories.tsx` file, which is Chromatic-disabled and uses + * `play` functions to exercise click-to-expand, dedupe, and the kind-specific + * verb. This file deliberately contains a single visual baseline — the + * collapsed `Read files …` row — so reviewers get one pinned snapshot of the + * canonical new behavior without pushing the global snapshot budget over. + */ + +function StoryLayout(props: { children: ReactNode }) { + return ( +
+
{props.children}
+
+ ); +} + +const NOOP = () => undefined; + +const meta = { + ...lightweightMeta, + title: "App/Chat/Tools/CoalescedToolCall (visual)", + component: CoalescedToolCall, + render: (args) => ( + + + + ), +} satisfies Meta; + +export default meta; + +type Story = StoryObj; + +/** + * Collapsed coalesce row for a two-file read burst — the headline new UX. The + * adjacent interactions file covers expansion, dedupe, and the `file_edit` + * verb in play functions. + */ +export const CollapsedReadFiles: Story = { + args: { + kind: "file_read", + filePaths: ["src/App.tsx", "src/main.ts"], + expanded: false, + onToggle: NOOP, + }, +};