Supports updating multiple locations of a file in one call of the apply_diff tool

[qdaxb]

Description

apply_diff tool (search/replace) seems to only be able to modify a continuous text at a time. Some changes require modifying multiple places in the file at once, which can only be achieved through multiple rounds of interaction.

This pr attempts to implement the function of updating multiple locations of a file in one apply_diff tool call by modifying the format of the apply_diff tool.

Tested with DeepSeek Model.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Checklist:

• My code follows the patterns of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation

Additional context

changes:

1. modify the parameters of <apply_diff>, remove start_line and end_line parameter.
2. modify the format of diff format, it's now likes:

<apply_diff>
<path>xxx</path>
<diff>
<<<<<<< SEARCH
:start_line:1
:end_line:2
------------
def calculate_sum(items):
    sum = 0
=======
def calculate_sum(items):
    sum = 0
>>>>>>> REPLACE

<<<<<<< SEARCH
:start_line:4
:end_line:5
------------
        total += item
    return total
=======
        sum += item
    return sum 
>>>>>>> REPLACE
</diff>
</apply_diff>

3. update regex
4. Support applying only the valid parts when apply_diff returns multiple changes at once. Add fail_parts field in DiffResult to record these failures.
5. If failure happens, prompt the model to re-read the file and retry.
6. Update buffer line count to 40.

result:

(Ignore the prompt word format, I was still testing it at the time)

Related Issues

Reviewers

---

Important

Enhances applyDiff in search-replace.ts to support multiple search/replace operations in one call by modifying the diff format.

• New Feature:
  • applyDiff in search-replace.ts now supports multiple search/replace operations in one call.
  • Modifies diff format to include multiple search/replace blocks with line numbers.
• Functionality:
  • Iterates over matched blocks, applying replacements sequentially.
  • Updates tool description and usage examples to reflect new capability.

^{This description was created by for 79414745a2bd1c6a9d31d94b27ae97112aefa655. It will automatically update as commits are pushed.}

[changeset-bot]

⚠️ No Changeset found

Latest commit: 8a51a6c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[samhvw8]

cool

[qdaxb]

@mrubens I think we can see if the idea is ok first. If this idea works, I will try to add a new strategy class and corresponding experiments, and update the test cases.

I did some testing on some files > 1000 lines and DeepSeek models, and the results seem to be ok.

[mrubens]

@mrubens I think we can see if the idea is ok first. If this idea works, I will try to add a new strategy class and corresponding experiments, and update the test cases.

I did some testing on some files > 1000 lines and DeepSeek models, and the results seem to be ok.

Great, I will try it out!

[KJ7LNW]

this pr is amazing! sonnet-3.7 was making really large search replace blocks, so I gave it the following additional instructions in my Code profile. I recommend that you integrate these into the global system instructions for the tool, because it makes for very short targeted replacements which keeps the context nice and small with low latency editing changes in multiple locations throughout the single file in one AI response:

- ALWAYS use search replace via `apply_diff` to edit existing files.
- NEVER use `write_to_file` to edit existing files.
- ALWAYS make as many changes in a single `apply_diff` request as possible using multiple SEARCH/REPLACE blocks
- In `apply_diff`: be more precise with SEARCH and REPLACE patterns: only match the lines that you wish to match plus two extra lines above and below for context. Do not replace entire code blocks unless the entire block needs to be replaced.

[KJ7LNW]

the model was deleting a few lines of code and did the following:

<<<<<<< SEARCH
:start_line:111
:end_line:115
------------
	
	// Add debug listeners to track events
	terminalProcess.on("stream_available", () => console.log("DEBUG: stream_available event received by TerminalProcess"))
	terminalProcess.on("shell_execution_complete", () => console.log("DEBUG: shell_execution_complete event received by TerminalProcess"))
	
=======
	
=======
>>>>>>> REPLACE

which of course replaced the lines with

=======

so I have added this additional instruction:

- in `apply_diff`: only use a single line of `=======` between search and replacement content, because multiple `=======` will corrupt the file.

to which it properly replied:

<<<<<<< SEARCH
:start_line:111
:end_line:115
------------
	
	// Add debug listeners to track events
	terminalProcess.on("stream_available", () => console.log("DEBUG: stream_available event received by TerminalProcess"))
	terminalProcess.on("shell_execution_complete", () => console.log("DEBUG: shell_execution_complete event received by TerminalProcess"))
	
=======
	
>>>>>>> REPLACE

[mrubens]

Great! Should we put it in a new diff strategy and work to get it live? 🚀

[KJ7LNW]

part of the differences failed to apply, so Roo gave this message:

Changes successfully applied to src/integrations/terminal/__tests__/TerminalProcess.test.ts:

But unable to apply all diff parts to file: /home/ewheeler/src/roo-cline/src/integrations/terminal/__tests__/TerminalProcess.test.ts, silently use <read_file> tool to check newest file version and re-apply diffs

in response, the AI wanted to read_file, which makes sense. However, if your reply provides a "unified diff" format of what actually changed, the AI will understand what changed figure it out without re-reading the whole file and polluting the context.

For example, you might consider the following:

Changes successfully applied to src/integrations/terminal/__tests__/TerminalProcess.test.ts:

But unable to apply all diff parts to file: /home/ewheeler/src/roo-cline/src/integrations/terminal/__tests__/TerminalProcess.test.ts

These are the changes that were applied:

