diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 01c4796c16..9e5a354092 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -8,7 +8,7 @@ import { LRUCache } from "lru-cache" import { useDebounceEffect } from "@src/utils/useDebounceEffect" import { appendImages } from "@src/utils/imageUtils" import { getCostBreakdownIfNeeded } from "@src/utils/costFormatting" -import { batchConsecutive } from "@src/utils/batchConsecutive" +import { batchNearby } from "@src/utils/batchNearby" import type { ClineAsk, ClineSayTool, ClineMessage, ExtensionMessage, AudioType, SuggestionItem } from "@roo-code/types" import { getCompletionCheckpoint, getSuggestionMode, isRetiredProvider } from "@roo-code/types" @@ -1268,10 +1268,64 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + if (msg.type !== "say") return false + return ( + msg.say === "api_req_started" || + msg.say === "api_req_finished" || + (msg.say === "text" && !msg.text?.trim()) || + msg.say === "reasoning" + ) + } + + // Semantic boundaries that stop batching. When we hit one of these, + // any current batch is finalized and the boundary message is preserved as-is: + // - user feedback / new user messages + // - visible assistant text (the model spoke to the user) + // - completion result (turn ended) + // - checkpoint saved + // - errors + const isBoundary = (msg: ClineMessage): boolean => { + if (msg.type !== "say") return false + return ( + msg.say === "user_feedback" || + msg.say === "user_feedback_diff" || + (msg.say === "text" && !!msg.text?.trim()) || + msg.say === "completion_result" || + msg.say === "checkpoint_saved" || + msg.say === "error" || + msg.say === "condense_context" || + msg.say === "codebase_search_result" + ) + } + + // Consolidate tool asks into batches, allowing ignorable messages between targets. + // Unlike batchConsecutive which only merges truly adjacent items, batchNearby + // skips over api_req_started/finished, empty text rows, and reasoning rows that + // models like qwen insert between tool calls during streaming. + const readFileBatched = batchNearby(filtered, { + isTarget: isReadFileAsk, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeReadFileBatch, + }) + const listFilesBatched = batchNearby(readFileBatched, { + isTarget: isListFilesAsk, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeListFilesBatch, + }) + const result = batchNearby(listFilesBatched, { + isTarget: isEditFileAsk, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeEditFileBatch, + }) if (isCondensing) { result.push({ diff --git a/webview-ui/src/utils/__tests__/batchNearby.spec.ts b/webview-ui/src/utils/__tests__/batchNearby.spec.ts new file mode 100644 index 0000000000..b73fb2ca7d --- /dev/null +++ b/webview-ui/src/utils/__tests__/batchNearby.spec.ts @@ -0,0 +1,397 @@ +import { batchNearby } from "../batchNearby" + +interface TestItem { + ts: number + type: string + text: string + say?: string +} + +/** Helper: create a minimal test item with an identifiable text field. */ +function msg(text: string, type = "say", say?: string): TestItem { + return { ts: Date.now(), type, text, say } +} + +/** Predicate: matches items whose text starts with "match". */ +const isMatch = (m: TestItem) => !!m.text?.startsWith("match") + +/** Predicate for realistic qwen tests: matches JSON tool calls. */ +const isToolCall = (m: TestItem) => !!m.text?.startsWith("{") + +/** Ignorable: api_req_started/finished, empty text, reasoning. */ +const isIgnorableBetweenTargets = (m: TestItem): boolean => { + if (m.type !== "say") return false + return ( + m.say === "api_req_started" || + m.say === "api_req_finished" || + (m.say === "text" && !m.text?.trim()) || + m.say === "reasoning" + ) +} + +/** Boundary: user_feedback, visible text, completion_result, checkpoint_saved, error. */ +const isBoundary = (m: TestItem): boolean => { + if (m.type !== "say") return false + return ( + m.say === "user_feedback" || + m.say === "user_feedback_diff" || + (m.say === "text" && !!m.text?.trim()) || + m.say === "completion_result" || + m.say === "checkpoint_saved" || + m.say === "error" || + m.say === "condense_context" + ) +} + +/** Synthesize: merges a batch into a single item with a "BATCH:" marker. */ +const synthesizeBatch = (batch: TestItem[]): TestItem => ({ + ...batch[0], + text: `BATCH:${batch.map((m) => m.text).join(",")}`, +}) + +describe("batchNearby", () => { + test("empty input returns empty output", () => { + expect( + batchNearby([], { isTarget: isMatch, isIgnorableBetweenTargets, isBoundary, synthesize: synthesizeBatch }), + ).toEqual([]) + }) + + test("no matches returns passthrough", () => { + const messages = [msg("a"), msg("b"), msg("c")] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toEqual(messages) + }) + + test("single match is passed through without batching", () => { + const messages = [msg("a"), msg("match-1", "ask"), msg("b")] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(3) + expect(result[1].text).toBe("match-1") + }) + + test("two consecutive matches produce one synthetic message", () => { + const messages = [msg("a"), msg("match-1", "ask"), msg("match-2", "ask"), msg("b")] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(3) + expect(result[0].text).toBe("a") + expect(result[1].text).toBe("BATCH:match-1,match-2") + expect(result[2].text).toBe("b") + }) + + test("non-consecutive matches separated by ignorable messages ARE batched", () => { + const messages = [msg("a"), msg("match-1", "ask"), msg("", "say", "api_req_started"), msg("match-2", "ask")] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(2) + expect(result[0].text).toBe("a") + expect(result[1].text).toBe("BATCH:match-1,match-2") + }) + + test("non-consecutive matches separated by empty text are batched", () => { + const messages = [msg("match-1", "ask"), msg("", "say", "text"), msg("match-2", "ask")] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(1) + expect(result[0].text).toBe("BATCH:match-1,match-2") + }) + + test("boundary message stops batching", () => { + const messages = [msg("match-1", "ask"), msg("visible text", "say", "text"), msg("match-2", "ask")] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(3) + expect(result[0].text).toBe("match-1") + expect(result[1].text).toBe("visible text") + expect(result[2].text).toBe("match-2") + }) + + test("boundary message stops batching with ignorable before it", () => { + const messages = [ + msg("match-1", "ask"), + msg("", "say", "api_req_started"), + msg("visible text", "say", "text"), + msg("match-2", "ask"), + ] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(4) + expect(result[0].text).toBe("match-1") + expect(result[1].text).toBe("") // api_req_started restored after single target + expect(result[2].text).toBe("visible text") + expect(result[3].text).toBe("match-2") + }) + + test("multiple batches separated by boundaries", () => { + const messages = [ + msg("match-1", "ask"), + msg("", "say", "api_req_started"), + msg("match-2", "ask"), + msg("visible text", "say", "text"), + msg("match-3", "ask"), + msg("", "say", "reasoning"), + msg("match-4", "ask"), + ] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(3) + expect(result[0].text).toBe("BATCH:match-1,match-2") + expect(result[1].text).toBe("visible text") + expect(result[2].text).toBe("BATCH:match-3,match-4") + }) + + test("user_feedback stops batching", () => { + const messages = [ + msg("match-1", "ask"), + msg("", "say", "api_req_started"), + msg("match-2", "ask"), + msg("feedback", "say", "user_feedback"), + ] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(2) // bridge succeeded → pending consumed; [BATCH:match-1,match-2, "feedback"] + expect(result[0].text).toBe("BATCH:match-1,match-2") + expect(result[1].text).toBe("feedback") + }) + + test("error stops batching", () => { + const messages = [ + msg("match-1", "ask"), + msg("", "say", "api_req_started"), + msg("match-2", "ask"), + msg("err", "say", "error"), + ] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(2) // bridge succeeded → pending consumed; [BATCH:match-1,match-2, "err"] + expect(result[0].text).toBe("BATCH:match-1,match-2") + expect(result[1].text).toBe("err") + }) + + test("checkpoint_saved stops batching", () => { + const messages = [ + msg("match-1", "ask"), + msg("", "say", "api_req_started"), + msg("match-2", "ask"), + msg("ck", "say", "checkpoint_saved"), + ] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(2) // bridge succeeded → pending consumed; [BATCH:match-1,match-2, "ck"] + expect(result[0].text).toBe("BATCH:match-1,match-2") + expect(result[1].text).toBe("ck") + }) + + test("completion_result stops batching", () => { + const messages = [ + msg("match-1", "ask"), + msg("", "say", "api_req_started"), + msg("match-2", "ask"), + msg("done", "say", "completion_result"), + ] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(2) // bridge succeeded → pending consumed; [BATCH:match-1,match-2, "done"] + expect(result[0].text).toBe("BATCH:match-1,match-2") + expect(result[1].text).toBe("done") + }) + + test("non-ignorable non-target message stops batching", () => { + const messages = [msg("match-1", "ask"), msg("command_output", "say", "command_output"), msg("match-2", "ask")] + const result = batchNearby(messages, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(3) + expect(result[0].text).toBe("match-1") + expect(result[1].text).toBe("command_output") + expect(result[2].text).toBe("match-2") + }) + + test("all items match → single synthetic message", () => { + const items = [msg("match-1", "ask"), msg("match-2", "ask"), msg("match-3", "ask")] + const result = batchNearby(items, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(1) + expect(result[0].text).toBe("BATCH:match-1,match-2,match-3") + }) + + test("does not mutate the input array", () => { + const items = [msg("match-1", "ask"), msg("match-2", "ask")] + const original = [...items] + batchNearby(items, { isTarget: isMatch, isIgnorableBetweenTargets, isBoundary, synthesize: synthesizeBatch }) + expect(items).toHaveLength(2) + expect(items).toEqual(original) + }) + + test("returns a new array, not the same reference", () => { + const items = [msg("a"), msg("b")] + const result = batchNearby(items, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).not.toBe(items) + }) + + test("synthesize callback receives the correct batches", () => { + const spy = vi.fn(synthesizeBatch) + const items = [ + msg("match-1", "ask"), + msg("", "say", "api_req_started"), + msg("match-2", "ask"), + msg("other"), + msg("match-3", "ask"), + msg("", "say", "reasoning"), + msg("match-4", "ask"), + ] + batchNearby(items, { isTarget: isMatch, isIgnorableBetweenTargets, isBoundary, synthesize: spy }) + expect(spy).toHaveBeenCalledTimes(2) + expect(spy.mock.calls[0][0]).toHaveLength(2) + expect(spy.mock.calls[1][0]).toHaveLength(2) + }) + + test("batch at the end of the array", () => { + const items = [msg("other"), msg("match-1", "ask"), msg("", "say", "api_req_started"), msg("match-2", "ask")] + const result = batchNearby(items, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(2) + expect(result[0].text).toBe("other") + expect(result[1].text).toBe("BATCH:match-1,match-2") + }) + + test("batch at the beginning of the array", () => { + const items = [msg("match-1", "ask"), msg("", "say", "api_req_finished"), msg("match-2", "ask"), msg("other")] + const result = batchNearby(items, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(2) + expect(result[0].text).toBe("BATCH:match-1,match-2") + expect(result[1].text).toBe("other") + }) + + test("multiple ignorable messages between targets", () => { + const items = [ + msg("match-1", "ask"), + msg("", "say", "api_req_started"), + msg("", "say", "reasoning"), + msg("", "say", "api_req_finished"), + msg("match-2", "ask"), + ] + const result = batchNearby(items, { + isTarget: isMatch, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(1) + expect(result[0].text).toBe("BATCH:match-1,match-2") + }) + + test("realistic qwen scenario: tool calls with api_req rows between them", () => { + const messages = [ + msg('{"tool":"readFile","path":"a.ts"}', "ask"), + msg("", "say", "api_req_started"), + msg("", "say", "text"), // empty streaming row + msg('{"tool":"readFile","path":"b.ts"}', "ask"), + msg("", "say", "api_req_finished"), + msg('{"tool":"editFile","path":"c.ts"}', "ask"), + ] + const result = batchNearby(messages, { + isTarget: isToolCall, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(1) // all JSON tool calls batched together (no boundary between them) + expect(result[0].text).toBe( + 'BATCH:{"tool":"readFile","path":"a.ts"},{"tool":"readFile","path":"b.ts"},{"tool":"editFile","path":"c.ts"}', + ) + }) + + test("realistic qwen scenario: two turns separated by user feedback", () => { + const messages = [ + msg('{"tool":"readFile","path":"a.ts"}', "ask"), + msg("", "say", "api_req_started"), + msg('{"tool":"readFile","path":"b.ts"}', "ask"), + msg("feedback", "say", "user_feedback"), // boundary + msg('{"tool":"readFile","path":"c.ts"}', "ask"), + msg("", "say", "api_req_started"), + msg('{"tool":"readFile","path":"d.ts"}', "ask"), + ] + const result = batchNearby(messages, { + isTarget: isToolCall, + isIgnorableBetweenTargets, + isBoundary, + synthesize: synthesizeBatch, + }) + expect(result).toHaveLength(3) // [BATCH:a,b, "feedback", BATCH:c,d] — api_req_started skipped as ignorable + expect(result[0].text).toBe('BATCH:{"tool":"readFile","path":"a.ts"},{"tool":"readFile","path":"b.ts"}') + expect(result[1].text).toBe("feedback") + expect(result[2].text).toBe('BATCH:{"tool":"readFile","path":"c.ts"},{"tool":"readFile","path":"d.ts"}') + }) +}) diff --git a/webview-ui/src/utils/batchNearby.ts b/webview-ui/src/utils/batchNearby.ts new file mode 100644 index 0000000000..d76e2ee149 --- /dev/null +++ b/webview-ui/src/utils/batchNearby.ts @@ -0,0 +1,83 @@ +/** + * Batch tool asks that are near each other, allowing ignorable messages in between. + * + * Unlike `batchConsecutive` which only merges truly adjacent items, this function + * merges items of the same type even when separated by low-information or invisible + * messages (e.g., api_req_started/finished, empty text rows, partial streaming). + * + * It stops merging when it hits a "semantic boundary": user feedback, visible assistant + * text, completion result, different tool group, checkpoint, error, etc. + */ + +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 + /** 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 */ + synthesize: (batch: T[]) => T +} + +/** + * Walk an item array and batch runs of items matching `isTarget`, allowing + * ignorable messages between them. Stops at semantic boundaries. + * + * - Runs of length 1 are passed through unchanged. + * - Runs of length >= 2 are replaced by a single synthetic item. + * - Non-matching / boundary items are preserved in-order. + */ +export function batchNearby(items: T[], options: BatchNearbyOptions): T[] { + const { isTarget, isIgnorableBetweenTargets, isBoundary, synthesize } = options + + const result: T[] = [] + let i = 0 + + while (i < items.length) { + if (isBoundary(items[i])) { + // Boundary stops any current batch and is preserved as-is + result.push(items[i]) + i++ + } else if (isTarget(items[i])) { + // Start collecting a batch of targets, skipping ignorable messages in between + const batch: T[] = [items[i]] + let j = i + 1 + const pendingIgnorable: T[] = [] + + while (j < items.length) { + if (isBoundary(items[j])) { + break // boundary stops the batch + } + if (isTarget(items[j])) { + batch.push(items[j]) + j++ + } else if (isIgnorableBetweenTargets(items[j])) { + pendingIgnorable.push(items[j]) // track but don't commit yet + j++ + } else { + break // non-ignorable, non-target message stops the batch + } + } + + if (batch.length > 1) { + // Bridge succeeded — pending ignorable items are metadata consumed by the batch + result.push(synthesize(batch)) + } else { + // Bridge failed — restore pending ignorable items to preserve in-order semantics + result.push(batch[0]) + if (pendingIgnorable.length > 0) { + result.push(...pendingIgnorable) + } + } + + i = j // items[j] was not consumed — re-examine it on next iteration + } else { + // Non-target, non-boundary item — preserve as-is + result.push(items[i]) + i++ + } + } + + return result +}