Skip to content

Commit 8a86af0

Browse files
roomoteroomote
committed
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
1 parent a8aea14 commit 8a86af0

File tree

3 files changed

+228
-18
lines changed

3 files changed

+228
-18
lines changed

‎src/core/diff/strategies/__tests__/identical-diff.spec.ts‎

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
import { MultiSearchReplaceDiffStrategy } from "../multi-search-replace"
2+
import { MultiFileSearchReplaceDiffStrategy } from "../multi-file-search-replace"
3+
4+
describe("Identical diff handling", () => {
5+
describe("MultiSearchReplaceDiffStrategy", () => {
6+
let strategy: MultiSearchReplaceDiffStrategy
7+
8+
beforeEach(() => {
9+
strategy = new MultiSearchReplaceDiffStrategy()
10+
})
11+
12+
it("should treat identical search and replace content as successful no-op", async () => {
13+
const originalContent = `function test() {
14+
console.log("hello");
15+
return true;
16+
}`
17+
const diffContent = `test.ts
18+
<<<<<<< SEARCH
19+
console.log("hello");
20+
=======
21+
console.log("hello");
22+
>>>>>>> REPLACE`
23+
24+
const result = await strategy.applyDiff(originalContent, diffContent)
25+
expect(result.success).toBe(true)
26+
if (result.success) {
27+
// Content should remain unchanged
28+
expect(result.content).toBe(originalContent)
29+
}
30+
})
31+
32+
it("should handle multiple diffs where some are identical (no-op)", async () => {
33+
const originalContent = `function test() {
34+
console.log("hello");
35+
console.log("world");
36+
return true;
37+
}`
38+
const diffContent = `test.ts
39+
<<<<<<< SEARCH
40+
console.log("hello");
41+
=======
42+
console.log("hello");
43+
>>>>>>> REPLACE
44+
45+
<<<<<<< SEARCH
46+
console.log("world");
47+
=======
48+
console.log("universe");
49+
>>>>>>> REPLACE`
50+
51+
const result = await strategy.applyDiff(originalContent, diffContent)
52+
expect(result.success).toBe(true)
53+
if (result.success) {
54+
// First diff is no-op, second diff should apply
55+
expect(result.content).toBe(`function test() {
56+
console.log("hello");
57+
console.log("universe");
58+
return true;
59+
}`)
60+
}
61+
})
62+
63+
it("should handle all identical diffs as successful no-op", async () => {
64+
const originalContent = `class Example {
65+
constructor() {
66+
this.value = 0;
67+
}
68+
}`
69+
const diffContent = `test.ts
70+
<<<<<<< SEARCH
71+
constructor() {
72+
this.value = 0;
73+
}
74+
=======
75+
constructor() {
76+
this.value = 0;
77+
}
78+
>>>>>>> REPLACE
79+
80+
<<<<<<< SEARCH
81+
class Example {
82+
=======
83+
class Example {
84+
>>>>>>> REPLACE`
85+
86+
const result = await strategy.applyDiff(originalContent, diffContent)
87+
expect(result.success).toBe(true)
88+
if (result.success) {
89+
// All diffs are no-op, content should remain unchanged
90+
expect(result.content).toBe(originalContent)
91+
}
92+
})
93+
94+
it("should handle identical diffs with line numbers as no-op", async () => {
95+
const originalContent = `function test() {
96+
const x = 1;
97+
return x;
98+
}`
99+
const diffContent = `test.ts
100+
<<<<<<< SEARCH
101+
:start_line:2
102+
-------
103+
const x = 1;
104+
=======
105+
const x = 1;
106+
>>>>>>> REPLACE`
107+
108+
const result = await strategy.applyDiff(originalContent, diffContent)
109+
expect(result.success).toBe(true)
110+
if (result.success) {
111+
// Content should remain unchanged
112+
expect(result.content).toBe(originalContent)
113+
}
114+
})
115+
})
116+
117+
describe("MultiFileSearchReplaceDiffStrategy", () => {
118+
let strategy: MultiFileSearchReplaceDiffStrategy
119+
120+
beforeEach(() => {
121+
strategy = new MultiFileSearchReplaceDiffStrategy()
122+
})
123+
124+
it("should treat identical search and replace content as successful no-op", async () => {
125+
const originalContent = `function test() {
126+
console.log("hello");
127+
return true;
128+
}`
129+
const diffContent = `test.ts
130+
<<<<<<< SEARCH
131+
console.log("hello");
132+
=======
133+
console.log("hello");
134+
>>>>>>> REPLACE`
135+
136+
const result = await strategy.applyDiff(originalContent, diffContent)
137+
expect(result.success).toBe(true)
138+
if (result.success) {
139+
// Content should remain unchanged
140+
expect(result.content).toBe(originalContent)
141+
}
142+
})
143+
144+
it("should handle array of diffs with identical content as no-op", async () => {
145+
const originalContent = `function test() {
146+
console.log("hello");
147+
console.log("world");
148+
return true;
149+
}`
150+
const diffItems = [
151+
{
152+
content: `<<<<<<< SEARCH
153+
console.log("hello");
154+
=======
155+
console.log("hello");
156+
>>>>>>> REPLACE`,
157+
startLine: undefined,
158+
},
159+
{
160+
content: `<<<<<<< SEARCH
161+
console.log("world");
162+
=======
163+
console.log("universe");
164+
>>>>>>> REPLACE`,
165+
startLine: undefined,
166+
},
167+
]
168+
169+
const result = await strategy.applyDiff(originalContent, diffItems)
170+
expect(result.success).toBe(true)
171+
if (result.success) {
172+
// First diff is no-op, second diff should apply
173+
expect(result.content).toBe(`function test() {
174+
console.log("hello");
175+
console.log("universe");
176+
return true;
177+
}`)
178+
}
179+
})
180+
181+
it("should handle all identical diffs in array as successful no-op", async () => {
182+
const originalContent = `class Example {
183+
constructor() {
184+
this.value = 0;
185+
}
186+
}`
187+
const diffItems = [
188+
{
189+
content: `<<<<<<< SEARCH
190+
constructor() {
191+
this.value = 0;
192+
}
193+
=======
194+
constructor() {
195+
this.value = 0;
196+
}
197+
>>>>>>> REPLACE`,
198+
startLine: undefined,
199+
},
200+
{
201+
content: `<<<<<<< SEARCH
202+
class Example {
203+
=======
204+
class Example {
205+
>>>>>>> REPLACE`,
206+
startLine: undefined,
207+
},
208+
]
209+
210+
const result = await strategy.applyDiff(originalContent, diffItems)
211+
expect(result.success).toBe(true)
212+
if (result.success) {
213+
// All diffs are no-op, content should remain unchanged
214+
expect(result.content).toBe(originalContent)
215+
}
216+
})
217+
})
218+
})

