fix(patch): guard against None match in hunk header extraction#2322
Closed
fix(patch): guard against None match in hunk header extraction#2322
Conversation
In `decouple_and_convert_to_hunks_with_lines_numbers` and `extract_hunk_lines_from_patch`, the call to `extract_hunk_headers(match)` was outside proper `if match:` guards. If a line starts with `@@` but doesn't match `RE_HUNK_HEADER`, `match` is None, causing an `AttributeError` crash on `match.groups()`. Move the `extract_hunk_headers` call inside the match guard in both functions, and skip malformed hunk header lines gracefully.
Contributor
Review Summary by QodoGuard against None match in hunk header extraction
WalkthroughsDescription• Guard extract_hunk_headers() calls against None match results • Move extraction logic inside match validation blocks • Skip malformed hunk header lines gracefully instead of crashing • Prevent AttributeError when @@ lines don't match regex pattern Diagramflowchart LR
A["Line starts with @@"] --> B{"RE_HUNK_HEADER.match()"}
B -->|Valid match| C["Extract hunk headers"]
B -->|None/Invalid| D["Skip line gracefully"]
C --> E["Process hunk content"]
D --> E
File Changes1. pr_agent/algo/git_patch_processing.py
|
Contributor
Code Review by Qodo
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+379
to
+380
| else: | ||
| continue # skip lines that start with @@ but don't match the hunk header pattern |
Contributor
There was a problem hiding this comment.
1. Malformed hunk handling untested 📘 Rule violation ☼ Reliability
This PR changes patch parsing behavior to skip malformed @@ hunk headers, but no corresponding pytest coverage is added/updated in this change set. Without tests, regressions could reintroduce crashes or silently drop hunks when encountering unusual diff output.
Agent Prompt
## Issue description
The new guard logic for malformed `@@` hunk headers changes runtime behavior (skip/ignore malformed hunks) but is not covered by tests.
## Issue Context
Previously, malformed `@@` lines could cause `extract_hunk_headers(None)` crashes. The fix now skips those headers; add unit tests to ensure (1) no exception is raised and (2) behavior is stable for both valid and malformed headers.
## Fix Focus Areas
- pr_agent/algo/git_patch_processing.py[376-380]
- pr_agent/algo/git_patch_processing.py[433-436]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
… @@ lines The decouple_and_convert_to_hunks_with_lines_numbers() function overwrote `match` with the new RE_HUNK_HEADER result before checking whether the previous hunk needed to be finalized. When a malformed @@ line produced match=None, the flush condition `if match and (new/old_content_lines)` was False, silently dropping the previous hunk's content. Fix: save `prev_match` before overwriting and use it for the flush decision. Also use `prev_header_line` instead of `match`/`header_line` in the post-loop finalization, so a trailing malformed @@ cannot suppress the last valid hunk. Adds 7 unit tests covering malformed @@ scenarios: crash safety, content preservation, trailing malformed headers, line-number accuracy, all-malformed patches, and deletion-only hunks.
Contributor
|
Persistent review updated to latest commit 89fbb48 |
Comment on lines
355
to
362
| if line.startswith('@@'): | ||
| header_line = line | ||
| prev_match = match # save previous match before overwriting | ||
| match = RE_HUNK_HEADER.match(line) | ||
| if match and (new_content_lines or old_content_lines): # found a new hunk, split the previous lines | ||
| if prev_match and (new_content_lines or old_content_lines): # flush the previous hunk | ||
| if prev_header_line: | ||
| patch_with_lines_str += f'\n{prev_header_line}\n' | ||
| is_plus_lines = is_minus_lines = False |
Contributor
There was a problem hiding this comment.
1. Orphan lines join next hunk 🐞 Bug ≡ Correctness
In decouple_and_convert_to_hunks_with_lines_numbers(), a malformed @@ line sets match=None and continues without clearing new_content_lines/old_content_lines, so any subsequent patch lines are buffered and then incorrectly emitted under the next valid hunk header (wrong hunk association and wrong line numbers). This can corrupt the diff context passed downstream to PR processing and suggestions generation.
Agent Prompt
### Issue description
`decouple_and_convert_to_hunks_with_lines_numbers()` can mis-attribute diff lines that appear after a malformed `@@` header to the next valid hunk because it `continue`s without clearing `new_content_lines` / `old_content_lines`.
### Issue Context
A malformed `@@` header sets `match=None`. Any subsequent `+`/`-`/context lines are still appended to the buffers, and the next valid header won’t flush them because `prev_match` is `None`.
### Fix Focus Areas
- pr_agent/algo/git_patch_processing.py[355-395]
### Implementation notes
- When encountering any `@@` line, after flushing the previous valid hunk (if any), reset `new_content_lines` and `old_content_lines` regardless of whether the previous header matched.
- Additionally, when `match` is `None` (malformed header), ensure buffers are cleared (and optionally reset `start1/start2`) so subsequent non-header lines are treated as orphan content and not attached to the next valid hunk.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Author
|
Closing to reopen from fork (lost push access to org branch). |
3 tasks
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
1 participant
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.
Summary
extract_hunk_headers(match)calls againstNonematch results in two functions withinpr_agent/algo/git_patch_processing.pydecouple_and_convert_to_hunks_with_lines_numbers(line ~379): movedextract_hunk_headersinside the existingif match:block and addedcontinuefor non-matching@@linesextract_hunk_lines_from_patch(line ~434): added an explicitif not match:guard that setsskip_hunk = Trueand continues, preventing the crashBug
If a line starts with
@@but doesn't fully match theRE_HUNK_HEADERregex pattern,re.match()returnsNone. The code then callsextract_hunk_headers(None), which invokesNone.groups()and raisesAttributeError: 'NoneType' object has no attribute 'groups'.This can occur with malformed or unusual diff output (e.g. conflict markers, truncated patches, or non-standard
@@lines).Test plan
pytest tests/ -v)@@line (e.g.@@ invalid header @@) to confirm it no longer crashes🤖 Generated with Claude Code