From 4a114a23db5a702badc1bfd3c7fe50fd853d5dd7 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Sat, 23 Aug 2025 21:40:44 +0000 Subject: [PATCH] fix: enhance null edit detection for Gemini models - Add detailed error messages when search and replace content are identical - Provide clear guidance on common causes (model hallucination, copy/paste errors) - Add comprehensive tests for null edit detection - Help AI models (especially Gemini) avoid producing phantom edits Fixes #7360 --- .../__tests__/null-edit-detection.spec.ts | 134 ++++++++++++++++++ .../strategies/multi-file-search-replace.ts | 13 +- .../diff/strategies/multi-search-replace.ts | 13 +- 3 files changed, 154 insertions(+), 6 deletions(-) create mode 100644 src/core/diff/strategies/__tests__/null-edit-detection.spec.ts diff --git a/src/core/diff/strategies/__tests__/null-edit-detection.spec.ts b/src/core/diff/strategies/__tests__/null-edit-detection.spec.ts new file mode 100644 index 0000000000..a1dbdf3fc8 --- /dev/null +++ b/src/core/diff/strategies/__tests__/null-edit-detection.spec.ts @@ -0,0 +1,134 @@ +import { MultiSearchReplaceDiffStrategy } from "../multi-search-replace" + +describe("null edit detection", () => { + let strategy: MultiSearchReplaceDiffStrategy + + beforeEach(() => { + strategy = new MultiSearchReplaceDiffStrategy() + }) + + it("should detect and reject null edits (identical search and replace)", async () => { + const originalContent = 'function hello() {\n console.log("hello")\n}\n' + const diffContent = [ + "<<<<<<< SEARCH", + "function hello() {", + ' console.log("hello")', + "}", + "=======", + "function hello() {", + ' console.log("hello")', + "}", + ">>>>>>> REPLACE", + ].join("\n") + + const result = await strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(false) + if (!result.success && result.failParts && result.failParts.length > 0) { + const firstError = result.failParts[0] + if (!firstError.success && firstError.error) { + expect(firstError.error).toContain("NULL EDIT DETECTED") + expect(firstError.error).toContain("Search and replace content are identical") + expect(firstError.error).toContain("This is a common issue with AI models (especially Gemini)") + expect(firstError.error).toContain("Model hallucination") + } + } + }) + + it("should detect null edits in multi-block diffs", async () => { + const originalContent = + 'function hello() {\n console.log("hello")\n}\nfunction world() {\n return "world"\n}' + const diffContent = [ + "<<<<<<< SEARCH", + "function hello() {", + ' console.log("hello")', + "}", + "=======", + "function hello() {", + ' console.log("hello world")', + "}", + ">>>>>>> REPLACE", + "", + "<<<<<<< SEARCH", + "function world() {", + ' return "world"', + "}", + "=======", + "function world() {", + ' return "world"', + "}", + ">>>>>>> REPLACE", + ].join("\n") + + const result = await strategy.applyDiff(originalContent, diffContent) + // Should partially succeed but report the null edit + expect(result.success).toBe(true) + if (result.success) { + expect(result.failParts).toBeDefined() + if (result.failParts && result.failParts[0] && !result.failParts[0].success) { + expect(result.failParts[0].error).toContain("NULL EDIT DETECTED") + } + } + }) + + it("should detect null edits with line numbers", async () => { + const originalContent = "function test() {\n return true;\n}\n" + const diffContent = [ + "<<<<<<< SEARCH", + ":start_line:1", + "-------", + "function test() {", + " return true;", + "}", + "=======", + "function test() {", + " return true;", + "}", + ">>>>>>> REPLACE", + ].join("\n") + + const result = await strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(false) + if (!result.success && result.failParts && result.failParts.length > 0) { + const firstError = result.failParts[0] + if (!firstError.success && firstError.error) { + expect(firstError.error).toContain("NULL EDIT DETECTED") + expect(firstError.error).toContain("phantom") + expect(firstError.error).toContain("Ensure the REPLACE block contains the actual modified content") + } + } + }) + + it("should not trigger null edit detection for legitimate empty replacements (deletions)", async () => { + const originalContent = "function test() {\n // Remove this comment\n return true;\n}\n" + const diffContent = ["<<<<<<< SEARCH", " // Remove this comment", "=======", ">>>>>>> REPLACE"].join("\n") + + const result = await strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + // Should not contain null edit error + if (!result.success && result.error) { + expect(result.error).not.toContain("NULL EDIT DETECTED") + } + }) + + it("should not trigger for actual changes even if similar", async () => { + const originalContent = "function test() {\n return true;\n}\n" + const diffContent = [ + "<<<<<<< SEARCH", + "function test() {", + " return true;", + "}", + "=======", + "function test() {", + " return false;", + "}", + ">>>>>>> REPLACE", + ].join("\n") + + const result = await strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + // Should not contain null edit error + if (!result.success && result.error) { + expect(result.error).not.toContain("NULL EDIT DETECTED") + } + }) +}) diff --git a/src/core/diff/strategies/multi-file-search-replace.ts b/src/core/diff/strategies/multi-file-search-replace.ts index a212cf2b8e..3aa5799d80 100644 --- a/src/core/diff/strategies/multi-file-search-replace.ts +++ b/src/core/diff/strategies/multi-file-search-replace.ts @@ -515,10 +515,17 @@ Each file requires its own path, start_line, and diff elements. diffResults.push({ success: false, error: - `Search and replace content are identical - no changes would be made\n\n` + + `NULL EDIT DETECTED: Search and replace content are identical - no changes would be made\n\n` + + `This is a common issue with AI models (especially Gemini) producing "phantom" edits.\n\n` + `Debug Info:\n` + - `- Search and replace must be different to make changes\n` + - `- Use read_file to verify the content you want to change`, + `- The SEARCH block and REPLACE block contain exactly the same content\n` + + `- This means the edit would have zero effect on the file\n` + + `- To fix: Ensure the REPLACE block contains the actual modified content\n` + + `- Use read_file to verify the current file content before making changes\n\n` + + `Common causes:\n` + + `1. Model hallucination - thinking it made changes when it didn't\n` + + `2. Incorrect copy/paste of content between blocks\n` + + `3. Missing the actual modification in the REPLACE block`, }) continue } diff --git a/src/core/diff/strategies/multi-search-replace.ts b/src/core/diff/strategies/multi-search-replace.ts index a6a9913203..f2f9be2615 100644 --- a/src/core/diff/strategies/multi-search-replace.ts +++ b/src/core/diff/strategies/multi-search-replace.ts @@ -432,10 +432,17 @@ Only use a single line of '=======' between search and replacement content, beca diffResults.push({ success: false, error: - `Search and replace content are identical - no changes would be made\n\n` + + `NULL EDIT DETECTED: Search and replace content are identical - no changes would be made\n\n` + + `This is a common issue with AI models (especially Gemini) producing "phantom" edits.\n\n` + `Debug Info:\n` + - `- Search and replace must be different to make changes\n` + - `- Use read_file to verify the content you want to change`, + `- The SEARCH block and REPLACE block contain exactly the same content\n` + + `- This means the edit would have zero effect on the file\n` + + `- To fix: Ensure the REPLACE block contains the actual modified content\n` + + `- Use read_file to verify the current file content before making changes\n\n` + + `Common causes:\n` + + `1. Model hallucination - thinking it made changes when it didn't\n` + + `2. Incorrect copy/paste of content between blocks\n` + + `3. Missing the actual modification in the REPLACE block`, }) continue }