From 8a86af01385679f126cd91606aed5506c7e1102a Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 18 Aug 2025 14:22:10 +0000 Subject: [PATCH] fix: accept identical diffs as no-op instead of error - Modified multi-search-replace.ts and multi-file-search-replace.ts to treat identical search/replace content as successful no-op operations - Added comprehensive tests for the new behavior - Fixes #7183 where Gemini and other AI providers sometimes generate identical diffs --- .../__tests__/identical-diff.spec.ts | 218 ++++++++++++++++++ .../strategies/multi-file-search-replace.ts | 14 +- .../diff/strategies/multi-search-replace.ts | 14 +- 3 files changed, 228 insertions(+), 18 deletions(-) create mode 100644 src/core/diff/strategies/__tests__/identical-diff.spec.ts diff --git a/src/core/diff/strategies/__tests__/identical-diff.spec.ts b/src/core/diff/strategies/__tests__/identical-diff.spec.ts new file mode 100644 index 0000000000..4ef1228234 --- /dev/null +++ b/src/core/diff/strategies/__tests__/identical-diff.spec.ts @@ -0,0 +1,218 @@ +import { MultiSearchReplaceDiffStrategy } from "../multi-search-replace" +import { MultiFileSearchReplaceDiffStrategy } from "../multi-file-search-replace" + +describe("Identical diff handling", () => { + describe("MultiSearchReplaceDiffStrategy", () => { + let strategy: MultiSearchReplaceDiffStrategy + + beforeEach(() => { + strategy = new MultiSearchReplaceDiffStrategy() + }) + + it("should treat identical search and replace content as successful no-op", async () => { + const originalContent = `function test() { + console.log("hello"); + return true; +}` + const diffContent = `test.ts +<<<<<<< SEARCH + console.log("hello"); +======= + console.log("hello"); +>>>>>>> REPLACE` + + const result = await strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + // Content should remain unchanged + expect(result.content).toBe(originalContent) + } + }) + + it("should handle multiple diffs where some are identical (no-op)", async () => { + const originalContent = `function test() { + console.log("hello"); + console.log("world"); + return true; +}` + const diffContent = `test.ts +<<<<<<< SEARCH + console.log("hello"); +======= + console.log("hello"); +>>>>>>> REPLACE + +<<<<<<< SEARCH + console.log("world"); +======= + console.log("universe"); +>>>>>>> REPLACE` + + const result = await strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + // First diff is no-op, second diff should apply + expect(result.content).toBe(`function test() { + console.log("hello"); + console.log("universe"); + return true; +}`) + } + }) + + it("should handle all identical diffs as successful no-op", async () => { + const originalContent = `class Example { + constructor() { + this.value = 0; + } +}` + const diffContent = `test.ts +<<<<<<< SEARCH + constructor() { + this.value = 0; + } +======= + constructor() { + this.value = 0; + } +>>>>>>> REPLACE + +<<<<<<< SEARCH +class Example { +======= +class Example { +>>>>>>> REPLACE` + + const result = await strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + // All diffs are no-op, content should remain unchanged + expect(result.content).toBe(originalContent) + } + }) + + it("should handle identical diffs with line numbers as no-op", async () => { + const originalContent = `function test() { + const x = 1; + return x; +}` + const diffContent = `test.ts +<<<<<<< SEARCH +:start_line:2 +------- + const x = 1; +======= + const x = 1; +>>>>>>> REPLACE` + + const result = await strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + // Content should remain unchanged + expect(result.content).toBe(originalContent) + } + }) + }) + + describe("MultiFileSearchReplaceDiffStrategy", () => { + let strategy: MultiFileSearchReplaceDiffStrategy + + beforeEach(() => { + strategy = new MultiFileSearchReplaceDiffStrategy() + }) + + it("should treat identical search and replace content as successful no-op", async () => { + const originalContent = `function test() { + console.log("hello"); + return true; +}` + const diffContent = `test.ts +<<<<<<< SEARCH + console.log("hello"); +======= + console.log("hello"); +>>>>>>> REPLACE` + + const result = await strategy.applyDiff(originalContent, diffContent) + expect(result.success).toBe(true) + if (result.success) { + // Content should remain unchanged + expect(result.content).toBe(originalContent) + } + }) + + it("should handle array of diffs with identical content as no-op", async () => { + const originalContent = `function test() { + console.log("hello"); + console.log("world"); + return true; +}` + const diffItems = [ + { + content: `<<<<<<< SEARCH + console.log("hello"); +======= + console.log("hello"); +>>>>>>> REPLACE`, + startLine: undefined, + }, + { + content: `<<<<<<< SEARCH + console.log("world"); +======= + console.log("universe"); +>>>>>>> REPLACE`, + startLine: undefined, + }, + ] + + const result = await strategy.applyDiff(originalContent, diffItems) + expect(result.success).toBe(true) + if (result.success) { + // First diff is no-op, second diff should apply + expect(result.content).toBe(`function test() { + console.log("hello"); + console.log("universe"); + return true; +}`) + } + }) + + it("should handle all identical diffs in array as successful no-op", async () => { + const originalContent = `class Example { + constructor() { + this.value = 0; + } +}` + const diffItems = [ + { + content: `<<<<<<< SEARCH + constructor() { + this.value = 0; + } +======= + constructor() { + this.value = 0; + } +>>>>>>> REPLACE`, + startLine: undefined, + }, + { + content: `<<<<<<< SEARCH +class Example { +======= +class Example { +>>>>>>> REPLACE`, + startLine: undefined, + }, + ] + + const result = await strategy.applyDiff(originalContent, diffItems) + expect(result.success).toBe(true) + if (result.success) { + // All diffs are no-op, content should remain unchanged + expect(result.content).toBe(originalContent) + } + }) + }) +}) diff --git a/src/core/diff/strategies/multi-file-search-replace.ts b/src/core/diff/strategies/multi-file-search-replace.ts index a212cf2b8e..7bcc8a6e51 100644 --- a/src/core/diff/strategies/multi-file-search-replace.ts +++ b/src/core/diff/strategies/multi-file-search-replace.ts @@ -510,16 +510,12 @@ Each file requires its own path, start_line, and diff elements. replaceContent = stripLineNumbers(replaceContent) } - // Validate that search and replace content are not identical + // If search and replace content are identical, treat as no-op (skip silently) if (searchContent === replaceContent) { - diffResults.push({ - success: false, - error: - `Search and replace content are identical - no changes would be made\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`, - }) + // This is a no-op diff, skip it without error + // Some AI providers (like Gemini) sometimes generate identical diffs + // We treat these as successful no-ops rather than errors + appliedCount++ // Count it as applied since it's intentionally a no-op continue } diff --git a/src/core/diff/strategies/multi-search-replace.ts b/src/core/diff/strategies/multi-search-replace.ts index a6a9913203..1f45df229f 100644 --- a/src/core/diff/strategies/multi-search-replace.ts +++ b/src/core/diff/strategies/multi-search-replace.ts @@ -427,16 +427,12 @@ Only use a single line of '=======' between search and replacement content, beca replaceContent = stripLineNumbers(replaceContent) } - // Validate that search and replace content are not identical + // If search and replace content are identical, treat as no-op (skip silently) if (searchContent === replaceContent) { - diffResults.push({ - success: false, - error: - `Search and replace content are identical - no changes would be made\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`, - }) + // This is a no-op diff, skip it without error + // Some AI providers (like Gemini) sometimes generate identical diffs + // We treat these as successful no-ops rather than errors + appliedCount++ // Count it as applied since it's intentionally a no-op continue }