agent-ecosystem/docs/research/diff-view-fix-plans.md
iliya d177d22dba feat: enhance diff view fix plans with deep research upgrades
- Updated the diff view fix plans to reflect findings from deep research conducted by 4 parallel and 3 deep research agents.
- Improved line counting methods, upgrading confidence from 7/10 to 9.5/10, and introduced a unified line counting utility for better accuracy.
- Enhanced hunk mapping confidence from 5/10 to 9/10 with a new HunkSnippetMatcher class, optimizing matching processes.
- Added a new function to resolve team display names for improved notification handling, ensuring accurate user communication.
- Included additional fixes and improvements in the summary table, marking several tasks as completed and pending implementation.
2026-02-26 19:44:27 +02:00

32 KiB
Raw Blame History

Diff View — Detailed Fix Plans from Deep Research

Date: 2026-02-26 Source: 4 parallel research agents + 3 deep research agents (Round 2) Last updated: 2026-02-26 (Round 2 deep research — 3 agents, 280k+ tokens total)


Fix #11: Cache TTL 3min → 30sec

Confidence: 10/10 Effort: 2 lines

Files to Change

  1. src/main/services/team/ChangeExtractorService.ts:40
// OLD
private readonly CACHE_TTL = 3 * 60 * 1000; // 3 мин

// NEW
private readonly CACHE_TTL = 30 * 1000; // 30 sec
  1. src/main/services/team/FileContentResolver.ts:32
// OLD
private readonly cacheTtl = 3 * 60 * 1000; // 3 мин

// NEW
private readonly cacheTtl = 30 * 1000; // 30 sec

Cache Architecture Details

Both services use Map<string, CacheEntry> with TTL:

  • ChangeExtractorService: key = ${teamName}:${memberName}, stores AgentChangeSet + mtime + expiresAt
  • FileContentResolver: key = file path, stores original | modified | source + expiresAt
  • ChangeExtractorService stores file mtime but NEVER uses it for validation
  • FileContentResolver has invalidateFile(filePath) but only called in ONE place: review.ts:265 after save

FileWatcher Coverage

FileWatcher ALREADY watches the right directories:

  • ~/.claude/projects/ (JSONL session files)
  • ~/.claude/todos/ (todo JSON files)
  • ~/.claude/teams/ (team config files)
  • ~/.claude/tasks/ (task JSON files)

But NO service-level cache invalidation hooks exist beyond the single invalidateFile() call.

Thundering Herd Risk: NONE

  • Each cache entry expires independently, staggered by client refresh timing
  • 30sec = 120 cache refreshes/hour per user, negligible CPU
  • Each team member has separate cache key; concurrent misses don't cascade

Future Phase (Optional): FileWatcher Integration

Would require:

  1. Add ChangeExtractorService and FileContentResolver to ServiceContext
  2. Wire FileWatcher events to precise cache invalidation
  3. Map ${teamName}:${memberName} cache keys to affected files
  4. Complex wiring, not worth it until TTL proves insufficient

Fix #10: OOM Safeguard for DiffViewer LCS

Confidence: 9/10 Effort: ~30 LOC

Memory Analysis

LCS matrix: (m+1) × (n+1) entries, each number = ~8 bytes in V8:

  • 1000×1000 = 1M entries ≈ 8MB ✓ Safe
  • 3000×3000 = 9M entries ≈ 72MB ⚠️ Manageable
  • 5000×5000 = 25M entries ≈ 200MB ✗ Dangerous
  • 10000×10000 = 100M entries ≈ 800MB ✗ OOM

Recommended threshold: MAX_CELLS = 1_000_000 (~1000×1000 lines)

diffLines() Return Format

From npm diff package:

Array<{
  value: string;       // The actual line(s) + newline
  count?: number;      // Number of lines
  added?: boolean;     // true = new lines
  removed?: boolean;   // true = removed lines
  // If neither added/removed: unchanged context lines
}>

DiffLine Type (DiffViewer)

interface DiffLine {
  type: 'removed' | 'added' | 'context';
  content: string;
  lineNumber: number;
}