‎src/core/diff/strategies/multi-file-search-replace.ts‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -510,16 +510,12 @@ Each file requires its own path, start_line, and diff elements.
510510
replaceContent = stripLineNumbers(replaceContent)
511511
}
512512

513-
// Validate that search and replace content are not identical
513+
// If search and replace content are identical, treat as no-op (skip silently)
514514
if (searchContent === replaceContent) {
515-
diffResults.push({
516-
success: false,
517-
error:
518-
`Search and replace content are identical - no changes would be made\n\n` +
519-
`Debug Info:\n` +
520-
`- Search and replace must be different to make changes\n` +
521-
`- Use read_file to verify the content you want to change`,
522-
})
515+
// This is a no-op diff, skip it without error
516+
// Some AI providers (like Gemini) sometimes generate identical diffs
517+
// We treat these as successful no-ops rather than errors
518+
appliedCount++ // Count it as applied since it's intentionally a no-op
523519
continue
524520
}
525521

‎src/core/diff/strategies/multi-search-replace.ts‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -427,16 +427,12 @@ Only use a single line of '=======' between search and replacement content, beca
427427
replaceContent = stripLineNumbers(replaceContent)
428428
}
429429

430-
// Validate that search and replace content are not identical
430+
// If search and replace content are identical, treat as no-op (skip silently)
431431
if (searchContent === replaceContent) {
432-
diffResults.push({
433-
success: false,
434-
error:
435-
`Search and replace content are identical - no changes would be made\n\n` +
436-
`Debug Info:\n` +
437-
`- Search and replace must be different to make changes\n` +
438-
`- Use read_file to verify the content you want to change`,
439-
})
432+
// This is a no-op diff, skip it without error
433+
// Some AI providers (like Gemini) sometimes generate identical diffs
434+
// We treat these as successful no-ops rather than errors
435+
appliedCount++ // Count it as applied since it's intentionally a no-op
440436
continue
441437
}
442438

0 commit comments

Comments
 (0)