Validate to_replace in edit_file_by_replace AgentSkill#3073
Merged
Conversation
li-boxuan
force-pushed
the
agentskills/edit-replace
branch
from
July 23, 2024 00:57
e143631 to
2343587
Compare
yufansong
approved these changes
Jul 23, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the problem that this fixes or functionality that this introduces? Does it fix any open issues?
Issue 1
I was running a small subset of SWE-bench-lite and saw LLM doing some dumb stuff:
Diff checker told me the
to_replaceandnew_contentare exactly the same. This edit is meaningless, wastes tokens, and increases context length. For this specific task, the LLM kept repeating this dumb edit and eventually our loop detector terminated the task - not sure if it's related to the fact that the edit doesn't make "any progress".Issue 2
Again running SWE-bench-lite, I noticed that sometimes there are more than one occurrence of
to_replacein the same file. Although the prompt instructs LLM to include enough context to be able to uniquely identify a location, sometimes LLM just forgets about the full context of the entire file, OR even hasn't seen the full file yet. When this happens, there's a chance that we end up editing the wrong place.Give a summary of what the PR does, explaining any non-trivial design decisions
Add sanity checks for
to_replaceparameter. Tell LLM the edit is wrong and hope it can fix the error.Other references
My intuition is this won't change evaluation scores (beyond statistical error), but does more help than harm in general.