Skip to content

Commit a8024f8

Browse files
ampagentampagent
committed
fix(webview): batch repeated tool preambles
Amp-Thread-ID: https://ampcode.com/threads/T-01a0aa02-b6de-7763-97b8-1650ea4b46da
1 parent ba46d1f commit a8024f8

4 files changed

Lines changed: 101 additions & 7 deletions

File tree

‎webview-ui/src/components/chat/__tests__/ChatView.spec.tsx‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,44 @@ describe("ChatView - Tool Batching Tests", () => {
463463
expect(toolRow?.text).toContain('"path":"b.ts"')
464464
})
465465
})
466+
467+
it("shows a repeated assistant preamble once while batching readFile asks", async () => {
468+
renderChatView()
469+
const preamble = "I'll read the files now."
470+
471+
mockPostMessage({
472+
clineMessages: [
473+
{ type: "say", say: "task", ts: 1, text: "Read the relevant files." },
474+
{ type: "say", say: "text", ts: 2, text: preamble },
475+
{
476+
type: "ask",
477+
ask: "tool",
478+
ts: 3,
479+
text: JSON.stringify({ tool: "readFile", path: "a.ts" }),
480+
},
481+
{ type: "say", say: "text", ts: 4, text: preamble },
482+
{
483+
type: "ask",
484+
ask: "tool",
485+
ts: 5,
486+
text: JSON.stringify({ tool: "readFile", path: "b.ts" }),
487+
},
488+
],
489+
})
490+
491+
await waitFor(() => {
492+
const textRows = mockVirtuosoState.lastData.filter(
493+
(message) => message.type === "say" && message.say === "text" && message.text === preamble,
494+
)
495+
const toolRows = mockVirtuosoState.lastData.filter(
496+
(message) => message.type === "ask" && message.ask === "tool",
497+
)
498+
499+
expect(textRows).toHaveLength(1)
500+
expect(toolRows).toHaveLength(1)
501+
expect(toolRows[0]?.text).toContain('"batchFiles"')
502+
})
503+
})
466504
})
467505

468506
describe("ChatView - Aggregated Costs Lifecycle", () => {

‎webview-ui/src/utils/__tests__/batchNearby.spec.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,46 @@ describe("batchNearby", () => {
9494
expect(result[0].text).toBe("BATCH:match-1,match-2")
9595
})
9696

97+
test("repeated assistant preambles between matching tools are consumed by a successful batch", () => {
98+
const preamble = "I'll read the files now."
99+
const messages = [
100+
msg(preamble, "say", "text"),
101+
msg("match-1", "ask"),
102+
msg("", "say", "api_req_started"),
103+
msg(preamble, "say", "text"),
104+
msg("match-2", "ask"),
105+
msg(preamble, "say", "text"),
106+
msg("match-3", "ask"),
107+
]
108+
const result = batchNearby(messages, {
109+
isTarget: isMatch,
110+
isIgnorableBetweenTargets,
111+
isBoundary,
112+
synthesize: synthesizeBatch,
113+
})
114+
115+
expect(result).toHaveLength(2)
116+
expect(result[0].text).toBe(preamble)
117+
expect(result[1].text).toBe("BATCH:match-1,match-2,match-3")
118+
})
119+
120+
test("distinct assistant text between matching tools remains a boundary", () => {
121+
const messages = [
122+
msg("I'll read the files now.", "say", "text"),
123+
msg("match-1", "ask"),
124+
msg("I found something important.", "say", "text"),
125+
msg("match-2", "ask"),
126+
]
127+
const result = batchNearby(messages, {
128+
isTarget: isMatch,
129+
isIgnorableBetweenTargets,
130+
isBoundary,
131+
synthesize: synthesizeBatch,
132+
})
133+
134+
expect(result).toEqual(messages)
135+
})
136+
97137
test("boundary message stops batching", () => {
98138
const messages = [msg("match-1", "ask"), msg("visible text", "say", "text"), msg("match-2", "ask")]
99139
const result = batchNearby(messages, {

‎webview-ui/src/utils/batchNearby.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export interface BatchNearbyOptions<T> {
1212
/** Returns true if this item is the target type to batch (e.g., readFile ask) */
1313
isTarget: (item: T) => boolean
1414
/** Returns true if this item can be skipped over when looking for more targets */
15-
isIgnorableBetweenTargets: (item: T) => boolean
15+
isIgnorableBetweenTargets: (item: T, batchContext?: T) => boolean
1616
/** Returns true if this item is a semantic boundary that stops merging */
1717
isBoundary: (item: T) => boolean
1818
/** Synthesize a batch of items into a single item */
@@ -43,17 +43,24 @@ export function batchNearby<T>(items: T[], options: BatchNearbyOptions<T>): T[]
4343
const batch: T[] = [items[i]]
4444
let j = i + 1
4545
const pendingIgnorable: T[] = []
46+
let batchContext: T | undefined
4647

47-
while (j < items.length) {
48-
if (isBoundary(items[j])) {
49-
break // boundary stops the batch
48+
for (let contextIndex = i - 1; contextIndex >= 0; contextIndex--) {
49+
if (!isIgnorableBetweenTargets(items[contextIndex])) {
50+
batchContext = items[contextIndex]
51+
break
5052
}
53+
}
54+
55+
while (j < items.length) {
5156
if (isTarget(items[j])) {
5257
batch.push(items[j])
5358
j++
54-
} else if (isIgnorableBetweenTargets(items[j])) {
59+
} else if (isIgnorableBetweenTargets(items[j], batchContext)) {
5560
pendingIgnorable.push(items[j]) // track but don't commit yet
5661
j++
62+
} else if (isBoundary(items[j])) {
63+
break // boundary stops the batch
5764
} else {
5865
break // non-ignorable, non-target message stops the batch
5966
}

‎webview-ui/src/utils/chatBatchingPredicates.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,18 @@ interface BatchableMessage {
88
* Messages that can be safely skipped over when batching tool asks.
99
* These are low-information or invisible messages that don't affect semantics.
1010
*/
11-
export const isIgnorableBetweenTargets = (msg: BatchableMessage): boolean => {
11+
export const isIgnorableBetweenTargets = (msg: BatchableMessage, batchContext?: BatchableMessage): boolean => {
1212
if (msg.type !== "say") return false
13-
return msg.say === "api_req_started" || (msg.say === "text" && !msg.text?.trim()) || msg.say === "reasoning"
13+
return (
14+
msg.say === "api_req_started" ||
15+
(msg.say === "text" &&
16+
(!msg.text?.trim() ||
17+
(batchContext?.type === "say" &&
18+
batchContext.say === "text" &&
19+
!!batchContext.text?.trim() &&
20+
msg.text === batchContext.text))) ||
21+
msg.say === "reasoning"
22+
)
1423
}
1524

1625
/**

0 commit comments

Comments
 (0)