Implementation

Import to add (DiffViewer.tsx top):

import { diffLines as semanticDiffLines } from 'diff';

New constant:

/** Max LCS matrix cells before falling back to semantic diff.
 *  1M cells ≈ 8MB RAM — safe for all platforms. */
const MAX_LCS_CELLS = 1_000_000;

Fallback function:

/**
 * Fallback diff using semantic line-diffing from npm `diff` package.
 * Used when LCS matrix would exceed memory threshold.
 * Output format matches LCS-based generateDiff().
 */
function generateDiffFallback(oldLines: string[], newLines: string[]): DiffLine[] {
  const oldText = oldLines.join('\n');
  const newText = newLines.join('\n');
  const changes = semanticDiffLines(oldText, newText);

  const result: DiffLine[] = [];
  let lineNumber = 1;

  for (const change of changes) {
    // Split change value into individual lines, removing trailing newline
    const changeLines = change.value.replace(/\r?\n$/, '').split(/\r?\n/);

    for (const content of changeLines) {
      if (change.added) {
        result.push({ type: 'added', content, lineNumber: lineNumber++ });
      } else if (change.removed) {
        result.push({ type: 'removed', content, lineNumber: lineNumber++ });
      } else {
        result.push({ type: 'context', content, lineNumber: lineNumber++ });
      }
    }
  }

  return result;
}

Modified generateDiff():

function generateDiff(oldLines: string[], newLines: string[]): DiffLine[] {
  // Fallback to semantic diffing for large files to prevent OOM
  if (oldLines.length * newLines.length > MAX_LCS_CELLS) {
    return generateDiffFallback(oldLines, newLines);
  }

  // Original LCS-based algorithm
  const matrix = computeLCSMatrix(oldLines, newLines);
  // ... rest unchanged ...
}

Visual Behavior Difference

File Pair Strategy Visual Quality
< 1000×1000 LCS Precise character-level alignment
> 1000×1000 Semantic Groups consecutive changes differently, but correct

The fallback is semantically correct but may group consecutive changes differently. For most real code diffs, the visual difference is negligible.

Precedent in Codebase

ReviewDiffContent.tsx already uses diffLines() successfully:

const diffResult = diffLines(original ?? '', modified ?? '');

Fix #1+#2: Unified Line Counting

Confidence: 7/10 → UPGRADED to 9.5/10 (after deep research) Effort: ~4-6 hours

Deep Research Upgrade (2026-02-26)

Deep research agent (76.1k tokens, 12 tool uses) found a significantly more reliable approach:

  1. UnifiedLineCounter — единая утилита на diffLines() для ALL counting paths
  2. filesSeen tracking — Set для отслеживания Write (новый файл vs перезапись)
  3. File-history backups — использование ~/.claude/file-history/ для получения оригинального контента при Write-update (вместо приблизительной оценки "assume ~same amount removed")
  4. buildTimeline fix — ChangeExtractorService.buildTimeline тоже должен использовать diffLines() вместо split('\n').length

Ключевое улучшение: вместо linesRemoved += added (assume ~same) для Write-update, агент обнаружил что можно получить оригинальный контент через тот же FileContentResolver/file-history pipeline и сделать точный diff.

Current State: 3 Independent Algorithms

MemberStatsComputer (src/main/services/team/MemberStatsComputer.ts)

Edit (lines 189-202):

const oldLines = oldStr ? oldStr.split('\n').length : 0;
const newLines = newStr ? newStr.split('\n').length : 0;
const fileAdded = newLines > oldLines ? newLines - oldLines : 0;
const fileRemoved = oldLines > newLines ? oldLines - newLines : 0;
  • WRONG when content changes but line count stays same

Write (lines 204-214):

const fileAdded = writeContent.split('\n').length;
linesAdded += fileAdded;
addFileLines(input.file_path, fileAdded, 0);  // Always removals = 0!
  • NEVER counts removals

MultiEdit (lines 216-229):

  • Same pattern as Write: only additions, no removals

