mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
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
This commit is contained in:
parent
a8aea14078
commit
8a86af0138
3 changed files with 228 additions and 18 deletions
218
src/core/diff/strategies/__tests__/identical-diff.spec.ts
Normal file
218
src/core/diff/strategies/__tests__/identical-diff.spec.ts
Normal file
|
|
@ -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)
|
||||
}
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue