feat(diff-viewer): add inline diff computation with line-level and character-level highlights#545
Merged
feat(diff-viewer): add inline diff computation with line-level and character-level highlights#545
Conversation
Add inlineDiff.ts with line-level classification and character-level diff highlighting. Uses LCS for line pairing, sequential greedy matching for modified line detection, and word-level diff for inline highlights. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add getLineClass() and getCharHighlights() helpers to bridge alignment data with inline diff computation for use in the DiffViewer component. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add --diff-modified-bg and --diff-modified-inline-bg for distinguishing modified lines from pure additions/deletions in diff view. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update DiffViewer.svelte to classify lines as removed/added/modified within changed alignments. Modified lines get a yellow/orange background, and character-level changes within modified lines are highlighted inline. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ffsets The greedy two-pointer matching in computeLineDiff only advanced the before pointer on mismatch, causing insertions before a modification to break pairing. Now peeks at the next after-line when similarity is low; if it's above threshold, the current after-line is skipped as a pure insertion and the match proceeds correctly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cover computeLineDiff (identical lines, pure additions/removals, modified line detection, peek-ahead insertion offsets, character highlights, and complex realistic scenarios), getLineDiffResult caching, getLineClass, and getCharHighlights helper functions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The greedy two-pointer in computeLineDiff only peeked 1 step ahead when looking for a modified-line match, so re-indented lines preceded by many insertions (e.g. code wrapped in an if-else block) were misclassified as removed/added instead of modified. Replace the single peek-ahead with a forward scan through all remaining unmatched after-lines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add a test using exact hunk data from a real diff where code was wrapped in an if-else block, causing re-indentation. Verifies that context lines remain unchanged and re-indented lines are classified as modified. Keep the multi-insertion scan-ahead edge case test separately. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The line-to-alignment maps stored indices into the changedAlignments array (filtered subset), but getLineClass/getCharHighlights indexed into activeAlignments (the full array). This caused lookups to hit the wrong alignment, so inline diff computation used incorrect line ranges and failed to detect modifications like re-indentation changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ified pairs Character-level LCS inflated similarity scores for unrelated code lines that shared indentation whitespace (e.g. `r#"<action>"` vs `&branch,` scored 0.63 due to 8 shared leading spaces). Compare trimmed lines when computing similarity for line pairing, and raise the threshold from 0.4 to 0.5. This prevents unrelated lines from being classified as "modified" while preserving detection of re-indentation changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nent Limit the greedy matcher's forward scan to 10 unmatched after-lines (MAX_SCAN_AHEAD) to bound worst-case O(n² × L²) similarity computation. Replace the module-level cache with a createLineDiffCache() factory so each DiffViewer component instance owns its cache, preventing stale entries from previous diffs from occupying slots. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…d pairs Lines sharing structural tokens (e.g. JSDoc descriptions with common words like `segments`, `highlights`) scored 0.515 similarity — just above the 0.5 threshold — causing unrelated lines to be classified as "modified" instead of removed/added. Raise threshold from 0.5 to 0.55; all legitimate modification pairs (re-indentation, single-word edits) score well above 0.6. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tLineDiffResult The getLineClass/getCharHighlights tests were missing the required LineDiffCache parameter added in the cache-scoping refactor. Also remove the deprecated module-level getLineDiffResult singleton — tests now use createLineDiffCache() directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3ae07d067
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+210
to
+211
| const scanLimit = Math.min(ai + 1 + MAX_SCAN_AHEAD, unmatchedAfter.length); | ||
| for (let scan = ai + 1; scan < scanLimit; scan++) { |
There was a problem hiding this comment.
The matching loop only scans MAX_SCAN_AHEAD entries in unmatchedAfter, so if a modified counterpart appears more than 10 added lines later in the same changed alignment, it is never considered and the before-line is forced to removed while the true counterpart is later marked added. This produces incorrect line classes and suppresses inline char highlights in larger insertion-heavy hunks (exactly the scenario this logic is meant to improve).
Useful? React with 👍 / 👎.
Comment on lines
+29
to
+31
| const dp: number[][] = Array.from({ length: m + 1 }, () => | ||
| new Array<number>(n + 1).fill(0), | ||
| ); |
There was a problem hiding this comment.
This allocates a full (m+1) x (n+1) matrix for every line-diff computation, which can consume very large memory and stall the UI on big changed alignments (for example, multi-thousand-line hunks). Because this runs in the viewer path via computeLineDiff, large diffs can become unresponsive or crash due to browser memory pressure.
Useful? React with 👍 / 👎.
…nding start startBranchSessionWithPendingItem creates a non-queued pending item before the backend assigns a sessionId. During that window, willQueue did not account for the in-flight start, so drainQueuedSessions or another startOrQueueSession call could launch a concurrent session on the same branch. Include isSessionStartPending in willQueue so any session request during a pending start is queued instead of started. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
Test plan
inlineDiff.test.ts)diffViewerHelpers.inlineDiff.test.ts)🤖 Generated with Claude Code