[editor] Stabilize Dynamic Diff Parsing#978
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
22b8b65 to
add79c9
Compare
add79c9 to
3110382
Compare
3110382 to
9e61224
Compare
9e61224 to
c13c8a5
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c13c8a5673
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
createHunkBuilder seeds each side's start as "next line to consume" (lines.length + 1), but getHunkSideStartBoundary assumes the unified N,0 convention where a zero-count side's start names the line the hunk applies after. A whole-file conflict with an empty side (no context anywhere) finalized with that start unnormalized, inventing a phantom collapsedBefore row and inflating split/unified totals by one. Normalize each zero-count side down by one in finalizeActiveHunk so emitted hunks match what parsePatchFiles produces from real git patches (headers now read +0,0/-0,0 like git). Also corrects the benchmark checksum documented in the file header, which was stale before this branch (the parser output hasn't changed: parent, HEAD, and this fix all produce 31314920). Addresses codex review feedback on #978. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
An attempt to ensure the edited diff better matches the fully parsed diff at the end. It's not perfect, and I think it can still break in certain ways, but I think it's a bit better. Will need more testing. I am a little worried about this code, it touches a lot of things, but it does appear to fix things... It's one of those things that I don't fully understand, which scares me a little.
createHunkBuilder seeds each side's start as "next line to consume" (lines.length + 1), but getHunkSideStartBoundary assumes the unified N,0 convention where a zero-count side's start names the line the hunk applies after. A whole-file conflict with an empty side (no context anywhere) finalized with that start unnormalized, inventing a phantom collapsedBefore row and inflating split/unified totals by one. Normalize each zero-count side down by one in finalizeActiveHunk so emitted hunks match what parsePatchFiles produces from real git patches (headers now read +0,0/-0,0 like git). Also corrects the benchmark checksum documented in the file header, which was stale before this branch (the parser output hasn't changed: parent, HEAD, and this fix all produce 31314920). Addresses codex review feedback on #978. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The cached per-line addition HAST is looked up by line index, but applyDocumentChange only truncated/appended it — a mid-document insert or delete never shifted the surviving entries, so rows hidden behind collapsed context rendered another line's stale tokens once the edit session ended and they became visible (a duplicated line in the diff). - realignAdditionHastLines shifts the cached entries to their new indexes on a line-count edit (common prefix/suffix preserved, the changed window plain-filled for the editor's next tokenize pass). - refreshHighlightedResult re-highlights the final diff in the background at genuine session exit, so plain-filled and shifted lines regain color. The current result keeps rendering until the fresh one lands, so no interim paint drops highlighting (no un-highlighted flash). Called from completeEditSession and the self-heal backstop. - applyDocumentChange now throws on a partial diff instead of half-applying the line rewrite, matching updateRenderCache's guard. Adds regression tests covering the realign shift and the exit re-highlight restoring colors without an interim un-highlighted render. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…edge slideBlankBoundaryBlocksUp moves a pure blank insert/delete to the top of its blank run so the change anchors to the content above it (the caret line after an Enter). When the run outruns the diff context, the slide was instead stopped by the hunk's leading edge — a context-window cut, not the run's top — parking the block at an arbitrary mid-run position that abuts a hunk separator and anchors to nothing. Make the slide all-or-nothing: skip it when it would consume the hunk's entire leading context, keeping the diff library's bottom-of-run anchor, which sits against the content below the run (and matches what plain git diff shows for the same files). Slides that come to rest beneath in-hunk content are unchanged. The rule runs inside the shared canonical parse, so mid-session and exit renderings agree by construction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
b527d72 to
e6dc046
Compare
An attempt to ensure the edited diff better matches the fully parsed diff at the end. It's not perfect, and I think it can still break in certain ways, but I think it's a bit better.
Will need more testing.
I am a little worried about this code, it touches a lot of things, but it does appear to fix things... It's one of those things that I don't fully understand, which scares me a little.