Skip to content

Commit 0f504cf

Browse files
committed
feat: address PR feedback for more tolerant search/replace
- Use SEARCH_PATTERN.source consistently in error messages - Add explanatory comment about Sonnet 4 behavior - Add edge case tests for multiple >, mixed cases, and whitespace variations - Update regex comment to mention optional > character
1 parent 8db5890 commit 0f504cf

File tree

3 files changed

+51
-7
lines changed

3 files changed

+51
-7
lines changed

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,46 @@ describe("MultiSearchReplaceDiffStrategy", () => {
1818
expect(strategy["validateMarkerSequencing"](diff).success).toBe(true)
1919
})
2020

21+
it("validates correct marker sequence with multiple > in SEARCH", () => {
22+
const diff = "<<<<<<< SEARCH>>\n" + "some content\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE"
23+
expect(strategy["validateMarkerSequencing"](diff).success).toBe(false)
24+
})
25+
26+
it("validates mixed cases with and without extra > in the same diff", () => {
27+
const diff =
28+
"<<<<<<< SEARCH>\n" +
29+
"content1\n" +
30+
"=======\n" +
31+
"new1\n" +
32+
">>>>>>> REPLACE\n\n" +
33+
"<<<<<<< SEARCH\n" +
34+
"content2\n" +
35+
"=======\n" +
36+
"new2\n" +
37+
">>>>>>> REPLACE"
38+
expect(strategy["validateMarkerSequencing"](diff).success).toBe(true)
39+
})
40+
41+
it("validates extra > with whitespace variations", () => {
42+
const diff1 = "<<<<<<< SEARCH> \n" + "some content\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE"
43+
expect(strategy["validateMarkerSequencing"](diff1).success).toBe(true)
44+
45+
const diff2 = "<<<<<<< SEARCH >\n" + "some content\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE"
46+
expect(strategy["validateMarkerSequencing"](diff2).success).toBe(false)
47+
})
48+
49+
it("validates extra > with line numbers", () => {
50+
const diff =
51+
"<<<<<<< SEARCH>\n" +
52+
":start_line:10\n" +
53+
"-------\n" +
54+
"content1\n" +
55+
"=======\n" +
56+
"new1\n" +
57+
">>>>>>> REPLACE"
58+
expect(strategy["validateMarkerSequencing"](diff).success).toBe(true)
59+
})
60+
2161
it("validates multiple correct marker sequences", () => {
2262
const diff =
2363
"<<<<<<< SEARCH\n" +

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -259,8 +259,10 @@ Each file requires its own path, start_line, and diff elements.
259259

260260
const state = { current: State.START, line: 0 }
261261

262-
const SEARCH = "<<<<<<< SEARCH"
262+
// Pattern allows optional '>' after SEARCH to handle AI-generated diffs
263+
// (e.g., Sonnet 4 sometimes adds an extra '>')
263264
const SEARCH_PATTERN = /^<<<<<<< SEARCH>?$/
265+
const SEARCH = SEARCH_PATTERN.source.replace(/[\^$]/g, "") // Remove regex anchors for display
264266
const SEP = "======="
265267
const REPLACE = ">>>>>>> REPLACE"
266268
const SEARCH_PREFIX = "<<<<<<< "
@@ -363,15 +365,15 @@ Each file requires its own path, start_line, and diff elements.
363365
break
364366

365367
case State.AFTER_SEARCH:
366-
if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH, SEP)
368+
if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH_PATTERN.source, SEP)
367369
if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH)
368370
if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEP)
369371
if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH)
370372
if (marker === SEP) state.current = State.AFTER_SEPARATOR
371373
break
372374

373375
case State.AFTER_SEPARATOR:
374-
if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH, REPLACE)
376+
if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH_PATTERN.source, REPLACE)
375377
if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, REPLACE)
376378
if (marker === SEP)
377379
return likelyBadStructure
@@ -457,7 +459,7 @@ Each file requires its own path, start_line, and diff elements.
457459

458460
/* Regex parts:
459461
1. (?:^|\n) Ensures the first marker starts at the beginning of the file or right after a newline.
460-
2. (?<!\\)<<<<<<< SEARCH\s*\n Matches the line "<<<<<<< SEARCH" (ignoring any trailing spaces) – the negative lookbehind makes sure it isn't escaped.
462+
2. (?<!\\)<<<<<<< SEARCH>?\s*\n Matches the line "<<<<<<< SEARCH" with optional '>' (ignoring any trailing spaces) – the negative lookbehind makes sure it isn't escaped.
461463
3. ((?:\:start_line:\s*(\d+)\s*\n))? Optionally matches a ":start_line:" line. The outer capturing group is group 1 and the inner (\d+) is group 2.
462464
4. ((?:\:end_line:\s*(\d+)\s*\n))? Optionally matches a ":end_line:" line. Group 3 is the whole match and group 4 is the digits.
463465
5. ((?<!\\)-------\s*\n)? Optionally matches the "-------" marker line (group 5).

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,10 @@ Only use a single line of '=======' between search and replacement content, beca
198198
}
199199
const state = { current: State.START, line: 0 }
200200

201-
const SEARCH = "<<<<<<< SEARCH"
201+
// Pattern allows optional '>' after SEARCH to handle AI-generated diffs
202+
// (e.g., Sonnet 4 sometimes adds an extra '>')
202203
const SEARCH_PATTERN = /^<<<<<<< SEARCH>?$/
204+
const SEARCH = SEARCH_PATTERN.source.replace(/[\^$]/g, "") // Remove regex anchors for display
203205
const SEP = "======="
204206
const REPLACE = ">>>>>>> REPLACE"
205207
const SEARCH_PREFIX = "<<<<<<<"
@@ -302,15 +304,15 @@ Only use a single line of '=======' between search and replacement content, beca
302304
break
303305

304306
case State.AFTER_SEARCH:
305-
if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH, SEP)
307+
if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH_PATTERN.source, SEP)
306308
if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH)
307309
if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEP)
308310
if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH)
309311
if (marker === SEP) state.current = State.AFTER_SEPARATOR
310312
break
311313

312314
case State.AFTER_SEPARATOR:
313-
if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH, REPLACE)
315+
if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH_PATTERN.source, REPLACE)
314316
if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, REPLACE)
315317
if (marker === SEP)
316318
return likelyBadStructure

0 commit comments

Comments
 (0)