Skip to content

Commit d509c20

Browse files
roomoteroomote
committed
fix: improve diff parsing for Grok models with malformed content
- Add detection for Grok-specific malformed diff patterns - Provide better error messages when AI models generate incorrect syntax - Add debugging information to help users understand what went wrong - Include actionable suggestions for fixing diff issues - Add comprehensive test coverage for Grok scenarios This addresses issue #7750 where Grok models were generating malformed diffs that caused confusing error messages about markers that were not actually present in the files being edited.
1 parent 247da38 commit d509c20

File tree

4 files changed

+549
-8
lines changed

4 files changed

+549
-8
lines changed

‎src/core/diff/strategies/__tests__/grok-malformed-diff.spec.ts‎

Lines changed: 327 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,327 @@
1+
import { MultiSearchReplaceDiffStrategy } from "../multi-search-replace"
2+
import { MultiFileSearchReplaceDiffStrategy } from "../multi-file-search-replace"
3+
4+
describe("Grok Malformed Diff Detection", () => {
5+
describe("MultiSearchReplaceDiffStrategy", () => {
6+
let strategy: MultiSearchReplaceDiffStrategy
7+
8+
beforeEach(() => {
9+
strategy = new MultiSearchReplaceDiffStrategy()
10+
})
11+
12+
describe("detectGrokMalformedDiff", () => {
13+
it("detects consecutive separators", () => {
14+
const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n>>>>>>> REPLACE"
15+
const result = strategy["detectGrokMalformedDiff"](diff)
16+
expect(result).toBe(true)
17+
})
18+
19+
it("detects separator before search marker", () => {
20+
const diff = "=======\n<<<<<<< SEARCH\ncontent\n>>>>>>> REPLACE"
21+
const result = strategy["detectGrokMalformedDiff"](diff)
22+
expect(result).toBe(true)
23+
})
24+
25+
it("detects too many separators", () => {
26+
const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n=======\n>>>>>>> REPLACE"
27+
const result = strategy["detectGrokMalformedDiff"](diff)
28+
expect(result).toBe(true)
29+
})
30+
31+
it("returns false for valid diff", () => {
32+
const diff = "<<<<<<< SEARCH\ncontent\n=======\nreplacement\n>>>>>>> REPLACE"
33+
const result = strategy["detectGrokMalformedDiff"](diff)
34+
expect(result).toBe(false)
35+
})
36+
37+
it("returns false for multiple valid diffs", () => {
38+
const diff =
39+
"<<<<<<< SEARCH\ncontent1\n=======\nreplace1\n>>>>>>> REPLACE\n\n" +
40+
"<<<<<<< SEARCH\ncontent2\n=======\nreplace2\n>>>>>>> REPLACE"
41+
const result = strategy["detectGrokMalformedDiff"](diff)
42+
expect(result).toBe(false)
43+
})
44+
})
45+
46+
describe("analyzeDiffStructure", () => {
47+
it("provides debugging info for malformed diff", () => {
48+
const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n>>>>>>> REPLACE"
49+
const result = strategy["analyzeDiffStructure"](diff)
50+
51+
expect(result).toContain("Found 1 SEARCH markers")
52+
expect(result).toContain("Found 2 separator (=======) markers")
53+
expect(result).toContain("Found 1 REPLACE markers")
54+
expect(result).toContain("WARNING: More separators than SEARCH blocks")
55+
expect(result).toContain("ERROR: Consecutive separators")
56+
})
57+
58+
it("provides debugging info for unbalanced markers", () => {
59+
const diff = "<<<<<<< SEARCH\ncontent\n=======\nreplace\n<<<<<<< SEARCH\nanother"
60+
const result = strategy["analyzeDiffStructure"](diff)
61+
62+
expect(result).toContain("Found 2 SEARCH markers")
63+
expect(result).toContain("Found 1 separator (=======) markers")
64+
expect(result).toContain("Found 0 REPLACE markers")
65+
expect(result).toContain("WARNING: Unbalanced markers")
66+
})
67+
})
68+
69+
describe("validateMarkerSequencing with Grok detection", () => {
70+
it("returns helpful error for Grok-style malformed diff", () => {
71+
const diff = "=======\ncontent\n>>>>>>> REPLACE"
72+
const result = strategy["validateMarkerSequencing"](diff)
73+
74+
expect(result.success).toBe(false)
75+
expect(result.error).toContain("The diff content appears to be malformed")
76+
expect(result.error).toContain("This often happens when AI models generate incorrect diff syntax")
77+
expect(result.error).toContain("DEBUGGING INFO:")
78+
expect(result.error).toContain("SUGGESTIONS:")
79+
expect(result.debugInfo).toBeDefined()
80+
})
81+
82+
it("returns helpful error for consecutive separators", () => {
83+
const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n>>>>>>> REPLACE"
84+
const result = strategy["validateMarkerSequencing"](diff)
85+
86+
expect(result.success).toBe(false)
87+
expect(result.error).toContain("The diff content appears to be malformed")
88+
expect(result.error).toContain("Try using the read_file tool first")
89+
expect(result.error).toContain("Use simpler, smaller diff blocks")
90+
})
91+
92+
it("still validates normal diffs correctly", () => {
93+
const diff = "<<<<<<< SEARCH\ncontent\n=======\nreplacement\n>>>>>>> REPLACE"
94+
const result = strategy["validateMarkerSequencing"](diff)
95+
96+
expect(result.success).toBe(true)
97+
})
98+
})
99+
100+
describe("applyDiff with malformed content", () => {
101+
it("returns error with debug info for malformed diff", async () => {
102+
const originalContent = "function test() {\n return true;\n}"
103+
const malformedDiff = "=======\nfunction test() {\n return false;\n}\n>>>>>>> REPLACE"
104+
105+
const result = await strategy.applyDiff(originalContent, malformedDiff)
106+
107+
expect(result.success).toBe(false)
108+
if (!result.success) {
109+
expect(result.error).toContain("The diff content appears to be malformed")
110+
}
111+
})
112+
113+
it("handles Grok-style issues gracefully", async () => {
114+
const originalContent = "const x = 1;"
115+
const grokDiff = "<<<<<<< SEARCH\nconst x = 1;\n=======\n=======\nconst x = 2;\n>>>>>>> REPLACE"
116+
117+
const result = await strategy.applyDiff(originalContent, grokDiff)
118+
119+
expect(result.success).toBe(false)
120+
if (!result.success) {
121+
expect(result.error).toContain("The diff content appears to be malformed")
122+
expect(result.error).toContain("ERROR: Consecutive separators")
123+
}
124+
})
125+
})
126+
})
127+
128+
describe("MultiFileSearchReplaceDiffStrategy", () => {
129+
let strategy: MultiFileSearchReplaceDiffStrategy
130+
131+
beforeEach(() => {
132+
strategy = new MultiFileSearchReplaceDiffStrategy()
133+
})
134+
135+
describe("detectGrokMalformedDiff", () => {
136+
it("detects consecutive separators", () => {
137+
const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n>>>>>>> REPLACE"
138+
const result = strategy["detectGrokMalformedDiff"](diff)
139+
expect(result).toBe(true)
140+
})
141+
142+
it("detects separator before search marker", () => {
143+
const diff = "=======\n<<<<<<< SEARCH\ncontent\n>>>>>>> REPLACE"
144+
const result = strategy["detectGrokMalformedDiff"](diff)
145+
expect(result).toBe(true)
146+
})
147+
148+
it("detects too many separators", () => {
149+
const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n=======\n>>>>>>> REPLACE"
150+
const result = strategy["detectGrokMalformedDiff"](diff)
151+
expect(result).toBe(true)
152+
})
153+
154+
it("returns false for valid diff", () => {
155+
const diff = "<<<<<<< SEARCH\ncontent\n=======\nreplacement\n>>>>>>> REPLACE"
156+
const result = strategy["detectGrokMalformedDiff"](diff)
157+
expect(result).toBe(false)
158+
})
159+
160+
it("returns false for multiple valid diffs", () => {
161+
const diff =
162+
"<<<<<<< SEARCH\ncontent1\n=======\nreplace1\n>>>>>>> REPLACE\n\n" +
163+
"<<<<<<< SEARCH\ncontent2\n=======\nreplace2\n>>>>>>> REPLACE"
164+
const result = strategy["detectGrokMalformedDiff"](diff)
165+
expect(result).toBe(false)
166+
})
167+
})
168+
169+
describe("analyzeDiffStructure", () => {
170+
it("provides debugging info for malformed diff", () => {
171+
const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n>>>>>>> REPLACE"
172+
const result = strategy["analyzeDiffStructure"](diff)
173+
174+
expect(result).toContain("Found 1 SEARCH markers")
175+
expect(result).toContain("Found 2 separator (=======) markers")
176+
expect(result).toContain("Found 1 REPLACE markers")
177+
expect(result).toContain("WARNING: More separators than SEARCH blocks")
178+
expect(result).toContain("ERROR: Consecutive separators")
179+
})
180+
181+
it("provides debugging info for unbalanced markers", () => {
182+
const diff = "<<<<<<< SEARCH\ncontent\n=======\nreplace\n<<<<<<< SEARCH\nanother"
183+
const result = strategy["analyzeDiffStructure"](diff)
184+
185+
expect(result).toContain("Found 2 SEARCH markers")
186+
expect(result).toContain("Found 1 separator (=======) markers")
187+
expect(result).toContain("Found 0 REPLACE markers")
188+
expect(result).toContain("WARNING: Unbalanced markers")
189+
})
190+
})
191+
192+
describe("validateMarkerSequencing with Grok detection", () => {
193+
it("returns helpful error for Grok-style malformed diff", () => {
194+
const diff = "=======\ncontent\n>>>>>>> REPLACE"
195+
const result = strategy["validateMarkerSequencing"](diff)
196+
197+
expect(result.success).toBe(false)
198+
expect(result.error).toContain("The diff content appears to be malformed")
199+
expect(result.error).toContain("This often happens when AI models generate incorrect diff syntax")
200+
expect(result.error).toContain("DEBUGGING INFO:")
201+
expect(result.error).toContain("SUGGESTIONS:")
202+
expect(result.debugInfo).toBeDefined()
203+
})
204+
205+
it("returns helpful error for consecutive separators", () => {
206+
const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n>>>>>>> REPLACE"
207+
const result = strategy["validateMarkerSequencing"](diff)
208+
209+
expect(result.success).toBe(false)
210+
expect(result.error).toContain("The diff content appears to be malformed")
211+
expect(result.error).toContain("Try using the read_file tool first")
212+
expect(result.error).toContain("Use simpler, smaller diff blocks")
213+
})
214+
215+
it("still validates normal diffs correctly", () => {
216+
const diff = "<<<<<<< SEARCH\ncontent\n=======\nreplacement\n>>>>>>> REPLACE"
217+
const result = strategy["validateMarkerSequencing"](diff)
218+
219+
expect(result.success).toBe(true)
220+
})
221+
})
222+
223+
describe("applyDiff with malformed content", () => {
224+
it("returns error with debug info for malformed diff", async () => {
225+
const originalContent = "function test() {\n return true;\n}"
226+
const malformedDiff = "=======\nfunction test() {\n return false;\n}\n>>>>>>> REPLACE"
227+
228+
const result = await strategy.applyDiff(originalContent, malformedDiff)
229+
230+
expect(result.success).toBe(false)
231+
if (!result.success) {
232+
expect(result.error).toContain("The diff content appears to be malformed")
233+
}
234+
})
235+
236+
it("handles Grok-style issues gracefully", async () => {
237+
const originalContent = "const x = 1;"
238+
const grokDiff = "<<<<<<< SEARCH\nconst x = 1;\n=======\n=======\nconst x = 2;\n>>>>>>> REPLACE"
239+
240+
const result = await strategy.applyDiff(originalContent, grokDiff)
241+
242+
expect(result.success).toBe(false)
243+
if (!result.success) {
244+
expect(result.error).toContain("The diff content appears to be malformed")
245+
expect(result.error).toContain("ERROR: Consecutive separators")
246+
}
247+
})
248+
})
249+
})
250+
251+
describe("Real-world Grok scenarios", () => {
252+
let strategy: MultiSearchReplaceDiffStrategy
253+
254+
beforeEach(() => {
255+
strategy = new MultiSearchReplaceDiffStrategy()
256+
})
257+
258+
it("handles case where Grok adds consecutive separators", async () => {
259+
const originalContent = `function processPayment(amount) {
260+
// Process payment logic
261+
return { success: true };
262+
}`
263+
264+
// Simulating a Grok model that incorrectly adds consecutive separators
265+
// This triggers Grok detection
266+
const grokDiff = [
267+
"<<<<<<< SEARCH",
268+
"function processPayment(amount) {",
269+
" // Process payment logic",
270+
"=======",
271+
"=======",
272+
"function processPayment(amount, currency) {",
273+
" // Enhanced payment logic",
274+
">>>>>>> REPLACE",
275+
].join("\n")
276+
277+
const result = await strategy.applyDiff(originalContent, grokDiff)
278+
279+
expect(result.success).toBe(false)
280+
if (!result.success) {
281+
expect(result.error).toContain("The diff content appears to be malformed")
282+
}
283+
})
284+
285+
it("handles case where separator appears before search marker", async () => {
286+
// This simulates the exact issue @mrbm reported
287+
const originalContent = `export function calculateTotal(items) {
288+
let total = 0;
289+
for (const item of items) {
290+
total += item.price;
291+
}
292+
return total;
293+
}`
294+
295+
// Malformed diff that Grok might generate - separator before search
296+
const malformedDiff = [
297+
"=======",
298+
"<<<<<<< SEARCH",
299+
"export function calculateTotal(items) {",
300+
" let total = 0;",
301+
"=======",
302+
"export function calculateTotal(items, taxRate = 0) {",
303+
" let total = 0;",
304+
">>>>>>> REPLACE",
305+
].join("\n")
306+
307+
const result = await strategy.applyDiff(originalContent, malformedDiff)
308+
309+
expect(result.success).toBe(false)
310+
if (!result.success) {
311+
expect(result.error).toContain("The diff content appears to be malformed")
312+
expect(result.error).not.toContain("When removing merge conflict markers")
313+
expect(result.error).toContain("AI models generate incorrect diff syntax")
314+
}
315+
})
316+
317+
it("provides actionable suggestions for Grok users", () => {
318+
const diff = "=======\ncontent\n>>>>>>> REPLACE"
319+
const result = strategy["validateMarkerSequencing"](diff)
320+
321+
expect(result.error).toContain("Try using the read_file tool first")
322+
expect(result.error).toContain("Ensure your SEARCH block exactly matches")
323+
expect(result.error).toContain("Use simpler, smaller diff blocks")
324+
expect(result.error).toContain("CORRECT FORMAT:")
325+
})
326+
})
327+
})

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,9 @@ describe("MultiSearchReplaceDiffStrategy", () => {
9696
const diff = "=======\n" + "content\n" + ">>>>>>> REPLACE"
9797
const result = strategy["validateMarkerSequencing"](diff)
9898
expect(result.success).toBe(false)
99-
expect(result.error).toContain("'=======' found in your diff content")
100-
expect(result.error).toContain("Diff block is malformed")
99+
// This now triggers Grok detection instead of the regular error
100+
expect(result.error).toContain("The diff content appears to be malformed")
101+
expect(result.error).toContain("AI models generate incorrect diff syntax")
101102
})
102103

103104
it("detects missing separator", () => {
@@ -112,8 +113,9 @@ describe("MultiSearchReplaceDiffStrategy", () => {
112113
const diff = "<<<<<<< SEARCH\n" + "content\n" + "=======\n" + "=======\n" + ">>>>>>> REPLACE"
113114
const result = strategy["validateMarkerSequencing"](diff)
114115
expect(result.success).toBe(false)
115-
expect(result.error).toContain("'=======' found in your diff content")
116-
expect(result.error).toContain("When removing merge conflict markers")
116+
// This now triggers Grok detection for consecutive separators
117+
expect(result.error).toContain("The diff content appears to be malformed")
118+
expect(result.error).toContain("AI models generate incorrect diff syntax")
117119
})
118120

119121
it("detects replace before separator (merge conflict message)", () => {

0 commit comments

Comments
 (0)