forked from OpenHands/OpenHands
-
Notifications
You must be signed in to change notification settings - Fork 0
Replace str_replace_editor with new apply_patch Tool
#3
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
Open
GuyPaddock
wants to merge
41
commits into
main
Choose a base branch
from
feature/replace_str_replace_editor_with_apply_patch
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Replace str_replace_editor with new apply_patch Tool
#3
GuyPaddock
wants to merge
41
commits into
main
from
feature/replace_str_replace_editor_with_apply_patch
Conversation
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
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This restores a feature that was previously part of `str_replace_editor` for some reason.
This is essential for the LLM to see the outcome of a patch. Otherwise, they will loop repeatedly.
We don't need to specially handle "success"/"error" observations. Also, since both "patch_success" and "patch_error" are handled the same way, we can just handle it with a single method.
This was inadvertently dropped and did not need to be removed to implement this feature.
None of the other observation types differ by outcome type, so ApplyPatch should follow the existing convention.
4421af5 to
fa4c776
Compare
This was supposed to be a correlation ID for patches, but we didn't use it at all, and I didn't see a way it's that different from built-in tool call IDs.
dcc39b5 to
419dde4
Compare
v0 conversations automatically look up the localized text by action type and don't provide an existing switch/case statement injection point to dynamically provide arguments to the localized text. At the same time, it's really not necessary for us to make the title dynamic because a patch can theoretically touch multiple files, so it's best to expose this info in the action details.
06bef48 to
00b3dab
Compare
- Put patch after file summary. - Use the correct loop structure for the data structure.
00b3dab to
c46382c
Compare
4f5b644 to
0077cd2
Compare
e7b47fc to
6aba3bb
Compare
6aba3bb to
2cb0ffa
Compare
We already have an in-context example of how to use this tool, so this change revises that existing example to include the details we want about including context in the patch.
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.
No description provided.