Bash (lines 232-243):

  • Heuristic estimateBashLinesChanged() (~30-40% coverage)

ChangeExtractorService (src/main/services/team/ChangeExtractorService.ts)

countLines (lines 463-473): Uses diffLines() — CORRECT buildTimeline (lines 426-427): Uses split('\n').length — INCONSISTENT with own countLines!

FileContentResolver (src/main/services/team/FileContentResolver.ts)

Lines 141-156: Uses diffLines() — CORRECT

Who Consumes These Counts

Source Consumer UI
MemberStatsComputer MemberStatsTab.tsx Session analytics "+X / -Y"
ChangeExtractorService ChangeStatsBadge.tsx File tree badges
ChangeExtractorService ReviewApplierService.ts Diff hunks
FileContentResolver CodeMirrorDiffView.tsx Full file diff display

Proposed Fix

Phase 1: Create UnifiedLineCounter

// src/main/services/team/UnifiedLineCounter.ts
import { diffLines } from 'diff';

export class UnifiedLineCounter {
  static countLines(oldStr: string, newStr: string): { added: number; removed: number } {
    if (!oldStr && !newStr) return { added: 0, removed: 0 };
    const changes = diffLines(oldStr, newStr);
    let added = 0;
    let removed = 0;
    for (const c of changes) {
      if (c.added) added += c.count ?? 0;
      if (c.removed) removed += c.count ?? 0;
    }
    return { added, removed };
  }
}

Phase 2: Migrate MemberStatsComputer Edit

// Replace lines 189-202
const { added: fileAdded, removed: fileRemoved } = UnifiedLineCounter.countLines(oldStr, newStr);

