Skip to content

Remove the end_line from the multi_diff instructions and logic #2615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mrubens merged 1 commit into from
Apr 15, 2025
Merged

Remove the end_line from the multi_diff instructions and logic #2615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 8 additions & 60 deletions src/core/diff/strategies/__tests__/multi-search-replace.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,13 @@ describe("MultiSearchReplaceDiffStrategy", () => {
const diff =
"<<<<<<< SEARCH\n" +
":start_line:10\n" +
":end_line:11\n" +
"-------\n" +
"content1\n" +
"=======\n" +
"new1\n" +
">>>>>>> REPLACE\n\n" +
"<<<<<<< SEARCH\n" +
":start_line:10\n" +
":end_line:11\n" +
"-------\n" +
"content2\n" +
"=======\n" +
Expand Down Expand Up @@ -141,15 +139,13 @@ function helloWorld() {
const diffContent = `test.ts
<<<<<<< SEARCH
:start_line:1
:end_line:1
-------
function hello() {
=======
function helloWorld() {
>>>>>>> REPLACE
<<<<<<< SEARCH
:start_line:2
:end_line:2
-------
console.log("hello")
=======
Expand Down Expand Up @@ -741,7 +737,7 @@ function five() {
// Search around the middle (function three)
// Even though all functions contain the target text,
// it should match the one closest to line 9 first
const result = await strategy.applyDiff(originalContent, diffContent, 9, 9)
const result = await strategy.applyDiff(originalContent, diffContent, 9)
expect(result.success).toBe(true)
if (result.success) {
expect(result.content).toBe(`function one() {
Expand Down Expand Up @@ -843,7 +839,6 @@ function five() {
const diffContent = [
"<<<<<<< SEARCH",
":start_line:1",
":end_line:3",
"-------",
"1 | function test() {",
" return true;", // missing line number
Expand All @@ -868,7 +863,6 @@ function five() {
const diffContent = [
"<<<<<<< SEARCH",
":start_line:1",
":end_line:3",
"-------",
"| function test() {",
"| return true;",
Expand Down Expand Up @@ -1634,7 +1628,6 @@ function five() {
const diffContent = `
<<<<<<< SEARCH
:start_line:2
:end_line:2
-------
2 | line to delete
=======
Expand Down Expand Up @@ -1768,7 +1761,7 @@ function two() {
}
>>>>>>> REPLACE`

const result = await strategy.applyDiff(originalContent, diffContent, 5, 7)
const result = await strategy.applyDiff(originalContent, diffContent, 5)
expect(result.success).toBe(true)
if (result.success) {
expect(result.content).toBe(`function one() {
Expand Down Expand Up @@ -1812,7 +1805,7 @@ function three() {

// Even though we specify lines 5-7, it should still find the match at lines 9-11
// because it's within the 5-line buffer zone
const result = await strategy.applyDiff(originalContent, diffContent, 5, 7)
const result = await strategy.applyDiff(originalContent, diffContent, 5)
expect(result.success).toBe(true)
if (result.success) {
expect(result.content).toBe(`function one() {
Expand Down Expand Up @@ -1866,7 +1859,6 @@ pointer-events: auto; /* Enable clicks on the promotion dialog */
const diffContent = `test.ts
<<<<<<< SEARCH
:start_line:12
:end_line:13
-------
.overlay {
=======
Expand Down Expand Up @@ -1946,7 +1938,6 @@ function five() {
const diffContent = `test.ts
<<<<<<< SEARCH
:start_line:5
:end_line:7
-------
function five() {
return 5;
Expand Down Expand Up @@ -1984,7 +1975,7 @@ function one() {
}
>>>>>>> REPLACE`

const result = await strategy.applyDiff(originalContent, diffContent, 1, 3)
const result = await strategy.applyDiff(originalContent, diffContent, 1)
expect(result.success).toBe(true)
if (result.success) {
expect(result.content).toBe(`function one() {
Expand Down Expand Up @@ -2018,7 +2009,7 @@ function two() {
}
>>>>>>> REPLACE`

const result = await strategy.applyDiff(originalContent, diffContent, 5, 7)
const result = await strategy.applyDiff(originalContent, diffContent, 5)
expect(result.success).toBe(true)
if (result.success) {
expect(result.content).toBe(`function one() {
Expand Down Expand Up @@ -2064,7 +2055,7 @@ function processData(data) {
>>>>>>> REPLACE`

// Target the second instance of processData
const result = await strategy.applyDiff(originalContent, diffContent, 10, 12)
const result = await strategy.applyDiff(originalContent, diffContent, 10)
expect(result.success).toBe(true)
if (result.success) {
expect(result.content).toBe(`function processData(data) {
Expand Down Expand Up @@ -2131,49 +2122,6 @@ function three() {
}
})

it("should search from start of file to end line when only end_line is provided", async () => {
const originalContent = `
function one() {
return 1;
}

function two() {
return 2;
}

function three() {
return 3;
}
`.trim()
const diffContent = `test.ts
<<<<<<< SEARCH
function one() {
return 1;
}
=======
function one() {
return "one";
}
>>>>>>> REPLACE`

// Only provide end_line, should search from start of file to there
const result = await strategy.applyDiff(originalContent, diffContent, undefined, 4)
expect(result.success).toBe(true)
if (result.success) {
expect(result.content).toBe(`function one() {
return "one";
}

function two() {
return 2;
}

function three() {
return 3;
}`)
}
})

it("should prioritize exact line match over expanded search", async () => {
const originalContent = `
function one() {
Expand Down Expand Up @@ -2204,7 +2152,7 @@ function process() {

// Should match the second instance exactly at lines 10-12
// even though the first instance at 6-8 is within the expanded search range
const result = await strategy.applyDiff(originalContent, diffContent, 10, 12)
const result = await strategy.applyDiff(originalContent, diffContent, 10)
expect(result.success).toBe(true)
if (result.success) {
expect(result.content).toBe(`
Expand Down Expand Up @@ -2252,7 +2200,7 @@ function process() {

// Specify wrong line numbers (3-5), but content exists at 6-8
// Should still find and replace it since it's within the expanded range
const result = await strategy.applyDiff(originalContent, diffContent, 3, 5)
const result = await strategy.applyDiff(originalContent, diffContent, 3)
expect(result.success).toBe(true)
if (result.success) {
expect(result.content).toBe(`function one() {
Expand Down
45 changes: 17 additions & 28 deletions src/core/diff/strategies/multi-search-replace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ Diff format:
\`\`\`
<<<<<<< SEARCH
:start_line: (required) The line number of original content where the search block starts.
:end_line: (required) The line number of original content where the search block ends.
-------
[exact content to find including whitespace]
=======
Expand All @@ -132,7 +131,6 @@ Search/Replace content:
\`\`\`
<<<<<<< SEARCH
:start_line:1
:end_line:5
-------
def calculate_total(items):
total = 0
Expand All @@ -151,7 +149,6 @@ Search/Replace content with multi edits:
\`\`\`
<<<<<<< SEARCH
:start_line:1
:end_line:2
-------
def calculate_total(items):
sum = 0
Expand All @@ -162,7 +159,6 @@ def calculate_sum(items):

<<<<<<< SEARCH
:start_line:4
:end_line:5
-------
total += item
return total
Expand Down Expand Up @@ -190,7 +186,6 @@ Only use a single line of '=======' between search and replacement content, beca
.replace(/^\\=======/gm, "=======")
.replace(/^\\>>>>>>>/gm, ">>>>>>>")
.replace(/^\\-------/gm, "-------")
.replace(/^\\:end_line:/gm, ":end_line:")
.replace(/^\\:start_line:/gm, ":start_line:")
}

Expand Down Expand Up @@ -240,7 +235,6 @@ Only use a single line of '=======' between search and replacement content, beca
"CORRECT FORMAT:\n\n" +
"<<<<<<< SEARCH\n" +
":start_line: (required) The line number of original content where the search block starts.\n" +
":end_line: (required) The line number of original content where the search block ends.\n" +
"-------\n" +
"[exact content to find including whitespace]\n" +
"=======\n" +
Expand Down Expand Up @@ -328,35 +322,32 @@ Only use a single line of '=======' between search and replacement content, beca
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.

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.

5. ((?<!\\)-------\s*\n)?
4. ((?<!\\)-------\s*\n)?
  Optionally matches the “-------” marker line (group 5).

6. ([\s\S]*?)(?:\n)?
5. ([\s\S]*?)(?:\n)?
  Non‐greedy match for the “search content” (group 6) up to the next marker.

7. (?:(?<=\n)(?<!\\)=======\s*\n)
6. (?:(?<=\n)(?<!\\)=======\s*\n)
  Matches the “=======” marker on its own line.

8. ([\s\S]*?)(?:\n)?
7. ([\s\S]*?)(?:\n)?
  Non‐greedy match for the “replace content” (group 7).

9. (?:(?<=\n)(?<!\\)>>>>>>> REPLACE)(?=\n|$)
8. (?:(?<=\n)(?<!\\)>>>>>>> REPLACE)(?=\n|$)
  Matches the final “>>>>>>> REPLACE” marker on its own line (and requires a following newline or the end of file).
*/

let matches = [
...diffContent.matchAll(
/(?:^|\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,
/(?:^|\n)(?<!\\)<<<<<<< SEARCH\s*\n((?:\:start_line:\s*(\d+)\s*\n))?((?<!\\)-------\s*\n)?([\s\S]*?)(?:\n)?(?:(?<=\n)(?<!\\)=======\s*\n)([\s\S]*?)(?:\n)?(?:(?<=\n)(?<!\\)>>>>>>> REPLACE)(?=\n|$)/g,
),
]

if (matches.length === 0) {
return {
success: false,
error: `Invalid diff format - missing required sections\n\nDebug Info:\n- Expected Format: <<<<<<< SEARCH\\n:start_line: start line\\n:end_line: end line\\n-------\\n[search content]\\n=======\\n[replace content]\\n>>>>>>> REPLACE\n- Tip: Make sure to include start_line/end_line/SEARCH/=======/REPLACE sections with correct markers on new lines`,
error: `Invalid diff format - missing required sections\n\nDebug Info:\n- Expected Format: <<<<<<< SEARCH\\n:start_line: start line\\n-------\\n[search content]\\n=======\\n[replace content]\\n>>>>>>> REPLACE\n- Tip: Make sure to include start_line/SEARCH/=======/REPLACE sections with correct markers on new lines`,
}
}
// Detect line ending from original content
Expand All @@ -368,8 +359,8 @@ Only use a single line of '=======' between search and replacement content, beca
const replacements = matches
.map((match) => ({
startLine: Number(match[2] ?? 0),
searchContent: match[6],
replaceContent: match[7],
searchContent: match[4],
replaceContent: match[5],
}))
.sort((a, b) => a.startLine - b.startLine)

Expand Down Expand Up @@ -430,15 +421,16 @@ Only use a single line of '=======' between search and replacement content, beca
let searchEndIndex = resultLines.length

// Validate and handle line range if provided
if (startLine && endLine) {
if (startLine) {
// Convert to 0-based index
const exactStartIndex = startLine - 1
const exactEndIndex = endLine - 1
const searchLen = searchLines.length
const exactEndIndex = exactStartIndex + searchLen - 1

if (exactStartIndex < 0 || exactEndIndex > resultLines.length || exactStartIndex > exactEndIndex) {
if (exactStartIndex < 0 || exactEndIndex >= resultLines.length) {
diffResults.push({
success: false,
error: `Line range ${startLine}-${endLine} is invalid (file has ${resultLines.length} lines)\n\nDebug Info:\n- Requested Range: lines ${startLine}-${endLine}\n- File Bounds: lines 1-${resultLines.length}`,
error: `Line range ${startLine}-${startLine + searchLen - 1} is invalid (file has ${resultLines.length} lines)\n\nDebug Info:\n- Requested Range: lines ${startLine}-${startLine + searchLen - 1}\n- File Bounds: lines 1-${resultLines.length}`,
})
continue
}
Expand All @@ -453,7 +445,7 @@ Only use a single line of '=======' between search and replacement content, beca
} else {
// Set bounds for buffered search
searchStartIndex = Math.max(0, startLine - (this.bufferLines + 1))
searchEndIndex = Math.min(resultLines.length, endLine + this.bufferLines)
searchEndIndex = Math.min(resultLines.length, startLine + searchLines.length + this.bufferLines)
}
}

Expand Down Expand Up @@ -512,14 +504,11 @@ Only use a single line of '=======' between search and replacement content, beca
? `\n\nBest Match Found:\n${addLineNumbers(bestMatchContent, matchIndex + 1)}`
: `\n\nBest Match Found:\n(no match)`

const lineRange =
startLine || endLine
? ` at ${startLine ? `start: ${startLine}` : "start"} to ${endLine ? `end: ${endLine}` : "end"}`
: ""
const lineRange = startLine ? ` at line: ${startLine}` : ""

diffResults.push({
success: false,
error: `No sufficiently similar match found${lineRange} (${Math.floor(bestMatchScore * 100)}% similar, needs ${Math.floor(this.fuzzyThreshold * 100)}%)\n\nDebug Info:\n- Similarity Score: ${Math.floor(bestMatchScore * 100)}%\n- Required Threshold: ${Math.floor(this.fuzzyThreshold * 100)}%\n- Search Range: ${startLine && endLine ? `lines ${startLine}-${endLine}` : "start to end"}\n- Tried both standard and aggressive line number stripping\n- Tip: Use the read_file tool to get the latest content of the file before attempting to use the apply_diff tool again, as the file content may have changed\n\nSearch Content:\n${searchChunk}${bestMatchSection}${originalContentSection}`,
error: `No sufficiently similar match found${lineRange} (${Math.floor(bestMatchScore * 100)}% similar, needs ${Math.floor(this.fuzzyThreshold * 100)}%)\n\nDebug Info:\n- Similarity Score: ${Math.floor(bestMatchScore * 100)}%\n- Required Threshold: ${Math.floor(this.fuzzyThreshold * 100)}%\n- Search Range: ${startLine ? `starting at line ${startLine}` : "start to end"}\n- Tried both standard and aggressive line number stripping\n- Tip: Use the read_file tool to get the latest content of the file before attempting to use the apply_diff tool again, as the file content may have changed\n\nSearch Content:\n${searchChunk}${bestMatchSection}${originalContentSection}`,
})
continue
}
Expand Down
4 changes: 0 additions & 4 deletions src/core/prompts/__tests__/__snapshots__/system.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4055,7 +4055,6 @@ Diff format:
\`\`\`
<<<<<<< SEARCH
:start_line: (required) The line number of original content where the search block starts.
:end_line: (required) The line number of original content where the search block ends.
-------
[exact content to find including whitespace]
=======
Expand All @@ -4080,7 +4079,6 @@ Search/Replace content:
\`\`\`
<<<<<<< SEARCH
:start_line:1
:end_line:5
-------
def calculate_total(items):
total = 0
Expand All @@ -4099,7 +4097,6 @@ Search/Replace content with multi edits:
\`\`\`
<<<<<<< SEARCH
:start_line:1
:end_line:2
-------
def calculate_total(items):
sum = 0
Expand All @@ -4110,7 +4107,6 @@ def calculate_sum(items):

<<<<<<< SEARCH
:start_line:4
:end_line:5
-------
total += item
return total
Expand Down
1 change: 0 additions & 1 deletion src/core/tools/applyDiffTool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ export async function applyDiffTool(
originalContent,
diffContent,
parseInt(block.params.start_line ?? ""),
parseInt(block.params.end_line ?? ""),
)) ?? {
success: false,
error: "No diff strategy available",
Expand Down