diff --git a/core/tools/systemMessageTools/complexToolCall.vitest.ts b/core/tools/systemMessageTools/complexToolCall.vitest.ts deleted file mode 100644 index 954427ee2..000000000 --- a/core/tools/systemMessageTools/complexToolCall.vitest.ts +++ /dev/null @@ -1,304 +0,0 @@ -import { beforeEach, describe, expect, it } from "vitest"; -import { AssistantChatMessage, ChatMessage, PromptLog } from "../.."; -import { interceptSystemToolCalls } from "./interceptSystemToolCalls"; - -describe("interceptSystemToolCalls - Complex Tool Call Test", () => { - let abortController: AbortController; - - beforeEach(() => { - abortController = new AbortController(); - }); - - const createAsyncGenerator = async function* ( - messages: ChatMessage[][], - ): AsyncGenerator { - for (const messageGroup of messages) { - yield messageGroup; - } - return undefined; - }; - - function isAssistantMessageWithToolCalls( - message: ChatMessage, - ): message is AssistantChatMessage { - return ( - message.role === "assistant" && - "toolCalls" in message && - message.toolCalls !== undefined && - message.toolCalls.length > 0 - ); - } - - async function collectAllResults( - generator: AsyncGenerator, - ): Promise { - const results: ChatMessage[][] = []; - while (true) { - const result = await generator.next(); - if (result.done) break; - if (result.value) results.push(result.value); - } - return results; - } - - it("handles complex multi-line tool arguments with nested content and special characters", async () => { - // This test case is based on the example.txt which has complex nested content - // that includes JSON arrays, code blocks, escape characters, and the word "assistant" - const complexDiffContent = `I'll add an exp method to the Calculator class in test.js. This method will raise the current result to the power of a given number. - -\`\`\`tool -TOOL_NAME: search_and_replace_in_file -BEGIN_ARG: filepath -test.js -END_ARG -BEGIN_ARG: diff -[ -"------- SEARCH - /** - * Divide the current result by a number - * @param {number} number - The number to divide by - * @ -assistant -returns {Calculator} - Returns this instance for method chaining - * @throws {Error} - Throws error if attempting to divide by zero - */ - divide(number) { - if (number === 0) { - throw new Error(\"Cannot divide by zero\"); - } - this.result /= number; - return this; - } - -======= - /** - * Divide the current result by a number - * @param {number} number - The number to -assistant - divide by - * @returns {Calculator} - Returns this instance for method chaining - * @throws {Error} - Throws error if attempting to divide by zero - */ - divide(number) { - if (number === 0) { - throw new Error(\"Cannot divide by zero\"); - } - this.result /= number; - return this; - } - - /** - * Raise the current result to the power of a number - * -assistant - @param {number} number - The exponent to raise to - * @returns {Calculator} - Returns this instance for method chaining - */ - exp(number) { - this.result = Math.pow(this.result, number); - return this; - } - -+++++++ REPLACE" -] -END_ARG -\`\`\``; - - const messages: ChatMessage[][] = [ - [ - { - role: "assistant", - content: - "I'll add an exp method to the Calculator class in test.js.\n\n", - }, - ], - [{ role: "assistant", content: "```tool\n" }], - [ - { - role: "assistant", - content: "TOOL_NAME: search_and_replace_in_file\n", - }, - ], - [{ role: "assistant", content: "BEGIN_ARG: filepath\n" }], - [{ role: "assistant", content: "test.js\n" }], - [{ role: "assistant", content: "END_ARG\n" }], - [{ role: "assistant", content: "BEGIN_ARG: diffs\n" }], - [{ role: "assistant", content: complexDiffContent }], - [{ role: "assistant", content: "\nEND_ARG\n" }], - [{ role: "assistant", content: "```" }], - ]; - - const generator = interceptSystemToolCalls( - createAsyncGenerator(messages), - abortController, - ); - - const allResults = await collectAllResults(generator); - - // Should have processed intro text - expect(allResults[0]).toEqual([ - { - role: "assistant", - content: [ - { - type: "text", - text: "I'll add an exp method to the Calculator class in test.js.", - }, - ], - }, - ]); - - // Should have detected and parsed the tool call - const toolCallResults = allResults.filter( - (r): r is AssistantChatMessage[] => - r && r[0] && isAssistantMessageWithToolCalls(r[0]), - ); - - expect(toolCallResults.length).toBeGreaterThan(0); - - // Tool name should be correct - const toolNameResult = toolCallResults.find( - (r) => - r[0].toolCalls?.[0]?.function?.name === "search_and_replace_in_file", - ); - expect(toolNameResult).toBeTruthy(); - - // Reconstruct the full arguments from all deltas - let fullArgs = ""; - for (const toolResult of toolCallResults) { - fullArgs += toolResult[0].toolCalls?.[0]?.function?.arguments || ""; - } - - console.log("Full args reconstructed:", fullArgs); - - // Should be valid JSON - expect(() => JSON.parse(fullArgs)).not.toThrow(); - - const parsed = JSON.parse(fullArgs); - expect(parsed.filepath).toBe("test.js"); - expect(parsed.diffs).toBeTruthy(); - - // The diffs should contain the complex content including special markers - const diffContent1 = - typeof parsed.diffs === "string" - ? parsed.diffs - : JSON.stringify(parsed.diffs); - expect(diffContent1).toContain("SEARCH"); - expect(diffContent1).toContain("REPLACE"); - expect(diffContent1).toContain("Cannot divide by zero"); - expect(diffContent1).toContain("assistant"); // Word should be preserved in content - }); - - it("handles arguments with embedded JSON and special escape sequences", async () => { - // Test with JSON array containing embedded quotes and backslashes - const jsonArrayContent = `["first item", "second \\"quoted\\" item", "third\\nitem"]`; - - const messages: ChatMessage[][] = [ - [{ role: "assistant", content: "```tool\n" }], - [{ role: "assistant", content: "TOOL_NAME: test_tool\n" }], - [{ role: "assistant", content: "BEGIN_ARG: json_array\n" }], - [{ role: "assistant", content: jsonArrayContent }], - [{ role: "assistant", content: "\nEND_ARG\n" }], - [{ role: "assistant", content: "```" }], - ]; - - const generator = interceptSystemToolCalls( - createAsyncGenerator(messages), - abortController, - ); - - const allResults = await collectAllResults(generator); - - // Should have parsed the tool call - const toolCallResults = allResults.filter( - (r): r is AssistantChatMessage[] => - r && r[0] && isAssistantMessageWithToolCalls(r[0]), - ); - - expect(toolCallResults.length).toBeGreaterThan(0); - - // Reconstruct the full arguments - let fullArgs = ""; - for (const toolResult of toolCallResults) { - fullArgs += toolResult[0].toolCalls?.[0]?.function?.arguments || ""; - } - - // Should be valid JSON - expect(() => JSON.parse(fullArgs)).not.toThrow(); - - const parsed = JSON.parse(fullArgs); - expect(parsed.json_array).toBeTruthy(); - - // The parsed array should be correct - const arrayValue = - typeof parsed.json_array === "string" - ? JSON.parse(parsed.json_array) - : parsed.json_array; - - expect(Array.isArray(arrayValue)).toBe(true); - expect(arrayValue).toHaveLength(3); - expect(arrayValue[0]).toBe("first item"); - expect(arrayValue[1]).toBe('second "quoted" item'); - expect(arrayValue[2]).toBe("third\nitem"); - }); - - it("simulates the exact user scenario that causes parsing issues", async () => { - // This simulates the exact scenario from example.txt that was causing issues - const userProblemDiff = `[ -"------- SEARCH - /** - * Divide the current result by a number - * @param {number} number - The number to divide by - * @returns {Calculator} - Returns this instance for method chaining - * @throws {Error} - Throws error if attempting to divide by zero - */ - divide(number) { - if (number === 0) { - throw new Error(\"Cannot divide by zero\"); - } - this.result /= number; - return this; - } -======= - /** - * Divide the current result by a number - * @param {number} number - The number to divide by - * @returns {Calculator} - Returns this instance for method chaining - * @throws {Error} - Throws error if attempting to divide by zero - */ - divide(number) { - if (number === 0) { - throw new Error(\"Cannot divide by zero\"); - } - this.result /= number; - return this; - } - - /** - * Raise the current result to the power of a number - * @param {number} number - The exponent to raise to - * @returns {Calculator} - Returns this instance for method chaining - */ - exp(number) { - this.result = Math.pow(this.result, number); - return this; - } -+++++++ REPLACE" -]`; - - // Test that the JSON string can be properly parsed and used - let parsed; - expect(() => { - parsed = JSON.parse(userProblemDiff); - }).not.toThrow(); - - expect(Array.isArray(parsed)).toBe(true); - expect(parsed!.length).toBe(1); - - const diffContent = parsed![0]; - expect(diffContent).toContain("------- SEARCH"); - expect(diffContent).toContain("======="); - expect(diffContent).toContain("+++++++ REPLACE"); - expect(diffContent).toContain("Cannot divide by zero"); - }); -}); diff --git a/gui/src/util/clientTools/searchReplaceImpl.ts b/gui/src/util/clientTools/searchReplaceImpl.ts index bdd04a5e7..1fcf649e5 100644 --- a/gui/src/util/clientTools/searchReplaceImpl.ts +++ b/gui/src/util/clientTools/searchReplaceImpl.ts @@ -10,45 +10,13 @@ export const searchReplaceToolImpl: ClientToolImpl = async ( toolCallId, extras, ) => { - const { filepath } = args; - let { diffs } = args; - - // Handle legacy parameter name and system message tools edge cases - if (!diffs && (args as any).diff) { - diffs = (args as any).diff; - } - - if (!diffs) { - throw new Error("Missing required parameter: diffs (or diff)"); - } + const { filepath, diffs } = args; const state = extras.getState(); const allowAnonymousTelemetry = state.config.config.allowAnonymousTelemetry; const streamId = uuid(); - // Handle the case where system message tools provide diffs as a JSON string - // instead of an actual array (due to JSON parsing/stringifying in system message tools) - let processedDiffs: string[]; - if (typeof diffs === "string") { - try { - const parsed = JSON.parse(diffs); - if (Array.isArray(parsed)) { - processedDiffs = parsed; - } else { - processedDiffs = [diffs]; - } - } catch (e) { - processedDiffs = [diffs]; - } - } else if (Array.isArray(diffs)) { - processedDiffs = diffs; - } else { - throw new Error( - "diffs parameter must be an array of strings or a JSON string representing an array", - ); - } - // Resolve the file path const resolvedFilepath = await resolveRelativePathInDir( filepath, @@ -60,16 +28,11 @@ export const searchReplaceToolImpl: ClientToolImpl = async ( // Parse all search/replace blocks from all diff strings const allBlocks = []; - for (let diffIndex = 0; diffIndex < processedDiffs.length; diffIndex++) { - const blocks = parseAllSearchReplaceBlocks(processedDiffs[diffIndex]); + for (let diffIndex = 0; diffIndex < diffs.length; diffIndex++) { + const blocks = parseAllSearchReplaceBlocks(diffs[diffIndex]); if (blocks.length === 0) { throw new Error( - `No complete search/replace blocks found in diff ${diffIndex + 1}. Each diff must contain at least one complete SEARCH/REPLACE block with the format: -------- SEARCH -[content to find] -======= -[replacement content] -+++++++ REPLACE`, + `No complete search/replace blocks found in diff ${diffIndex + 1}`, ); } allBlocks.push(...blocks);