diff --git a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx index 8f7de5c459..a1e918b63c 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx @@ -463,6 +463,45 @@ describe("ChatView - Tool Batching Tests", () => { expect(toolRow?.text).toContain('"path":"b.ts"') }) }) + + it("shows a repeated assistant preamble once while batching readFile asks", async () => { + renderChatView() + const preamble = "I'll read the files now." + + mockPostMessage({ + clineMessages: [ + { type: "say", say: "task", ts: 1, text: "Read the relevant files." }, + { type: "say", say: "text", ts: 2, text: preamble }, + { + type: "ask", + ask: "tool", + ts: 3, + text: JSON.stringify({ tool: "readFile", path: "a.ts" }), + }, + { type: "say", say: "text", ts: 4, text: preamble }, + { + type: "ask", + ask: "tool", + ts: 5, + text: JSON.stringify({ tool: "readFile", path: "b.ts" }), + }, + ], + }) + + await waitFor(() => { + const textRows = mockVirtuosoState.lastData.filter( + (message) => message.type === "say" && message.say === "text" && message.text === preamble, + ) + const toolRows = mockVirtuosoState.lastData.filter( + (message) => message.type === "ask" && message.ask === "tool", + ) + + expect(textRows).toHaveLength(1) + expect(toolRows).toHaveLength(1) + const toolPayload = JSON.parse(toolRows[0]?.text ?? "{}") as { batchFiles?: Array<{ path?: string }> } + expect(toolPayload.batchFiles?.map(({ path }) => path)).toEqual(["a.ts", "b.ts"]) + }) + }) }) describe("ChatView - Aggregated Costs Lifecycle", () => { diff --git a/webview-ui/src/utils/__tests__/batchNearby.spec.ts b/webview-ui/src/utils/__tests__/batchNearby.spec.ts index c6fa640aea..5ac35de93d 100644 --- a/webview-ui/src/utils/__tests__/batchNearby.spec.ts +++ b/webview-ui/src/utils/__tests__/batchNearby.spec.ts @@ -94,6 +94,76 @@ describe("batchNearby", () => { expect(result[0].text).toBe("BATCH:match-1,match-2") }) + test("repeated assistant preambles between matching tools are consumed by a successful batch", () => { + const preamble = "I'll read the files now." + const messages = [ + msg(preamble, "say", "text"), + msg("match-1", "ask"), + msg("", "say", "api_req_started"), + msg(preamble, "say", "text"), + msg("match-2", "ask"), + msg(preamble, "say", "text"), + msg("match-3", "ask"), + ] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + + expect(result).toHaveLength(2) + expect(result[0].text).toBe(preamble) + expect(result[1].text).toBe("BATCH:match-1,match-2,match-3") + }) + + test("distinct assistant text between matching tools remains a boundary", () => { + const messages = [ + msg("I'll read the files now.", "say", "text"), + msg("match-1", "ask"), + msg("I found something important.", "say", "text"), + msg("match-2", "ask"), + ] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + + expect(result).toEqual(messages) + }) + + test("restores a repeated preamble when no later target is found", () => { + const preamble = "I'll read the files now." + const messages = [ + msg(preamble, "say", "text"), + msg("match-1", "ask"), + msg(preamble, "say", "text"), + msg("different tool", "ask"), + ] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + + expect(result).toEqual(messages) + }) + + test("an item matching target and boundary stops the current batch", () => { + const messages = [msg("match-1", "ask"), msg("match-boundary", "ask"), msg("match-2", "ask")] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets: () => false, + isBoundary: (item) => item.text === "match-boundary", + synthesize: synthesizeBatch, + }) + + expect(result).toEqual(messages) + }) + test("boundary message stops batching", () => { const messages = [msg("match-1", "ask"), msg("visible text", "say", "text"), msg("match-2", "ask")] const result = batchNearby(messages, { diff --git a/webview-ui/src/utils/batchNearby.ts b/webview-ui/src/utils/batchNearby.ts index 201eef5078..ab49be2b22 100644 --- a/webview-ui/src/utils/batchNearby.ts +++ b/webview-ui/src/utils/batchNearby.ts @@ -12,7 +12,7 @@ export interface BatchNearbyOptions { /** Returns true if this item is the target type to batch (e.g., readFile ask) */ isTarget: (item: T) => boolean /** Returns true if this item can be skipped over when looking for more targets */ - isIgnorableBetweenTargets: (item: T) => boolean + isIgnorableBetweenTargets: (item: T, batchContext?: T) => boolean /** Returns true if this item is a semantic boundary that stops merging */ isBoundary: (item: T) => boolean /** Synthesize a batch of items into a single item */ @@ -43,15 +43,22 @@ export function batchNearby(items: T[], options: BatchNearbyOptions): T[] const batch: T[] = [items[i]] let j = i + 1 const pendingIgnorable: T[] = [] + let batchContext: T | undefined + + for (let contextIndex = i - 1; contextIndex >= 0; contextIndex--) { + if (!isIgnorableBetweenTargets(items[contextIndex])) { + batchContext = items[contextIndex] + break + } + } while (j < items.length) { - if (isBoundary(items[j])) { + if (isBoundary(items[j]) && !isIgnorableBetweenTargets(items[j], batchContext)) { break // boundary stops the batch - } - if (isTarget(items[j])) { + } else if (isTarget(items[j])) { batch.push(items[j]) j++ - } else if (isIgnorableBetweenTargets(items[j])) { + } else if (isIgnorableBetweenTargets(items[j], batchContext)) { pendingIgnorable.push(items[j]) // track but don't commit yet j++ } else { diff --git a/webview-ui/src/utils/chatBatchingPredicates.ts b/webview-ui/src/utils/chatBatchingPredicates.ts index 9fe65a98fa..b011605642 100644 --- a/webview-ui/src/utils/chatBatchingPredicates.ts +++ b/webview-ui/src/utils/chatBatchingPredicates.ts @@ -8,9 +8,18 @@ interface BatchableMessage { * Messages that can be safely skipped over when batching tool asks. * These are low-information or invisible messages that don't affect semantics. */ -export const isIgnorableBetweenTargets = (msg: BatchableMessage): boolean => { +export const isIgnorableBetweenTargets = (msg: BatchableMessage, batchContext?: BatchableMessage): boolean => { if (msg.type !== "say") return false - return msg.say === "api_req_started" || (msg.say === "text" && !msg.text?.trim()) || msg.say === "reasoning" + return ( + msg.say === "api_req_started" || + (msg.say === "text" && + (!msg.text?.trim() || + (batchContext?.type === "say" && + batchContext.say === "text" && + !!batchContext.text?.trim() && + msg.text === batchContext.text))) || + msg.say === "reasoning" + ) } /**