Skip to content

Commit 999ea68

Browse files
daniel-lxsdaniel-lxs
authored
fix: resolve apply_diff performance regression from PR #9456 (#9474)
1 parent ee93530 commit 999ea68

File tree

2 files changed

+197
-7
lines changed

2 files changed

+197
-7
lines changed

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

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
import { describe, it, expect } from "vitest"
2+
import { MultiFileSearchReplaceDiffStrategy } from "../multi-file-search-replace"
3+
4+
describe("MultiFileSearchReplaceDiffStrategy - 8-character marker support", () => {
5+
it("should handle 8 '<' characters in SEARCH marker (PR #9456 use case)", async () => {
6+
const strategy = new MultiFileSearchReplaceDiffStrategy()
7+
const originalContent = "line 1\nline 2\nline 3"
8+
9+
const diff = `<<<<<<<< SEARCH
10+
:start_line:1
11+
-------
12+
line 1
13+
=======
14+
modified line 1
15+
>>>>>>>> REPLACE`
16+
17+
const result = await strategy.applyDiff(originalContent, diff)
18+
19+
expect(result.success).toBe(true)
20+
if (result.success) {
21+
expect(result.content).toBe("modified line 1\nline 2\nline 3")
22+
}
23+
})
24+
25+
it("should handle 7 '<' characters in SEARCH marker (standard)", async () => {
26+
const strategy = new MultiFileSearchReplaceDiffStrategy()
27+
const originalContent = "line 1\nline 2\nline 3"
28+
29+
const diff = `<<<<<<< SEARCH
30+
:start_line:1
31+
-------
32+
line 1
33+
=======
34+
modified line 1
35+
>>>>>>> REPLACE`
36+
37+
const result = await strategy.applyDiff(originalContent, diff)
38+
39+
expect(result.success).toBe(true)
40+
if (result.success) {
41+
expect(result.content).toBe("modified line 1\nline 2\nline 3")
42+
}
43+
})
44+
45+
it("should handle 8 '>' characters in REPLACE marker", async () => {
46+
const strategy = new MultiFileSearchReplaceDiffStrategy()
47+
const originalContent = "line 1\nline 2\nline 3"
48+
49+
const diff = `<<<<<<< SEARCH
50+
:start_line:2
51+
-------
52+
line 2
53+
=======
54+
modified line 2
55+
>>>>>>>> REPLACE`
56+
57+
const result = await strategy.applyDiff(originalContent, diff)
58+
59+
expect(result.success).toBe(true)
60+
if (result.success) {
61+
expect(result.content).toBe("line 1\nmodified line 2\nline 3")
62+
}
63+
})
64+
65+
it("should handle optional '<' at end of REPLACE marker", async () => {
66+
const strategy = new MultiFileSearchReplaceDiffStrategy()
67+
const originalContent = "line 1\nline 2\nline 3"
68+
69+
const diff = `<<<<<<< SEARCH
70+
:start_line:3
71+
-------
72+
line 3
73+
=======
74+
modified line 3
75+
>>>>>>> REPLACE<`
76+
77+
const result = await strategy.applyDiff(originalContent, diff)
78+
79+
expect(result.success).toBe(true)
80+
if (result.success) {
81+
expect(result.content).toBe("line 1\nline 2\nmodified line 3")
82+
}
83+
})
84+
85+
it("should handle mixed 7 and 8 character markers in same diff", async () => {
86+
const strategy = new MultiFileSearchReplaceDiffStrategy()
87+
const originalContent = "line 1\nline 2\nline 3"
88+
89+
const diff = `<<<<<<<< SEARCH
90+
:start_line:1
91+
-------
92+
line 1
93+
=======
94+
modified line 1
95+
>>>>>>> REPLACE
96+
97+
<<<<<<< SEARCH
98+
:start_line:3
99+
-------
100+
line 3
101+
=======
102+
modified line 3
103+
>>>>>>>> REPLACE`
104+
105+
const result = await strategy.applyDiff(originalContent, diff)
106+
107+
expect(result.success).toBe(true)
108+
if (result.success) {
109+
expect(result.content).toBe("modified line 1\nline 2\nmodified line 3")
110+
}
111+
})
112+
113+
it("should reject markers with too many characters (9+)", async () => {
114+
const strategy = new MultiFileSearchReplaceDiffStrategy()
115+
const originalContent = "line 1\nline 2\nline 3"
116+
117+
const diff = `<<<<<<<<< SEARCH
118+
:start_line:1
119+
-------
120+
line 1
121+
=======
122+
modified line 1
123+
>>>>>>> REPLACE`
124+
125+
const result = await strategy.applyDiff(originalContent, diff)
126+
127+
expect(result.success).toBe(false)
128+
if (!result.success) {
129+
expect(result.error).toContain("Diff block is malformed")
130+
}
131+
})
132+
133+
it("should reject markers with too few characters (6-)", async () => {
134+
const strategy = new MultiFileSearchReplaceDiffStrategy()
135+
const originalContent = "line 1\nline 2\nline 3"
136+
137+
const diff = `<<<<<< SEARCH
138+
:start_line:1
139+
-------
140+
line 1
141+
=======
142+
modified line 1
143+
>>>>>>> REPLACE`
144+
145+
const result = await strategy.applyDiff(originalContent, diff)
146+
147+
expect(result.success).toBe(false)
148+
if (!result.success) {
149+
expect(result.error).toContain("Diff block is malformed")
150+
}
151+
})
152+
153+
it("should handle validation with 8 character markers", () => {
154+
const strategy = new MultiFileSearchReplaceDiffStrategy()
155+
156+
const diff = `<<<<<<<< SEARCH
157+
:start_line:1
158+
-------
159+
content
160+
=======
161+
new content
162+
>>>>>>>> REPLACE`
163+
164+
const result = strategy["validateMarkerSequencing"](diff)
165+
166+
expect(result.success).toBe(true)
167+
})
168+
169+
it("should detect merge conflict with 8 character prefix", () => {
170+
const strategy = new MultiFileSearchReplaceDiffStrategy()
171+
172+
const diff = `<<<<<<<< SEARCH
173+
:start_line:1
174+
-------
175+
content
176+
<<<<<<<< HEAD
177+
conflict content
178+
=======
179+
new content
180+
>>>>>>>> REPLACE`
181+
182+
const result = strategy["validateMarkerSequencing"](diff)
183+
184+
expect(result.success).toBe(false)
185+
if (!result.success) {
186+
expect(result.error).toContain("merge conflict")
187+
}
188+
})
189+
})

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -253,14 +253,15 @@ Each file requires its own path, start_line, and diff elements.
253253

