From a8024f8542f4d46b56c6e42886a924bcd2bcd83f Mon Sep 17 00:00:00 2001 From: Amp Date: Wed, 16 Sep 2026 11:45:00 +0000 Subject: [PATCH 1/2] fix(webview): batch repeated tool preambles Amp-Thread-ID: https://ampcode.com/threads/T-01a0aa02-b6de-7763-97b8-1650ea4b46da --- .../chat/__tests__/ChatView.spec.tsx | 38 ++++++++++++++++++ .../src/utils/__tests__/batchNearby.spec.ts | 40 +++++++++++++++++++ webview-ui/src/utils/batchNearby.ts | 17 +++++--- .../src/utils/chatBatchingPredicates.ts | 13 +++++- 4 files changed, 101 insertions(+), 7 deletions(-) diff --git a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx index 8f7de5c459..19c069628e 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx @@ -463,6 +463,44 @@ 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) + expect(toolRows[0]?.text).toContain('"batchFiles"') + }) + }) }) 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..1290c98812 100644 --- a/webview-ui/src/utils/__tests__/batchNearby.spec.ts +++ b/webview-ui/src/utils/__tests__/batchNearby.spec.ts @@ -94,6 +94,46 @@ 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("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..0b1ba05f12 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,17 +43,24 @@ export function batchNearby(items: T[], options: BatchNearbyOptions): T[] const batch: T[] = [items[i]] let j = i + 1 const pendingIgnorable: T[] = [] + let batchContext: T | undefined - while (j < items.length) { - if (isBoundary(items[j])) { - break // boundary stops the batch + for (let contextIndex = i - 1; contextIndex >= 0; contextIndex--) { + if (!isIgnorableBetweenTargets(items[contextIndex])) { + batchContext = items[contextIndex] + break } + } + + while (j < items.length) { 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 if (isBoundary(items[j])) { + break // boundary stops the batch } else { break // non-ignorable, non-target message stops the batch } 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" + ) } /** From 83ad7dafb1539765f258e677d1156351359532eb Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Wed, 16 Sep 2026 16:55:08 +0200 Subject: [PATCH 2/2] fix(webview): preserve batching boundaries --- .../chat/__tests__/ChatView.spec.tsx | 3 +- .../src/utils/__tests__/batchNearby.spec.ts | 30 +++++++++++++++++++ webview-ui/src/utils/batchNearby.ts | 6 ++-- 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx index 19c069628e..a1e918b63c 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx @@ -498,7 +498,8 @@ describe("ChatView - Tool Batching Tests", () => { expect(textRows).toHaveLength(1) expect(toolRows).toHaveLength(1) - expect(toolRows[0]?.text).toContain('"batchFiles"') + const toolPayload = JSON.parse(toolRows[0]?.text ?? "{}") as { batchFiles?: Array<{ path?: string }> } + expect(toolPayload.batchFiles?.map(({ path }) => path)).toEqual(["a.ts", "b.ts"]) }) }) }) diff --git a/webview-ui/src/utils/__tests__/batchNearby.spec.ts b/webview-ui/src/utils/__tests__/batchNearby.spec.ts index 1290c98812..5ac35de93d 100644 --- a/webview-ui/src/utils/__tests__/batchNearby.spec.ts +++ b/webview-ui/src/utils/__tests__/batchNearby.spec.ts @@ -134,6 +134,36 @@ describe("batchNearby", () => { 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 0b1ba05f12..ab49be2b22 100644 --- a/webview-ui/src/utils/batchNearby.ts +++ b/webview-ui/src/utils/batchNearby.ts @@ -53,14 +53,14 @@ export function batchNearby(items: T[], options: BatchNearbyOptions): T[] } while (j < items.length) { - if (isTarget(items[j])) { + if (isBoundary(items[j]) && !isIgnorableBetweenTargets(items[j], batchContext)) { + break // boundary stops the batch + } else if (isTarget(items[j])) { batch.push(items[j]) j++ } else if (isIgnorableBetweenTargets(items[j], batchContext)) { pendingIgnorable.push(items[j]) // track but don't commit yet j++ - } else if (isBoundary(items[j])) { - break // boundary stops the batch } else { break // non-ignorable, non-target message stops the batch }