fix(chat): share tool batching predicates

Author: easonliang28

webview-ui/src/components/chat/ChatView.tsx

@@ -9,6 +9,7 @@ import { useDebounceEffect } from "@src/utils/useDebounceEffect"
 import { appendImages } from "@src/utils/imageUtils"
 import { getCostBreakdownIfNeeded } from "@src/utils/costFormatting"
 import { batchNearby } from "@src/utils/batchNearby"
+import { isBoundary, isIgnorableBetweenTargets } from "@src/utils/chatBatchingPredicates"
 
 import type { ClineAsk, ClineSayTool, ClineMessage, ExtensionMessage, AudioType, SuggestionItem } from "@roo-code/types"
 import { getCompletionCheckpoint, getSuggestionMode, isRetiredProvider } from "@roo-code/types"
@@ -1279,37 +1280,6 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
 			}
 		}
 
-		// Messages that can be safely skipped over when batching tool asks.
-		// These are low-information or invisible messages that don't affect semantics:
-		// - api_req_started (API request metadata row)
-		// - empty text rows (partial streaming with no visible content)
-		// - reasoning rows (hidden from user by default)
-		const isIgnorableBetweenTargets = (msg: ClineMessage): boolean => {
-			if (msg.type !== "say") return false
-			return msg.say === "api_req_started" || (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.
 		// batchNearby skips over api_req_started, empty text rows, and reasoning rows that
 		// models like qwen insert between tool calls during streaming.

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

@@ -16,6 +16,7 @@ const mockVirtuosoState = vi.hoisted(() => ({
 		defaultItemHeight?: number
 		increaseViewportBy?: number | { top?: number; bottom?: number }
 	} | null,
+	lastData: [] as ClineMessage[],
 }))
 
 // Define minimal types needed for testing
@@ -111,6 +112,7 @@ vi.mock("react-virtuoso", () => ({
 			defaultItemHeight,
 			increaseViewportBy,
 		}
+		mockVirtuosoState.lastData = data
 
 		return (
 			<div data-testid="virtuoso-item-list">
@@ -287,6 +289,22 @@ vi.mock("@vscode/webview-ui-toolkit/react", () => ({
 			</button>
 		)
 	},
+	VSCodeCheckbox: function MockVSCodeCheckbox({
+		children,
+		checked,
+		onChange,
+	}: {
+		children: React.ReactNode
+		checked?: boolean
+		onChange?: (event: React.ChangeEvent<HTMLInputElement>) => void
+	}) {
+		return (
+			<label>
+				<input type="checkbox" checked={checked} onChange={onChange} />
+				{children}
+			</label>
+		)
+	},
 	VSCodeTextField: function MockVSCodeTextField({
 		value,
 		onInput,
@@ -308,6 +326,9 @@ vi.mock("@vscode/webview-ui-toolkit/react", () => ({
 	VSCodeLink: function MockVSCodeLink({ children, href }: { children: React.ReactNode; href?: string }) {
 		return <a href={href}>{children}</a>
 	},
+	VSCodeProgressRing: function MockVSCodeProgressRing() {
+		return <div data-testid="vscode-progress-ring" />
+	},
 }))
 
 // Mock window.postMessage to trigger state hydration
@@ -349,6 +370,55 @@ const renderChatView = (props: Partial<ChatViewProps> = {}) => {
 	)
 }
 
+describe("ChatView - Tool Batching Tests", () => {
+	beforeEach(() => vi.clearAllMocks())
+
+	it("batches readFile asks separated by an API request row", async () => {
+		renderChatView()
+
+		mockPostMessage({
+			clineMessages: [
+				{
+					type: "say",
+					say: "task",
+					ts: 1,
+					text: "Initial task",
+				},
+				{
+					type: "ask",
+					ask: "tool",
+					ts: 2,
+					text: JSON.stringify({ tool: "readFile", path: "a.ts" }),
+				},
+				{
+					type: "say",
+					say: "api_req_started",
+					ts: 3,
+					text: JSON.stringify({ apiProtocol: "anthropic" }),
+				},
+				{
+					type: "ask",
+					ask: "tool",
+					ts: 4,
+					text: JSON.stringify({ tool: "readFile", path: "b.ts" }),
+				},
+			],
+		})
+
+		await waitFor(() => {
+			const toolRows = mockVirtuosoState.lastData.filter(
+				(message) => message.type === "ask" && message.ask === "tool",
+			)
+			const [toolRow] = toolRows
+
+			expect(toolRows).toHaveLength(1)
+			expect(toolRow?.text).toContain('"batchFiles"')
+			expect(toolRow?.text).toContain('"path":"a.ts"')
+			expect(toolRow?.text).toContain('"path":"b.ts"')
+		})
+	})
+})
+
 describe("ChatView - Sound Playing Tests", () => {
 	beforeEach(() => vi.clearAllMocks())
 

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

@@ -1,4 +1,5 @@
 import { batchNearby } from "../batchNearby"
+import { isBoundary, isIgnorableBetweenTargets } from "../chatBatchingPredicates"
 
 interface TestItem {
 	ts: number
@@ -18,32 +19,6 @@ 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" ||
-		m.say === "codebase_search_result"
-	)
-}
-
 /** Synthesize: merges a batch into a single item with a "BATCH:" marker. */
 const synthesizeBatch = (batch: TestItem[]): TestItem => ({
 	...batch[0],
@@ -346,25 +321,11 @@ describe("batchNearby", () => {
 		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, {
@@ -383,7 +344,7 @@ describe("batchNearby", () => {
 			msg("", "say", "api_req_started"),
 			msg("", "say", "text"), // empty streaming row
 			msg('{"tool":"readFile","path":"b.ts"}', "ask"),
-			msg("", "say", "api_req_finished"),
+			msg("", "say", "reasoning"),
 			msg('{"tool":"editFile","path":"c.ts"}', "ask"),
 		]
 		const result = batchNearby(messages, {

webview-ui/src/utils/chatBatchingPredicates.ts

@@ -0,0 +1,32 @@
+interface BatchableMessage {
+	type?: string
+	say?: string
+	text?: string
+}
+
+/**
+ * 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 => {
+	if (msg.type !== "say") return false
+	return msg.say === "api_req_started" || (msg.say === "text" && !msg.text?.trim()) || msg.say === "reasoning"
+}
+
+/**
+ * Semantic boundaries that stop batching. When batching hits one of these,
+ * the current batch is finalized and the boundary message is preserved as-is.
+ */
+export const isBoundary = (msg: BatchableMessage): 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"
+	)
+}