Skip to content

Conversation

dylan-hurd-oai
Copy link
Collaborator

Summary

GPT-OSS and gpt-5-mini have training artifacts that cause the models to occasionally use applypatch instead of apply_patch. I think long-term we'll want to provide apply_patch as a first class tool, but for now let's silently handle this case to avoid hurting model performance

Testing

  • Added unit test

@@ -81,9 +81,10 @@ pub struct ApplyPatchArgs {
pub hunks: Vec<Hunk>,
}

const APPLY_PATCH_COMMANDS: [&str; 2] = ["apply_patch", "applypatch"];
Copy link
Collaborator

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call - addressed

@dylan-hurd-oai dylan-hurd-oai added the codex-rust-review Perform a detailed review of Rust changes. label Aug 11, 2025
@github-actions github-actions bot added codex-rust-review-in-progress and removed codex-rust-review Perform a detailed review of Rust changes. labels Aug 11, 2025
Copy link

Summary

Adds applypatch as a supported command alias in codex-apply-patch by:

  • introducing APPLY_PATCH_COMMANDS constant and using contains for detection.
  • extending test coverage with test_literal_applypatch.

Review

Looks good—small, focused change with matching tests.

  • ✅ Code is clear; constant keeps aliases in one place.
  • ✅ No public API break; existing behaviour unchanged.
  • 🔍 Minor: consider defining the constant closer to maybe_parse_apply_patch (keeps related logic grouped) and documenting why both spellings are needed.

View workflow run

@dylan-hurd-oai dylan-hurd-oai merged commit 5f8984a into main Aug 11, 2025
11 checks passed
@dylan-hurd-oai dylan-hurd-oai deleted the dh--patch-apply-patch branch August 11, 2025 20:11
@github-actions github-actions bot locked and limited conversation to collaborators Aug 11, 2025
@@ -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"];
Copy link
Collaborator

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants