Skip to content

fix(apply-patch): skip initial new-file hunk header emitted by some AI models #695

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
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

msmhrt
Copy link

@msmhrt msmhrt commented Apr 27, 2025

What & Why

Some AI-generated patches (e.g. from Gemini 2.0 Flash) prepend a full diff hunk header like:

@@ -0,0 +1,2 @@ SectionName

before any + lines. Our parser treats that as invalid, causing parse failures or stray blank lines when adding new files.
This PR makes both the TypeScript and Rust parsers ignore only the first such "new-file" hunk header, then proceed normally.


How

TypeScript

  • Introduce NEW_FILE_HUNK regex + isFirst flag in parse_add_file()
  • Skip the very first header matching that pattern

Rust

  • Import regex::Regex
  • Apply the same new_file_re + "first" logic in parse_one_hunk()

Tests

  • TS

    • process_patch – add file skips full hunk headers
    • Regex self-test
  • Rust

    • test_add_file_skip_full_hunk_header_with_section
    • test_new_file_hunk_regex

Copy link

github-actions bot commented Apr 27, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@msmhrt
Copy link
Author

msmhrt commented Apr 27, 2025

I have read the CLA Document and I hereby sign the CLA

Gemini 2.0 Flash emits hunk headers in Add File directives (e.g. "@@ -0,0 +1 @@" or "@@ -0,0 +1,5 @@"). Use "/^@@\s+-0,0\s+\+1(?:,[1-9]\d*)?\s+@@/" to detect and skip these headers.

Signed-off-by: Masami HIRATA <[email protected]>
@msmhrt msmhrt force-pushed the fix/apply-patch-skip-gemini-hunk branch from 77bdda0 to 389d877 Compare April 28, 2025 01:13
msmhrt added 3 commits April 28, 2025 15:01
…I models and add unit

    tests

        - TypeScript parser: introduce NEW_FILE_HUNK regex + isFirst flag
        - Rust parser: same regex+first logic in parse_one_hunk
        - Tests (TS & Rust) covering skip-header behavior and regex self-test

Signed-off-by: Masami HIRATA <[email protected]>
@msmhrt msmhrt changed the title apply-patch: skip gemini-2.0-flash hunk headers in Add File blocks fix(apply-patch): skip initial new-file hunk header emitted by some AI models Apr 28, 2025
@bolinfest
Copy link
Collaborator

I know we are on the hook for putting up an actual proposal, but we are planning to introduce a concept of "plugins." Accepting a patch format that OpenAI models consider to be invalid output (from its own internal specification) seems like something that should live in a plugin rather in the core of Codex itself.

@msmhrt
Copy link
Author

msmhrt commented Apr 29, 2025

Hi @bolinfest,

Thank you for your comment and for sharing the insight about the planned "plugins" concept. That sounds like an interesting direction for enhancing extensibility in the long run.

Regarding the immediate issue with the apply_patch command, I wanted to provide a bit more context on the problems it's causing. Specifically, there are two main concerns:

  1. Strict Input Requirements: The command currently lacks tolerance for patch formats that slightly deviate from the expected structure, even if they are conceptually valid.
  2. Cryptic Error Messages: The error messages provided upon failure are often unclear and don't offer enough information to pinpoint the exact issue, making debugging very difficult, especially for an AI agent.

These issues combined create a significant usability problem. AI agents interacting with apply_patch often get stuck in inefficient loops: they generate a patch, it fails (sometimes for unclear reasons), the error message isn't helpful for correction, leading to repeated attempts, wasted token consumption, and ultimately, task failure. This severely impacts the ability to reliably use Codex CLI for automating patch applications right now.

I understand from your comment that you envision functionality like accepting varied patch formats potentially living in a plugin, rather than the core. That makes sense from an architectural perspective to keep the core clean.

However, given the significant usability hurdle the current behavior presents, and acknowledging that developing the plugin system might take time, I'd like to ask if exploring any interim improvements to the core apply_patch command might be possible. Would you be open to considering merging the proposed tolerance improvements PR #695, perhaps as a temporary measure or even optionally, just to alleviate the immediate pain point and make the tool more usable in the meantime? The current strictness and poor error feedback are major blockers.

Looking ahead to the plugin system, I think it's a great opportunity. To ensure plugins (and potentially core components too) work effectively in collaboration with AI agents, I'd strongly suggest considering these two principles as part of the design guidelines or best practices:

  1. Robustness/Forgiveness towards AI-generated Input: Components handling AI-generated content should anticipate and tolerate slight imperfections or variations in format, which is inherent in AI generation.
  2. Clear and Actionable Error Messages: Errors should be reported in a way that helps the user (AI or human) understand what went wrong and how to potentially fix it. This is crucial for enabling an effective learning and iteration process.

Incorporating these principles would greatly enhance the value and usability of Codex CLI as an AI-collaborative tool.

Thank you again for engaging with this feedback and considering the PR. I'm happy to discuss further.

Best regards,

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

Successfully merging this pull request may close these issues.

2 participants