agent-ecosystem/docs/research/diff-view-audit.md

Diff View Feature — Full Audit Report

Date: 2026-02-26 Verified: 2026-02-26 (4 parallel agents cross-checked every bug against actual source code)

Comprehensive audit of the changes/diff viewing feature covering line count reliability, hunk parsing, stat-to-hunk consistency, and UI rendering.

---

Architecture Overview

The app uses two distinct diff strategies:

1. Chat Viewer Diffs (read-only, simple display):

   • DiffViewer.tsx — custom LCS-based line-by-line diff
   • Pure string comparison, NO hunk structure

2. Team Review Diffs (interactive, hunk-aware):

   • CodeMirror Merge plugin (@codemirror/merge)
   • ReviewDiffContent.tsx — uses diffLines from diff package
   • CodeMirrorDiffView.tsx — full merge view with hunk navigation
   • ReviewApplierService.ts — applies/rejects hunks to disk

Three independent stat computation paths:

Service	Algorithm	Used For
ChangeExtractorService	diffLines() from npm diff	Team member changes (file badges)
MemberStatsComputer	split('\n').length arithmetic	Session analytics
FileContentResolver	diffLines() from npm diff	Full file content diffs (CodeMirror)

---

Evaluation Summary

#	Bug	Real?	Confidence	Status
1	Two conflicting line-counting methods	YES	9/10	Open
2	Write never counts removals	YES	10/10	Open
3	Trailing newline off-by-one	PARTIAL	6/10	Resolves with #1
4	FileContentResolver overwrites stats	PARTIAL	5/10	Design issue
5	computeHunkIndexAtPos → 0 fallback	YES	10/10	Open
6	Hunk ≠ snippet mapping	YES	9/10	Open
7	indexOf duplicates in rejection	YES	10/10	Open
8	Skeleton flash after save	FIXED	9/10	Done
9	CRLF → false diffs	YES	9/10	Open
10	OOM on large files (LCS)	YES	8/10	Open
11	Race condition disk vs cache	YES	8/10	Open
12	Empty string inconsistency	YES	7/10	Resolves with #1
13	Bash estimation ~30-40%	PARTIAL	6/10	Design limitation
14	Echo escape handling wrong	YES	8/10	Open
15	portionCollapse edge case	PARTIAL	5/10	Needs testing
16	No-newline-at-EOF hidden	YES	8/10	Open
17	Three-way merge labels	NO	9/10	False positive
18	Zero-change files invisible	YES	8/10	Open
19	Viewed threshold mismatch	YES	9/10	Open
20	useEffect no deps array	YES	10/10	Open
21	Hunk count ≠ snippet count	YES	8/10	Open
22	Toolbar off-screen narrow viewport	YES	7/10	Open
23	Deleted files not marked	YES	8/10	Open
24	write-update reconstruction null	YES	9/10	Open
25	Bash relative paths	YES	9/10	Open
26	Empty line → space	YES	7/10	Open
27	No keyboard nav in tree	YES	8/10	Feature
28	Collapse state not persisted	YES	9/10	Open
29	No stats summary	NO	3/10	Feature request
30	Binary files not detected	YES	8/10	Open
31	No max file size	YES	7/10	Open
32	Whitespace changes not distinguished	PARTIAL	6/10	Optional feature