254254
// Pattern allows optional extra '<' or '>' for SEARCH to handle AI-generated diffs
255255
// (e.g., Sonnet 4 sometimes adds extra markers)
256-
const SEARCH_PATTERN = /^<{7,8} SEARCH>?$/
256+
// Using explicit alternation instead of quantifiers to avoid regex backtracking
257+
const SEARCH_PATTERN = /^(?:<<<<<<< |<<<<<<<< )SEARCH>?$/
257258
const SEARCH = "<<<<<<< SEARCH" // Simplified for display
258259
const SEP = "======="
259260
// Pattern allows optional extra '>' or '<' for REPLACE
260-
const REPLACE_PATTERN = /^>{7,8} REPLACE<?$/
261+
const REPLACE_PATTERN = /^(?:>>>>>>> |>>>>>>>> )REPLACE<?$/
261262
const REPLACE = ">>>>>>> REPLACE" // Simplified for display
262-
const SEARCH_PREFIX_PATTERN = /^<{7,8} /
263-
const REPLACE_PREFIX_PATTERN = /^>{7,8} /
263+
const SEARCH_PREFIX_PATTERN = /^(?:<<<<<<< |<<<<<<<< )/
264+
const REPLACE_PREFIX_PATTERN = /^(?:>>>>>>> |>>>>>>>> )/
264265

265266
const reportMergeConflictError = (found: string, _expected: string) => ({
266267
success: false,
@@ -453,18 +454,18 @@ Each file requires its own path, start_line, and diff elements.
453454

454455
/* Regex parts:
455456
1. (?:^|\n) Ensures the first marker starts at the beginning of the file or right after a newline.
456-
2. (?<!\\)<{7,8} SEARCH>?\s*\n Matches "<<<<<<< SEARCH" or "<<<<<<< SEARCH>" or "<<<<<<<<" with 7-8 '<' chars (ignoring any trailing spaces) – the negative lookbehind makes sure it isn't escaped.
457+
2. (?<!\\)(?:<<<<<<< |<<<<<<<< )SEARCH>?\s*\n Matches "<<<<<<< SEARCH" or "<<<<<<< SEARCH>" or "<<<<<<<< SEARCH" (7 or 8 '<' chars) (ignoring any trailing spaces) – the negative lookbehind makes sure it isn't escaped. Uses explicit alternation to avoid backtracking.
457458
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.
458459
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.
459460
5. ((?<!\\)-------\s*\n)? Optionally matches the "-------" marker line (group 5).
460461
6. ([\s\S]*?)(?:\n)? Non‐greedy match for the "search content" (group 6) up to the next marker.
461462
7. (?:(?<=\n)(?<!\\)=======\s*\n) Matches the "=======" marker on its own line.
462463
8. ([\s\S]*?)(?:\n)? Non‐greedy match for the "replace content" (group 7).
463-
9. (?:(?<=\n)(?<!\\)>{7,8} REPLACE<?)(?=\n|$) Matches ">>>>>>> REPLACE" or ">>>>>>> REPLACE<" or ">>>>>>>>" with 7-8 '>' chars on its own line (and requires a following newline or the end of file).
464+
9. (?:(?<=\n)(?<!\\)(?:>>>>>>> |>>>>>>>> )REPLACE<?)(?=\n|$) Matches ">>>>>>> REPLACE" or ">>>>>>> REPLACE<" or ">>>>>>>> REPLACE" (7 or 8 '>' chars) on its own line (and requires a following newline or the end of file). Uses explicit alternation to avoid backtracking.
464465
*/
465466
let matches = [
466467
...diffContent.matchAll(
467-
/(?:^|\n)(?<!\\)<{7,8} 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)(?<!\\)>{7,8} REPLACE<?)(?=\n|$)/g,
468+
/(?:^|\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,
468469
),
469470
]
470471

0 commit comments

Comments
 (0)