diff --git a/packages/coding-agent/src/modes/components/read-tool-group.ts b/packages/coding-agent/src/modes/components/read-tool-group.ts index d973a57a6..1aab424dc 100644 --- a/packages/coding-agent/src/modes/components/read-tool-group.ts +++ b/packages/coding-agent/src/modes/components/read-tool-group.ts @@ -43,27 +43,30 @@ export function readArgsCollapseIntoGroup(args: unknown): boolean { } /** - * Return the collapsed read calls that can own a turn's usage row. Visible - * assistant content and mixed-tool turns keep their usage as a standalone row - * so the metrics are not misleadingly attributed to one tool. + * Return the collapsed read calls that can own a turn's usage row. Mixed-tool + * turns and visible content after a read keep the standalone row so request + * metrics retain their transcript ordering. */ export function groupedReadUsageCallIds(message: AssistantMessage): string[] | undefined { - const hasVisibleContent = message.content.some( - content => - content.type === "image" || - (content.type === "text" && canonicalizeMessage(content.text)) || - (content.type === "thinking" && canonicalizeMessage(content.thinking)), - ); - if (hasVisibleContent) return undefined; - - const toolCalls = message.content.filter(content => content.type === "toolCall"); - if ( - toolCalls.length === 0 || - toolCalls.some(content => content.name !== "read" || !readArgsCollapseIntoGroup(content.arguments)) - ) { - return undefined; + const toolCallIds: string[] = []; + let sawToolCall = false; + for (const content of message.content) { + if (content.type === "toolCall") { + if (content.name !== "read" || !readArgsCollapseIntoGroup(content.arguments)) return undefined; + sawToolCall = true; + toolCallIds.push(content.id); + continue; + } + if ( + sawToolCall && + (content.type === "image" || + (content.type === "text" && canonicalizeMessage(content.text)) || + (content.type === "thinking" && canonicalizeMessage(content.thinking))) + ) { + return undefined; + } } - return toolCalls.map(content => content.id); + return toolCallIds.length > 0 ? toolCallIds : undefined; } type ReadRenderArgs = { @@ -122,6 +125,7 @@ type ReadEntry = { }; type ReadUsageRow = { + toolCallIds: readonly string[]; usage: Usage; durationMs?: number; ttftMs?: number; @@ -325,6 +329,7 @@ function formatMergedSelectorParts(selectors: string[]): string { export class ReadToolGroupComponent extends Container implements ToolExecutionHandle { #entries = new Map(); #usageRows = new Map(); + #usageBatchByToolCallId = new Map(); #text: Text; #expanded = false; #showContentPreview: boolean; @@ -440,16 +445,18 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa ttftMs?: number, timestamp?: number, ): boolean { + const attachedToolCallIds: string[] = []; let anchorId: string | undefined; - for (let index = toolCallIds.length - 1; index >= 0; index--) { - const toolCallId = toolCallIds[index]!; - if (this.#entries.has(toolCallId)) { - anchorId = toolCallId; - break; - } + for (const toolCallId of toolCallIds) { + if (!this.#entries.has(toolCallId)) continue; + attachedToolCallIds.push(toolCallId); + anchorId = toolCallId; } if (!anchorId) return false; - this.#usageRows.set(anchorId, { usage, durationMs, ttftMs, timestamp }); + for (const toolCallId of attachedToolCallIds) { + this.#usageBatchByToolCallId.set(toolCallId, anchorId); + } + this.#usageRows.set(anchorId, { toolCallIds: attachedToolCallIds, usage, durationMs, ttftMs, timestamp }); this.#updateDisplay(); return true; } @@ -544,27 +551,37 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa } #buildSummaryRows(targets: ReadDisplayTarget[]): ReadSummaryRow[] { - const selectorTargetsByBasePath = new Map(); + const selectorTargetsByBasePathAndBatch = new Map>(); for (const target of targets) { if (!target.selector) continue; - const existing = selectorTargetsByBasePath.get(target.basePath); + let targetsByBatch = selectorTargetsByBasePathAndBatch.get(target.basePath); + if (!targetsByBatch) { + targetsByBatch = new Map(); + selectorTargetsByBasePathAndBatch.set(target.basePath, targetsByBatch); + } + const batchId = this.#usageBatchByToolCallId.get(target.entry.toolCallId); + const existing = targetsByBatch.get(batchId); if (existing) existing.push(target); - else selectorTargetsByBasePath.set(target.basePath, [target]); + else targetsByBatch.set(batchId, [target]); } - const mergeableBasePaths = new Set(); - for (const [basePath, baseTargets] of selectorTargetsByBasePath) { - if (basePath && baseTargets.length > 1) { - mergeableBasePaths.add(basePath); + const mergedTargetsByTarget = new Map(); + for (const [basePath, targetsByBatch] of selectorTargetsByBasePathAndBatch) { + if (!basePath) continue; + for (const groupedTargets of targetsByBatch.values()) { + if (groupedTargets.length <= 1) continue; + for (const target of groupedTargets) { + mergedTargetsByTarget.set(target, groupedTargets); + } } } - const emittedMergedRows = new Set(); + const emittedMergedTargets = new Set(); const rows: ReadSummaryRow[] = []; for (const target of targets) { - if (target.selector && mergeableBasePaths.has(target.basePath)) { - if (!emittedMergedRows.has(target.basePath)) { - const mergedTargets = selectorTargetsByBasePath.get(target.basePath) ?? [target]; + const mergedTargets = mergedTargetsByTarget.get(target); + if (mergedTargets) { + if (!emittedMergedTargets.has(mergedTargets)) { rows.push({ targetPath: `${target.basePath}:${formatMergedSelectorParts( mergedTargets @@ -574,7 +591,7 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa basePath: target.basePath, targets: mergedTargets, }); - emittedMergedRows.add(target.basePath); + emittedMergedTargets.add(mergedTargets); } continue; } @@ -602,17 +619,27 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa } #usageRowsBySummaryRow(rows: ReadSummaryRow[]): Map { - const usageRowsByIndex = new Map(); - for (const [toolCallId, usageRow] of this.#usageRows) { - for (let index = rows.length - 1; index >= 0; index--) { - const row = rows[index]!; - if (!row.targets.some(target => target.entry.toolCallId === toolCallId)) continue; - const usageRows = usageRowsByIndex.get(index); - if (usageRows) usageRows.push(usageRow); - else usageRowsByIndex.set(index, [usageRow]); - break; + const lastRowIndexByToolCallId = new Map(); + for (const [index, row] of rows.entries()) { + for (const target of row.targets) { + lastRowIndexByToolCallId.set(target.entry.toolCallId, index); } } + + const usageRowsByIndex = new Map(); + for (const usageRow of this.#usageRows.values()) { + let lastRowIndex: number | undefined; + for (const toolCallId of usageRow.toolCallIds) { + const index = lastRowIndexByToolCallId.get(toolCallId); + if (index !== undefined && (lastRowIndex === undefined || index > lastRowIndex)) { + lastRowIndex = index; + } + } + if (lastRowIndex === undefined) continue; + const usageRows = usageRowsByIndex.get(lastRowIndex); + if (usageRows) usageRows.push(usageRow); + else usageRowsByIndex.set(lastRowIndex, [usageRow]); + } return usageRowsByIndex; } diff --git a/packages/coding-agent/test/modes/controllers/event-controller-read-grouping.test.ts b/packages/coding-agent/test/modes/controllers/event-controller-read-grouping.test.ts index ad20e4ddf..82292d27c 100644 --- a/packages/coding-agent/test/modes/controllers/event-controller-read-grouping.test.ts +++ b/packages/coding-agent/test/modes/controllers/event-controller-read-grouping.test.ts @@ -135,7 +135,7 @@ describe("EventController read-group accretion", () => { it("nests a read-only completion's usage inside the active group", async () => { settings.set("display.showTokenUsage", true); const { controller, chatContainer } = createFixture(); - const message = assistantMessage([read("usage.ts:1-50")]); + const message = assistantMessage([thinking("Reviewing the target"), read("usage.ts:1-50")]); message.usage = { input: 1234, output: 7, @@ -158,6 +158,33 @@ describe("EventController read-group accretion", () => { expect(usageBlocks).toEqual([group!]); }); + it("keeps usage standalone when visible content follows a read", async () => { + settings.set("display.showTokenUsage", true); + const { controller, chatContainer } = createFixture(); + const message = assistantMessage([read("usage.ts:1-50"), thinking("Read complete")]); + message.usage = { + input: 1234, + output: 7, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 1241, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }; + message.timestamp = new Date(2026, 0, 2, 3, 4, 5).getTime(); + + await controller.handleEvent({ type: "message_start", message } as AgentSessionEvent); + await controller.handleEvent({ type: "message_update", message } as AgentSessionEvent); + await controller.handleEvent({ type: "message_end", message } as AgentSessionEvent); + + const [group] = readGroups(chatContainer); + expect(group).toBeDefined(); + const usageBlocks = chatContainer.children.filter(component => + Bun.stripANSI(component.render(120).join("\n")).includes("2026-01-02 03:04:05"), + ); + expect(usageBlocks).toHaveLength(1); + expect(usageBlocks[0]).not.toBe(group!); + }); + it("starts a new group after a completion that renders visible reasoning", async () => { const { controller, chatContainer } = createFixture(); diff --git a/packages/coding-agent/test/read-tool-group.test.ts b/packages/coding-agent/test/read-tool-group.test.ts index 44c96b3ce..be1ac1832 100644 --- a/packages/coding-agent/test/read-tool-group.test.ts +++ b/packages/coding-agent/test/read-tool-group.test.ts @@ -101,8 +101,8 @@ describe("ReadToolGroupComponent", () => { const twoPath = path.resolve("/tmp/two.ts"); const threePath = path.resolve("/tmp/three.ts"); component.updateArgs({ path: onePath }, "read-one"); - component.updateArgs({ path: twoPath }, "read-two"); - component.updateArgs({ path: threePath }, "read-three"); + component.updateArgs({ path: `${twoPath}:1-2,${threePath}:1-2` }, "read-two"); + component.updateArgs({ path: `${twoPath}:3-4` }, "read-three"); component.updateResult({ content: [{ type: "text", text: "one" }] }, false, "read-one"); component.updateResult({ content: [{ type: "text", text: "two" }] }, false, "read-two"); component.updateResult({ content: [{ type: "text", text: "three" }] }, false, "read-three"); @@ -280,6 +280,35 @@ describe("ReadToolGroupComponent", () => { expect(matches).toBe(1); }); + it("keeps usage below an inline preview when the summary row is suppressed", () => { + const component = new ReadToolGroupComponent({ showContentPreview: true }); + const examplePath = path.resolve("/tmp/example.ts"); + component.updateArgs({ path: examplePath }, "read-preview"); + component.updateResult({ content: [{ type: "text", text: "line 1\nline 2" }] }, false, "read-preview"); + component.attachUsage( + ["read-preview"], + { + input: 1234, + output: 7, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 1241, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + 1000, + 500, + new Date(2026, 0, 2, 3, 4, 5).getTime(), + ); + + const lines = Bun.stripANSI(component.render(120).join("\n")).split("\n"); + const previewIndex = lines.findIndex(line => line.includes("line 2")); + const usageIndices = lines + .map((line, index) => (line.includes("2026-01-02 03:04:05") ? index : -1)) + .filter(index => index >= 0); + expect(usageIndices).toHaveLength(1); + expect(usageIndices[0]).toBeGreaterThan(previewIndex); + }); + it("links grouped summary paths to resolved filesystem paths and selector lines", () => { settings.override("tui.hyperlinks", "always"); const component = new ReadToolGroupComponent();