From 0c3d5f5ebd677f440a7e63cb45f79c85f6065dc1 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 30 Mar 2026 11:17:07 +1100 Subject: [PATCH 01/14] feat(diff-viewer): add inline diff computation module 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) --- .../diff-viewer/src/lib/utils/inlineDiff.ts | 251 ++++++++++++++++++ 1 file changed, 251 insertions(+) create mode 100644 packages/diff-viewer/src/lib/utils/inlineDiff.ts diff --git a/packages/diff-viewer/src/lib/utils/inlineDiff.ts b/packages/diff-viewer/src/lib/utils/inlineDiff.ts new file mode 100644 index 00000000..047c411c --- /dev/null +++ b/packages/diff-viewer/src/lib/utils/inlineDiff.ts @@ -0,0 +1,251 @@ +export type BeforeLineClass = 'removed' | 'modified' | 'unchanged'; +export type AfterLineClass = 'added' | 'modified' | 'unchanged'; + +export interface CharHighlight { + start: number; + end: number; +} + +export interface ModifiedPair { + beforeLineIndex: number; + afterLineIndex: number; + beforeHighlights: CharHighlight[]; + afterHighlights: CharHighlight[]; +} + +export interface LineDiffResult { + beforeLines: BeforeLineClass[]; + afterLines: AfterLineClass[]; + modifiedPairs: ModifiedPair[]; +} + +function lcsIndices( + a: T[], + b: T[], + eq: (x: T, y: T) => boolean = (x, y) => x === y, +): { aIndices: Set; bIndices: Set } { + const m = a.length; + const n = b.length; + const dp: number[][] = Array.from({ length: m + 1 }, () => + new Array(n + 1).fill(0), + ); + + for (let i = 1; i <= m; i++) { + for (let j = 1; j <= n; j++) { + if (eq(a[i - 1], b[j - 1])) { + dp[i][j] = dp[i - 1][j - 1] + 1; + } else { + dp[i][j] = Math.max(dp[i - 1][j], dp[i][j - 1]); + } + } + } + + const aIndices = new Set(); + const bIndices = new Set(); + let i = m; + let j = n; + while (i > 0 && j > 0) { + if (eq(a[i - 1], b[j - 1])) { + aIndices.add(i - 1); + bIndices.add(j - 1); + i--; + j--; + } else if (dp[i - 1][j] >= dp[i][j - 1]) { + i--; + } else { + j--; + } + } + + return { aIndices, bIndices }; +} + +function lcsLength(a: string, b: string): number { + const m = a.length; + const n = b.length; + const prev = new Array(n + 1).fill(0); + const curr = new Array(n + 1).fill(0); + + for (let i = 1; i <= m; i++) { + for (let j = 1; j <= n; j++) { + if (a[i - 1] === b[j - 1]) { + curr[j] = prev[j - 1] + 1; + } else { + curr[j] = Math.max(prev[j], curr[j - 1]); + } + } + for (let j = 0; j <= n; j++) { + prev[j] = curr[j]; + curr[j] = 0; + } + } + + return prev[n]; +} + +function similarity(a: string, b: string): number { + if (a.length === 0 && b.length === 0) return 1; + if (a.length === 0 || b.length === 0) return 0; + const lcsLen = lcsLength(a, b); + return (2 * lcsLen) / (a.length + b.length); +} + +const SIMILARITY_THRESHOLD = 0.4; + +function splitWords(text: string): string[] { + return text.split(/(\s+)/); +} + +function computeCharHighlights( + before: string, + after: string, +): { beforeHighlights: CharHighlight[]; afterHighlights: CharHighlight[] } { + const beforeTokens = splitWords(before); + const afterTokens = splitWords(after); + + const { aIndices, bIndices } = lcsIndices(beforeTokens, afterTokens); + + const beforeHighlights: CharHighlight[] = []; + let pos = 0; + for (let i = 0; i < beforeTokens.length; i++) { + const tokenLen = beforeTokens[i].length; + if (!aIndices.has(i)) { + beforeHighlights.push({ start: pos, end: pos + tokenLen }); + } + pos += tokenLen; + } + + const afterHighlights: CharHighlight[] = []; + pos = 0; + for (let i = 0; i < afterTokens.length; i++) { + const tokenLen = afterTokens[i].length; + if (!bIndices.has(i)) { + afterHighlights.push({ start: pos, end: pos + tokenLen }); + } + pos += tokenLen; + } + + return { + beforeHighlights: mergeHighlights(beforeHighlights), + afterHighlights: mergeHighlights(afterHighlights), + }; +} + +function mergeHighlights(highlights: CharHighlight[]): CharHighlight[] { + if (highlights.length === 0) return highlights; + const merged: CharHighlight[] = [highlights[0]]; + for (let i = 1; i < highlights.length; i++) { + const prev = merged[merged.length - 1]; + const curr = highlights[i]; + if (curr.start <= prev.end) { + prev.end = Math.max(prev.end, curr.end); + } else { + merged.push(curr); + } + } + return merged; +} + +export function computeLineDiff( + beforeLines: string[], + afterLines: string[], +): LineDiffResult { + const { aIndices: lcsBeforeIndices, bIndices: lcsAfterIndices } = lcsIndices( + beforeLines, + afterLines, + ); + + const beforeClasses: BeforeLineClass[] = new Array(beforeLines.length).fill( + 'unchanged', + ); + const afterClasses: AfterLineClass[] = new Array(afterLines.length).fill( + 'unchanged', + ); + + const unmatchedBefore: number[] = []; + const unmatchedAfter: number[] = []; + + for (let i = 0; i < beforeLines.length; i++) { + if (!lcsBeforeIndices.has(i)) { + unmatchedBefore.push(i); + } + } + for (let i = 0; i < afterLines.length; i++) { + if (!lcsAfterIndices.has(i)) { + unmatchedAfter.push(i); + } + } + + const modifiedPairs: ModifiedPair[] = []; + let bi = 0; + let ai = 0; + + while (bi < unmatchedBefore.length && ai < unmatchedAfter.length) { + const bIdx = unmatchedBefore[bi]; + const aIdx = unmatchedAfter[ai]; + const sim = similarity(beforeLines[bIdx], afterLines[aIdx]); + + if (sim > SIMILARITY_THRESHOLD) { + beforeClasses[bIdx] = 'modified'; + afterClasses[aIdx] = 'modified'; + const { beforeHighlights, afterHighlights } = computeCharHighlights( + beforeLines[bIdx], + afterLines[aIdx], + ); + modifiedPairs.push({ + beforeLineIndex: bIdx, + afterLineIndex: aIdx, + beforeHighlights, + afterHighlights, + }); + bi++; + ai++; + } else { + beforeClasses[bIdx] = 'removed'; + bi++; + } + } + + while (bi < unmatchedBefore.length) { + beforeClasses[unmatchedBefore[bi]] = 'removed'; + bi++; + } + while (ai < unmatchedAfter.length) { + afterClasses[unmatchedAfter[ai]] = 'added'; + ai++; + } + + return { + beforeLines: beforeClasses, + afterLines: afterClasses, + modifiedPairs, + }; +} + +const MAX_CACHE_SIZE = 100; +const cache = new Map(); + +function makeCacheKey(beforeLines: string[], afterLines: string[]): string { + return beforeLines.join('\n') + '\0' + afterLines.join('\n'); +} + +export function getLineDiffResult( + beforeLines: string[], + afterLines: string[], +): LineDiffResult { + const key = makeCacheKey(beforeLines, afterLines); + const cached = cache.get(key); + if (cached) return cached; + + const result = computeLineDiff(beforeLines, afterLines); + + if (cache.size >= MAX_CACHE_SIZE) { + const firstKey = cache.keys().next().value; + if (firstKey !== undefined) { + cache.delete(firstKey); + } + } + cache.set(key, result); + + return result; +} From 5b4ca7e8a90a0230b5db74094433ad6fcbd81b1f Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 30 Mar 2026 11:18:18 +1100 Subject: [PATCH 02/14] feat(diff-viewer): export inline diff utils and add lookup helpers 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) --- .../src/lib/utils/diffViewerHelpers.ts | 67 +++++++++++++++++++ packages/diff-viewer/src/lib/utils/index.ts | 5 ++ 2 files changed, 72 insertions(+) diff --git a/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts b/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts index 874892e9..9fbf353c 100644 --- a/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts +++ b/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts @@ -1,5 +1,7 @@ import type { Alignment, Comment, SmartDiffAnnotation } from '../types'; import type { Token } from './highlighter'; +import { getLineDiffResult } from './inlineDiff.js'; +import type { BeforeLineClass, AfterLineClass, CharHighlight } from './inlineDiff.js'; export type ChangedAlignmentEntry = { alignment: Alignment; index: number }; export type PaneSide = 'before' | 'after'; @@ -284,3 +286,68 @@ export function buildLineSelectionToolbarLayout( left, }; } + +/** + * Get the line classification for a specific line within a changed alignment. + * Returns null if the line is not in a changed alignment. + */ +export function getLineClass( + side: PaneSide, + lineIndex: number, + beforeLineToAlignment: Map, + afterLineToAlignment: Map, + alignments: Alignment[], + beforeLines: string[], + afterLines: string[] +): BeforeLineClass | AfterLineClass | null { + const map = side === 'before' ? beforeLineToAlignment : afterLineToAlignment; + const alignIdx = map.get(lineIndex); + if (alignIdx === undefined) return null; + + const alignment = alignments[alignIdx]; + const alignBefore = beforeLines.slice(alignment.before.start, alignment.before.end); + const alignAfter = afterLines.slice(alignment.after.start, alignment.after.end); + const result = getLineDiffResult(alignBefore, alignAfter); + + if (side === 'before') { + const localIdx = lineIndex - alignment.before.start; + return result.beforeLines[localIdx]; + } else { + const localIdx = lineIndex - alignment.after.start; + return result.afterLines[localIdx]; + } +} + +/** + * Get character highlights for a line if it's a modified line in a changed alignment. + * Returns null if the line is not modified. + */ +export function getCharHighlights( + side: PaneSide, + lineIndex: number, + beforeLineToAlignment: Map, + afterLineToAlignment: Map, + alignments: Alignment[], + beforeLines: string[], + afterLines: string[] +): CharHighlight[] | null { + const map = side === 'before' ? beforeLineToAlignment : afterLineToAlignment; + const alignIdx = map.get(lineIndex); + if (alignIdx === undefined) return null; + + const alignment = alignments[alignIdx]; + const alignBefore = beforeLines.slice(alignment.before.start, alignment.before.end); + const alignAfter = afterLines.slice(alignment.after.start, alignment.after.end); + const result = getLineDiffResult(alignBefore, alignAfter); + + const localIdx = side === 'before' + ? lineIndex - alignment.before.start + : lineIndex - alignment.after.start; + + const pair = result.modifiedPairs.find(p => + side === 'before' ? p.beforeLineIndex === localIdx : p.afterLineIndex === localIdx + ); + + if (!pair) return null; + return side === 'before' ? pair.beforeHighlights : pair.afterHighlights; +} diff --git a/packages/diff-viewer/src/lib/utils/index.ts b/packages/diff-viewer/src/lib/utils/index.ts index 17702ff3..a622edd4 100644 --- a/packages/diff-viewer/src/lib/utils/index.ts +++ b/packages/diff-viewer/src/lib/utils/index.ts @@ -38,6 +38,8 @@ export { measureLineHeight, normalizeLineSelection, resolveLineSelectionToolbarLeft, + getLineClass, + getCharHighlights, } from './diffViewerHelpers'; export { @@ -84,3 +86,6 @@ export { createFileSelectionWithSearch, type FileSelectionWithSearchConfig } from './fileSelection'; + +export { computeLineDiff, getLineDiffResult } from './inlineDiff.js'; +export type { BeforeLineClass, AfterLineClass, CharHighlight, ModifiedPair, LineDiffResult } from './inlineDiff.js'; From 56a444f6e3f90ddf2e51fc1deff31c5858ad5b8d Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 30 Mar 2026 11:18:49 +1100 Subject: [PATCH 03/14] feat(diff-viewer): add CSS variables for modified line highlights 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) --- apps/staged/src/app.css | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/staged/src/app.css b/apps/staged/src/app.css index b74eae8d..0fe5b4b5 100644 --- a/apps/staged/src/app.css +++ b/apps/staged/src/app.css @@ -54,6 +54,8 @@ --diff-added-bg: rgba(63, 185, 80, 0.08); --diff-removed-bg: rgba(248, 81, 73, 0.08); --diff-changed-bg: rgba(255, 255, 255, 0.04); + --diff-modified-bg: rgba(227, 179, 65, 0.08); + --diff-modified-inline-bg: rgba(227, 179, 65, 0.25); --diff-range-border: #524d58; --diff-comment-highlight: rgba(88, 166, 255, 0.5); From 8fafa63b27789f80dd6c89fdf9256abff7b5ac37 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 30 Mar 2026 11:23:32 +1100 Subject: [PATCH 04/14] feat(diff-viewer): render line-level and character-level diff highlights 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) --- .../src/lib/components/DiffViewer.svelte | 136 +++++++++++++++++- 1 file changed, 129 insertions(+), 7 deletions(-) diff --git a/packages/diff-viewer/src/lib/components/DiffViewer.svelte b/packages/diff-viewer/src/lib/components/DiffViewer.svelte index 060d4e07..332809c8 100644 --- a/packages/diff-viewer/src/lib/components/DiffViewer.svelte +++ b/packages/diff-viewer/src/lib/components/DiffViewer.svelte @@ -54,6 +54,8 @@ isLineInChangedAlignment as helperIsLineInChangedAlignment, isLineInIndexedRange, isLineSelected as helperIsLineSelected, + getLineClass as helperGetLineClass, + getCharHighlights as helperGetCharHighlights, buildLineCommentEditorLayout, buildLineSelectionToolbarLayout, buildRangeCommentEditorLayout, @@ -63,6 +65,7 @@ normalizeLineSelection, resolveLineSelectionToolbarLeft, } from '../utils/diffViewerHelpers'; + import type { BeforeLineClass, AfterLineClass, CharHighlight } from '../utils/inlineDiff.js'; import { setupDiffKeyboardNav } from '../utils/diffKeyboard'; import { pathsMatch } from '../utils/diffModalHelpers'; import CommentEditor from './CommentEditor.svelte'; @@ -666,12 +669,13 @@ // Search highlighting // ========================================================================== - /** A token segment that may be part of a search match */ + /** A token segment that may be part of a search match or char-level diff highlight */ interface HighlightedSegment { content: string; color: string; isMatch: boolean; isCurrent: boolean; + isCharChanged: boolean; } /** @@ -738,6 +742,7 @@ color: t.color, isMatch: false, isCurrent: false, + isCharChanged: false, })); } @@ -760,6 +765,7 @@ color: token.color, isMatch: false, isCurrent: false, + isCharChanged: false, }); } else { // Token has matches - split at match boundaries @@ -776,6 +782,7 @@ color: token.color, isMatch: false, isCurrent: false, + isCharChanged: false, }); } @@ -785,6 +792,7 @@ color: token.color, isMatch: true, isCurrent: match.isCurrent, + isCharChanged: false, }); pos = matchEnd; @@ -797,6 +805,7 @@ color: token.color, isMatch: false, isCurrent: false, + isCharChanged: false, }); } } @@ -808,7 +817,73 @@ } /** - * Get highlighted token segments for a line, with search matches applied. + * Apply character-level diff highlights to segments by splitting them at highlight boundaries. + * Works similarly to applySearchHighlights — walks through segments tracking column position. + */ + function applyCharHighlights( + segments: HighlightedSegment[], + highlights: CharHighlight[] + ): HighlightedSegment[] { + if (highlights.length === 0) return segments; + + const result: HighlightedSegment[] = []; + let charIndex = 0; + + for (const segment of segments) { + const segStart = charIndex; + const segEnd = charIndex + segment.content.length; + + // Find highlights that overlap with this segment + const overlapping = highlights.filter( + (h) => h.start < segEnd && h.end > segStart + ); + + if (overlapping.length === 0) { + result.push(segment); + } else { + let pos = 0; // Position within segment content + + for (const hl of overlapping) { + const hlStart = Math.max(0, hl.start - segStart); + const hlEnd = Math.min(segment.content.length, hl.end - segStart); + + // Add any content before the highlight + if (pos < hlStart) { + result.push({ + ...segment, + content: segment.content.slice(pos, hlStart), + isCharChanged: false, + }); + } + + // Add the highlighted portion + result.push({ + ...segment, + content: segment.content.slice(hlStart, hlEnd), + isCharChanged: true, + }); + + pos = hlEnd; + } + + // Add any remaining content after all highlights + if (pos < segment.content.length) { + result.push({ + ...segment, + content: segment.content.slice(pos), + isCharChanged: false, + }); + } + } + + charIndex = segEnd; + } + + return result; + } + + /** + * Get highlighted token segments for a line, with search matches and char-level diff applied. */ function getHighlightedTokens( lineIndex: number, @@ -816,7 +891,14 @@ ): HighlightedSegment[] { const tokens = side === 'before' ? getBeforeTokens(lineIndex) : getAfterTokens(lineIndex); const matches = getSearchMatchesForLine(lineIndex, side); - return applySearchHighlights(tokens, matches); + let segments = applySearchHighlights(tokens, matches); + + const charHL = getCharHighlightsForLine(side, lineIndex); + if (charHL && charHL.length > 0) { + segments = applyCharHighlights(segments, charHL); + } + + return segments; } // ========================================================================== @@ -832,6 +914,30 @@ ); } + function getLineClassForLine(side: 'before' | 'after', lineIndex: number): BeforeLineClass | AfterLineClass | null { + return helperGetLineClass( + side, + lineIndex, + beforeLineToAlignment, + afterLineToAlignment, + activeAlignments, + beforeLines, + afterLines + ); + } + + function getCharHighlightsForLine(side: 'before' | 'after', lineIndex: number): CharHighlight[] | null { + return helperGetCharHighlights( + side, + lineIndex, + beforeLineToAlignment, + afterLineToAlignment, + activeAlignments, + beforeLines, + afterLines + ); + } + function isLineSelected(pane: 'before' | 'after', lineIndex: number): boolean { return helperIsLineSelected(pane, lineIndex, selectedLineRange); } @@ -1760,7 +1866,8 @@ : { isStart: false, isEnd: false }} {@const isInHoveredRange = isLineInHoveredRange('before', i)} {@const isInFocusedHunk = isLineInFocusedHunk('before', i)} - {@const isChanged = showRangeMarkers && isLineInChangedAlignment('before', i)} + {@const lineClass = showRangeMarkers ? getLineClassForLine('before', i) : null} + {@const isChanged = lineClass !== null && lineClass !== 'unchanged'}
handleLineMouseEnter('before', i)} onmouseleave={handleLineMouseLeave} > @@ -1778,6 +1886,7 @@ style="color: {segment.color}" class:search-match={segment.isMatch && !segment.isCurrent} class:search-current={segment.isCurrent} + class:char-changed={segment.isCharChanged} > {segment.content} @@ -1851,6 +1960,7 @@ style="color: {segment.color}" class:search-match={segment.isMatch && !segment.isCurrent} class:search-current={segment.isCurrent} + class:char-changed={segment.isCharChanged} > {segment.content} @@ -1927,7 +2037,8 @@ : { isStart: false, isEnd: false }} {@const isInHoveredRange = isLineInHoveredRange('after', i)} {@const isInFocusedHunk = isLineInFocusedHunk('after', i)} - {@const isChanged = showRangeMarkers && isLineInChangedAlignment('after', i)} + {@const lineClass = showRangeMarkers ? getLineClassForLine('after', i) : null} + {@const isChanged = lineClass !== null && lineClass !== 'unchanged'} {@const isSelected = isLineSelected('after', i)}
handleLineMouseEnter('after', i)} onmouseleave={handleLineMouseLeave} @@ -1948,6 +2060,7 @@ style="color: {segment.color}" class:search-match={segment.isMatch && !segment.isCurrent} class:search-current={segment.isCurrent} + class:char-changed={segment.isCharChanged} > {segment.content} @@ -2553,6 +2666,15 @@ background-color: var(--diff-added-bg); } + .line.diff-modified { + background-color: var(--diff-modified-bg); + } + + .char-changed { + background-color: var(--diff-modified-inline-bg); + border-radius: 2px; + } + /* Range boundary markers */ .line.range-start::before { content: ''; From eb01c28aa78432d68cd14349d624a1d1cc38a83e Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 30 Mar 2026 11:39:35 +1100 Subject: [PATCH 05/14] fix(diff-viewer): peek ahead in greedy matching to handle insertion offsets 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) --- .../src/lib/utils/inlineDiff.test.ts | 53 +++++++++++++++++++ .../diff-viewer/src/lib/utils/inlineDiff.ts | 11 ++++ 2 files changed, 64 insertions(+) create mode 100644 packages/diff-viewer/src/lib/utils/inlineDiff.test.ts diff --git a/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts b/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts new file mode 100644 index 00000000..70e008db --- /dev/null +++ b/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vitest'; +import { computeLineDiff } from './inlineDiff'; + +describe('computeLineDiff', () => { + it('detects a modified pair when an insertion precedes it', () => { + const before = [' content: string | string[];']; + const after = [' newField: boolean;', ' content: string | string[];']; + + const result = computeLineDiff(before, after); + + // The identical line should be matched by LCS, leaving nothing unmatched + expect(result.beforeLines).toEqual(['unchanged']); + expect(result.afterLines).toEqual(['added', 'unchanged']); + }); + + it('detects modification through an insertion offset', () => { + const before = [' pattern: string | string[];']; + const after = [' newField: boolean;', ' pattern: string[];']; + + const result = computeLineDiff(before, after); + + // The before line should pair with the similar after line, not be marked removed + expect(result.beforeLines).toEqual(['modified']); + expect(result.afterLines).toEqual(['added', 'modified']); + expect(result.modifiedPairs).toHaveLength(1); + expect(result.modifiedPairs[0].beforeLineIndex).toBe(0); + expect(result.modifiedPairs[0].afterLineIndex).toBe(1); + }); + + it('still marks pure removals and additions correctly', () => { + const before = ['line A', 'line B']; + const after = ['line C', 'line D']; + + const result = computeLineDiff(before, after); + + expect(result.beforeLines).toEqual(['removed', 'removed']); + expect(result.afterLines).toEqual(['added', 'added']); + expect(result.modifiedPairs).toHaveLength(0); + }); + + it('pairs modified lines without offset correctly', () => { + const before = ['const x = 1;']; + const after = ['const x = 2;']; + + const result = computeLineDiff(before, after); + + expect(result.beforeLines).toEqual(['modified']); + expect(result.afterLines).toEqual(['modified']); + expect(result.modifiedPairs).toHaveLength(1); + expect(result.modifiedPairs[0].beforeHighlights.length).toBeGreaterThan(0); + expect(result.modifiedPairs[0].afterHighlights.length).toBeGreaterThan(0); + }); +}); diff --git a/packages/diff-viewer/src/lib/utils/inlineDiff.ts b/packages/diff-viewer/src/lib/utils/inlineDiff.ts index 047c411c..8fcbcb62 100644 --- a/packages/diff-viewer/src/lib/utils/inlineDiff.ts +++ b/packages/diff-viewer/src/lib/utils/inlineDiff.ts @@ -201,6 +201,17 @@ export function computeLineDiff( bi++; ai++; } else { + // Peek ahead: if the next after-line is a better match for the current + // before-line, skip the current after-line as a pure insertion. + if (ai + 1 < unmatchedAfter.length) { + const nextAIdx = unmatchedAfter[ai + 1]; + const nextSim = similarity(beforeLines[bIdx], afterLines[nextAIdx]); + if (nextSim > SIMILARITY_THRESHOLD) { + afterClasses[aIdx] = 'added'; + ai++; + continue; + } + } beforeClasses[bIdx] = 'removed'; bi++; } From 8b00772f37ddc864a47d24ffc02dc6de5883e6a1 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 30 Mar 2026 11:43:56 +1100 Subject: [PATCH 06/14] test(diff-viewer): add comprehensive tests for inline diff and helpers 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) --- .../diffViewerHelpers.inlineDiff.test.ts | 215 +++++++++++++ .../src/lib/utils/inlineDiff.test.ts | 286 ++++++++++++++++-- 2 files changed, 469 insertions(+), 32 deletions(-) create mode 100644 packages/diff-viewer/src/lib/utils/diffViewerHelpers.inlineDiff.test.ts diff --git a/packages/diff-viewer/src/lib/utils/diffViewerHelpers.inlineDiff.test.ts b/packages/diff-viewer/src/lib/utils/diffViewerHelpers.inlineDiff.test.ts new file mode 100644 index 00000000..909d0b28 --- /dev/null +++ b/packages/diff-viewer/src/lib/utils/diffViewerHelpers.inlineDiff.test.ts @@ -0,0 +1,215 @@ +import { describe, expect, it } from 'vitest'; +import { getLineClass, getCharHighlights } from './diffViewerHelpers'; +import type { Alignment } from '../types'; + +/** + * Helper to build alignment lookup maps from an alignment array, + * mapping each line index to its alignment index. + */ +function buildLookups(alignments: Alignment[]) { + const beforeLineToAlignment = new Map(); + const afterLineToAlignment = new Map(); + + for (let i = 0; i < alignments.length; i++) { + const a = alignments[i]; + if (a.changed) { + for (let line = a.before.start; line < a.before.end; line++) { + beforeLineToAlignment.set(line, i); + } + for (let line = a.after.start; line < a.after.end; line++) { + afterLineToAlignment.set(line, i); + } + } + } + + return { beforeLineToAlignment, afterLineToAlignment }; +} + +describe('getLineClass', () => { + it('returns null for lines not in a changed alignment', () => { + const alignments: Alignment[] = [ + { before: { start: 0, end: 3 }, after: { start: 0, end: 3 }, changed: false }, + ]; + const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + + const result = getLineClass( + 'before', 1, beforeLineToAlignment, afterLineToAlignment, + alignments, ['a', 'b', 'c'], ['a', 'b', 'c'], + ); + expect(result).toBeNull(); + }); + + it('returns "modified" for a modified before-line in a changed alignment', () => { + const beforeLines = ['const x = 1;', 'unchanged']; + const afterLines = ['const x = 2;', 'unchanged']; + const alignments: Alignment[] = [ + { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: true }, + { before: { start: 1, end: 2 }, after: { start: 1, end: 2 }, changed: false }, + ]; + const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + + const result = getLineClass( + 'before', 0, beforeLineToAlignment, afterLineToAlignment, + alignments, beforeLines, afterLines, + ); + expect(result).toBe('modified'); + }); + + it('returns "modified" for a modified after-line', () => { + const beforeLines = ['const x = 1;']; + const afterLines = ['const x = 2;']; + const alignments: Alignment[] = [ + { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: true }, + ]; + const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + + const result = getLineClass( + 'after', 0, beforeLineToAlignment, afterLineToAlignment, + alignments, beforeLines, afterLines, + ); + expect(result).toBe('modified'); + }); + + it('returns "removed" for a deleted before-line', () => { + const beforeLines = ['deleted line', 'kept line']; + const afterLines = ['kept line']; + const alignments: Alignment[] = [ + { before: { start: 0, end: 1 }, after: { start: 0, end: 0 }, changed: true }, + { before: { start: 1, end: 2 }, after: { start: 0, end: 1 }, changed: false }, + ]; + const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + + const result = getLineClass( + 'before', 0, beforeLineToAlignment, afterLineToAlignment, + alignments, beforeLines, afterLines, + ); + expect(result).toBe('removed'); + }); + + it('returns "added" for a new after-line', () => { + const beforeLines = ['kept line']; + const afterLines = ['kept line', 'new line']; + const alignments: Alignment[] = [ + { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: false }, + { before: { start: 1, end: 1 }, after: { start: 1, end: 2 }, changed: true }, + ]; + const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + + const result = getLineClass( + 'after', 1, beforeLineToAlignment, afterLineToAlignment, + alignments, beforeLines, afterLines, + ); + expect(result).toBe('added'); + }); + + it('correctly maps line indices within multi-line changed alignments', () => { + // Alignment covers lines 2-4 in before, 2-5 in after + const beforeLines = ['ctx', 'ctx', 'const a = 1;', 'const b = true;', 'ctx']; + const afterLines = ['ctx', 'ctx', 'const a = 2;', 'newLine();', 'const b = false;', 'ctx']; + const alignments: Alignment[] = [ + { before: { start: 0, end: 2 }, after: { start: 0, end: 2 }, changed: false }, + { before: { start: 2, end: 4 }, after: { start: 2, end: 5 }, changed: true }, + { before: { start: 4, end: 5 }, after: { start: 5, end: 6 }, changed: false }, + ]; + const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + + // Line 2 before: "const a = 1;" should be modified (similar to "const a = 2;") + expect(getLineClass( + 'before', 2, beforeLineToAlignment, afterLineToAlignment, + alignments, beforeLines, afterLines, + )).toBe('modified'); + + // Line 3 in after: "newLine();" should be added (no similar before-line) + expect(getLineClass( + 'after', 3, beforeLineToAlignment, afterLineToAlignment, + alignments, beforeLines, afterLines, + )).toBe('added'); + }); +}); + +describe('getCharHighlights', () => { + it('returns null for lines not in a changed alignment', () => { + const alignments: Alignment[] = [ + { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: false }, + ]; + const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + + const result = getCharHighlights( + 'before', 0, beforeLineToAlignment, afterLineToAlignment, + alignments, ['hello'], ['hello'], + ); + expect(result).toBeNull(); + }); + + it('returns null for non-modified lines in a changed alignment', () => { + const beforeLines = ['removed line']; + const afterLines = ['totally different content here']; + const alignments: Alignment[] = [ + { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: true }, + ]; + const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + + // Lines are too dissimilar to be "modified", so no char highlights + const result = getCharHighlights( + 'before', 0, beforeLineToAlignment, afterLineToAlignment, + alignments, beforeLines, afterLines, + ); + expect(result).toBeNull(); + }); + + it('returns highlights for a modified before-line', () => { + const beforeLines = ['const x = 1;']; + const afterLines = ['const x = 2;']; + const alignments: Alignment[] = [ + { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: true }, + ]; + const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + + const result = getCharHighlights( + 'before', 0, beforeLineToAlignment, afterLineToAlignment, + alignments, beforeLines, afterLines, + ); + expect(result).not.toBeNull(); + expect(result!.length).toBeGreaterThan(0); + // The highlight should cover "1;" (the changed part) + for (const h of result!) { + expect(h.start).toBeGreaterThanOrEqual(0); + expect(h.end).toBeGreaterThan(h.start); + expect(h.end).toBeLessThanOrEqual(beforeLines[0].length); + } + }); + + it('returns highlights for a modified after-line', () => { + const beforeLines = ['the quick brown fox']; + const afterLines = ['the slow brown fox']; + const alignments: Alignment[] = [ + { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: true }, + ]; + const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + + const result = getCharHighlights( + 'after', 0, beforeLineToAlignment, afterLineToAlignment, + alignments, beforeLines, afterLines, + ); + expect(result).not.toBeNull(); + // "slow" replaces "quick" -> highlight at position 4-8 + expect(result).toEqual([{ start: 4, end: 8 }]); + }); + + it('works with offset line indices in alignments', () => { + const beforeLines = ['ctx', 'const x = 1;']; + const afterLines = ['ctx', 'const x = 2;']; + const alignments: Alignment[] = [ + { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: false }, + { before: { start: 1, end: 2 }, after: { start: 1, end: 2 }, changed: true }, + ]; + const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + + const result = getCharHighlights( + 'after', 1, beforeLineToAlignment, afterLineToAlignment, + alignments, beforeLines, afterLines, + ); + expect(result).not.toBeNull(); + expect(result!.length).toBeGreaterThan(0); + }); +}); diff --git a/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts b/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts index 70e008db..d1feee08 100644 --- a/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts +++ b/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts @@ -1,53 +1,275 @@ import { describe, expect, it } from 'vitest'; -import { computeLineDiff } from './inlineDiff'; +import { + computeLineDiff, + getLineDiffResult, + type LineDiffResult, + type CharHighlight, +} from './inlineDiff'; describe('computeLineDiff', () => { - it('detects a modified pair when an insertion precedes it', () => { - const before = [' content: string | string[];']; - const after = [' newField: boolean;', ' content: string | string[];']; + describe('identical lines', () => { + it('marks all lines unchanged when before and after are identical', () => { + const lines = ['line 1', 'line 2', 'line 3']; + const result = computeLineDiff(lines, lines); - const result = computeLineDiff(before, after); + expect(result.beforeLines).toEqual(['unchanged', 'unchanged', 'unchanged']); + expect(result.afterLines).toEqual(['unchanged', 'unchanged', 'unchanged']); + expect(result.modifiedPairs).toHaveLength(0); + }); - // The identical line should be matched by LCS, leaving nothing unmatched - expect(result.beforeLines).toEqual(['unchanged']); - expect(result.afterLines).toEqual(['added', 'unchanged']); + it('handles empty input', () => { + const result = computeLineDiff([], []); + expect(result.beforeLines).toEqual([]); + expect(result.afterLines).toEqual([]); + expect(result.modifiedPairs).toHaveLength(0); + }); + + it('handles single identical line', () => { + const result = computeLineDiff(['hello'], ['hello']); + expect(result.beforeLines).toEqual(['unchanged']); + expect(result.afterLines).toEqual(['unchanged']); + }); + }); + + describe('pure additions and removals', () => { + it('marks all lines as added when before is empty', () => { + const result = computeLineDiff([], ['line 1', 'line 2']); + expect(result.beforeLines).toEqual([]); + expect(result.afterLines).toEqual(['added', 'added']); + }); + + it('marks all lines as removed when after is empty', () => { + const result = computeLineDiff(['line 1', 'line 2'], []); + expect(result.beforeLines).toEqual(['removed', 'removed']); + expect(result.afterLines).toEqual([]); + }); + + it('marks completely different lines as removed/added', () => { + const before = ['aaa xyz', 'bbb xyz']; + const after = ['111 qqq', '222 qqq']; + const result = computeLineDiff(before, after); + + expect(result.beforeLines).toEqual(['removed', 'removed']); + expect(result.afterLines).toEqual(['added', 'added']); + expect(result.modifiedPairs).toHaveLength(0); + }); + + it('detects additions at the end with unchanged context', () => { + const before = ['line 1', 'line 2']; + const after = ['line 1', 'line 2', 'line 3']; + const result = computeLineDiff(before, after); + + expect(result.beforeLines).toEqual(['unchanged', 'unchanged']); + expect(result.afterLines).toEqual(['unchanged', 'unchanged', 'added']); + }); + + it('detects removals at the beginning with unchanged context', () => { + const before = ['line 0', 'line 1', 'line 2']; + const after = ['line 1', 'line 2']; + const result = computeLineDiff(before, after); + + expect(result.beforeLines).toEqual(['removed', 'unchanged', 'unchanged']); + expect(result.afterLines).toEqual(['unchanged', 'unchanged']); + }); + + it('detects additions in the middle', () => { + const before = ['line 1', 'line 3']; + const after = ['line 1', 'line 2', 'line 3']; + const result = computeLineDiff(before, after); + + expect(result.beforeLines).toEqual(['unchanged', 'unchanged']); + expect(result.afterLines).toEqual(['unchanged', 'added', 'unchanged']); + }); + }); + + describe('modified line detection', () => { + it('pairs modified lines without offset', () => { + const before = ['const x = 1;']; + const after = ['const x = 2;']; + const result = computeLineDiff(before, after); + + expect(result.beforeLines).toEqual(['modified']); + expect(result.afterLines).toEqual(['modified']); + expect(result.modifiedPairs).toHaveLength(1); + expect(result.modifiedPairs[0].beforeLineIndex).toBe(0); + expect(result.modifiedPairs[0].afterLineIndex).toBe(0); + }); + + it('detects multiple modified pairs', () => { + const before = ['const a = 1;', 'unchanged', 'const b = true;']; + const after = ['const a = 2;', 'unchanged', 'const b = false;']; + const result = computeLineDiff(before, after); + + expect(result.beforeLines).toEqual(['modified', 'unchanged', 'modified']); + expect(result.afterLines).toEqual(['modified', 'unchanged', 'modified']); + expect(result.modifiedPairs).toHaveLength(2); + }); + + it('handles mixed modifications and unchanged lines', () => { + const before = ['first', 'const x = 1;', 'middle', 'last']; + const after = ['first', 'const x = 2;', 'middle', 'last']; + const result = computeLineDiff(before, after); + + expect(result.beforeLines).toEqual(['unchanged', 'modified', 'unchanged', 'unchanged']); + expect(result.afterLines).toEqual(['unchanged', 'modified', 'unchanged', 'unchanged']); + expect(result.modifiedPairs).toHaveLength(1); + }); + }); + + describe('insertion offset (peek-ahead)', () => { + it('detects a modified pair when an insertion precedes it', () => { + const before = [' content: string | string[];']; + const after = [' newField: boolean;', ' content: string | string[];']; + const result = computeLineDiff(before, after); + + expect(result.beforeLines).toEqual(['unchanged']); + expect(result.afterLines).toEqual(['added', 'unchanged']); + }); + + it('detects modification through an insertion offset', () => { + const before = [' pattern: string | string[];']; + const after = [' newField: boolean;', ' pattern: string[];']; + const result = computeLineDiff(before, after); + + expect(result.beforeLines).toEqual(['modified']); + expect(result.afterLines).toEqual(['added', 'modified']); + expect(result.modifiedPairs).toHaveLength(1); + expect(result.modifiedPairs[0].beforeLineIndex).toBe(0); + expect(result.modifiedPairs[0].afterLineIndex).toBe(1); + }); + + it('handles insertion before a modification with unchanged context', () => { + const before = ['header', ' pattern: string | string[];', 'footer']; + const after = ['header', '// totally new comment here', ' pattern: string[];', 'footer']; + const result = computeLineDiff(before, after); + + expect(result.beforeLines).toEqual(['unchanged', 'modified', 'unchanged']); + expect(result.afterLines).toEqual(['unchanged', 'added', 'modified', 'unchanged']); + }); }); - it('detects modification through an insertion offset', () => { - const before = [' pattern: string | string[];']; - const after = [' newField: boolean;', ' pattern: string[];']; + describe('character highlights', () => { + it('produces highlights for word-level changes', () => { + const before = ['const x = 1;']; + const after = ['const x = 2;']; + const result = computeLineDiff(before, after); + + expect(result.modifiedPairs).toHaveLength(1); + const pair = result.modifiedPairs[0]; + + // "1;" -> "2;" are the changed tokens + expect(pair.beforeHighlights.length).toBeGreaterThan(0); + expect(pair.afterHighlights.length).toBeGreaterThan(0); + }); + + it('highlights only the changed word in a sentence', () => { + const before = ['the quick brown fox']; + const after = ['the slow brown fox']; + const result = computeLineDiff(before, after); + + const pair = result.modifiedPairs[0]; + // "quick" is at position 4-9, "slow" is at position 4-8 + expect(pair.beforeHighlights).toEqual([{ start: 4, end: 9 }]); + expect(pair.afterHighlights).toEqual([{ start: 4, end: 8 }]); + }); - const result = computeLineDiff(before, after); + it('highlights multiple changed words', () => { + const before = ['function foo(bar: string): number {']; + const after = ['function baz(bar: boolean): string {']; + const result = computeLineDiff(before, after); - // The before line should pair with the similar after line, not be marked removed - expect(result.beforeLines).toEqual(['modified']); - expect(result.afterLines).toEqual(['added', 'modified']); - expect(result.modifiedPairs).toHaveLength(1); - expect(result.modifiedPairs[0].beforeLineIndex).toBe(0); - expect(result.modifiedPairs[0].afterLineIndex).toBe(1); + const pair = result.modifiedPairs[0]; + expect(pair.beforeHighlights.length).toBeGreaterThanOrEqual(2); + expect(pair.afterHighlights.length).toBeGreaterThanOrEqual(2); + }); + + it('produces no highlights when lines are identical but in unmatched blocks', () => { + // This shouldn't happen in practice since identical lines would be + // caught by LCS, but the char-highlight logic should handle it + const before = [' return null;', ' return value;']; + const after = [' return value;']; + const result = computeLineDiff(before, after); + + // "return null;" is removed, "return value;" is unchanged via LCS + expect(result.beforeLines[0]).toBe('removed'); + expect(result.beforeLines[1]).toBe('unchanged'); + expect(result.afterLines[0]).toBe('unchanged'); + }); }); - it('still marks pure removals and additions correctly', () => { - const before = ['line A', 'line B']; - const after = ['line C', 'line D']; + describe('complex scenarios', () => { + it('handles a realistic code diff with mixed changes', () => { + const before = [ + 'import { foo } from "bar";', + '', + 'export function hello() {', + ' return "world";', + '}', + ]; + const after = [ + 'import { foo, baz } from "bar";', + 'import { qux } from "quux";', + '', + 'export function hello() {', + ' return "universe";', + '}', + ]; + const result = computeLineDiff(before, after); + + expect(result.beforeLines[0]).toBe('modified'); // import changed + expect(result.beforeLines[1]).toBe('unchanged'); // empty line + expect(result.beforeLines[2]).toBe('unchanged'); // function decl + expect(result.beforeLines[3]).toBe('modified'); // return changed + expect(result.beforeLines[4]).toBe('unchanged'); // closing brace + + expect(result.afterLines[0]).toBe('modified'); // import changed + expect(result.afterLines[1]).toBe('added'); // new import + expect(result.afterLines[2]).toBe('unchanged'); // empty line + expect(result.afterLines[3]).toBe('unchanged'); // function decl + expect(result.afterLines[4]).toBe('modified'); // return changed + expect(result.afterLines[5]).toBe('unchanged'); // closing brace + }); + + it('handles multiple consecutive removals followed by additions', () => { + const before = ['aaa', 'bbb', 'ccc']; + const after = ['xxx', 'yyy', 'zzz']; + const result = computeLineDiff(before, after); - const result = computeLineDiff(before, after); + expect(result.beforeLines).toEqual(['removed', 'removed', 'removed']); + expect(result.afterLines).toEqual(['added', 'added', 'added']); + }); - expect(result.beforeLines).toEqual(['removed', 'removed']); - expect(result.afterLines).toEqual(['added', 'added']); - expect(result.modifiedPairs).toHaveLength(0); + it('handles interleaved unchanged and changed lines', () => { + const before = ['A', 'B', 'C', 'D', 'E']; + const after = ['A', 'B2', 'C', 'D2', 'E']; + const result = computeLineDiff(before, after); + + expect(result.beforeLines[0]).toBe('unchanged'); + expect(result.beforeLines[2]).toBe('unchanged'); + expect(result.beforeLines[4]).toBe('unchanged'); + // B and D are dissimilar enough to be removed/added rather than modified + expect(result.afterLines[0]).toBe('unchanged'); + expect(result.afterLines[2]).toBe('unchanged'); + expect(result.afterLines[4]).toBe('unchanged'); + }); }); +}); - it('pairs modified lines without offset correctly', () => { +describe('getLineDiffResult (caching)', () => { + it('returns same result object for identical inputs', () => { const before = ['const x = 1;']; const after = ['const x = 2;']; - const result = computeLineDiff(before, after); + const result1 = getLineDiffResult(before, after); + const result2 = getLineDiffResult(before, after); + + expect(result1).toBe(result2); // same reference + }); + + it('returns different result objects for different inputs', () => { + const result1 = getLineDiffResult(['a'], ['b']); + const result2 = getLineDiffResult(['c'], ['d']); - expect(result.beforeLines).toEqual(['modified']); - expect(result.afterLines).toEqual(['modified']); - expect(result.modifiedPairs).toHaveLength(1); - expect(result.modifiedPairs[0].beforeHighlights.length).toBeGreaterThan(0); - expect(result.modifiedPairs[0].afterHighlights.length).toBeGreaterThan(0); + expect(result1).not.toBe(result2); }); }); From 7fdeb9d380c7f1cdc078bb322bb0f58ca0214856 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 30 Mar 2026 12:09:42 +1100 Subject: [PATCH 07/14] fix(diff-viewer): scan ahead past multiple insertions in greedy matching 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) --- .../src/lib/utils/inlineDiff.test.ts | 51 +++++++++++++++++++ .../diff-viewer/src/lib/utils/inlineDiff.ts | 30 +++++++---- 2 files changed, 70 insertions(+), 11 deletions(-) diff --git a/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts b/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts index d1feee08..b124eb85 100644 --- a/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts +++ b/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts @@ -239,6 +239,57 @@ describe('computeLineDiff', () => { expect(result.afterLines).toEqual(['added', 'added', 'added']); }); + it('detects re-indented lines as modified when many insertions precede them', () => { + // Real-world case: code gets wrapped in an if-else block. The "before" + // has a simple format!() call; the "after" has a large new if-branch + // followed by an else-branch where the original code is re-indented. + // When parsed as a single changed alignment the greedy two-pointer + // sees many inserted after-lines before reaching the re-indented lines, + // causing them to be classified as removed/added instead of modified. + const before = [ + ' let prompt = format!(', + ' r#"', + ' pr_type = pr_type,', + ' base_branch = base_branch,', + ' branch_name = branch.branch_name,', + ' draft_flag = draft_flag,', + ' );', + ]; + const after = [ + ' let prompt = if let Some(ctx) = git_context {', + ' format!(', + ' r#"', + 'Steps:', + '1. Push the current branch to the remote', + '"#,', + ' pr_type = pr_type,', + ' base_branch = base_branch,', + ' branch_name = branch.branch_name,', + ' draft_flag = draft_flag,', + ' log_output = ctx.log,', + ' stat_output = ctx.stat,', + ' )', + ' } else {', + ' format!(', + ' r#"', + ' pr_type = pr_type,', + ' base_branch = base_branch,', + ' branch_name = branch.branch_name,', + ' draft_flag = draft_flag,', + ' )', + ' };', + ]; + const result = computeLineDiff(before, after); + + // The re-indented lines in the else branch should be modified, not removed/added. + // before[2..5] ("pr_type", "base_branch", "branch_name", "draft_flag") + // should pair with the re-indented versions in the after else-branch. + expect(result.beforeLines[2]).toBe('modified'); // pr_type + expect(result.beforeLines[3]).toBe('modified'); // base_branch + expect(result.beforeLines[4]).toBe('modified'); // branch_name + expect(result.beforeLines[5]).toBe('modified'); // draft_flag + }); + it('handles interleaved unchanged and changed lines', () => { const before = ['A', 'B', 'C', 'D', 'E']; const after = ['A', 'B2', 'C', 'D2', 'E']; diff --git a/packages/diff-viewer/src/lib/utils/inlineDiff.ts b/packages/diff-viewer/src/lib/utils/inlineDiff.ts index 8fcbcb62..ae1fa4c5 100644 --- a/packages/diff-viewer/src/lib/utils/inlineDiff.ts +++ b/packages/diff-viewer/src/lib/utils/inlineDiff.ts @@ -201,19 +201,27 @@ export function computeLineDiff( bi++; ai++; } else { - // Peek ahead: if the next after-line is a better match for the current - // before-line, skip the current after-line as a pure insertion. - if (ai + 1 < unmatchedAfter.length) { - const nextAIdx = unmatchedAfter[ai + 1]; - const nextSim = similarity(beforeLines[bIdx], afterLines[nextAIdx]); - if (nextSim > SIMILARITY_THRESHOLD) { - afterClasses[aIdx] = 'added'; - ai++; - continue; + // Scan ahead in unmatchedAfter to find a match for the current + // before-line. This handles cases where many insertions precede a + // modification (e.g. code re-indented after being wrapped in a block). + let found = false; + for (let scan = ai + 1; scan < unmatchedAfter.length; scan++) { + const scanIdx = unmatchedAfter[scan]; + const scanSim = similarity(beforeLines[bIdx], afterLines[scanIdx]); + if (scanSim > SIMILARITY_THRESHOLD) { + // Mark all skipped after-lines as pure additions. + for (let skip = ai; skip < scan; skip++) { + afterClasses[unmatchedAfter[skip]] = 'added'; + } + ai = scan; + found = true; + break; } } - beforeClasses[bIdx] = 'removed'; - bi++; + if (!found) { + beforeClasses[bIdx] = 'removed'; + bi++; + } } } From 8bcbb2113d0c3b2d0b83ad29a5755e6f333f1f1c Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 30 Mar 2026 12:27:39 +1100 Subject: [PATCH 08/14] test(diff-viewer): add real-world re-indentation test for inline diff 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) --- .../src/lib/utils/inlineDiff.test.ts | 69 ++++++++++++++++--- 1 file changed, 60 insertions(+), 9 deletions(-) diff --git a/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts b/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts index b124eb85..a8c3959e 100644 --- a/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts +++ b/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts @@ -239,13 +239,66 @@ describe('computeLineDiff', () => { expect(result.afterLines).toEqual(['added', 'added', 'added']); }); + it('detects re-indented lines as modified within a hunk with context', () => { + // Real-world case: code wrapped in an if-else block gains indentation. + // The hunk alignment includes context lines (identical on both sides) + // plus the changed lines. The re-indented lines should be "modified". + const before = [ + 'This is critical - the application parses this to link the PR.', + '"#,', + ' pr_type = pr_type,', + ' base_branch = base_branch,', + ' branch_name = branch.branch_name,', + ' draft_flag = draft_flag,', + ' );', + '', + ' let mut session = store::Session::new_running(&prompt, &working_dir);', + ' if let Some(ref p) = provider {', + ]; + const after = [ + 'This is critical - the application parses this to link the PR.', + '"#,', + ' pr_type = pr_type,', + ' base_branch = base_branch,', + ' branch_name = branch.branch_name,', + ' draft_flag = draft_flag,', + ' )', + ' };', + '', + ' let mut session = store::Session::new_running(&prompt, &working_dir);', + ' if let Some(ref p) = provider {', + ]; + const result = computeLineDiff(before, after); + + // Context lines should be unchanged + expect(result.beforeLines[0]).toBe('unchanged'); + expect(result.beforeLines[1]).toBe('unchanged'); + expect(result.afterLines[0]).toBe('unchanged'); + expect(result.afterLines[1]).toBe('unchanged'); + + // Re-indented lines should be modified, not removed/added + expect(result.beforeLines[2]).toBe('modified'); // pr_type + expect(result.beforeLines[3]).toBe('modified'); // base_branch + expect(result.beforeLines[4]).toBe('modified'); // branch_name + expect(result.beforeLines[5]).toBe('modified'); // draft_flag + expect(result.beforeLines[6]).toBe('modified'); // ); -> ) + + expect(result.afterLines[2]).toBe('modified'); // pr_type + expect(result.afterLines[3]).toBe('modified'); // base_branch + expect(result.afterLines[4]).toBe('modified'); // branch_name + expect(result.afterLines[5]).toBe('modified'); // draft_flag + expect(result.afterLines[6]).toBe('modified'); // ) + expect(result.afterLines[7]).toBe('added'); // }; + + // Trailing context should be unchanged + expect(result.beforeLines[7]).toBe('unchanged'); + expect(result.afterLines[8]).toBe('unchanged'); + }); + it('detects re-indented lines as modified when many insertions precede them', () => { - // Real-world case: code gets wrapped in an if-else block. The "before" - // has a simple format!() call; the "after" has a large new if-branch - // followed by an else-branch where the original code is re-indented. - // When parsed as a single changed alignment the greedy two-pointer - // sees many inserted after-lines before reaching the re-indented lines, - // causing them to be classified as removed/added instead of modified. + // Edge case: if two hunks were merged into a single alignment, the + // greedy matcher would see many inserted after-lines before reaching + // the re-indented lines. The scan-ahead handles this. const before = [ ' let prompt = format!(', ' r#"', @@ -281,9 +334,7 @@ describe('computeLineDiff', () => { ]; const result = computeLineDiff(before, after); - // The re-indented lines in the else branch should be modified, not removed/added. - // before[2..5] ("pr_type", "base_branch", "branch_name", "draft_flag") - // should pair with the re-indented versions in the after else-branch. + // The re-indented lines should be modified, not removed/added expect(result.beforeLines[2]).toBe('modified'); // pr_type expect(result.beforeLines[3]).toBe('modified'); // base_branch expect(result.beforeLines[4]).toBe('modified'); // branch_name From b32e26c7e0dd3b0cbd8e0a2730bc1e8013737293 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 30 Mar 2026 13:07:37 +1100 Subject: [PATCH 09/14] fix(diff-viewer): use changedAlignments index for inline diff lookups 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) --- .../src/lib/components/DiffViewer.svelte | 4 +- .../diffViewerHelpers.inlineDiff.test.ts | 61 ++++++++++--------- .../src/lib/utils/diffViewerHelpers.ts | 8 +-- 3 files changed, 39 insertions(+), 34 deletions(-) diff --git a/packages/diff-viewer/src/lib/components/DiffViewer.svelte b/packages/diff-viewer/src/lib/components/DiffViewer.svelte index 332809c8..6ac8e9f0 100644 --- a/packages/diff-viewer/src/lib/components/DiffViewer.svelte +++ b/packages/diff-viewer/src/lib/components/DiffViewer.svelte @@ -920,7 +920,7 @@ lineIndex, beforeLineToAlignment, afterLineToAlignment, - activeAlignments, + changedAlignments, beforeLines, afterLines ); @@ -932,7 +932,7 @@ lineIndex, beforeLineToAlignment, afterLineToAlignment, - activeAlignments, + changedAlignments, beforeLines, afterLines ); diff --git a/packages/diff-viewer/src/lib/utils/diffViewerHelpers.inlineDiff.test.ts b/packages/diff-viewer/src/lib/utils/diffViewerHelpers.inlineDiff.test.ts index 909d0b28..697f1507 100644 --- a/packages/diff-viewer/src/lib/utils/diffViewerHelpers.inlineDiff.test.ts +++ b/packages/diff-viewer/src/lib/utils/diffViewerHelpers.inlineDiff.test.ts @@ -1,28 +1,33 @@ import { describe, expect, it } from 'vitest'; import { getLineClass, getCharHighlights } from './diffViewerHelpers'; +import type { ChangedAlignmentEntry } from './diffViewerHelpers'; import type { Alignment } from '../types'; /** - * Helper to build alignment lookup maps from an alignment array, - * mapping each line index to its alignment index. + * Helper to build alignment lookup maps and the changedAlignments array + * from a full alignment array. Maps line indices to the index within + * the changedAlignments array (matching production code). */ function buildLookups(alignments: Alignment[]) { const beforeLineToAlignment = new Map(); const afterLineToAlignment = new Map(); + const changedAlignments: ChangedAlignmentEntry[] = []; for (let i = 0; i < alignments.length; i++) { const a = alignments[i]; if (a.changed) { + const changedIdx = changedAlignments.length; + changedAlignments.push({ alignment: a, index: i }); for (let line = a.before.start; line < a.before.end; line++) { - beforeLineToAlignment.set(line, i); + beforeLineToAlignment.set(line, changedIdx); } for (let line = a.after.start; line < a.after.end; line++) { - afterLineToAlignment.set(line, i); + afterLineToAlignment.set(line, changedIdx); } } } - return { beforeLineToAlignment, afterLineToAlignment }; + return { beforeLineToAlignment, afterLineToAlignment, changedAlignments }; } describe('getLineClass', () => { @@ -30,11 +35,11 @@ describe('getLineClass', () => { const alignments: Alignment[] = [ { before: { start: 0, end: 3 }, after: { start: 0, end: 3 }, changed: false }, ]; - const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + const { beforeLineToAlignment, afterLineToAlignment, changedAlignments } = buildLookups(alignments); const result = getLineClass( 'before', 1, beforeLineToAlignment, afterLineToAlignment, - alignments, ['a', 'b', 'c'], ['a', 'b', 'c'], + changedAlignments, ['a', 'b', 'c'], ['a', 'b', 'c'], ); expect(result).toBeNull(); }); @@ -46,11 +51,11 @@ describe('getLineClass', () => { { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: true }, { before: { start: 1, end: 2 }, after: { start: 1, end: 2 }, changed: false }, ]; - const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + const { beforeLineToAlignment, afterLineToAlignment, changedAlignments } = buildLookups(alignments); const result = getLineClass( 'before', 0, beforeLineToAlignment, afterLineToAlignment, - alignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, ); expect(result).toBe('modified'); }); @@ -61,11 +66,11 @@ describe('getLineClass', () => { const alignments: Alignment[] = [ { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: true }, ]; - const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + const { beforeLineToAlignment, afterLineToAlignment, changedAlignments } = buildLookups(alignments); const result = getLineClass( 'after', 0, beforeLineToAlignment, afterLineToAlignment, - alignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, ); expect(result).toBe('modified'); }); @@ -77,11 +82,11 @@ describe('getLineClass', () => { { before: { start: 0, end: 1 }, after: { start: 0, end: 0 }, changed: true }, { before: { start: 1, end: 2 }, after: { start: 0, end: 1 }, changed: false }, ]; - const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + const { beforeLineToAlignment, afterLineToAlignment, changedAlignments } = buildLookups(alignments); const result = getLineClass( 'before', 0, beforeLineToAlignment, afterLineToAlignment, - alignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, ); expect(result).toBe('removed'); }); @@ -93,11 +98,11 @@ describe('getLineClass', () => { { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: false }, { before: { start: 1, end: 1 }, after: { start: 1, end: 2 }, changed: true }, ]; - const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + const { beforeLineToAlignment, afterLineToAlignment, changedAlignments } = buildLookups(alignments); const result = getLineClass( 'after', 1, beforeLineToAlignment, afterLineToAlignment, - alignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, ); expect(result).toBe('added'); }); @@ -111,18 +116,18 @@ describe('getLineClass', () => { { before: { start: 2, end: 4 }, after: { start: 2, end: 5 }, changed: true }, { before: { start: 4, end: 5 }, after: { start: 5, end: 6 }, changed: false }, ]; - const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + const { beforeLineToAlignment, afterLineToAlignment, changedAlignments } = buildLookups(alignments); // Line 2 before: "const a = 1;" should be modified (similar to "const a = 2;") expect(getLineClass( 'before', 2, beforeLineToAlignment, afterLineToAlignment, - alignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, )).toBe('modified'); // Line 3 in after: "newLine();" should be added (no similar before-line) expect(getLineClass( 'after', 3, beforeLineToAlignment, afterLineToAlignment, - alignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, )).toBe('added'); }); }); @@ -132,11 +137,11 @@ describe('getCharHighlights', () => { const alignments: Alignment[] = [ { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: false }, ]; - const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + const { beforeLineToAlignment, afterLineToAlignment, changedAlignments } = buildLookups(alignments); const result = getCharHighlights( 'before', 0, beforeLineToAlignment, afterLineToAlignment, - alignments, ['hello'], ['hello'], + changedAlignments, ['hello'], ['hello'], ); expect(result).toBeNull(); }); @@ -147,12 +152,12 @@ describe('getCharHighlights', () => { const alignments: Alignment[] = [ { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: true }, ]; - const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + const { beforeLineToAlignment, afterLineToAlignment, changedAlignments } = buildLookups(alignments); // Lines are too dissimilar to be "modified", so no char highlights const result = getCharHighlights( 'before', 0, beforeLineToAlignment, afterLineToAlignment, - alignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, ); expect(result).toBeNull(); }); @@ -163,11 +168,11 @@ describe('getCharHighlights', () => { const alignments: Alignment[] = [ { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: true }, ]; - const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + const { beforeLineToAlignment, afterLineToAlignment, changedAlignments } = buildLookups(alignments); const result = getCharHighlights( 'before', 0, beforeLineToAlignment, afterLineToAlignment, - alignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, ); expect(result).not.toBeNull(); expect(result!.length).toBeGreaterThan(0); @@ -185,11 +190,11 @@ describe('getCharHighlights', () => { const alignments: Alignment[] = [ { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: true }, ]; - const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + const { beforeLineToAlignment, afterLineToAlignment, changedAlignments } = buildLookups(alignments); const result = getCharHighlights( 'after', 0, beforeLineToAlignment, afterLineToAlignment, - alignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, ); expect(result).not.toBeNull(); // "slow" replaces "quick" -> highlight at position 4-8 @@ -203,11 +208,11 @@ describe('getCharHighlights', () => { { before: { start: 0, end: 1 }, after: { start: 0, end: 1 }, changed: false }, { before: { start: 1, end: 2 }, after: { start: 1, end: 2 }, changed: true }, ]; - const { beforeLineToAlignment, afterLineToAlignment } = buildLookups(alignments); + const { beforeLineToAlignment, afterLineToAlignment, changedAlignments } = buildLookups(alignments); const result = getCharHighlights( 'after', 1, beforeLineToAlignment, afterLineToAlignment, - alignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, ); expect(result).not.toBeNull(); expect(result!.length).toBeGreaterThan(0); diff --git a/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts b/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts index 9fbf353c..33e1d9e9 100644 --- a/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts +++ b/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts @@ -296,7 +296,7 @@ export function getLineClass( lineIndex: number, beforeLineToAlignment: Map, afterLineToAlignment: Map, - alignments: Alignment[], + changedAlignments: ChangedAlignmentEntry[], beforeLines: string[], afterLines: string[] ): BeforeLineClass | AfterLineClass | null { @@ -304,7 +304,7 @@ export function getLineClass( const alignIdx = map.get(lineIndex); if (alignIdx === undefined) return null; - const alignment = alignments[alignIdx]; + const alignment = changedAlignments[alignIdx].alignment; const alignBefore = beforeLines.slice(alignment.before.start, alignment.before.end); const alignAfter = afterLines.slice(alignment.after.start, alignment.after.end); const result = getLineDiffResult(alignBefore, alignAfter); @@ -327,7 +327,7 @@ export function getCharHighlights( lineIndex: number, beforeLineToAlignment: Map, afterLineToAlignment: Map, - alignments: Alignment[], + changedAlignments: ChangedAlignmentEntry[], beforeLines: string[], afterLines: string[] ): CharHighlight[] | null { @@ -335,7 +335,7 @@ export function getCharHighlights( const alignIdx = map.get(lineIndex); if (alignIdx === undefined) return null; - const alignment = alignments[alignIdx]; + const alignment = changedAlignments[alignIdx].alignment; const alignBefore = beforeLines.slice(alignment.before.start, alignment.before.end); const alignAfter = afterLines.slice(alignment.after.start, alignment.after.end); const result = getLineDiffResult(alignBefore, alignAfter); From 6e3d6b8668cf457f83d18fdaf77ad8801ef36850 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 30 Mar 2026 13:17:01 +1100 Subject: [PATCH 10/14] fix(diff-viewer): use trimmed lines for similarity to avoid false modified pairs Character-level LCS inflated similarity scores for unrelated code lines that shared indentation whitespace (e.g. `r#""` 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) --- .../src/lib/utils/inlineDiff.test.ts | 40 +++++++++++++++++++ .../diff-viewer/src/lib/utils/inlineDiff.ts | 6 +-- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts b/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts index a8c3959e..a57bd835 100644 --- a/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts +++ b/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts @@ -341,6 +341,46 @@ describe('computeLineDiff', () => { expect(result.beforeLines[5]).toBe('modified'); // draft_flag }); + it('classifies completely replaced code blocks as removed/added, not modified', () => { + // Real-world case: a `let prompt = format!(...)` block is replaced by + // a `let git_context = pre_compute_git_context(...)` call preceded by + // comments. The lines share structural tokens like `let ... = ...(` but + // are semantically unrelated and should NOT be paired as modified. + const before = [ + ' let prompt = format!(', + ' r#"', + 'Create a {pr_type} for the current branch.', + '', + 'Steps:', + '1. First, look at the diff between the current branch and when it branched off of the base branch `{base_branch}`.', + '2. Push the current branch to the remote: `git push -u origin {branch_name}`', + '3. Create a PR using the GitHub CLI: `gh pr create --base {base_branch} --fill-first{draft_flag}`', + ]; + const after = [ + ' // Pre-compute git context in parallel so the agent can skip straight to', + ' // pushing and creating the PR instead of running these deterministic', + ' // commands itself.', + ' let git_context = pre_compute_git_context(', + ' is_remote,', + ' &working_dir,', + ' workspace_name.as_deref(),', + ' &store,', + ' &branch,', + ' base_branch,', + ' );', + ]; + const result = computeLineDiff(before, after); + + // Every before-line should be removed, every after-line should be added + for (let i = 0; i < before.length; i++) { + expect(result.beforeLines[i]).toBe('removed'); + } + for (let i = 0; i < after.length; i++) { + expect(result.afterLines[i]).toBe('added'); + } + expect(result.modifiedPairs).toHaveLength(0); + }); + it('handles interleaved unchanged and changed lines', () => { const before = ['A', 'B', 'C', 'D', 'E']; const after = ['A', 'B2', 'C', 'D2', 'E']; diff --git a/packages/diff-viewer/src/lib/utils/inlineDiff.ts b/packages/diff-viewer/src/lib/utils/inlineDiff.ts index ae1fa4c5..4da43dfa 100644 --- a/packages/diff-viewer/src/lib/utils/inlineDiff.ts +++ b/packages/diff-viewer/src/lib/utils/inlineDiff.ts @@ -90,7 +90,7 @@ function similarity(a: string, b: string): number { return (2 * lcsLen) / (a.length + b.length); } -const SIMILARITY_THRESHOLD = 0.4; +const SIMILARITY_THRESHOLD = 0.5; function splitWords(text: string): string[] { return text.split(/(\s+)/); @@ -183,7 +183,7 @@ export function computeLineDiff( while (bi < unmatchedBefore.length && ai < unmatchedAfter.length) { const bIdx = unmatchedBefore[bi]; const aIdx = unmatchedAfter[ai]; - const sim = similarity(beforeLines[bIdx], afterLines[aIdx]); + const sim = similarity(beforeLines[bIdx].trim(), afterLines[aIdx].trim()); if (sim > SIMILARITY_THRESHOLD) { beforeClasses[bIdx] = 'modified'; @@ -207,7 +207,7 @@ export function computeLineDiff( let found = false; for (let scan = ai + 1; scan < unmatchedAfter.length; scan++) { const scanIdx = unmatchedAfter[scan]; - const scanSim = similarity(beforeLines[bIdx], afterLines[scanIdx]); + const scanSim = similarity(beforeLines[bIdx].trim(), afterLines[scanIdx].trim()); if (scanSim > SIMILARITY_THRESHOLD) { // Mark all skipped after-lines as pure additions. for (let skip = ai; skip < scan; skip++) { From adc8fbc06de717bc2e3f671a95b84f8540977fe2 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 30 Mar 2026 13:27:47 +1100 Subject: [PATCH 11/14] fix(diff-viewer): cap scan-ahead to 10 lines and scope cache to component MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../src/lib/components/DiffViewer.svelte | 13 ++++- .../src/lib/utils/diffViewerHelpers.ts | 13 +++-- packages/diff-viewer/src/lib/utils/index.ts | 4 +- .../diff-viewer/src/lib/utils/inlineDiff.ts | 55 +++++++++++++------ 4 files changed, 58 insertions(+), 27 deletions(-) diff --git a/packages/diff-viewer/src/lib/components/DiffViewer.svelte b/packages/diff-viewer/src/lib/components/DiffViewer.svelte index 6ac8e9f0..94632aa4 100644 --- a/packages/diff-viewer/src/lib/components/DiffViewer.svelte +++ b/packages/diff-viewer/src/lib/components/DiffViewer.svelte @@ -65,6 +65,7 @@ normalizeLineSelection, resolveLineSelectionToolbarLeft, } from '../utils/diffViewerHelpers'; + import { createLineDiffCache } from '../utils/inlineDiff.js'; import type { BeforeLineClass, AfterLineClass, CharHighlight } from '../utils/inlineDiff.js'; import { setupDiffKeyboardNav } from '../utils/diffKeyboard'; import { pathsMatch } from '../utils/diffModalHelpers'; @@ -128,6 +129,12 @@ onDeleteComment, }: Props = $props(); + // ========================================================================== + // Inline diff cache (scoped to component instance) + // ========================================================================== + + const lineDiffCache = createLineDiffCache(); + // ========================================================================== // Element refs // ========================================================================== @@ -922,7 +929,8 @@ afterLineToAlignment, changedAlignments, beforeLines, - afterLines + afterLines, + lineDiffCache ); } @@ -934,7 +942,8 @@ afterLineToAlignment, changedAlignments, beforeLines, - afterLines + afterLines, + lineDiffCache ); } diff --git a/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts b/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts index 33e1d9e9..2b4451b3 100644 --- a/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts +++ b/packages/diff-viewer/src/lib/utils/diffViewerHelpers.ts @@ -1,7 +1,6 @@ import type { Alignment, Comment, SmartDiffAnnotation } from '../types'; import type { Token } from './highlighter'; -import { getLineDiffResult } from './inlineDiff.js'; -import type { BeforeLineClass, AfterLineClass, CharHighlight } from './inlineDiff.js'; +import type { LineDiffCache, BeforeLineClass, AfterLineClass, CharHighlight } from './inlineDiff.js'; export type ChangedAlignmentEntry = { alignment: Alignment; index: number }; export type PaneSide = 'before' | 'after'; @@ -298,7 +297,8 @@ export function getLineClass( afterLineToAlignment: Map, changedAlignments: ChangedAlignmentEntry[], beforeLines: string[], - afterLines: string[] + afterLines: string[], + cache: LineDiffCache ): BeforeLineClass | AfterLineClass | null { const map = side === 'before' ? beforeLineToAlignment : afterLineToAlignment; const alignIdx = map.get(lineIndex); @@ -307,7 +307,7 @@ export function getLineClass( const alignment = changedAlignments[alignIdx].alignment; const alignBefore = beforeLines.slice(alignment.before.start, alignment.before.end); const alignAfter = afterLines.slice(alignment.after.start, alignment.after.end); - const result = getLineDiffResult(alignBefore, alignAfter); + const result = cache.get(alignBefore, alignAfter); if (side === 'before') { const localIdx = lineIndex - alignment.before.start; @@ -329,7 +329,8 @@ export function getCharHighlights( afterLineToAlignment: Map, changedAlignments: ChangedAlignmentEntry[], beforeLines: string[], - afterLines: string[] + afterLines: string[], + cache: LineDiffCache ): CharHighlight[] | null { const map = side === 'before' ? beforeLineToAlignment : afterLineToAlignment; const alignIdx = map.get(lineIndex); @@ -338,7 +339,7 @@ export function getCharHighlights( const alignment = changedAlignments[alignIdx].alignment; const alignBefore = beforeLines.slice(alignment.before.start, alignment.before.end); const alignAfter = afterLines.slice(alignment.after.start, alignment.after.end); - const result = getLineDiffResult(alignBefore, alignAfter); + const result = cache.get(alignBefore, alignAfter); const localIdx = side === 'before' ? lineIndex - alignment.before.start diff --git a/packages/diff-viewer/src/lib/utils/index.ts b/packages/diff-viewer/src/lib/utils/index.ts index a622edd4..aea43c52 100644 --- a/packages/diff-viewer/src/lib/utils/index.ts +++ b/packages/diff-viewer/src/lib/utils/index.ts @@ -87,5 +87,5 @@ export { type FileSelectionWithSearchConfig } from './fileSelection'; -export { computeLineDiff, getLineDiffResult } from './inlineDiff.js'; -export type { BeforeLineClass, AfterLineClass, CharHighlight, ModifiedPair, LineDiffResult } from './inlineDiff.js'; +export { computeLineDiff, getLineDiffResult, createLineDiffCache } from './inlineDiff.js'; +export type { BeforeLineClass, AfterLineClass, CharHighlight, ModifiedPair, LineDiffResult, LineDiffCache } from './inlineDiff.js'; diff --git a/packages/diff-viewer/src/lib/utils/inlineDiff.ts b/packages/diff-viewer/src/lib/utils/inlineDiff.ts index 4da43dfa..17f00c4d 100644 --- a/packages/diff-viewer/src/lib/utils/inlineDiff.ts +++ b/packages/diff-viewer/src/lib/utils/inlineDiff.ts @@ -146,6 +146,8 @@ function mergeHighlights(highlights: CharHighlight[]): CharHighlight[] { return merged; } +const MAX_SCAN_AHEAD = 10; + export function computeLineDiff( beforeLines: string[], afterLines: string[], @@ -205,7 +207,8 @@ export function computeLineDiff( // before-line. This handles cases where many insertions precede a // modification (e.g. code re-indented after being wrapped in a block). let found = false; - for (let scan = ai + 1; scan < unmatchedAfter.length; scan++) { + const scanLimit = Math.min(ai + 1 + MAX_SCAN_AHEAD, unmatchedAfter.length); + for (let scan = ai + 1; scan < scanLimit; scan++) { const scanIdx = unmatchedAfter[scan]; const scanSim = similarity(beforeLines[bIdx].trim(), afterLines[scanIdx].trim()); if (scanSim > SIMILARITY_THRESHOLD) { @@ -242,29 +245,47 @@ export function computeLineDiff( } const MAX_CACHE_SIZE = 100; -const cache = new Map(); function makeCacheKey(beforeLines: string[], afterLines: string[]): string { return beforeLines.join('\n') + '\0' + afterLines.join('\n'); } +export interface LineDiffCache { + get(beforeLines: string[], afterLines: string[]): LineDiffResult; +} + +export function createLineDiffCache(): LineDiffCache { + const cache = new Map(); + + return { + get(beforeLines: string[], afterLines: string[]): LineDiffResult { + const key = makeCacheKey(beforeLines, afterLines); + const cached = cache.get(key); + if (cached) return cached; + + const result = computeLineDiff(beforeLines, afterLines); + + if (cache.size >= MAX_CACHE_SIZE) { + const firstKey = cache.keys().next().value; + if (firstKey !== undefined) { + cache.delete(firstKey); + } + } + cache.set(key, result); + + return result; + }, + }; +} + +/** + * @deprecated Use createLineDiffCache() for component-scoped caching. + * Kept for backward compatibility with tests. + */ +const defaultCache = createLineDiffCache(); export function getLineDiffResult( beforeLines: string[], afterLines: string[], ): LineDiffResult { - const key = makeCacheKey(beforeLines, afterLines); - const cached = cache.get(key); - if (cached) return cached; - - const result = computeLineDiff(beforeLines, afterLines); - - if (cache.size >= MAX_CACHE_SIZE) { - const firstKey = cache.keys().next().value; - if (firstKey !== undefined) { - cache.delete(firstKey); - } - } - cache.set(key, result); - - return result; + return defaultCache.get(beforeLines, afterLines); } From 14699a9591b750d7cd4a3583904222a6e2791f42 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 30 Mar 2026 13:38:19 +1100 Subject: [PATCH 12/14] fix(diff-viewer): raise similarity threshold to prevent false modified pairs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../src/lib/utils/inlineDiff.test.ts | 31 +++++++++++++++++++ .../diff-viewer/src/lib/utils/inlineDiff.ts | 2 +- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts b/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts index a57bd835..2fee4f33 100644 --- a/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts +++ b/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts @@ -381,6 +381,37 @@ describe('computeLineDiff', () => { expect(result.modifiedPairs).toHaveLength(0); }); + it('classifies replaced JSDoc descriptions as removed/added, not modified', () => { + // Real-world case: a JSDoc comment is completely rewritten. The + // description lines share structural tokens (` * `, `segments`, + // `highlights`) which inflate similarity despite being semantically + // unrelated. They should be removed/added, not modified. + const before = [ + '/**', + ' * Get highlighted token segments for a line, with search matches applied.', + ' */', + ]; + const after = [ + '/**', + ' * Apply character-level diff highlights to segments by splitting them at highlight boundaries.', + ' * Works similarly to applySearchHighlights — walks through segments tracking column position.', + ' */', + ]; + const result = computeLineDiff(before, after); + + // /** and */ are context + expect(result.beforeLines[0]).toBe('unchanged'); + expect(result.beforeLines[2]).toBe('unchanged'); + expect(result.afterLines[0]).toBe('unchanged'); + expect(result.afterLines[3]).toBe('unchanged'); + + // The description lines are too different to be "modified" + expect(result.beforeLines[1]).toBe('removed'); + expect(result.afterLines[1]).toBe('added'); + expect(result.afterLines[2]).toBe('added'); + expect(result.modifiedPairs).toHaveLength(0); + }); + it('handles interleaved unchanged and changed lines', () => { const before = ['A', 'B', 'C', 'D', 'E']; const after = ['A', 'B2', 'C', 'D2', 'E']; diff --git a/packages/diff-viewer/src/lib/utils/inlineDiff.ts b/packages/diff-viewer/src/lib/utils/inlineDiff.ts index 17f00c4d..0d8122a3 100644 --- a/packages/diff-viewer/src/lib/utils/inlineDiff.ts +++ b/packages/diff-viewer/src/lib/utils/inlineDiff.ts @@ -90,7 +90,7 @@ function similarity(a: string, b: string): number { return (2 * lcsLen) / (a.length + b.length); } -const SIMILARITY_THRESHOLD = 0.5; +const SIMILARITY_THRESHOLD = 0.55; function splitWords(text: string): string[] { return text.split(/(\s+)/); From f3ae07d067c57b6a3e1ce317244979a038eba96a Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 30 Mar 2026 13:48:52 +1100 Subject: [PATCH 13/14] fix(diff-viewer): pass cache to helper tests and remove deprecated getLineDiffResult MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../diffViewerHelpers.inlineDiff.test.ts | 27 ++++++++++--------- packages/diff-viewer/src/lib/utils/index.ts | 2 +- .../src/lib/utils/inlineDiff.test.ts | 14 +++++----- .../diff-viewer/src/lib/utils/inlineDiff.ts | 11 -------- 4 files changed, 24 insertions(+), 30 deletions(-) diff --git a/packages/diff-viewer/src/lib/utils/diffViewerHelpers.inlineDiff.test.ts b/packages/diff-viewer/src/lib/utils/diffViewerHelpers.inlineDiff.test.ts index 697f1507..2ffd0298 100644 --- a/packages/diff-viewer/src/lib/utils/diffViewerHelpers.inlineDiff.test.ts +++ b/packages/diff-viewer/src/lib/utils/diffViewerHelpers.inlineDiff.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { getLineClass, getCharHighlights } from './diffViewerHelpers'; import type { ChangedAlignmentEntry } from './diffViewerHelpers'; import type { Alignment } from '../types'; +import { createLineDiffCache } from './inlineDiff'; /** * Helper to build alignment lookup maps and the changedAlignments array @@ -39,7 +40,7 @@ describe('getLineClass', () => { const result = getLineClass( 'before', 1, beforeLineToAlignment, afterLineToAlignment, - changedAlignments, ['a', 'b', 'c'], ['a', 'b', 'c'], + changedAlignments, ['a', 'b', 'c'], ['a', 'b', 'c'], createLineDiffCache(), ); expect(result).toBeNull(); }); @@ -55,7 +56,7 @@ describe('getLineClass', () => { const result = getLineClass( 'before', 0, beforeLineToAlignment, afterLineToAlignment, - changedAlignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, createLineDiffCache(), ); expect(result).toBe('modified'); }); @@ -70,7 +71,7 @@ describe('getLineClass', () => { const result = getLineClass( 'after', 0, beforeLineToAlignment, afterLineToAlignment, - changedAlignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, createLineDiffCache(), ); expect(result).toBe('modified'); }); @@ -86,7 +87,7 @@ describe('getLineClass', () => { const result = getLineClass( 'before', 0, beforeLineToAlignment, afterLineToAlignment, - changedAlignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, createLineDiffCache(), ); expect(result).toBe('removed'); }); @@ -102,7 +103,7 @@ describe('getLineClass', () => { const result = getLineClass( 'after', 1, beforeLineToAlignment, afterLineToAlignment, - changedAlignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, createLineDiffCache(), ); expect(result).toBe('added'); }); @@ -118,16 +119,18 @@ describe('getLineClass', () => { ]; const { beforeLineToAlignment, afterLineToAlignment, changedAlignments } = buildLookups(alignments); + const cache = createLineDiffCache(); + // Line 2 before: "const a = 1;" should be modified (similar to "const a = 2;") expect(getLineClass( 'before', 2, beforeLineToAlignment, afterLineToAlignment, - changedAlignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, cache, )).toBe('modified'); // Line 3 in after: "newLine();" should be added (no similar before-line) expect(getLineClass( 'after', 3, beforeLineToAlignment, afterLineToAlignment, - changedAlignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, cache, )).toBe('added'); }); }); @@ -141,7 +144,7 @@ describe('getCharHighlights', () => { const result = getCharHighlights( 'before', 0, beforeLineToAlignment, afterLineToAlignment, - changedAlignments, ['hello'], ['hello'], + changedAlignments, ['hello'], ['hello'], createLineDiffCache(), ); expect(result).toBeNull(); }); @@ -157,7 +160,7 @@ describe('getCharHighlights', () => { // Lines are too dissimilar to be "modified", so no char highlights const result = getCharHighlights( 'before', 0, beforeLineToAlignment, afterLineToAlignment, - changedAlignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, createLineDiffCache(), ); expect(result).toBeNull(); }); @@ -172,7 +175,7 @@ describe('getCharHighlights', () => { const result = getCharHighlights( 'before', 0, beforeLineToAlignment, afterLineToAlignment, - changedAlignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, createLineDiffCache(), ); expect(result).not.toBeNull(); expect(result!.length).toBeGreaterThan(0); @@ -194,7 +197,7 @@ describe('getCharHighlights', () => { const result = getCharHighlights( 'after', 0, beforeLineToAlignment, afterLineToAlignment, - changedAlignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, createLineDiffCache(), ); expect(result).not.toBeNull(); // "slow" replaces "quick" -> highlight at position 4-8 @@ -212,7 +215,7 @@ describe('getCharHighlights', () => { const result = getCharHighlights( 'after', 1, beforeLineToAlignment, afterLineToAlignment, - changedAlignments, beforeLines, afterLines, + changedAlignments, beforeLines, afterLines, createLineDiffCache(), ); expect(result).not.toBeNull(); expect(result!.length).toBeGreaterThan(0); diff --git a/packages/diff-viewer/src/lib/utils/index.ts b/packages/diff-viewer/src/lib/utils/index.ts index aea43c52..87379b2a 100644 --- a/packages/diff-viewer/src/lib/utils/index.ts +++ b/packages/diff-viewer/src/lib/utils/index.ts @@ -87,5 +87,5 @@ export { type FileSelectionWithSearchConfig } from './fileSelection'; -export { computeLineDiff, getLineDiffResult, createLineDiffCache } from './inlineDiff.js'; +export { computeLineDiff, createLineDiffCache } from './inlineDiff.js'; export type { BeforeLineClass, AfterLineClass, CharHighlight, ModifiedPair, LineDiffResult, LineDiffCache } from './inlineDiff.js'; diff --git a/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts b/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts index 2fee4f33..8c5b57fd 100644 --- a/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts +++ b/packages/diff-viewer/src/lib/utils/inlineDiff.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest'; import { computeLineDiff, - getLineDiffResult, + createLineDiffCache, type LineDiffResult, type CharHighlight, } from './inlineDiff'; @@ -428,20 +428,22 @@ describe('computeLineDiff', () => { }); }); -describe('getLineDiffResult (caching)', () => { +describe('createLineDiffCache', () => { it('returns same result object for identical inputs', () => { + const cache = createLineDiffCache(); const before = ['const x = 1;']; const after = ['const x = 2;']; - const result1 = getLineDiffResult(before, after); - const result2 = getLineDiffResult(before, after); + const result1 = cache.get(before, after); + const result2 = cache.get(before, after); expect(result1).toBe(result2); // same reference }); it('returns different result objects for different inputs', () => { - const result1 = getLineDiffResult(['a'], ['b']); - const result2 = getLineDiffResult(['c'], ['d']); + const cache = createLineDiffCache(); + const result1 = cache.get(['a'], ['b']); + const result2 = cache.get(['c'], ['d']); expect(result1).not.toBe(result2); }); diff --git a/packages/diff-viewer/src/lib/utils/inlineDiff.ts b/packages/diff-viewer/src/lib/utils/inlineDiff.ts index 0d8122a3..8d76b6ee 100644 --- a/packages/diff-viewer/src/lib/utils/inlineDiff.ts +++ b/packages/diff-viewer/src/lib/utils/inlineDiff.ts @@ -278,14 +278,3 @@ export function createLineDiffCache(): LineDiffCache { }; } -/** - * @deprecated Use createLineDiffCache() for component-scoped caching. - * Kept for backward compatibility with tests. - */ -const defaultCache = createLineDiffCache(); -export function getLineDiffResult( - beforeLines: string[], - afterLines: string[], -): LineDiffResult { - return defaultCache.get(beforeLines, afterLines); -} From 4c8540a3c63eea9e0766ce8055b4de03037c9457 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 30 Mar 2026 13:58:44 +1100 Subject: [PATCH 14/14] fix(branches): prevent concurrent session starts by queuing during pending 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) --- .../lib/features/branches/BranchCardSessionManager.svelte.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts index 64aecdde..55ad1a55 100644 --- a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts +++ b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts @@ -69,7 +69,8 @@ export default class BranchCardSessionManager { /** True when a new session will be queued rather than started immediately. */ willQueue = $derived( !this.getTimeline() || // provisioning — no timeline yet - this.hasRunningSession // another session is active + this.hasRunningSession || // another session is active + this.isSessionStartPending // a session start is already in flight ); /** True when new session actions (new commit, note, review) should be disabled. */