Phase 3: Fix Write Operations (Bug #2)

Track file creation vs update during JSONL parse:

const filesSeen = new Set<string>();

// In Write handler:
if (toolName === 'Write') {
  const filePath = typeof input.file_path === 'string' ? input.file_path : '';
  const writeContent = typeof input.content === 'string' ? input.content : '';

  const isNewFile = !filesSeen.has(filePath);
  filesSeen.add(filePath);

  if (writeContent) {
    if (isNewFile) {
      // New file creation — all lines are additions
      const { added } = UnifiedLineCounter.countLines('', writeContent);
      linesAdded += added;
      if (filePath) addFileLines(filePath, added, 0);
    } else {
      // File replacement — assume full rewrite (conservative estimate)
      const { added } = UnifiedLineCounter.countLines('', writeContent);
      linesAdded += added;
      linesRemoved += added; // Assume ~same amount removed
      if (filePath) addFileLines(filePath, added, added);
    }
  }
}

Phase 4: Fix buildTimeline in ChangeExtractorService

// Replace lines 426-427
const { added, removed } = UnifiedLineCounter.countLines(s.oldString, s.newString);
// Use `added` and `removed` instead of split('\n').length arithmetic

Phase 5: Keep Bash As-Is

Fundamental limitation — command string has no execution output.

Risk Assessment

Risk Level Mitigation
Write removals estimation inaccurate HIGH filesSeen Set tracks if file existed before; conservative "full rewrite" estimate
Line count numbers change in UI MEDIUM Expected — numbers become MORE accurate
Historical data shows different numbers LOW Accept as one-time correction
Circular dependency (MemberStatsComputer → FileContentResolver) NONE UnifiedLineCounter is independent utility

Open Questions

  1. Write-update without oldString: filesSeen approach assumes sequential JSONL parsing. If messages are out of order, may misclassify. Need to verify JSONL ordering.
  2. Performance: diffLines() is heavier than split().length. For sessions with 1000+ Edit calls, could add latency. Need to benchmark.
  3. Bash estimation: Keep as-is or drop entirely? Current tooltip says "Approximate" — may be enough.

Fix #6+#7: Hunk↔Snippet Mapping + indexOf

Confidence: 5/10 → UPGRADED to 9/10 (after deep research) Effort: ~150 LOC

Deep Research Upgrade (2026-02-26)

Deep research agent (83.8k tokens, 24 tool uses) found a significantly more reliable approach:

  1. HunkSnippetMatcher — отдельный класс для маппинга с fallback chain:
    • Level 1: contextHash — хеш ±3 строк контекста вокруг edit site, вычисляется при извлечении snippet в ChangeExtractorService
    • Level 2: structuredPatch() content overlap — hunk added/removed lines vs snippet newString/oldString
    • Level 3: indexOf с disambiguation через oldString proximity scoring
  2. contextHash на SnippetDiff — новое поле, вычисляется один раз при создании snippet, используется для быстрого matching без повторного парсинга
  3. Position-aware rejection — вместо content.indexOf(snippet.newString) (первое вхождение), ищет ВСЕ вхождения и выбирает ближайшее к hunk position
  4. Fallback chain — если contextHash не матчит → content overlap → indexOf, каждый уровень менее точный но покрывает больше кейсов

Ключевое улучшение: добавление contextHash поля в SnippetDiff при извлечении (в ChangeExtractorService) даёт O(1) matching вместо O(n×m) content scanning.

Current Architecture Flow

CodeMirrorDiffView.tsx (lines 427, 443)
  ↓
computeHunkIndexAtPos(state, pos) → hunkIndex: number
  ↓
onRejectRef.current?.(idx)  // onHunkRejected callback
  ↓
IPC: team:applyReviewDecisions
  ↓
ReviewApplierService.rejectHunks(filePath, original, modified, hunkIndices, snippets)
  ↓
trySnippetLevelReject(modified, hunkIndices, snippets)
  ↓
snippetsToReject = hunkIndices.map(idx => validSnippets[idx])  // ← BUG: 1:1 assumption
  ↓
content.indexOf(snippet.newString)  // ← BUG: first occurrence only

Data Available in Snippets

From ChangeExtractorService (SnippetDiff type):

  • oldString / newString — actual content
  • toolName — Edit, Write, MultiEdit
  • toolUseId — unique ID
  • timestamp — when it happened
  • type — 'edit' | 'write-new' | 'write-update' | 'multi-edit'
  • isError — whether tool errored
  • replaceAll — for Edit with replace_all flag
  • NO line numbers — this is the core problem

Data Available in Hunks

From structuredPatch() (npm diff package):

interface StructuredPatchHunk {
  oldStart: number;     // Line number in original (1-based)
  oldLines: number;     // Line count in original
  newStart: number;     // Line number in modified (1-based)
  newLines: number;     // Line count in modified
  lines: string[];      // Actual diff lines (+, -, space context)
}

From CodeMirror's getChunks():

chunks: {
  fromA: number  // Original doc character position
  toA: number    // Original doc character position
  fromB: number  // Modified doc character position
  toB: number    // Modified doc character position
}[]

Proposed Fix: 3 Phases

Phase 1: buildHunkToSnippetMapping()

Build explicit mapping using content overlap detection:

private buildHunkToSnippetMapping(
  original: string,
  modified: string,
  hunkIndices: number[],
  snippets: SnippetDiff[]
): Map<number, Set<number>> {
  const patch = structuredPatch('file', 'file', original, modified);
  if (!patch.hunks || patch.hunks.length === 0) return new Map();

  const mapping = new Map<number, Set<number>>();

  for (const hunkIdx of hunkIndices) {
    if (hunkIdx < 0 || hunkIdx >= patch.hunks.length) continue;
    const hunk = patch.hunks[hunkIdx];
    const snippetSet = new Set<number>();

    // Extract added/removed content from hunk
    const addedLines = hunk.lines.filter(l => l.startsWith('+')).map(l => l.slice(1));
    const removedLines = hunk.lines.filter(l => l.startsWith('-')).map(l => l.slice(1));
    const addedContent = addedLines.join('\n');
    const removedContent = removedLines.join('\n');

    for (let sIdx = 0; sIdx < snippets.length; sIdx++) {
      const snippet = snippets[sIdx];
      if (snippet.isError) continue;

      const matchesNew = addedContent.includes(snippet.newString);
      const matchesOld = removedContent.includes(snippet.oldString);

      if (snippet.type === 'write-new' || snippet.type === 'write-update') {
        if (matchesNew) snippetSet.add(sIdx);
      } else {
        // For edits: require both old AND new match for higher confidence
        if (matchesNew && matchesOld) {
          snippetSet.add(sIdx);
        } else if (matchesNew) {
          snippetSet.add(sIdx); // Lower confidence fallback
        }
      }
    }

    mapping.set(hunkIdx, snippetSet);
  }

  return mapping;
}

Phase 2: Position-Aware findSnippetPosition()

Replace indexOf with context-aware search:

private findSnippetPosition(
  snippet: SnippetDiff,
  content: string
): number {
  const { newString, oldString } = snippet;

  // Fast path: newString is unique in content
  const firstPos = content.indexOf(newString);
  if (firstPos === -1) return -1;

  const lastPos = content.lastIndexOf(newString);
  if (firstPos === lastPos) return firstPos; // Only one occurrence — safe

  // Multiple occurrences — use oldString context to disambiguate
  // Search for each occurrence and check if surrounding context matches oldString
  const positions: number[] = [];
  let searchStart = 0;
  while (true) {
    const pos = content.indexOf(newString, searchStart);
    if (pos === -1) break;
    positions.push(pos);
    searchStart = pos + 1;
  }

  // For each candidate position, check if oldString context is nearby
  if (oldString) {
    for (const pos of positions) {
      // Look for oldString within ±1000 chars of this position
      // (in the original document, oldString would be at roughly the same position)
      const nearbyStart = Math.max(0, pos - 1000);
      const nearbyEnd = Math.min(content.length, pos + newString.length + 1000);
      const nearby = content.substring(nearbyStart, nearbyEnd);

      // If any unique token from oldString appears nearby, this is likely correct
      const oldTokens = oldString.split(/\s+/).filter(t => t.length > 3);
      const matchScore = oldTokens.filter(t => nearby.includes(t)).length;

      if (matchScore > oldTokens.length * 0.5) {
        return pos; // >50% of oldString tokens found nearby
      }
    }
  }

  // Last resort: return first position with warning
  return firstPos;
}

Phase 3: Update trySnippetLevelReject() Signature

// Pass `original` through to enable mapping and context matching
private trySnippetLevelReject(
  modified: string,
  hunkIndices: number[],
  snippets: SnippetDiff[],
  original: string  // NEW parameter
): RejectResult | null {
  const validSnippets = snippets.filter(s => !s.isError);
  if (validSnippets.length === 0) return null;

  // NEW: Build mapping instead of assuming 1:1
  const hunkToSnippets = this.buildHunkToSnippetMapping(
    original, modified, hunkIndices, validSnippets
  );

  // Collect all snippets to reject
  const snippetIndices = new Set<number>();
  for (const indices of hunkToSnippets.values()) {
    indices.forEach(idx => snippetIndices.add(idx));
  }

  const snippetsToReject = Array.from(snippetIndices)
    .map(idx => validSnippets[idx])
    .filter(Boolean);

  // NEW: Position-aware matching
  const positioned = snippetsToReject
    .map(snippet => ({
      snippet,
      pos: this.findSnippetPosition(snippet, modified)
    }))
    .filter(item => item.pos !== -1)
    .sort((a, b) => b.pos - a.pos); // Descending for safe replacement

  if (positioned.length !== snippetsToReject.length) {
    return null; // Fallback to hunk-level
  }

  let content = modified;
  for (const { snippet, pos } of positioned) {
    if (snippet.type === 'write-new') continue;
    if (snippet.replaceAll) {
      content = content.split(snippet.newString).join(snippet.oldString);
    } else {
      content = content.substring(0, pos) + snippet.oldString +
                content.substring(pos + snippet.newString.length);
    }
  }

  return { success: true, newContent: content, hadConflicts: false };
}

Edge Cases & Concerns

Case Current Behavior After Fix
Multiple Edit → 1 Hunk Assumes hunkIdx = snippetIdx (WRONG) Content overlap mapping (CORRECT)
1 Write → 2 Hunks Maps to wrong snippet Maps via content match
Duplicate code in file Corrupts first occurrence Context-aware disambiguation
Short snippet (1 line) indexOf works May still match wrong occurrence if context is ambiguous
No oldString context N/A Falls back to first indexOf match (same as before)
replaceAll snippets Works (replaces all) Still works (replaceAll logic unchanged)

Open Questions

  1. Short snippets: If newString is "return true;" and appears 10 times — context matching may fail. Need hunk line range to narrow search.
  2. Performance: structuredPatch() is called again in buildHunkToSnippetMapping (already called in rejectHunks). Should cache the patch result.
  3. MultiEdit: Creates 1 snippet but may affect multiple non-contiguous regions. buildHunkToSnippetMapping should handle this but needs testing.
  4. Overlapping snippets: Two snippets touching the same line range. Position-aware replacement from end (descending sort) handles this, but still fragile.
  5. original parameter: Need to thread it through from all callers: rejectHunks(), previewReject(), acceptHunks().

Why Confidence is Only 5/10

The core issue is that snippets have NO line numbers. All matching is content-based (heuristic). For short/common snippets, disambiguation may fail. A truly robust fix would require:

  1. Adding line number tracking to SnippetDiff during extraction
  2. Or using structuredPatch bidirectionally to map hunks to file regions

Both are larger architectural changes that go beyond the current fix scope.


Summary: Implementation Priority

Fix Confidence Effort Status
#11 Cache TTL 10/10 2 lines DONE (commit d97a757)
#10 OOM safeguard 9/10 ~30 LOC DONE (commit d97a757)
#1+#2 Line counting 9.5/10 (was 7) ~4-6h Deep research done, pending implementation
#6+#7 Hunk mapping 9/10 (was 5) ~150 LOC Deep research done, pending implementation

Also Fixed in d97a757

  • #5: computeHunkIndexAtPos → nearest hunk (CodeMirrorDiffView.tsx)
  • #8: Skeleton flash after save (changeReviewSlice.ts)
  • #9: CRLF normalization (DiffViewer.tsx)
  • #19: threshold 1.0 → 0.85 (CodeMirrorDiffView.tsx)
  • #20: useEffect dependency array (FileSectionDiff.tsx)
  • #25: bash relative paths (MemberStatsComputer.ts)
  • #26: empty line ?? fix (DiffViewer.tsx)

Round 2: Deep Research (3 parallel agents, 280k+ tokens)

Agent 1: UnifiedLineCounter — Exact Line Numbers & Code Paths

6 точек с неправильным подсчётом строк

# Файл Метод Строки Алгоритм Проблема Влияет на
1 MemberStatsComputer.ts parseFile (Edit) 193-196 split('\n').length diff Не считает реальные diff операции MemberFullStats → Team stats
2 MemberStatsComputer.ts parseFile (Write) 208 split('\n').length абсолют removed всегда 0 MemberFullStats → Team stats
3 MemberStatsComputer.ts parseFile (NotebookEdit) 220 split('\n').length абсолют Аналогично Write MemberFullStats → Team stats
4 ChangeExtractorService.ts buildTimeline() 426-427 split('\n').length diff Не использует собственный countLines()! FileEditTimeline → UI
5 ChangeExtractorService.ts generateEditSummary() 445, 449-450 split('\n').length Дублирует логику подсчёта FileEditEvent.summary
6 ChangeExtractorService.ts aggregateByFile() 391 countLines()diffLines() ПРАВИЛЬНО FileChangeSummary

Точные строки для замены

MemberStatsComputer.ts:193-196 (Edit):

// ТЕКУЩЕЕ (НЕПРАВИЛЬНО):
const oldLines = oldStr ? oldStr.split('\n').length : 0;
const newLines = newStr ? newStr.split('\n').length : 0;
const fileAdded = newLines > oldLines ? newLines - oldLines : 0;
const fileRemoved = oldLines > newLines ? oldLines - newLines : 0;
// ЗАМЕНА: const { added: fileAdded, removed: fileRemoved } = UnifiedLineCounter.countLines(oldStr, newStr);

MemberStatsComputer.ts:208 (Write):

// ТЕКУЩЕЕ (НЕПРАВИЛЬНО):
const fileAdded = writeContent.split('\n').length;
linesAdded += fileAdded;
addFileLines(input.file_path, fileAdded, 0);  // removed всегда 0!
// ЗАМЕНА: использовать diffLines('', writeContent) для write-new, отслеживать через filesSeen

MemberStatsComputer.ts:220 (NotebookEdit):

// ТЕКУЩЕЕ (НЕПРАВИЛЬНО):
const fileAdded = src.split('\n').length;
// ЗАМЕНА: аналогично Write

ChangeExtractorService.ts:426-427 (buildTimeline):

// ТЕКУЩЕЕ (НЕПРАВИЛЬНО):
linesAdded: Math.max(0, s.newString.split('\n').length - s.oldString.split('\n').length),
linesRemoved: Math.max(0, s.oldString.split('\n').length - s.newString.split('\n').length),
// ЗАМЕНА: const { added, removed } = this.countLines(s.oldString, s.newString);

ChangeExtractorService.ts:445,449-450 (generateEditSummary):

// ТЕКУЩЕЕ (НЕПРАВИЛЬНО):
const lines = snippet.oldString.split('\n').length;
const added = snippet.newString.split('\n').length;
const removed = snippet.oldString.split('\n').length;
// ЗАМЕНА: использовать this.countLines()

Критичные находки

  • diffLines НЕ импортирован в MemberStatsComputer — нужно добавить
  • seenFiles в ChangeExtractorService (строка 207-265) определяет write-new vs write-update, но НЕ учитывает файлы существовавшие ДО сессии
  • JSONL парсится строго последовательно — filesSeen паттерн безопасен
  • Performance: diffLines() для типичных snippets (<50 строк) = микросекунды, no risk
  • НЕТ ТЕСТОВ для countLines, buildTimeline, generateEditSummary!

Agent 2: HunkSnippetMatcher — Exact Bug Chain

Полная цепочка бага (от UI до backend)

1. UI: CodeMirrorDiffView.tsx → computeHunkIndexAtPos(state, pos) → chunk index
2. UI: FileSectionDiff.tsx:123-124 → onHunkRejected(file.filePath, idx)
3. Store: changeReviewSlice.ts:610 →
   for (let i = 0; i < file.snippets.length; i++) {
     hunkDecs[i] = hunkDecisions[`${filePath}:${i}`] ?? 'pending';
   }
   // ← СТРОИТ hunkDecs по SNIPPET INDICES, но hunk indices != snippet indices!
4. IPC: review.ts:191-203 → handleRejectHunks(teamName, filePath, original, modified, hunkIndices, snippets)
5. Backend: ReviewApplierService.ts:71 → trySnippetLevelReject(modified, hunkIndices, snippets)
6. Bug #1: строка 340-342 → hunkIndices.map(idx => validSnippets[idx]) // 1:1 ASSUMPTION
7. Bug #2: строка 353 → content.indexOf(snippet.newString) // ПЕРВОЕ ВХОЖДЕНИЕ
8. Fallback: строка 87 → tryHunkLevelReject() (structuredPatch + inverse)

SnippetDiff — полное определение (shared/types/review.ts:1-12)

export interface SnippetDiff {
  toolUseId: string;
  filePath: string;
  toolName: 'Edit' | 'Write' | 'MultiEdit';
  type: 'edit' | 'write-new' | 'write-update' | 'multi-edit';
  oldString: string;      // ← ДО изменения (пусто для write-new)
  newString: string;      // ← ПОСЛЕ изменения
  replaceAll: boolean;
  timestamp: string;
  isError: boolean;
  // НЕТ: contextHash, lineNumber, position — core problem!
}

Как создаются snippets (ChangeExtractorService.ts:176-308)

  • Edit (строки 239-258): берёт input.old_string, input.new_string из JSONL. НЕТ доступа к полному файлу.
  • Write (строки 259-277): newString = input.content (весь файл!). 1 Write → 1 snippet → N hunks при patch.
  • MultiEdit (строки 278-302): for (const edit of edits) → N snippets с ОДНИМ toolUseId.

Edge cases

Case Текущее поведение Опасность
snippet.newString === "" (deletion) indexOf("") = 0 Всегда находит позицию 0!
newString встречается 5+ раз indexOf = первое Неправильная позиция
replaceAll = true content.split(new).join(old) OK, но конфликт с другими snippets
MultiEdit N snippets с одним toolUseId Могут слиться в 1 hunk
1 Write → N hunks hunkIdx > snippets.length Out of bounds!

Proposed HunkSnippetMatcher Architecture

class HunkSnippetMatcher {
  // Fallback chain (от самого точного к менее точному):
  // 1. contextHash match (если добавлено к SnippetDiff)
  // 2. structuredPatch content overlap (hunk lines vs snippet strings)
  // 3. indexOf с disambiguation через oldString proximity

  matchAll(original, modified, hunks, snippets): Map<hunkIndex, SnippetDiff[]>
  // Один hunk может соответствовать нескольким snippets!
  // Один snippet может создать несколько hunks (Write)!
}

Agent 3: Integration Points & Conflicts

3 критичных конфликта между #1+#2 и #6+#7

КОНФЛИКТ #1: countLines() в ChangeExtractorService

  • #1+#2 исправляет countLines() (строки 463-473)
  • #6+#7 зависит от результатов countLines() в aggregateByFile()
  • Решение: Реализовать #1+#2 ПЕРВЫМ

КОНФЛИКТ #2: FileContentResolver переопределяет numbers

  • FileContentResolver.getFileContent() (строки 144-156) ПЕРЕСЧИТЫВАЕТ linesAdded/linesRemoved из full content
  • Может перезаписать значения из ChangeExtractorService
  • Решение: Это OK — FileContentResolver использует более точный метод (full content diff)

КОНФЛИКТ #3: Порядок snippets

  • Если #1+#2 изменит фильтрацию/порядок snippets → hunk indices в #6+#7 сломаются
  • Решение: #1+#2 НЕ меняет порядок snippets, только алгоритм подсчёта

Порядок реализации: #1+#2 ПЕРВЫМ, потом #6+#7

Карта зависимостей SnippetDiff (13 файлов):

Создание:
  ChangeExtractorService.parseJSONLFile() [строки 177-308]
    ↓
Агрегация:
  ChangeExtractorService.aggregateByFile() [строки 368-415]
    ↓
IPC передача (5 каналов):
  REVIEW_GET_AGENT_CHANGES, REVIEW_GET_TASK_CHANGES,
  REVIEW_GET_FILE_CONTENT, REVIEW_REJECT_HUNKS, REVIEW_PREVIEW_REJECT
    ↓
Потребители:
  FileContentResolver.resolveFileContent() — реконструкция
  ReviewApplierService.trySnippetLevelReject() — reject/accept
  ReviewDiffContent → SnippetDiffView — UI рендеринг
  ChangeStatsBadge — отображает +/-

Если добавить contextHash к SnippetDiff:

  • shared/types/review.ts — ОБЯЗАТЕЛЬНО (тип)
  • ChangeExtractorService.ts — ОБЯЗАТЕЛЬНО (вычисление при создании)
  • Остальные 11 файлов — НЕ ТРЕБУЮТ изменений (optional field, JSON-safe)
  • Preload/IPC — не требуют изменений (JSON сериализация OK)

Тестовая инфраструктура:

  • MemberStatsComputer.test.ts — ЕСТЬ (75 строк, только Bash эвристика)
  • ChangeExtractorService.test.ts — НЕ СУЩЕСТВУЕТ!
  • ReviewApplierService.test.ts — НЕ СУЩЕСТВУЕТ!
  • Нужно создать оба перед реализацией