-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: reduce redundant context in apply_diff error messages #5796
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.
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
Conversation
When apply_diff fails to find a match, the error message previously included: - The search content that was just sent (redundant since model already has it) - A large snippet of the original file (noisy and context-consuming) This change streamlines the error output to only show the "Best Match Found" section, which provides the most actionable information for debugging while conserving valuable context window space. The best match section shows exactly what was found in the file that was closest to the search pattern, enabling targeted corrections without re-reading the entire file. Fixes: #5795 Signed-off-by: Eric Wheeler <[email protected]>
|
rebased on v3.23.14 |
|
@roomote-agent Show me a before and after with this change, do not create a new PR or make any changes |
|
I'll show you the before and after comparison of the apply_diff error message changes. Let me examine the code changes first. |
Before and After Comparison: apply_diff Error MessagesThis PR significantly reduces the verbosity of apply_diff error messages by removing redundant content while keeping the most actionable debugging information. BEFORE (Previous Error Message):When apply_diff failed to find a match, it would return a very verbose error like this: AFTER (New Streamlined Error Message):Now the same error is much more concise: Key Changes:
Benefits:
The error message is now much more focused and efficient while still providing the critical information needed to understand why the match failed and how to fix it. |
daniel-lxs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
It's not clear to me that the original content isn't valuable here. Is there a way we can demonstrate that it does more harm than good? I don't think that the best match on a failed diff is always the intended match. Until we have a little more evidence that this is positive I'm hesitant to make this change. |
|
Run an eval? |
Context
When apply_diff fails to find a match, it previously returned an excessively verbose error message that included redundant information the model already had, wasting valuable context window space.
Implementation
This PR streamlines the error output by:
The best match section shows exactly what was found in the file that was closest to the search pattern, enabling targeted corrections without re-reading the entire file.
How to Test
Get in Touch
Discord: KJ7LNW
Fixes #5795
Important
Streamlines
apply_differror messages by removing redundant content and updating version to3.23.12.apply_diffby removing redundant search content and large original file content sections inmulti-file-search-replace.tsandmulti-search-replace.ts.3.23.12inpackage.jsonto reflect changes..changeset/v3.23.12.mdfile as part of cleanup.This description was created by
for e308c174f62c9a8775d3f5866f4dfedf3dd38465. You can customize this summary. It will automatically update as commits are pushed.