fix(diff): repair truncated Grok diffs with missing markers (#186)#230
Open
proyectoauraorg wants to merge 2 commits into
Open
fix(diff): repair truncated Grok diffs with missing markers (#186)#230proyectoauraorg wants to merge 2 commits into
proyectoauraorg wants to merge 2 commits into
Conversation
proyectoauraorg
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
Contributor
📝 WalkthroughWalkthroughThis PR adds automatic repair of truncated merge-conflict diffs to handle AI model output that cuts off mid-stream. A new private ChangesTruncated Diff Repair
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
edelauna
requested changes
May 21, 2026
Contributor
edelauna
left a comment
edelauna
left a comment
There was a problem hiding this comment.
Can you add a fixture based e2e test which reproduces the issue / acts as a regression guard.
…Zoo-Code-Org#186) Grok frequently truncates streamed diffs, leaving SEARCH blocks without the ======= separator and/or the >>>>>>> REPLACE closer, which makes applyDiff fail with 'Expected ======= was not found'. repairTruncatedDiff() detects incomplete blocks and reinserts the missing markers while preserving valid blocks and escaped markers. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…fs (Zoo-Code-Org#186) Per review feedback: end-to-end applyDiff() regression guards using realistic truncated-Grok fixtures (missing >>>>>>> REPLACE, missing ======= separator), plus a well-formed multi-block diff that must pass through unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
|
Thanks @edelauna — added fixture-based regression tests that run through the full |
proyectoauraorg
force-pushed
the
fix/diff-parser-grok-truncation
branch
from
af855d3 to
8c86955
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/core/diff/strategies/multi-search-replace.ts (1)
324-332: 💤 Low valueReassigning the
diffContentparameter.Minor: prefer a local
const repaired = this.repairTruncatedDiff(diffContent)and userepaireddownstream rather than mutating the parameter — keeps the original input observable for logging/diagnostics later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/diff/strategies/multi-search-replace.ts` around lines 324 - 332, The applyDiff method currently reassigns the diffContent parameter with this.repairTruncatedDiff(diffContent); instead, call this.repairTruncatedDiff(diffContent) into a new local const (e.g., const repairedDiff = this.repairTruncatedDiff(diffContent)) and use repairedDiff for all downstream logic inside applyDiff (while leaving the original diffContent parameter untouched) so the original input remains available for logging/diagnostics; update all references within applyDiff that currently use diffContent after the repair point to use the new local variable (refer to applyDiff and repairTruncatedDiff).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/diff/strategies/multi-search-replace.ts`:
- Around line 294-318: The repair branch mis-handles blocks that contain a
closer without a separator and blocks that include Grok header directives;
update the logic in multi-search-replace.ts around the variables
hasSeparator/hasCloser/block/repaired so that if hasCloser && !hasSeparator you
do not synthesize a second ">>>>>>> REPLACE" but instead insert a "======="
immediately before the existing closer (i.e., locate the existing ">>>>>>>
REPLACE" and splice in "=======\n" before it); additionally, before applying the
firstNewlineIdx / searchMatch heuristic (the code that computes content,
firstNewlineIdx, searchContent, replaceContent) strip any leading directive
lines like ":start_line:\d+", ":end_line:\d+" and lines of "-------" so the
first-line-is-SEARCH heuristic sees only real content and not header metadata.
---
Nitpick comments:
In `@src/core/diff/strategies/multi-search-replace.ts`:
- Around line 324-332: The applyDiff method currently reassigns the diffContent
parameter with this.repairTruncatedDiff(diffContent); instead, call
this.repairTruncatedDiff(diffContent) into a new local const (e.g., const
repairedDiff = this.repairTruncatedDiff(diffContent)) and use repairedDiff for
all downstream logic inside applyDiff (while leaving the original diffContent
parameter untouched) so the original input remains available for
logging/diagnostics; update all references within applyDiff that currently use
diffContent after the repair point to use the new local variable (refer to
applyDiff and repairTruncatedDiff).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ed8cc68c-09f7-4fd2-993c-cf40d9920974
📒 Files selected for processing (2)
src/core/diff/strategies/__tests__/multi-search-replace.spec.tssrc/core/diff/strategies/multi-search-replace.ts
Comment on lines
+294
to
+318
| if (hasSeparator && !hasCloser) { | ||
| // Has ======= but missing >>>>>>> REPLACE — append closing marker | ||
| const body = block.replace(/\s+$/, "") | ||
| repaired += body + "\n>>>>>>> REPLACE" + separator | ||
| } else { | ||
| // Missing both ======= and >>>>>>> REPLACE | ||
| const searchMatch = block.match(/^<<<<<<< SEARCH\n?([\s\S]*)$/) | ||
| const content = (searchMatch?.[1] ?? "").replace(/\s+$/, "") | ||
| const firstNewlineIdx = content.indexOf("\n") | ||
| if (firstNewlineIdx !== -1) { | ||
| // First line is SEARCH content, rest is REPLACE content | ||
| const searchContent = content.substring(0, firstNewlineIdx) | ||
| const replaceContent = content.substring(firstNewlineIdx + 1) | ||
| repaired += | ||
| "<<<<<<< SEARCH\n" + | ||
| searchContent + | ||
| "\n=======\n" + | ||
| replaceContent + | ||
| "\n>>>>>>> REPLACE" + | ||
| separator | ||
| } else { | ||
| // Single line — treat as empty SEARCH with content as REPLACE | ||
| repaired += "<<<<<<< SEARCH\n=======\n" + content + "\n>>>>>>> REPLACE" + separator | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Repair branch can mis-handle blocks that drop ======= but keep >>>>>>> REPLACE, or contain :start_line: / ------- directives.
Two edge cases in the "needs repair" branch are worth a guard:
!hasSeparator && hasCloser(block has>>>>>>> REPLACEbut no=======) falls into theelsebranch at line 298, which appends another>>>>>>> REPLACE. The original closer ends up inside the synthesized REPLACE content, producing a doubly-closed block downstream.- When both markers are missing and the block still includes
:start_line:Nand/or-------directives (a partial structured SEARCH header that Grok had emitted before truncation), the "first newline = search content" heuristic at lines 302–306 treats:start_line:5as the SEARCH body, dropping-------and the real content into REPLACE. The repaired diff then fails the marker validation or matches the wrong location.
Consider stripping any :start_line: / :end_line: / ------- header lines before applying the first-newline heuristic, and explicitly handling the hasCloser && !hasSeparator case (e.g., insert ======= just before the existing >>>>>>> REPLACE instead of synthesizing a new closer).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/diff/strategies/multi-search-replace.ts` around lines 294 - 318, The
repair branch mis-handles blocks that contain a closer without a separator and
blocks that include Grok header directives; update the logic in
multi-search-replace.ts around the variables
hasSeparator/hasCloser/block/repaired so that if hasCloser && !hasSeparator you
do not synthesize a second ">>>>>>> REPLACE" but instead insert a "======="
immediately before the existing closer (i.e., locate the existing ">>>>>>>
REPLACE" and splice in "=======\n" before it); additionally, before applying the
firstNewlineIdx / searchMatch heuristic (the code that computes content,
firstNewlineIdx, searchContent, replaceContent) strip any leading directive
lines like ":start_line:\d+", ":end_line:\d+" and lines of "-------" so the
first-line-is-SEARCH heuristic sees only real content and not header metadata.
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
2 participants
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
Fixes #186. Grok (4.3) frequently truncates streamed diffs mid-output, so a
<<<<<<< SEARCHblock can arrive without its=======separator and/or>>>>>>> REPLACEcloser. The parser then rejects the whole edit with "Expected '=======' was not found" and the user sees "Unable to apply diff".This adds a small recovery layer,
repairTruncatedDiff(), called at the start ofapplyDiff()that reconstructs the missing markers so a salvageable diff can still be applied.How it works
For each
<<<<<<< SEARCHblock the repair step:=======and>>>>>>> REPLACE) untouched, verbatim;>>>>>>> REPLACEis missing, appends it;SEARCHas the search content and the rest as the replacement, then inserts=======and>>>>>>> REPLACE;(?<!\\)look-behinds (won't touch\<<<<<<<content);>>>>>>> REPLACEnever gets glued to the next<<<<<<< SEARCH.Complete, valid diffs pass through unchanged, so behavior for well-formed input is identical.
Testing
Added coverage in
multi-search-replace.spec.ts(complete-diff passthrough, missing closer, missing both markers, empty-search edge case, multi-block truncation, no spurious trailing newline).Files
src/core/diff/strategies/multi-search-replace.ts—repairTruncatedDiff()+ call inapplyDiff()src/core/diff/strategies/__tests__/multi-search-replace.spec.ts— testsSummary by CodeRabbit