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

814 lines
32 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# 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`
```typescript
// OLD
private readonly CACHE_TTL = 3 * 60 * 1000; // 3 мин
// NEW
private readonly CACHE_TTL = 30 * 1000; // 30 sec
```
2. `src/main/services/team/FileContentResolver.ts:32`
```typescript
// 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:
```typescript
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)
```typescript
interface DiffLine {
type: 'removed' | 'added' | 'context';
content: string;
lineNumber: number;
}
```
### Implementation
**Import to add** (DiffViewer.tsx top):
```typescript
import { diffLines as semanticDiffLines } from 'diff';
```
**New constant**:
```typescript
/** 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**:
```typescript
/**
* 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()`**:
```typescript
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:
```typescript
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):
```typescript
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):
```typescript
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**
```typescript
// 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**
```typescript
// 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:
```typescript
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**
```typescript
// 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):
```typescript
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()`:
```typescript
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:
```typescript
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:
```typescript
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
```typescript
// 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):**
```typescript
// ТЕКУЩЕЕ (НЕПРАВИЛЬНО):
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):**
```typescript
// ТЕКУЩЕЕ (НЕПРАВИЛЬНО):
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):**
```typescript
// ТЕКУЩЕЕ (НЕПРАВИЛЬНО):
const fileAdded = src.split('\n').length;
// ЗАМЕНА: аналогично Write
```
**ChangeExtractorService.ts:426-427 (buildTimeline):**
```typescript
// ТЕКУЩЕЕ (НЕПРАВИЛЬНО):
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):**
```typescript
// ТЕКУЩЕЕ (НЕПРАВИЛЬНО):
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)
```typescript
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
```typescript
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 — **НЕ СУЩЕСТВУЕТ!**
- **Нужно создать оба перед реализацией**