From 951cefc0fc01cc70b1d2e8e2041182926457727d Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Fri, 4 Apr 2025 16:26:25 -0600 Subject: [PATCH] Refactor: Improve file tool context formatting and diff error guidance (#2278) * refactor: Change read_file result format to XML Modifies how the result of the `read_file` tool is presented in the conversation history sent to the AI model. Previously, the format was: [read_file for 'path/to/file'] Result: {content_or_error} This commit changes the format to use XML tags for better structure and potentially easier parsing by the model: path/to/file {content_or_error} This change only affects the `read_file` tool result formatting within the user context message constructed in `src/core/Cline.ts`. Other tool result formats remain unchanged. * fix: Update error message for better clarity in multi-search-replace strategy Refines the error message returned when no sufficiently similar match is found during the multi-search-replace operation. The message now includes a clearer instruction to use the read_file tool for obtaining the latest file content before attempting to apply the diff again. * refactor: Update readFileTool to return results in XML format Modifies the `readFileTool` function to format the output as XML, enhancing the structure of the returned file content. This change aligns with previous updates to ensure consistent result formatting across tools. * refactor: Simplify result handling for tool responses in Cline This update refines the handling of tool responses in the `Cline` class by removing the XML formatting for `read_file` results and consolidating the logic for pushing results to the user message content. The changes ensure that all tool results are processed uniformly, improving code clarity and maintainability. * test: Update read_file tests to validate XML formatted results This commit modifies the assertions in the `read_file` tool tests to check for the expected XML format in the results. The changes ensure that the output structure aligns with recent updates to the tool's response formatting, enhancing test accuracy and reliability. * test: Refactor assertions in read_file tests to use expected XML variable This commit updates the `read_file` tool tests to utilize a predefined variable for the expected XML output, improving readability and maintainability of the test code. The changes ensure consistency in the expected results across multiple test cases. --- src/core/__tests__/read-file-maxReadFileLine.test.ts | 9 +++++---- src/core/diff/strategies/multi-search-replace.ts | 2 +- src/core/tools/readFileTool.ts | 4 +++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/core/__tests__/read-file-maxReadFileLine.test.ts b/src/core/__tests__/read-file-maxReadFileLine.test.ts index 619ae3a605..c3bf80a043 100644 --- a/src/core/__tests__/read-file-maxReadFileLine.test.ts +++ b/src/core/__tests__/read-file-maxReadFileLine.test.ts @@ -48,6 +48,7 @@ describe("read_file tool with maxReadFileLine setting", () => { const fileContent = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5" const numberedFileContent = "1 | Line 1\n2 | Line 2\n3 | Line 3\n4 | Line 4\n5 | Line 5" const sourceCodeDef = "\n\n# file.txt\n1--5 | Content" + const expectedFullFileXml = `\n ${testFilePath}\n \n${numberedFileContent}\n \n` // Mocked functions with correct types const mockedCountFileLines = countFileLines as jest.MockedFunction @@ -147,7 +148,7 @@ describe("read_file tool with maxReadFileLine setting", () => { expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) expect(mockedReadLines).not.toHaveBeenCalled() expect(mockedParseSourceCodeDefinitionsForFile).not.toHaveBeenCalled() - expect(result).toBe(numberedFileContent) + expect(result).toBe(expectedFullFileXml) }) }) @@ -207,7 +208,7 @@ describe("read_file tool with maxReadFileLine setting", () => { // Verify expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) - expect(result).toBe(numberedFileContent) + expect(result).toBe(expectedFullFileXml) }) it("should read with extractTextFromFile when file has few lines", async () => { @@ -221,7 +222,7 @@ describe("read_file tool with maxReadFileLine setting", () => { // Verify expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) expect(mockedReadLines).not.toHaveBeenCalled() - expect(result).toBe(numberedFileContent) + expect(result).toBe(expectedFullFileXml) }) }) @@ -237,7 +238,7 @@ describe("read_file tool with maxReadFileLine setting", () => { // Verify expect(mockedExtractTextFromFile).toHaveBeenCalledWith(absoluteFilePath) expect(mockedReadLines).not.toHaveBeenCalled() - expect(result).toBe(numberedFileContent) + expect(result).toBe(expectedFullFileXml) }) }) diff --git a/src/core/diff/strategies/multi-search-replace.ts b/src/core/diff/strategies/multi-search-replace.ts index 78622c8f03..0f756896cc 100644 --- a/src/core/diff/strategies/multi-search-replace.ts +++ b/src/core/diff/strategies/multi-search-replace.ts @@ -480,7 +480,7 @@ Only use a single line of '=======' between search and replacement content, beca diffResults.push({ success: false, - error: `No sufficiently similar match found${lineRange} (${Math.floor(bestMatchScore * 100)}% similar, needs ${Math.floor(this.fuzzyThreshold * 100)}%)\n\nDebug Info:\n- Similarity Score: ${Math.floor(bestMatchScore * 100)}%\n- Required Threshold: ${Math.floor(this.fuzzyThreshold * 100)}%\n- Search Range: ${startLine && endLine ? `lines ${startLine}-${endLine}` : "start to end"}\n- Tip: Use read_file to get the latest content of the file before attempting the diff again, as the file content may have changed\n\nSearch Content:\n${searchChunk}${bestMatchSection}${originalContentSection}`, + error: `No sufficiently similar match found${lineRange} (${Math.floor(bestMatchScore * 100)}% similar, needs ${Math.floor(this.fuzzyThreshold * 100)}%)\n\nDebug Info:\n- Similarity Score: ${Math.floor(bestMatchScore * 100)}%\n- Required Threshold: ${Math.floor(this.fuzzyThreshold * 100)}%\n- Search Range: ${startLine && endLine ? `lines ${startLine}-${endLine}` : "start to end"}\n- Tip: Use the read_file tool to get the latest content of the file before attempting to use the apply_diff tool again, as the file content may have changed\n\nSearch Content:\n${searchChunk}${bestMatchSection}${originalContentSection}`, }) continue } diff --git a/src/core/tools/readFileTool.ts b/src/core/tools/readFileTool.ts index 8cbe89e570..0632a0a86f 100644 --- a/src/core/tools/readFileTool.ts +++ b/src/core/tools/readFileTool.ts @@ -171,7 +171,9 @@ export async function readFileTool( content += `\n\n[Showing only ${maxReadFileLine} of ${totalLines} total lines. Use start_line and end_line if you need to read more]${sourceCodeDef}` } - pushToolResult(content) + // Format the result into the required XML structure + const xmlResult = `\n ${relPath}\n \n${content}\n \n` + pushToolResult(xmlResult) } } catch (error) { await handleError("reading file", error)