-
Notifications
You must be signed in to change notification settings - Fork 4.9k
[apply-patch] Support applypatch command string #2186
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
codex-rs/apply-patch/src/lib.rs
Outdated
@@ -81,9 +81,10 @@ pub struct ApplyPatchArgs { | |||
pub hunks: Vec<Hunk>, | |||
} | |||
|
|||
const APPLY_PATCH_COMMANDS: [&str; 2] = ["apply_patch", "applypatch"]; |
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.
I think it's better to put it either on top of the file or inside the function.
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.
Good call - addressed
Summary Adds
Review Looks good—small, focused change with matching tests.
|
@@ -82,8 +82,9 @@ pub struct ApplyPatchArgs { | |||
} | |||
|
|||
pub fn maybe_parse_apply_patch(argv: &[String]) -> MaybeApplyPatch { | |||
const APPLY_PATCH_COMMANDS: [&str; 2] = ["apply_patch", "applypatch"]; |
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.
This should have a comment (and ideally to link to a GitHub issue or something with more information) that explains why we are accepting applypatch
. Ideally, it would also provide guidance about criteria for when we should stop recognizing applypatch
.
Summary
GPT-OSS and
gpt-5-mini
have training artifacts that cause the models to occasionally useapplypatch
instead ofapply_patch
. I think long-term we'll want to provideapply_patch
as a first class tool, but for now let's silently handle this case to avoid hurting model performanceTesting