--- a/src/integrations/terminal/TerminalManager.ts
+++ b/src/integrations/terminal/TerminalManager.ts
@@ -205,8 +205,10 @@ export class TerminalManager {
-a
+b
@@ -222,19 +224,20 @@ export class TerminalManager {
-c
+d
@@ ...

Reply with <apply_diff> to fix any changes you expected to apply that were not applied above.

Note that write_to_file already has some kind of diff mechanism for displaying to the chat window, so you could use its mechanism.

[KJ7LNW]

Great! Should we put it in a new diff strategy and work to get it live? 🚀

Definitely. Before going live I think these two items should be addressed to increase reliability:

1. add system instructions:

- in `apply_diff`: only use a single line of `=======` between search and replacement content, because multiple `=======` will corrupt the file.
- ALWAYS use search replace via `apply_diff` to edit existing files.
- NEVER use `write_to_file` to edit existing files.
- ALWAYS make as many changes in a single `apply_diff` request as possible using multiple SEARCH/REPLACE blocks
- In `apply_diff`: be more precise with SEARCH and REPLACE patterns: only match the lines that you wish to match plus two extra lines above and below for context. Do not replace entire code blocks unless the entire block needs to be replaced.

CRITICAL:

<<<<<<< SEARCH
MUST BE FOLLOWED BY
:start_line:123
:end_line:456
------------
WHICH MUST BE FOLLOWED BY
=======
WHICH MUST BE FOLLOWED BY
>>>>>>> REPLACE

NEVER interlace any separator until the required sequence above is complete.

2. implement the diff-based response for partial diff application, detailed in the comment above: Supports updating multiple locations of a file in one call of the apply_diff tool #1234 (comment)

[KJ7LNW]

updated system instruction suggestion above

[KJ7LNW]

I have not had any editing issues since adding the instructions in the comment above.

There may be some other edge cases that can be informed by system instructions, but for now I think the ones above solve at least all the issues that we were having. The changes we were making were lots of small 2- or 3-line changes.

This pr is now baked into my daily driver. Thank you so much for putting this together!

[qdaxb]

Great! Should we put it in a new diff strategy and work to get it live? 🚀

@mrubens Add a new strategy class called MultiSearchReplaceDiffStrategy and leave the original class unchanged, add a new experiment MULTI_SEARCH_AND_REPLACE.

@KJ7LNW picked some of instructions into tool description. I may need to test the others more and then gradually optimize them in the future.

In my fork, I only keep the latest version of each file's contents in the conversation, so read_file doesn't cause too much overhead. see #1374

[mrubens]

Great! Should we put it in a new diff strategy and work to get it live? 🚀

@mrubens Add a new strategy class called MultiSearchReplaceDiffStrategy and leave the original class unchanged, add a new experiment MULTI_SEARCH_AND_REPLACE.

@KJ7LNW picked some of instructions into tool description. I may need to test the others more and then gradually optimize them in the future.

In my fork, I only keep the latest version of each file's contents in the conversation, so read_file doesn't cause too much overhead. see #1374

Awesome! Will try it out asap.

[mrubens]

Just tried this out and it's amazing! Only thing I'm thinking about is how to corral all of these different diff options. It seems like ideally we would have a dropdown or radio buttons in the diff settings that lets you choose between the different diff strategies. Let me know if you have time to take a shot, or I'd also be happy to pick this up. Thank you!

[KJ7LNW]

Only thing I'm thinking about is how to corral all of these different diff options.

In the interest of merging this very useful feature and getting it out to the public, can the corralling of the different diff options be implemented as a separate PR after this is merged?

[mrubens]

Only thing I'm thinking about is how to corral all of these different diff options.

In the interest of merging this very useful feature and getting it out to the public, can the corralling of the different diff options be implemented as a separate PR after this is merged?

Yeah happy to merge! I'll clean up the UX as a fast follow.

[mrubens]

@qdaxb we made this the default now, nice work! Only thing is that it seems like we might not be giving the model enough information about partial diff apply failures. Any chance you have time to take a look? Thank you!

[KJ7LNW]

Only thing is that it seems like we might not be giving the model enough information about partial diff apply failures

Is there an issue for that, or can you elaborate with more detail about what is going on? i am asking because #1861 provides a very solid state machine for AI feedback when it misbehaves. This was part of fixing ======= replacement issues.

[mrubens]

Only thing is that it seems like we might not be giving the model enough information about partial diff apply failures

Is there an issue for that, or can you elaborate with more detail about what is going on? i am asking because #1861 provides a very solid state machine for AI feedback when it misbehaves. This was part of fixing ======= replacement issues.

Mostly anecdotal - I've seen the LLM be unsure about what happened when parts of the diff fail, but also this code doesn't seem quite right: https://github.com/RooVetGit/Roo-Code/blob/9823628a539211634c1e8a2a1cc6fca4d123df6d/src/core/tools/applyDiffTool.ts#L91-L101

For instance I don't think we do anything with partResults, and we might only keep the latest error message if there are multiple?

[KJ7LNW]

I agree the model does not always know exactly what was applied, and I think that triggers re-reads. This is something I have already planned to work on, the plan is as follows:

1. apply all diffs that work (as it currently does)
2. if any failed to apply, show the result in a unified diff format (which is more compact) and tell to apply any changes that were omitted

additionally,
@hannesrudolph found a (hopefully) reproducible bug that duplicates replacement lines, so I am going to see if I can reproduce it create a test and push a PR.

[mrubens]

I agree the model does not always know exactly what was applied, and I think that triggers re-reads. This is something I have already planned to work on, the plan is as follows:

1. apply all diffs that work (as it currently does)
2. if any failed to apply, show the result in a unified diff format (which is more compact) and tell to apply any changes that were omitted

additionally, @hannesrudolph found a (hopefully) reproducible bug that duplicates replacement lines, so I am going to see if I can reproduce it create a test and push a PR.

Sounds great!