diff --git a/src/loop.ts b/src/loop.ts index 84cc7f242..9f6e52c87 100644 --- a/src/loop.ts +++ b/src/loop.ts @@ -26,18 +26,49 @@ import type { import { Usage } from './usage'; import { randomUUID } from './utils/randomUUID'; import { safeParseJson } from './utils/safeParseJson'; +import { groupToolCallsForParallelExecution } from './utils/toolGrouping'; const DEFAULT_MAX_TURNS = 50; const DEFAULT_ERROR_RETRY_TURNS = 10; const debug = createDebug('neovate:loop'); +/** + * Tool execution result with unified type structure + */ +type ToolExecutionResult = { + toolCallId: string; + toolName: string; + input: any; + result: ToolResult; + approved: boolean; + deniedToolUse?: ToolUse; +}; + +/** + * Tool call input structure from AI model + */ +type ToolCallInput = { + toolCallId: string; + toolName: string; + input: string; + providerMetadata?: any; +}; + +/** + * Tool call parameters structure + */ +type ToolCallParams = { + file_path?: string; + [key: string]: any; +}; + async function exponentialBackoffWithCancellation( attempt: number, signal?: AbortSignal, ): Promise { const baseDelay = 1000; - const delay = baseDelay * Math.pow(2, attempt - 1); + const delay = baseDelay * 2 ** (attempt - 1); const checkInterval = 100; const startTime = Date.now(); @@ -238,19 +269,14 @@ export async function runLoop(opts: RunLoopOpts): Promise { let text = ''; let reasoning = ''; - const toolCalls: Array<{ - providerMetadata?: any; - toolCallId: string; - toolName: string; - input: string; - }> = []; + const toolCalls: ToolCallInput[] = []; const requestId = randomUUID(); const m: LanguageModelV2 = await opts.model._mCreator(); const tools = opts.tools.toLanguageV2Tools(); // Get thinking config based on model's reasoning capability - let thinkingConfig: Record | undefined = undefined; + let thinkingConfig: Record | undefined; if (shouldThinking && opts.thinking) { thinkingConfig = getThinkingConfig(opts.model, opts.thinking.effort); shouldThinking = false; @@ -455,7 +481,7 @@ export async function runLoop(opts: RunLoopOpts): Promise { toolUse.displayName = displayName; } if (toolCall.providerMetadata) { - // @ts-ignore + // @ts-expect-error toolUse.providerMetadata = toolCall.providerMetadata; } assistantContent.push(toolUse); @@ -477,8 +503,48 @@ export async function runLoop(opts: RunLoopOpts): Promise { break; } - const toolResults: any[] = []; - for (const toolCall of toolCalls) { + const toolResults: ToolExecutionResult[] = []; + + // Helper function to add tool results to history + const addToolResultsToHistory = async ( + results: ToolExecutionResult[], + ): Promise => { + await history.addMessage({ + role: 'tool', + content: results.map((tr) => ({ + type: 'tool-result' as const, + toolCallId: tr.toolCallId, + toolName: tr.toolName, + input: tr.input, + result: tr.result, + })), + } as any); + }; + + // Helper function to handle tool denial + const handleToolDenial = async ( + results: ToolExecutionResult[], + deniedResult: ToolExecutionResult, + ): Promise => { + await addToolResultsToHistory(results); + return { + success: false, + error: { + type: 'tool_denied', + message: 'Error: Tool execution was denied by user.', + details: { + toolUse: deniedResult.deniedToolUse, + history, + usage: totalUsage, + }, + }, + }; + }; + + // Helper function to execute a single tool call + const executeSingleToolCall = async ( + toolCall: ToolCallInput, + ): Promise => { let toolUse: ToolUse = { name: toolCall.toolName, params: safeParseJson(toolCall.input), @@ -488,7 +554,7 @@ export async function runLoop(opts: RunLoopOpts): Promise { toolUse = await opts.onToolUse(toolUse as ToolUse); } let approved = true; - let updatedParams: ToolParams | undefined = undefined; + let updatedParams: ToolParams | undefined; if (opts.onToolApprove) { const approvalResult = await opts.onToolApprove(toolUse as ToolUse); @@ -501,7 +567,6 @@ export async function runLoop(opts: RunLoopOpts): Promise { } if (approved) { - toolCallsCount++; if (updatedParams) { toolUse.params = { ...toolUse.params, ...updatedParams }; } @@ -512,14 +577,13 @@ export async function runLoop(opts: RunLoopOpts): Promise { if (opts.onToolResult) { toolResult = await opts.onToolResult(toolUse, toolResult, approved); } - toolResults.push({ + return { toolCallId: toolUse.callId, toolName: toolUse.name, input: toolUse.params, result: toolResult, - }); - // Prevent normal turns from being terminated due to exceeding the limit - turnsCount--; + approved: true, + }; } else { const message = 'Error: Tool execution was denied by user.'; let toolResult: ToolResult = { @@ -529,51 +593,174 @@ export async function runLoop(opts: RunLoopOpts): Promise { if (opts.onToolResult) { toolResult = await opts.onToolResult(toolUse, toolResult, approved); } - toolResults.push({ + return { toolCallId: toolUse.callId, toolName: toolUse.name, input: toolUse.params, result: toolResult, - }); - await history.addMessage({ - role: 'tool', - content: toolResults.map((tr) => { + approved: false, + deniedToolUse: toolUse, + }; + } + }; + + // Group tool calls for parallel execution + const toolGroups = groupToolCallsForParallelExecution( + toolCalls.map((tc) => ({ + ...tc, + params: safeParseJson(tc.input) as ToolCallParams, + })), + ); + + // Debug: Log grouping information + debug( + 'Tool calls grouped: %d total calls -> %d groups', + toolCalls.length, + toolGroups.length, + ); + toolGroups.forEach((group, index) => { + debug( + 'Group %d: %s, %d tools [%s]', + index, + group.canExecuteInParallel + ? group.isReadOnly + ? 'parallel/read-only' + : 'parallel/write' + : 'sequential', + group.toolCalls.length, + group.toolCalls.map((tc) => tc.toolName).join(', '), + ); + }); + + for (const group of toolGroups) { + // Execute based on canExecuteInParallel flag + // The grouping logic already handles read-only vs write conflicts + if (!group.canExecuteInParallel) { + // Execute sequentially + debug('Executing group sequentially: %d tools', group.toolCalls.length); + for (const toolCall of group.toolCalls) { + const result = await executeSingleToolCall(toolCall); + + if (result.approved) { + toolCallsCount++; + toolResults.push(result); + // Prevent normal turns from being terminated due to exceeding the limit + turnsCount--; + } else { + toolResults.push(result); + return handleToolDenial(toolResults, result); + } + } + } else { + // Execute in parallel - use group-level approval + debug( + 'Executing group in parallel: %d tools [%s]', + group.toolCalls.length, + group.toolCalls.map((tc) => tc.toolName).join(', '), + ); + // First, get approval for the first tool in the group + // This will also handle session-level approval settings (autoEdit, etc.) + const firstToolCall = group.toolCalls[0]; + let firstToolUse: ToolUse = { + name: firstToolCall.toolName, + params: safeParseJson(firstToolCall.input), + callId: firstToolCall.toolCallId, + }; + if (opts.onToolUse) { + firstToolUse = await opts.onToolUse(firstToolUse as ToolUse); + } + + let groupApproved = true; + let updatedParams: ToolParams | undefined; + + if (opts.onToolApprove) { + const approvalResult = await opts.onToolApprove( + firstToolUse as ToolUse, + ); + if (typeof approvalResult === 'object') { + groupApproved = approvalResult.approved; + updatedParams = approvalResult.params; + } else { + groupApproved = approvalResult; + } + } + + // If group is denied, create error result and return + if (!groupApproved) { + const message = 'Error: Tool execution was denied by user.'; + let toolResult: ToolResult = { + llmContent: message, + isError: true, + }; + if (opts.onToolResult) { + toolResult = await opts.onToolResult( + firstToolUse, + toolResult, + false, + ); + } + const deniedResult: ToolExecutionResult = { + toolCallId: firstToolUse.callId, + toolName: firstToolUse.name, + input: firstToolUse.params, + result: toolResult, + approved: false, + deniedToolUse: firstToolUse, + }; + toolResults.push(deniedResult); + return handleToolDenial(toolResults, deniedResult); + } + + // Group is approved - execute all tools in parallel without individual approval + const groupStartTime = Date.now(); + const groupResults = await Promise.all( + group.toolCalls.map(async (toolCall, index) => { + let toolUse: ToolUse = { + name: toolCall.toolName, + params: safeParseJson(toolCall.input), + callId: toolCall.toolCallId, + }; + if (opts.onToolUse) { + toolUse = await opts.onToolUse(toolUse as ToolUse); + } + + // Apply updated params to first tool if provided + if (index === 0 && updatedParams) { + toolUse.params = { ...toolUse.params, ...updatedParams }; + } + + // Execute without approval check (group already approved) + let toolResult = await opts.tools.invoke( + toolUse.name, + JSON.stringify(toolUse.params), + ); + if (opts.onToolResult) { + toolResult = await opts.onToolResult(toolUse, toolResult, true); + } return { - type: 'tool-result', - toolCallId: tr.toolCallId, - toolName: tr.toolName, - input: tr.input, - result: tr.result, + toolCallId: toolUse.callId, + toolName: toolUse.name, + input: toolUse.params, + result: toolResult, + approved: true, }; }), - } as any); - return { - success: false, - error: { - type: 'tool_denied', - message, - details: { - toolUse, - history, - usage: totalUsage, - }, - }, - }; + ); + + // All tools succeeded + const groupDuration = Date.now() - groupStartTime; + debug( + 'Parallel execution completed in %dms: %d tools', + groupDuration, + groupResults.length, + ); + toolCallsCount += groupResults.length; + turnsCount -= groupResults.length; + toolResults.push(...groupResults); } } if (toolResults.length) { - await history.addMessage({ - role: 'tool', - content: toolResults.map((tr) => { - return { - type: 'tool-result', - toolCallId: tr.toolCallId, - toolName: tr.toolName, - input: tr.input, - result: tr.result, - }; - }), - } as any); + await addToolResultsToHistory(toolResults); } } const duration = Date.now() - startTime; diff --git a/src/systemPrompt.ts b/src/systemPrompt.ts index 99cd17bdc..37c5d89e2 100644 --- a/src/systemPrompt.ts +++ b/src/systemPrompt.ts @@ -56,13 +56,40 @@ I've found some existing telemetry code. Let me mark the first todo as in_progre # Doing tasks The user will primarily request you perform software engineering tasks. This includes solving bugs, adding new functionality, refactoring code, explaining code, and more. For these tasks the following steps are recommended: - Use the ${TOOL_NAMES.TODO_WRITE} tool to plan the task if required -- Use the available search tools to understand the codebase and the user's query. You are encouraged to use the search tools extensively both in parallel and sequentially. +- Use the available search tools to understand the codebase and the user's query. You are encouraged to use the search tools extensively. - Implement the solution using all tools available to you - Verify the solution if possible with tests. NEVER assume specific test framework or test script. Check the README or search codebase to determine the testing approach. - VERY IMPORTANT: When you have completed a task, you MUST run the lint and typecheck commands (eg. npm run lint, npm run typecheck, ruff, etc.) with ${TOOL_NAMES.BASH} if they were provided to you to ensure your code is correct. If you are unable to find the correct command, ask the user for the command to run and if they supply it, proactively suggest writing it to ${productName}.md so that you will know to run it next time. NEVER commit changes unless the user explicitly asks you to. It is VERY IMPORTANT to only commit when explicitly asked, otherwise the user will feel that you are being too proactive. IMPORTANT: Always use the ${TOOL_NAMES.TODO_WRITE} tool to plan and track tasks throughout the conversation. + +# Tool Call Optimization +You have the capability to call multiple tools in a single response. Always look for opportunities to maximize parallel tool execution. + +**CRITICAL: Maximize Parallel Tool Calls** +For maximum efficiency, whenever you perform multiple independent operations, invoke all relevant tools simultaneously rather than sequentially. + +**When to use parallel tool calls:** +- Reading multiple files → call all read tools simultaneously +- Searching different patterns → run all ${TOOL_NAMES.GREP} or glob tools in parallel +- Gathering independent information → execute all information-gathering tools together +- Writing to different files → call all write or edit tools for different files in parallel + +**Sequential execution required ONLY when:** +- Output of one tool is required as input for the next tool +- Writing to the same file multiple times (must be done in order) +- Running bash commands that have dependencies +- Operations that modify global state (${TOOL_NAMES.BASH}, ${TOOL_NAMES.TODO_WRITE}) + +**Examples:** +- ✅ GOOD: Read 3 files → use 3 concurrent read calls in one response +- ✅ GOOD: Write to file1.ts, file2.ts, file3.ts → use 3 concurrent write calls +- ✅ GOOD: Search for "import", "export", "function" → use 3 concurrent ${TOOL_NAMES.GREP} calls +- ❌ BAD: Read file1, wait, then read file2, wait, then read file3 (use parallel instead) +- ❌ BAD: Write to same file twice in parallel (must be sequential) + +**DEFAULT TO PARALLEL:** Unless you have a specific reason why operations MUST be sequential (output of A required for input of B), always execute multiple tools simultaneously. This is not just an optimization - it's the expected behavior. `; } @@ -76,7 +103,11 @@ export function generateSystemPrompt(opts: { const { outputStyle } = opts; const isDefaultOutputStyle = outputStyle.isDefault(); return ` -You are an interactive CLI tool that helps users ${isDefaultOutputStyle ? 'with software engineering tasks.' : `according to your "Output Style" below, which describes how you should respond to user queries.`} Use the instructions below and the tools available to you to assist the user. +You are an interactive CLI tool that helps users ${ + isDefaultOutputStyle + ? 'with software engineering tasks.' + : `according to your "Output Style" below, which describes how you should respond to user queries.` + } Use the instructions below and the tools available to you to assist the user. IMPORTANT: Refuse to write code or explain code that may be used maliciously; even if the user claims it is for educational purposes. ${ diff --git a/src/utils/toolGrouping.test.ts b/src/utils/toolGrouping.test.ts new file mode 100644 index 000000000..6537dcb41 --- /dev/null +++ b/src/utils/toolGrouping.test.ts @@ -0,0 +1,573 @@ +import { describe, expect, it } from 'vitest'; +import { + TOOL_EXECUTION_CATEGORIES, + getToolExecutionCategory, + groupToolCallsForParallelExecution, + normalizeFilePath, + type ToolCall, + type ToolCallGroup, +} from './toolGrouping'; + +describe('toolGrouping', () => { + describe('TOOL_EXECUTION_CATEGORIES', () => { + it('should contain correct safe parallel tools', () => { + expect(TOOL_EXECUTION_CATEGORIES.SAFE_PARALLEL.has('read')).toBe(true); + expect(TOOL_EXECUTION_CATEGORIES.SAFE_PARALLEL.has('ls')).toBe(true); + expect(TOOL_EXECUTION_CATEGORIES.SAFE_PARALLEL.has('glob')).toBe(true); + expect(TOOL_EXECUTION_CATEGORIES.SAFE_PARALLEL.has('grep')).toBe(true); + expect(TOOL_EXECUTION_CATEGORIES.SAFE_PARALLEL.has('fetch')).toBe(true); + expect(TOOL_EXECUTION_CATEGORIES.SAFE_PARALLEL.has('todo_read')).toBe( + true, + ); + expect(TOOL_EXECUTION_CATEGORIES.SAFE_PARALLEL.has('bash_output')).toBe( + true, + ); + }); + + it('should contain correct file write tools', () => { + expect(TOOL_EXECUTION_CATEGORIES.FILE_WRITE.has('write')).toBe(true); + expect(TOOL_EXECUTION_CATEGORIES.FILE_WRITE.has('edit')).toBe(true); + }); + + it('should contain correct global effect tools', () => { + expect(TOOL_EXECUTION_CATEGORIES.GLOBAL_EFFECT.has('bash')).toBe(true); + expect(TOOL_EXECUTION_CATEGORIES.GLOBAL_EFFECT.has('todo_write')).toBe( + true, + ); + expect(TOOL_EXECUTION_CATEGORIES.GLOBAL_EFFECT.has('kill_bash')).toBe( + true, + ); + expect( + TOOL_EXECUTION_CATEGORIES.GLOBAL_EFFECT.has('ask_user_question'), + ).toBe(true); + }); + }); + + describe('getToolExecutionCategory', () => { + it('should return safe_parallel for read-only tools', () => { + expect(getToolExecutionCategory('read')).toBe('safe_parallel'); + expect(getToolExecutionCategory('ls')).toBe('safe_parallel'); + expect(getToolExecutionCategory('glob')).toBe('safe_parallel'); + expect(getToolExecutionCategory('grep')).toBe('safe_parallel'); + expect(getToolExecutionCategory('fetch')).toBe('safe_parallel'); + expect(getToolExecutionCategory('todo_read')).toBe('safe_parallel'); + }); + + it('should return file_write for file write tools', () => { + expect(getToolExecutionCategory('write')).toBe('file_write'); + expect(getToolExecutionCategory('edit')).toBe('file_write'); + }); + + it('should return global_effect for global effect tools', () => { + expect(getToolExecutionCategory('bash')).toBe('global_effect'); + expect(getToolExecutionCategory('todo_write')).toBe('global_effect'); + expect(getToolExecutionCategory('kill_bash')).toBe('global_effect'); + expect(getToolExecutionCategory('ask_user_question')).toBe( + 'global_effect', + ); + }); + + it('should return global_effect for MCP tools', () => { + expect(getToolExecutionCategory('mcp__some_tool')).toBe('global_effect'); + expect(getToolExecutionCategory('mcp__another_tool')).toBe( + 'global_effect', + ); + }); + + it('should return safe_parallel for unknown tools', () => { + expect(getToolExecutionCategory('unknown_tool')).toBe('safe_parallel'); + expect(getToolExecutionCategory('custom_tool')).toBe('safe_parallel'); + }); + }); + + describe('normalizeFilePath', () => { + it('should normalize valid file paths', () => { + expect(normalizeFilePath('/path/to/file.ts')).toBe('/path/to/file.ts'); + expect(normalizeFilePath('relative/path/file.ts')).toBe( + 'relative/path/file.ts', + ); + }); + + it('should convert backslashes to forward slashes', () => { + expect(normalizeFilePath('path\\to\\file.ts')).toBe('path/to/file.ts'); + expect(normalizeFilePath('C:\\Users\\test\\file.ts')).toContain('/'); + }); + + it('should return null for invalid inputs', () => { + expect(normalizeFilePath(undefined)).toBeNull(); + expect(normalizeFilePath('')).toBeNull(); + expect(normalizeFilePath(null as any)).toBeNull(); + expect(normalizeFilePath(123 as any)).toBeNull(); + }); + }); + + describe('groupToolCallsForParallelExecution', () => { + it('should group safe parallel tools together', () => { + const toolCalls: ToolCall[] = [ + { toolName: 'read', toolCallId: '1', input: '', params: {} }, + { toolName: 'ls', toolCallId: '2', input: '', params: {} }, + { toolName: 'grep', toolCallId: '3', input: '', params: {} }, + ]; + + const groups = groupToolCallsForParallelExecution(toolCalls); + + expect(groups).toHaveLength(1); + expect(groups[0].canExecuteInParallel).toBe(true); + expect(groups[0].toolCalls).toHaveLength(3); + }); + + it('should isolate global effect tools in separate groups', () => { + const toolCalls: ToolCall[] = [ + { toolName: 'read', toolCallId: '1', input: '', params: {} }, + { toolName: 'bash', toolCallId: '2', input: '', params: {} }, + { toolName: 'ls', toolCallId: '3', input: '', params: {} }, + ]; + + const groups = groupToolCallsForParallelExecution(toolCalls); + + expect(groups).toHaveLength(3); + expect(groups[0].canExecuteInParallel).toBe(true); + expect(groups[0].toolCalls).toHaveLength(1); // read + expect(groups[1].canExecuteInParallel).toBe(false); + expect(groups[1].toolCalls).toHaveLength(1); // bash + expect(groups[2].canExecuteInParallel).toBe(true); + expect(groups[2].toolCalls).toHaveLength(1); // ls + }); + + it('should group different file writes in the same group', () => { + const toolCalls: ToolCall[] = [ + { + toolName: 'write', + toolCallId: '1', + input: '', + params: { file_path: '/path/to/file1.ts' }, + }, + { + toolName: 'edit', + toolCallId: '2', + input: '', + params: { file_path: '/path/to/file2.ts' }, + }, + { + toolName: 'write', + toolCallId: '3', + input: '', + params: { file_path: '/path/to/file3.ts' }, + }, + ]; + + const groups = groupToolCallsForParallelExecution(toolCalls); + + expect(groups).toHaveLength(1); + expect(groups[0].canExecuteInParallel).toBe(true); + expect(groups[0].toolCalls).toHaveLength(3); + }); + + it('should separate same file writes into different groups', () => { + const toolCalls: ToolCall[] = [ + { + toolName: 'write', + toolCallId: '1', + input: '', + params: { file_path: '/path/to/file.ts' }, + }, + { + toolName: 'edit', + toolCallId: '2', + input: '', + params: { file_path: '/path/to/file.ts' }, + }, + ]; + + const groups = groupToolCallsForParallelExecution(toolCalls); + + expect(groups).toHaveLength(2); + expect(groups[0].canExecuteInParallel).toBe(true); + expect(groups[0].toolCalls).toHaveLength(1); + expect(groups[1].canExecuteInParallel).toBe(true); + expect(groups[1].toolCalls).toHaveLength(1); + }); + + it('should maintain order for multiple writes to same file', () => { + const toolCalls: ToolCall[] = [ + { + toolName: 'write', + toolCallId: '1', + input: '', + params: { file_path: '/path/to/file.ts' }, + }, + { + toolName: 'read', + toolCallId: '2', + input: '', + params: { file_path: '/path/to/other.ts' }, + }, + { + toolName: 'edit', + toolCallId: '3', + input: '', + params: { file_path: '/path/to/file.ts' }, + }, + { + toolName: 'write', + toolCallId: '4', + input: '', + params: { file_path: '/path/to/file.ts' }, + }, + ]; + + const groups = groupToolCallsForParallelExecution(toolCalls); + + // Group 0: write file.ts + // Group 1: read other.ts (read-only, separated from writes) + // Group 2: edit file.ts (same file as group 0, so new group) + // Group 3: write file.ts (same file as group 2, so new group) + expect(groups).toHaveLength(4); + expect(groups[0].toolCalls).toHaveLength(1); + expect(groups[0].toolCalls[0].toolCallId).toBe('1'); + expect(groups[1].toolCalls).toHaveLength(1); + expect(groups[1].toolCalls[0].toolCallId).toBe('2'); + expect(groups[2].toolCalls).toHaveLength(1); + expect(groups[2].toolCalls[0].toolCallId).toBe('3'); + expect(groups[3].toolCalls).toHaveLength(1); + expect(groups[3].toolCalls[0].toolCallId).toBe('4'); + }); + + it('should handle file writes without file_path', () => { + const toolCalls: ToolCall[] = [ + { toolName: 'write', toolCallId: '1', input: '', params: {} }, + { + toolName: 'edit', + toolCallId: '2', + input: '', + params: { file_path: undefined }, + }, + { + toolName: 'write', + toolCallId: '3', + input: '', + params: { file_path: '/path/to/file.ts' }, + }, + ]; + + const groups = groupToolCallsForParallelExecution(toolCalls); + + expect(groups).toHaveLength(1); + expect(groups[0].canExecuteInParallel).toBe(true); + expect(groups[0].toolCalls).toHaveLength(3); + }); + + it('should handle mixed tool types correctly', () => { + const toolCalls: ToolCall[] = [ + { toolName: 'read', toolCallId: '1', input: '', params: {} }, + { + toolName: 'write', + toolCallId: '2', + input: '', + params: { file_path: '/path/to/file.ts' }, + }, + { toolName: 'bash', toolCallId: '3', input: '', params: {} }, + { toolName: 'grep', toolCallId: '4', input: '', params: {} }, + { + toolName: 'edit', + toolCallId: '5', + input: '', + params: { file_path: '/path/to/other.ts' }, + }, + ]; + + const groups = groupToolCallsForParallelExecution(toolCalls); + + expect(groups).toHaveLength(5); + // Group 0: read (read-only) + expect(groups[0].canExecuteInParallel).toBe(true); + expect(groups[0].toolCalls).toHaveLength(1); + expect(groups[0].toolCalls[0].toolCallId).toBe('1'); + // Group 1: write (write operation, separated from reads) + expect(groups[1].canExecuteInParallel).toBe(true); + expect(groups[1].toolCalls).toHaveLength(1); + expect(groups[1].toolCalls[0].toolCallId).toBe('2'); + // Group 2: bash (isolated global effect) + expect(groups[2].canExecuteInParallel).toBe(false); + expect(groups[2].toolCalls).toHaveLength(1); + expect(groups[2].toolCalls[0].toolCallId).toBe('3'); + // Group 3: grep (read-only) + expect(groups[3].canExecuteInParallel).toBe(true); + expect(groups[3].toolCalls).toHaveLength(1); + expect(groups[3].toolCalls[0].toolCallId).toBe('4'); + // Group 4: edit (write operation, separated from reads) + expect(groups[4].canExecuteInParallel).toBe(true); + expect(groups[4].toolCalls).toHaveLength(1); + expect(groups[4].toolCalls[0].toolCallId).toBe('5'); + }); + + it('should handle MCP tools as global effects', () => { + const toolCalls: ToolCall[] = [ + { toolName: 'read', toolCallId: '1', input: '', params: {} }, + { + toolName: 'mcp__custom_tool', + toolCallId: '2', + input: '', + params: {}, + }, + { toolName: 'ls', toolCallId: '3', input: '', params: {} }, + ]; + + const groups = groupToolCallsForParallelExecution(toolCalls); + + expect(groups).toHaveLength(3); + expect(groups[0].canExecuteInParallel).toBe(true); + expect(groups[1].canExecuteInParallel).toBe(false); // MCP tool + expect(groups[2].canExecuteInParallel).toBe(true); + }); + + it('should handle empty tool calls array', () => { + const groups = groupToolCallsForParallelExecution([]); + expect(groups).toHaveLength(0); + }); + + it('should handle single tool call', () => { + const toolCalls: ToolCall[] = [ + { toolName: 'read', toolCallId: '1', input: '', params: {} }, + ]; + const groups = groupToolCallsForParallelExecution(toolCalls); + + expect(groups).toHaveLength(1); + expect(groups[0].canExecuteInParallel).toBe(true); + expect(groups[0].toolCalls).toHaveLength(1); + }); + + it('should normalize file paths for conflict detection', () => { + const toolCalls: ToolCall[] = [ + { + toolName: 'write', + toolCallId: '1', + input: '', + params: { file_path: '/path/to/file.ts' }, + }, + { + toolName: 'edit', + toolCallId: '2', + input: '', + params: { file_path: '/path/to/../to/file.ts' }, + }, + ]; + + const groups = groupToolCallsForParallelExecution(toolCalls); + + // Should detect conflict after normalization + expect(groups).toHaveLength(2); + }); + + it('should handle consecutive global effect tools', () => { + const toolCalls: ToolCall[] = [ + { toolName: 'bash', toolCallId: '1', input: '', params: {} }, + { toolName: 'todo_write', toolCallId: '2', input: '', params: {} }, + { + toolName: 'ask_user_question', + toolCallId: '3', + input: '', + params: {}, + }, + ]; + + const groups = groupToolCallsForParallelExecution(toolCalls); + + expect(groups).toHaveLength(3); + groups.forEach((group) => { + expect(group.canExecuteInParallel).toBe(false); + expect(group.toolCalls).toHaveLength(1); + }); + }); + + it('should track file access across multiple groups', () => { + const toolCalls: ToolCall[] = [ + { + toolName: 'write', + toolCallId: '1', + input: '', + params: { file_path: '/path/to/file1.ts' }, + }, + { + toolName: 'write', + toolCallId: '2', + input: '', + params: { file_path: '/path/to/file2.ts' }, + }, + { toolName: 'bash', toolCallId: '3', input: '', params: {} }, // Force new group + { + toolName: 'edit', + toolCallId: '4', + input: '', + params: { file_path: '/path/to/file1.ts' }, + }, // Should conflict with toolCall 1 + ]; + + const groups = groupToolCallsForParallelExecution(toolCalls); + + expect(groups).toHaveLength(3); + // Group 1: write file1 + write file2 + expect(groups[0].toolCalls).toHaveLength(2); + // Group 2: bash + expect(groups[1].toolCalls).toHaveLength(1); + // Group 3: edit file1 (new group due to previous access in group 0) + expect(groups[2].toolCalls).toHaveLength(1); + expect(groups[2].toolCalls[0].toolCallId).toBe('4'); + }); + + it('should group multiple consecutive read operations together', () => { + const toolCalls: ToolCall[] = [ + { + toolName: 'read', + toolCallId: '1', + input: '', + params: { file_path: '/path/to/file1.txt' }, + }, + { + toolName: 'read', + toolCallId: '2', + input: '', + params: { file_path: '/path/to/file2.txt' }, + }, + { + toolName: 'read', + toolCallId: '3', + input: '', + params: { file_path: '/path/to/file3.txt' }, + }, + ]; + + const groups = groupToolCallsForParallelExecution(toolCalls); + + expect(groups).toHaveLength(1); + expect(groups[0].canExecuteInParallel).toBe(true); + expect(groups[0].isReadOnly).toBe(true); + expect(groups[0].toolCalls).toHaveLength(3); + expect(groups[0].toolCalls.map((tc) => tc.toolCallId)).toEqual([ + '1', + '2', + '3', + ]); + }); + + it('should group multiple consecutive write operations to different files together', () => { + const toolCalls: ToolCall[] = [ + { + toolName: 'write', + toolCallId: '1', + input: '', + params: { file_path: '/path/to/file4.txt' }, + }, + { + toolName: 'write', + toolCallId: '2', + input: '', + params: { file_path: '/path/to/file5.txt' }, + }, + { + toolName: 'write', + toolCallId: '3', + input: '', + params: { file_path: '/path/to/file6.txt' }, + }, + { + toolName: 'write', + toolCallId: '4', + input: '', + params: { file_path: '/path/to/file7.txt' }, + }, + { + toolName: 'write', + toolCallId: '5', + input: '', + params: { file_path: '/path/to/file8.txt' }, + }, + { + toolName: 'write', + toolCallId: '6', + input: '', + params: { file_path: '/path/to/file9.txt' }, + }, + { + toolName: 'write', + toolCallId: '7', + input: '', + params: { file_path: '/path/to/file10.txt' }, + }, + ]; + + const groups = groupToolCallsForParallelExecution(toolCalls); + + expect(groups).toHaveLength(1); + expect(groups[0].canExecuteInParallel).toBe(true); + expect(groups[0].isReadOnly).toBe(false); + expect(groups[0].toolCalls).toHaveLength(7); + expect(groups[0].toolCalls.map((tc) => tc.toolCallId)).toEqual([ + '1', + '2', + '3', + '4', + '5', + '6', + '7', + ]); + }); + + it('should handle read-then-write-then-todoWrite pattern correctly', () => { + const toolCalls: ToolCall[] = [ + { + toolName: 'read', + toolCallId: '1', + input: '', + params: { file_path: '/path/to/file1.txt' }, + }, + { + toolName: 'read', + toolCallId: '2', + input: '', + params: { file_path: '/path/to/file2.txt' }, + }, + { + toolName: 'read', + toolCallId: '3', + input: '', + params: { file_path: '/path/to/file3.txt' }, + }, + { toolName: 'todo_write', toolCallId: '4', input: '', params: {} }, + { + toolName: 'write', + toolCallId: '5', + input: '', + params: { file_path: '/path/to/file4.txt' }, + }, + { + toolName: 'write', + toolCallId: '6', + input: '', + params: { file_path: '/path/to/file5.txt' }, + }, + { toolName: 'todo_write', toolCallId: '7', input: '', params: {} }, + ]; + + const groups = groupToolCallsForParallelExecution(toolCalls); + + expect(groups).toHaveLength(4); + // Group 0: 3 reads (parallel) + expect(groups[0].canExecuteInParallel).toBe(true); + expect(groups[0].isReadOnly).toBe(true); + expect(groups[0].toolCalls).toHaveLength(3); + // Group 1: todo_write (sequential) + expect(groups[1].canExecuteInParallel).toBe(false); + expect(groups[1].toolCalls).toHaveLength(1); + expect(groups[1].toolCalls[0].toolCallId).toBe('4'); + // Group 2: 2 writes (parallel) + expect(groups[2].canExecuteInParallel).toBe(true); + expect(groups[2].isReadOnly).toBe(false); + expect(groups[2].toolCalls).toHaveLength(2); + // Group 3: todo_write (sequential) + expect(groups[3].canExecuteInParallel).toBe(false); + expect(groups[3].toolCalls).toHaveLength(1); + expect(groups[3].toolCalls[0].toolCallId).toBe('7'); + }); + }); +}); diff --git a/src/utils/toolGrouping.ts b/src/utils/toolGrouping.ts new file mode 100644 index 000000000..af5b5560f --- /dev/null +++ b/src/utils/toolGrouping.ts @@ -0,0 +1,225 @@ +import path from 'pathe'; + +/** + * Tool call parameters structure + */ +export type ToolCallParams = { + file_path?: string; + [key: string]: any; +}; + +/** + * Tool call structure with parsed parameters + */ +export type ToolCall = { + toolCallId: string; + toolName: string; + input: string; + params?: ToolCallParams; + providerMetadata?: any; +}; + +/** + * Tool execution categories based on side effects + */ +export const TOOL_EXECUTION_CATEGORIES = { + // No side effects, can be safely executed in parallel + SAFE_PARALLEL: new Set([ + 'read', + 'ls', + 'glob', + 'grep', + 'fetch', + 'todo_read', + 'bash_output', + ]), + + // File write operations, need conflict checking + FILE_WRITE: new Set(['write', 'edit']), + + // Global side effects, must be executed sequentially + GLOBAL_EFFECT: new Set([ + 'bash', + 'todo_write', + 'kill_bash', + 'ask_user_question', + ]), +} as const; + +export type ToolCallGroup = { + toolCalls: ToolCall[]; + canExecuteInParallel: boolean; + isReadOnly: boolean; // Indicates if all tools in group are read-only +}; + +/** + * Get tool execution category + */ +export function getToolExecutionCategory(toolName: string): string { + if (TOOL_EXECUTION_CATEGORIES.SAFE_PARALLEL.has(toolName)) { + return 'safe_parallel'; + } + if (TOOL_EXECUTION_CATEGORIES.FILE_WRITE.has(toolName)) { + return 'file_write'; + } + if (TOOL_EXECUTION_CATEGORIES.GLOBAL_EFFECT.has(toolName)) { + return 'global_effect'; + } + // MCP tools are treated as global effect for safety + if (toolName.startsWith('mcp__')) { + return 'global_effect'; + } + // Unknown tools default to safe parallel + return 'safe_parallel'; +} + +/** + * Normalize file path for conflict detection + */ +export function normalizeFilePath(filePath: string | undefined): string | null { + if (!filePath || typeof filePath !== 'string') return null; + return path.normalize(filePath).replace(/\\/g, '/'); +} + +/** + * Group tool calls for parallel execution + * + * This function implements intelligent tool grouping to maximize parallel execution + * while maintaining correctness and proper ordering. + * + * Grouping Strategy: + * 1. **Safe Parallel Tools** (read-only): Grouped together for maximum parallelism + * - Examples: read, ls, glob, grep, fetch, todo_read + * + * 2. **File Write Tools**: Grouped based on file conflicts + * - Different files: Execute in parallel within same group + * - Same file: Split into separate sequential groups to maintain ordering + * - File path normalization ensures accurate conflict detection + * + * 3. **Global Effect Tools**: Always isolated into separate sequential groups + * - Examples: bash, todo_write, kill_bash, ask_user_question + * - These tools have side effects beyond file system + * + * 4. **MCP Tools**: Treated as global effect tools (sequential) + * + * File Tracking: + * - Maintains a map of file -> last group index that touched it + * - When a file is accessed again, creates new group to preserve ordering + * - Normalized paths prevent false conflicts (e.g., ./file vs file) + * + * Example Flow: + * Input: [read(a), read(b), write(c), write(d), write(c)] + * Groups: + * 1. [read(a), read(b)] - parallel safe reads + * 2. [write(c), write(d)] - parallel writes to different files + * 3. [write(c)] - new group because 'c' was accessed in group 2 + * + * @param toolCalls - Array of tool calls with parsed parameters + * @returns Array of tool call groups with parallel execution flags + */ +export function groupToolCallsForParallelExecution( + toolCalls: ToolCall[], +): ToolCallGroup[] { + const groups: ToolCallGroup[] = []; + let currentReadOnlyGroup: ToolCall[] = []; + let currentWriteGroup: ToolCall[] = []; + const currentAffectedFiles = new Set(); + // Track which group index last touched each file + const fileToLastGroupIndex = new Map(); + + // Helper function to finalize read-only group + const finalizeReadOnlyGroup = () => { + if (currentReadOnlyGroup.length > 0) { + groups.push({ + toolCalls: currentReadOnlyGroup, + canExecuteInParallel: true, + isReadOnly: true, + }); + currentReadOnlyGroup = []; + } + }; + + // Helper function to finalize write group + const finalizeWriteGroup = () => { + if (currentWriteGroup.length > 0) { + const groupIndex = groups.length; + groups.push({ + toolCalls: currentWriteGroup, + canExecuteInParallel: true, + isReadOnly: false, + }); + // Update last access index for all files in this group + currentAffectedFiles.forEach((file) => { + fileToLastGroupIndex.set(file, groupIndex); + }); + currentWriteGroup = []; + currentAffectedFiles.clear(); + } + }; + + for (const toolCall of toolCalls) { + const category = getToolExecutionCategory(toolCall.toolName); + + // Global effect tools → isolated group, sequential execution + if (category === 'global_effect') { + // Save current groups first + finalizeReadOnlyGroup(); + finalizeWriteGroup(); + // Global effect tool in separate group (cannot execute in parallel) + groups.push({ + toolCalls: [toolCall], + canExecuteInParallel: false, + isReadOnly: false, + }); + continue; + } + + // File write tools → check file conflicts + if (category === 'file_write') { + // Only finalize read-only group if it exists (avoid unnecessary splits) + if (currentReadOnlyGroup.length > 0) { + finalizeReadOnlyGroup(); + } + + const filePath = normalizeFilePath(toolCall.params?.file_path); + + if (filePath) { + const lastGroupIdx = fileToLastGroupIndex.get(filePath); + + // Check if this file was accessed before (in a previous finalized group) + if (lastGroupIdx !== undefined) { + // File was accessed before - must start new group to ensure ordering + finalizeWriteGroup(); + currentWriteGroup.push(toolCall); + currentAffectedFiles.add(filePath); + } else if (currentAffectedFiles.has(filePath)) { + // Same file already in current group → start new group + finalizeWriteGroup(); + currentWriteGroup.push(toolCall); + currentAffectedFiles.add(filePath); + } else { + // Different file → add to current group + currentWriteGroup.push(toolCall); + currentAffectedFiles.add(filePath); + } + } else { + // No file path → add to current group + currentWriteGroup.push(toolCall); + } + continue; + } + + // Safe parallel tools (read-only) → add to read-only group + // Only finalize write group if it exists (avoid unnecessary splits) + if (currentWriteGroup.length > 0) { + finalizeWriteGroup(); + } + currentReadOnlyGroup.push(toolCall); + } + + // Handle remaining groups + finalizeReadOnlyGroup(); + finalizeWriteGroup(); + + return groups; +}