Totals: 24 real bugs, 3 false positives (#4, #17, #29), 5 partial/design (#3, #13, #15, #32, #4), 1 fixed (#8)

---

CRITICAL BUGS

1. Two Conflicting Line-Counting Methods

Real bug: YES — Confidence: 9/10

Impact: Line count badges in file tree may not match actual hunks in editor.

Details:

• ChangeExtractorService (src/main/services/team/ChangeExtractorService.ts:463-473) uses diffLines() — semantic line diffing
• MemberStatsComputer (src/main/services/team/MemberStatsComputer.ts:193-196) uses naive split('\n').length — newLines - oldLines

Example divergence:

File: 10 lines rewritten completely (same line count)
diffLines(): added=10, removed=10  (correct — all lines changed)
split arithmetic: added=0, removed=0  (wrong — same line count)

Best fix: Create unified line-counting utility using diffLines() as source of truth. Replace MemberStatsComputer's arithmetic with shared utility. Risk: Line count numbers will change post-fix; must test edge cases.

2. Write Operations Never Count Removals

Real bug: YES — Confidence: 10/10

Location: MemberStatsComputer.ts:204-214

if (toolName === 'Write') {
  const writeContent = typeof input.content === 'string' ? input.content : '';
  if (writeContent) {
    const fileAdded = writeContent.split('\n').length;
    linesAdded += fileAdded;
    addFileLines(input.file_path, fileAdded, 0);  // Always 0 removals!
  }
}

Impact: Write replacing 100-line file with 50-line file shows +50 / -0 instead of accurate counts.

Best fix: Use FileContentResolver to access original state before Write, calculate true delta. Risk: Requires coordination with FileContentResolver; may introduce coupling.

3. Trailing Newline Off-by-One

Real bug: PARTIAL — Confidence: 6/10

Resolves automatically when #1 is fixed (migration to diffLines()). Within MemberStatsComputer's own logic, the delta arithmetic is roughly self-consistent (both old and new over-count by 1, so the difference is correct). The issue is only visible when comparing MemberStatsComputer output against ChangeExtractorService output.

4. FileContentResolver Overwrites Stats

Real bug: PARTIAL (design issue) — Confidence: 5/10

FileContentResolver.getFileContent() recalculates stats from full content using diffLines(), overwriting input stats. This is actually MORE ACCURATE than snippet-based counts. The "overwrite" is intentional improvement, not a bug. The inconsistency is that file tree badges (pre-CM load) use snippet counts, while CodeMirror view uses recalculated counts.

Verdict: Not a code bug. Design choice with minor visual inconsistency during loading.

5. computeHunkIndexAtPos Returns 0 as Fallback

Real bug: YES — Confidence: 10/10

Location: CodeMirrorDiffView.tsx:129-143

function computeHunkIndexAtPos(state: EditorState, pos: number): number {
  const chunks = getChunks(state);
  if (!chunks) return 0;
  let index = 0;
  for (const chunk of chunks.chunks) {
    if (pos >= chunk.fromB && pos <= chunk.toB) {
      return index;
    }
    index++;
  }
  return 0;  // ← Always returns first hunk if no match!
}

Impact: Clicking Accept/Reject when cursor is between hunks applies action to the FIRST hunk, not the nearest one. Confirmed: callers at lines 416-420 and 432-435 trust this value unconditionally.

Best fix: Find nearest chunk by minimum distance: Math.min(|pos - chunk.fromB|, |pos - chunk.toB|) for each chunk, return index of nearest. Alternative: Return -1 for "no match" and require caller handling. Risk: Need tie-breaking rule when cursor is equidistant from two chunks.

6. Hunk Index ≠ Snippet Index (False 1:1 Assumption)

Real bug: YES — Confidence: 9/10

Location: ReviewApplierService.ts:337-342

const snippetsToReject = hunkIndices
  .filter((idx) => idx >= 0 && idx < validSnippets.length)
  .map((idx) => validSnippets[idx]);

Problem: Assumes hunk #N corresponds to snippet #N. But:

• Multiple Edit calls can merge into one hunk in structuredPatch
• One Write call can produce multiple hunks
• MultiEdit creates 1 snippet with multiple logical changes

Best fix: Build hunk-to-snippet mapping using position matching. For each hunk, find snippets whose newString appears in that hunk region. Store hunkIndex -> Set<snippetIndices>. Risk: Complex implementation, requires re-running diff analysis.

7. Snippet Rejection via indexOf — Duplicate Content Bug

Real bug: YES — Confidence: 10/10

Location: ReviewApplierService.ts:353

const pos = content.indexOf(snippet.newString);

indexOf() finds FIRST occurrence only. If identical code patterns exist elsewhere in the file, rejection corrupts the wrong section.

Best fix: Position-aware matching: calculate approximate line/column of original edit, search for newString near that position (±5 lines tolerance), require context match. Risk: More complex logic, false negatives if context too strict.

8. Skeleton Flash After File Save — FIXED

Location: changeReviewSlice.ts:649-666

After saving, fileContents[filePath] was deleted from cache, causing hasContent = false → skeleton placeholder shown until lazy re-fetch completes.

Fix applied: Instead of deleting, update modifiedFullContent with saved content in-place. contentSource set to 'disk-current'.

---

HIGH PRIORITY BUGS

9. CRLF Line Endings → False Diffs

Real bug: YES — Confidence: 9/10

Location: DiffViewer.tsx:297

const oldLines = oldString.split('\n');

Windows files with \r\n leave trailing \r on each line. Every line shows as "changed" even if content is identical.

Fix: Use split(/\r?\n/) or normalize before diffing. Risk: Very low. Standard regex, no side effects.

10. OOM on Large Files (DiffViewer LCS)

Real bug: YES — Confidence: 8/10

Location: DiffViewer.tsx:50-68

LCS algorithm is O(m×n) space. Two 5000-line files = 25M matrix entries ≈ 100MB RAM. No safeguards, no fallback for large files.

Best fix: Add size check: if m * n > MAX_CELLS (e.g., 1M), fallback to diffLines() from npm diff package. Risk: Fallback produces different visual output (semantic vs LCS). Need to test.

11. Race Condition: File Disk State vs Cache

Real bug: YES — Confidence: 8/10

Stats and hunks come from SEPARATE sources:

• Stats: JSONL tool_use blocks (snapshot at parse time)
• Hunks: current file on disk (read at view time)

3-minute cache TTL on both ChangeExtractorService and FileContentResolver. If file changes on disk between fetches, stats and hunks desynchronize.

No cache invalidation on FileWatcher events → caches stay stale until TTL expires.

Best fix: Hook FileWatcher to evict caches when files change. Or reduce TTL to 30s. Risk: Must ensure invalidation doesn't create new race conditions.

12. Empty String Handling Inconsistency

Real bug: YES — Confidence: 7/10

Location: MemberStatsComputer.ts:193

Empty string '' is falsy → returns 0. But ''.split('\n').length === 1.

Resolves with #1 — migrating to diffLines() handles this correctly.

13. Bash Line Estimation Covers ~30-40% of Patterns

Real bug: PARTIAL — Confidence: 6/10

Location: MemberStatsComputer.ts:314-416

Verdict: Fundamental limitation, not a code bug. The JSONL only stores command strings, not execution output. Without running the shell, accurate counting is impossible. Code comments acknowledge this. Best approach: document limitation in UI with tooltip.

14. Echo Escape Sequence Handling Wrong

Real bug: YES — Confidence: 8/10

Location: MemberStatsComputer.ts:371

added += content.split('\\n').length;

Splits on literal \\n in quoted string. But echo "line1\nline2" does NOT expand \n without -e flag. Counter is wrong for standard echo.

Best fix: Check for -e flag before splitting on \\n. Without -e, treat as single line. Risk: Hacky logic, any change may break other cases. Conservative: don't count echo lines at all.

---

MEDIUM PRIORITY BUGS

15. portionCollapse Line Position Edge Case

Real bug: PARTIAL — Confidence: 5/10

Location: portionCollapse.ts:140

Rare edge case at exact line boundaries. Needs unit tests with edge cases to confirm. Verdict: Low probability, needs testing before fixing.

16. No-Newline-At-End-Of-File Not Shown

Real bug: YES — Confidence: 8/10

Location: ReviewDiffContent.tsx:46-48

const lines = part.value.replace(/\n$/, '').split('\n');

Strips trailing newline. If original has no final newline but modified adds one, diff shows them as identical.

Best fix: Add visual indicator for no-newline-at-EOF. Risk: Need to update rendering without breaking existing layout.

17. Three-Way Merge Labels Confusing — FALSE POSITIVE

Confidence: 9/10 that this is NOT a bug

Labels correctly follow diff3 semantics: <<<<<<< current = disk state, >>>>>>> original = pre-change state. This is standard and correct. The audit was wrong.

18. Zero-Change Files Invisible

Real bug: YES — Confidence: 8/10

Location: ChangeStatsBadge.tsx

if (linesAdded === 0 && linesRemoved === 0) return null;

Files modified with equal adds/removes (e.g., 5 lines rewritten) show no badge. Missing modified boolean flag.

Best fix: Add modified: boolean flag to FileChangeSummary. Show neutral badge for zero-net-change files. Risk: Requires data structure change, but straightforward.

19. Viewed File Threshold Mismatch

Real bug: YES — Confidence: 9/10

• FileSectionDiff.tsx: threshold: 0.85
• CodeMirrorDiffView.tsx: threshold: 1.0

Best fix: Standardize on 0.85 everywhere. One-line change. Risk: None.

20. Missing Dependency Array in useEffect

Real bug: YES — Confidence: 10/10

Location: FileSectionDiff.tsx:50-56

useEffect(() => {
  if (localEditorViewRef.current) {
    onEditorViewReady(file.filePath, localEditorViewRef.current);
  }
});  // ← No dependency array! Runs EVERY render.

Best fix: Add [file.filePath, onEditorViewReady] dependency array. Risk: Low. Need to ensure onEditorViewReady is memoized with useCallback.

21. Hunk Counts Mismatch in UI

Real bug: YES — Confidence: 8/10

snippets.length used as fallback before CodeMirror loads, then replaced by chunks.length. User sees progress jump (e.g., "1 of 1" → "1 of 5").

Best fix: Pre-compute chunk count from diffLines() at load time, not from snippet count. Risk: Medium — adds computation step, but improves correctness.

22. Merge Toolbar Off-Screen in Narrow Viewport

Real bug: YES — Confidence: 7/10

Buttons at insetInlineEnd: '8px' get clipped in narrow viewports.

Best fix: CSS-only: use insetInlineStart or add maxWidth: '100%' overflow handling. Risk: None. CSS-only change.

23. Deleted Files Not Marked

Real bug: YES — Confidence: 8/10

No isDeleted flag in FileChangeSummary. Deleted files show as regular changes.

Best fix: Add isDeleted: boolean field. Set when original has content and modified is empty. Risk: Medium — requires data structure change.

24. Snippet Reconstruction Returns null for Write-Update

Real bug: YES — Confidence: 9/10

Location: FileContentResolver.ts:392-394

write-update can't be reconstructed because JSONL doesn't include oldString. Falls back to disk-current which may differ from actual original.

Best fix: Store original content when detecting write-update during extraction. Risk: Medium — requires data structure enrichment.

25. Bash Relative Paths Not Captured

Real bug: YES — Confidence: 9/10

Location: MemberStatsComputer.ts:373-376

Only captures absolute paths (startsWith('/')). Misses relative paths.

Best fix: Remove startsWith('/') check, just validate non-empty string. Risk: Very low. 1-line fix.

---

LOW PRIORITY

26. DiffViewer Empty Line Rendering

Real bug: YES — Confidence: 7/10

{line.content || ' '}

Empty lines show as single space. Can't distinguish from space-only lines.

Best fix: {line.content ?? ' '} (only use space if truly undefined). Risk: None. 1-line fix.

27. No Keyboard Navigation in File Tree

Real bug: YES — Confidence: 8/10

Mouse-only navigation. No WAI-ARIA tree widget support.

Verdict: UX feature, ~150 LOC. Not a correctness bug.

28. Folder Collapse State Not Persisted

Real bug: YES — Confidence: 9/10

State lost on dialog close. Uses useState with fresh Set().

Best fix: Persist to localStorage or Zustand store. Risk: Low-medium. localStorage has size limits but file tree data is tiny.

29. No Diff Stats Summary at Top — FALSE POSITIVE

Confidence: 3/10 that this is a bug

Missing feature, not a bug. File-by-file stats are visible. Aggregate summary would be nice UX but isn't a correctness issue.

30. Binary Files Not Detected

Real bug: YES — Confidence: 8/10

diffLines() called on binary content without check. Can cause corrupted display.

Best fix: Check for null bytes (content.includes('\0')) before diffing. Skip binary files. Risk: Very low. Simple guard.

31. No Maximum File Size Handling

Real bug: YES — Confidence: 7/10

No progress indicator for large files. Browser freezes for 2-3s on 1MB+ files.

Best fix: Add size check, show "file too large" fallback for >5MB. Risk: Low. Graceful degradation.

32. Whitespace-Only Changes Not Distinguished

Real bug: PARTIAL — Confidence: 6/10

No visual distinction between content vs whitespace-only changes.

Verdict: Optional enhancement. Add ignoreWhitespace toggle. Not a correctness bug.

---

Race Condition Severity Matrix

Scenario	Probability	Severity
Disk modification between stat cache and hunk fetch	Medium	High
File deletion after stats cached	Low-Medium	High
JSONL appended while parsing	Low	Medium
Git repo state change (checkout, rebase)	Low	Medium
Snippet reconstruction chain broken	Medium	High

---

Fix Categories

Safe to Fix Now (isolated, low-risk, clear approach)

• #5: computeHunkIndexAtPos → nearest hunk (local function change)
• #9: CRLF normalization split(/\r?\n/) (1-line regex change)
• #19: Threshold 0.85 vs 1.0 → standardize (1-line constant)
• #20: useEffect dependency array (1-line addition)
• #25: Bash relative paths (remove startsWith('/') check)
• #26: Empty line rendering (|| → ??)

Need More Research Before Fixing (multi-file, complex, risky)

• #1: Unify line counting (affects 3 services, all consumers)
• #2: Write removals (needs original file content during stats)
• #6: Hunk↔snippet mapping (algorithm redesign)
• #7: indexOf → position-aware matching (could break existing logic)
• #10: OOM safeguard (need to test fallback visual consistency)
• #11: Cache invalidation (FileWatcher integration complexity)
• #23: Deleted files flag (data structure change, migration)
• #24: write-update reconstruction (data enrichment needed)

Feature Requests / Won't Fix

• #13: Bash estimation — fundamental limitation
• #17: Three-way labels — NOT a bug
• #27: Keyboard nav — UX feature
• #29: Stats summary — UX feature
• #32: Whitespace toggle — optional enhancement

---

Key Files Reference

File	Role
src/main/services/team/ChangeExtractorService.ts	Parses JSONL, aggregates per-file changes
src/main/services/team/MemberStatsComputer.ts	Session analytics, line counting
src/main/services/team/FileContentResolver.ts	Full file content resolution
src/main/services/team/ReviewApplierService.ts	Apply/reject hunks, save files
src/renderer/components/chat/viewers/DiffViewer.tsx	Simple LCS diff viewer
src/renderer/components/team/review/CodeMirrorDiffView.tsx	Advanced CodeMirror merge view
src/renderer/components/team/review/ReviewDiffContent.tsx	Fallback snippet-based diff
src/renderer/components/team/review/FileSectionDiff.tsx	Diff section wrapper
src/renderer/components/team/review/FileSectionHeader.tsx	File header with Save button
src/renderer/components/team/review/ContinuousScrollView.tsx	Scroll container, skeleton logic
src/renderer/components/team/review/portionCollapse.ts	Smart collapse of unchanged regions
src/renderer/components/team/review/ReviewFileTree.tsx	File tree with stats badges
src/renderer/store/slices/changeReviewSlice.ts	Store: file contents, save, decisions
src/renderer/hooks/useDiffNavigation.ts	Keyboard navigation between hunks
src/shared/types/review.ts	Shared types: FileChangeSummary, FileChangeWithContent