Skip to content

Commit 5c105ac

Browse files
refactor(diff): address CodeRabbit review on truncated-diff repair (#186)
- Use a local repairedDiff in applyDiff instead of reassigning the diffContent parameter, keeping the original input observable. - When a block has a closer but no ======= separator, splice the separator in before the existing >>>>>>> REPLACE rather than synthesizing a second closer. - Strip leading Grok header directives (:start_line:, :end_line:, -------) before the first-line-is-SEARCH heuristic so metadata isn't treated as content; the directives are preserved on the SEARCH section. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 8c86955 commit 5c105ac

2 files changed

Lines changed: 66 additions & 5 deletions

File tree

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,6 +1286,44 @@ function sum(a, b) {
12861286
const result = strategy["repairTruncatedDiff"](diff)
12871287
expect(result).toBe("<<<<<<< SEARCH\n" + "original\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE")
12881288
})
1289+
1290+
it("inserts ======= before an existing closer instead of synthesizing a second one", () => {
1291+
// Has >>>>>>> REPLACE but no ======= separator.
1292+
const diff = "<<<<<<< SEARCH\n" + "old line\n" + ">>>>>>> REPLACE"
1293+
const result = strategy["repairTruncatedDiff"](diff)
1294+
expect(result).toBe("<<<<<<< SEARCH\n" + "old line\n" + "=======\n" + ">>>>>>> REPLACE")
1295+
// Exactly one closer, exactly one separator.
1296+
expect(result.match(/>>>>>>> REPLACE/g)).toHaveLength(1)
1297+
expect(result.match(/^=======$/gm)).toHaveLength(1)
1298+
})
1299+
1300+
it("preserves :start_line: / ------- directives instead of treating them as SEARCH content", () => {
1301+
const diff = "<<<<<<< SEARCH\n" + ":start_line:5\n" + "-------\n" + "old line\n" + "new line"
1302+
const result = strategy["repairTruncatedDiff"](diff)
1303+
expect(result).toBe(
1304+
"<<<<<<< SEARCH\n" +
1305+
":start_line:5\n" +
1306+
"-------\n" +
1307+
"old line\n" +
1308+
"=======\n" +
1309+
"new line\n" +
1310+
">>>>>>> REPLACE",
1311+
)
1312+
})
1313+
1314+
it("treats a single content line after a directive header as the SEARCH target", () => {
1315+
const diff = "<<<<<<< SEARCH\n" + ":start_line:5\n" + "-------\n" + "old line"
1316+
const result = strategy["repairTruncatedDiff"](diff)
1317+
expect(result).toBe(
1318+
"<<<<<<< SEARCH\n" +
1319+
":start_line:5\n" +
1320+
"-------\n" +
1321+
"old line\n" +
1322+
"=======\n" +
1323+
"\n" +
1324+
">>>>>>> REPLACE",
1325+
)
1326+
})
12891327
})
12901328

12911329
// Regression guards for #186: Grok sometimes truncates the streamed diff and drops

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

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -295,22 +295,45 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy {
295295
// Has ======= but missing >>>>>>> REPLACE — append closing marker
296296
const body = block.replace(/\s+$/, "")
297297
repaired += body + "\n>>>>>>> REPLACE" + separator
298+
} else if (hasCloser && !hasSeparator) {
299+
// Has >>>>>>> REPLACE but missing the ======= separator. Don't synthesize a
300+
// second closer; splice the separator in right before the existing closer so
301+
// everything above it becomes the SEARCH section.
302+
const body = block.replace(/\s+$/, "")
303+
repaired += body.replace(/(\n)(>>>>>>> REPLACE)(?=\n|$)/, "$1=======\n$2") + separator
298304
} else {
299-
// Missing both ======= and >>>>>>> REPLACE
305+
// Missing both ======= and >>>>>>> REPLACE.
300306
const searchMatch = block.match(/^<<<<<<< SEARCH\n?([\s\S]*)$/)
301-
const content = (searchMatch?.[1] ?? "").replace(/\s+$/, "")
307+
let content = (searchMatch?.[1] ?? "").replace(/\s+$/, "")
308+
309+
// Peel off any leading Grok header directives (:start_line:, :end_line:, -------)
310+
// so the "first line is SEARCH" heuristic sees real content, not metadata. The
311+
// directives are preserved as a header on the SEARCH section.
312+
let header = ""
313+
const directiveLine = /^(?::start_line:\s*\d+|:end_line:\s*\d+|-------)\s*$/
314+
let nlIdx: number
315+
while ((nlIdx = content.indexOf("\n")) !== -1 && directiveLine.test(content.slice(0, nlIdx))) {
316+
header += content.slice(0, nlIdx + 1)
317+
content = content.slice(nlIdx + 1)
318+
}
319+
302320
const firstNewlineIdx = content.indexOf("\n")
303321
if (firstNewlineIdx !== -1) {
304322
// First line is SEARCH content, rest is REPLACE content
305323
const searchContent = content.substring(0, firstNewlineIdx)
306324
const replaceContent = content.substring(firstNewlineIdx + 1)
307325
repaired +=
308326
"<<<<<<< SEARCH\n" +
327+
header +
309328
searchContent +
310329
"\n=======\n" +
311330
replaceContent +
312331
"\n>>>>>>> REPLACE" +
313332
separator
333+
} else if (header) {
334+
// Only a directive header plus a single content line: that line is the SEARCH
335+
// target (the user pinned it with start_line); the REPLACE section is empty.
336+
repaired += "<<<<<<< SEARCH\n" + header + content + "\n=======\n\n>>>>>>> REPLACE" + separator
314337
} else {
315338
// Single line — treat as empty SEARCH with content as REPLACE
316339
repaired += "<<<<<<< SEARCH\n=======\n" + content + "\n>>>>>>> REPLACE" + separator
@@ -329,9 +352,9 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy {
329352
): Promise<DiffResult> {
330353
// Repair truncated diffs before validation (common with Grok and other models
331354
// whose output gets cut off mid-stream, leaving missing ======= and >>>>>>> REPLACE markers)
332-
diffContent = this.repairTruncatedDiff(diffContent)
355+
const repairedDiff = this.repairTruncatedDiff(diffContent)
333356

334-
const validseq = this.validateMarkerSequencing(diffContent)
357+
const validseq = this.validateMarkerSequencing(repairedDiff)
335358
if (!validseq.success) {
336359
return {
337360
success: false,
@@ -371,7 +394,7 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy {
371394
*/
372395

373396
let matches = [
374-
...diffContent.matchAll(
397+
...repairedDiff.matchAll(
375398
/(?:^|\n)(?<!\\)<<<<<<< SEARCH>?\s*\n((?:\:start_line:\s*(\d+)\s*\n))?((?:\:end_line:\s*(\d+)\s*\n))?((?<!\\)-------\s*\n)?([\s\S]*?)(?:\n)?(?:(?<=\n)(?<!\\)=======\s*\n)([\s\S]*?)(?:\n)?(?:(?<=\n)(?<!\\)>>>>>>> REPLACE)(?=\n|$)/g,
376399
),
377400
]

0 commit comments

Comments
 (0)