diff --git a/docs/iterations/diff-view/phase-2-accept-reject.md b/docs/iterations/diff-view/phase-2-accept-reject.md index 25269b6c..54afec60 100644 --- a/docs/iterations/diff-view/phase-2-accept-reject.md +++ b/docs/iterations/diff-view/phase-2-accept-reject.md @@ -113,12 +113,140 @@ export class FileContentResolver { async resolveFileContent( teamName: string, memberName: string, - filePath: string + filePath: string, + snippets: SnippetDiff[] ): Promise<{ original: string | null; modified: string | null; source: 'file-history' | 'snippet-reconstruction' | 'disk-current' | 'git-fallback' | 'unavailable'; - }>; + }> { + // Level 1: file-history-snapshot backup + const fromBackup = await this.tryFileHistoryBackup(teamName, memberName, filePath); + if (fromBackup) return { ...fromBackup, source: 'file-history' }; + + // Level 2: Snippet chain reconstruction + const fromSnippets = await this.trySnippetReconstruction(filePath, snippets); + if (fromSnippets) return { ...fromSnippets, source: 'snippet-reconstruction' }; + + // Level 3: Текущий файл на диске (worst case — может быть уже изменён) + try { + const currentContent = await readFile(filePath, 'utf8'); + return { original: currentContent, modified: currentContent, source: 'disk-current' }; + } catch { + return { original: null, modified: null, source: 'unavailable' }; + } + } + + /** + * Level 2: Реконструкция original содержимого через обратное применение snippet chain. + * + * Алгоритм: + * 1. Читаем ТЕКУЩИЙ файл с диска (modified state) + * 2. Берём все SnippetDiff[] для этого файла (из ChangeExtractorService) + * 3. Применяем snippets в ОБРАТНОМ порядке (от последнего к первому) + * 4. Для каждого snippet: заменяем newString → oldString в текущем содержимом + * 5. Результат = "original" содержимое до всех изменений + * + * Ограничения: + * - Работает ТОЛЬКО если все snippets корректны и покрывают все изменения + * - Если какой-то snippet не найден в текущем файле → return null (fallback на Level 3) + * - Write-new тип: original = '' (пустой файл), modified = текущий файл + * - Write-update тип: невозможно восстановить original → return null + * - replaceAll: true — заменяем ВСЕ вхождения newString → oldString + */ + private async trySnippetReconstruction( + filePath: string, + snippets: SnippetDiff[] + ): Promise<{ original: string; modified: string } | null> { + // Нет snippets — нечего реконструировать + if (!snippets || snippets.length === 0) return null; + + // Читаем текущий файл с диска — это "modified" state (после всех изменений агента) + let currentContent: string; + try { + currentContent = await readFile(filePath, 'utf8'); + } catch { + // Файл не существует на диске — невозможно реконструировать + return null; + } + + const modified = currentContent; + + // Сортируем snippets по timestamp УБЫВАНИЯ — от последнего к первому. + // Обратный порядок нужен, потому что мы "откатываем" изменения: + // последний snippet применился последним → откатываем его первым. + const sorted = [...snippets].sort((a, b) => { + // timestamp — ISO string или epoch, сравниваем как строки (ISO) или числа + const timeA = typeof a.timestamp === 'number' ? a.timestamp : new Date(a.timestamp).getTime(); + const timeB = typeof b.timestamp === 'number' ? b.timestamp : new Date(b.timestamp).getTime(); + return timeB - timeA; // УБЫВАНИЕ — от нового к старому + }); + + // Применяем обратные замены + let content = currentContent; + + for (const snippet of sorted) { + // Write-new: агент СОЗДАЛ файл с нуля. + // Original = '' (пустой), Modified = текущее содержимое. + // Не нужно reverse-apply — просто знаем, что до этого файла не было. + if (snippet.type === 'write-new') { + return { original: '', modified }; + } + + // Write-update: агент ПЕРЕЗАПИСАЛ файл целиком (Write без old_string). + // Невозможно восстановить предыдущее содержимое — нет old_string. + // Fallback на Level 3 (текущий диск). + if (snippet.type === 'write-update') { + return null; + } + + // Edit / Multi-edit: у нас есть oldString и newString + // Reverse: заменяем newString → oldString + if (snippet.type === 'edit' || snippet.type === 'multi-edit') { + // Guard: пустой newString означает удаление (oldString → ''). + // Reverse = вставить oldString обратно, но без позиционного контекста + // не знаем КУДА вставлять → невозможно reverse → return null. + if (!snippet.newString && snippet.oldString) { + return null; + } + + // Guard: пустой oldString означает вставку ('' → newString). + // Reverse = удалить newString из файла. + // Но если newString не уникален — опасно. Проверяем ниже. + + if (snippet.replaceAll) { + // replaceAll: true — агент заменил ВСЕ вхождения oldString → newString. + // Reverse: заменяем ВСЕ вхождения newString → oldString. + if (snippet.newString && !content.includes(snippet.newString)) { + // newString не найден — chain сломана + return null; + } + content = content.replaceAll(snippet.newString, snippet.oldString); + continue; + } + + // Обычная замена (первое вхождение) + if (snippet.newString && !content.includes(snippet.newString)) { + // newString не найден в текущем содержимом — + // значит chain неполный или файл был изменён после агента. + // Fallback на Level 3. + return null; + } + + // Заменяем ПЕРВОЕ вхождение newString → oldString + if (snippet.newString) { + const idx = content.indexOf(snippet.newString); + content = + content.slice(0, idx) + + snippet.oldString + + content.slice(idx + snippet.newString.length); + } + } + } + + // content теперь содержит "original" — состояние файла ДО всех изменений агента + return { original: content, modified }; + } /** * Batch resolve для всех файлов в changeSet. @@ -154,13 +282,27 @@ export class FileContentResolver { 4. **Нужная версия**: Последний snapshot ПЕРЕД первым tool_use для данного файла 5. **Если snapshot отсутствует**: Fallback на snippet reconstruction -**Snippet chain reconstruction:** +**Snippet chain reconstruction (Level 2 — `trySnippetReconstruction`):** -1. Собрать все Edit tool_use для файла в хронологическом порядке -2. Начать с `original = ''` (или текущий файл, если есть) -3. Для каждого Edit: `content = content.replace(old_string, new_string)` -4. `modified` = финальный результат, `original` = начальное состояние -5. **Проблема**: Нет гарантии что chain полный (Write без old нарушает цепочку) +Подход: **обратное применение** (reverse-apply) — начинаем с текущего файла на диске и откатываем snippets от последнего к первому. + +1. Читаем ТЕКУЩИЙ файл с диска (= modified state после всех изменений агента) +2. Сортируем snippets по timestamp УБЫВАНИЯ (от последнего к первому) +3. Для каждого snippet в обратном порядке: + - `write-new` → original = `''`, modified = текущий файл (агент создал файл с нуля) + - `write-update` → return null (невозможно восстановить — нет oldString) + - `edit` / `multi-edit` + `replaceAll: true` → `content.replaceAll(newString, oldString)` + - `edit` / `multi-edit` → `content.replace(newString, oldString)` (первое вхождение) + - Если `newString` не найден в content → return null (chain сломана, fallback на Level 3) +4. После всех reverse-замен: `content` = original, текущий файл = modified + +**Ограничения:** +- Работает ТОЛЬКО если все snippets корректны и покрывают ВСЕ изменения +- `write-update` (Write без old_string) ломает цепочку → fallback на Level 3 +- Пустой `newString` (delete-операция) требует позиционный контекст → return null +- Неуникальный короткий `newString` может привести к неверной замене (но для Level 2 это приемлемо — Level 1 обычно покрывает ~85% кейсов) + +Полная реализация метода — см. `trySnippetReconstruction()` в классе выше. ### 3. Сервис: `src/main/services/team/ReviewApplierService.ts` (NEW) @@ -550,14 +692,15 @@ async function handleGetFileContent( ): Promise> { return wrapReviewHandler('review:getFileContent', async () => { const resolver = getContentResolver(); - const resolved = await resolver.resolveFileContent(teamName, memberName, filePath); - // ВАЖНО: resolver возвращает { original, modified, source }, - // но ReviewAPI.getFileContent обещает FileChangeWithContent. - // Нужно маппить в полный тип: + // ВАЖНО: сначала получаем snippets из extractor — они нужны для Level 2 reconstruction const extractor = getChangeExtractor(); const changeSet = await extractor.getAgentChanges(teamName, memberName); const fileSummary = changeSet.files.find(f => f.filePath === filePath); + const snippets = fileSummary?.snippets ?? []; + + // Передаём snippets в resolver для Level 2 (snippet chain reconstruction) + const resolved = await resolver.resolveFileContent(teamName, memberName, filePath, snippets); return { filePath, @@ -1213,28 +1356,128 @@ interface ConflictDialogProps { 2. "Use Agent's Original" — восстановить до-агентное состояние 3. "Edit Manually" — открыть CodeMirror для ручного редактирования +#### `src/renderer/components/team/review/DiffErrorBoundary.tsx` (NEW) + +**Задача**: React ErrorBoundary вокруг CodeMirror. CodeMirror может бросать исключения при malformed content, DOM manipulation issues, race conditions при destroy/create. ErrorBoundary перехватывает ошибки, логирует, показывает fallback с raw diff текстом. + +```typescript +// src/renderer/components/team/review/DiffErrorBoundary.tsx (NEW) +import React from 'react'; +import { AlertTriangle } from 'lucide-react'; + +interface DiffErrorBoundaryProps { + children: React.ReactNode; + filePath: string; + /** Fallback: показать raw text diff */ + oldString?: string; + newString?: string; + onRetry?: () => void; +} + +interface DiffErrorBoundaryState { + hasError: boolean; + error: Error | null; +} + +export class DiffErrorBoundary extends React.Component { + state: DiffErrorBoundaryState = { hasError: false, error: null }; + + static getDerivedStateFromError(error: Error): DiffErrorBoundaryState { + return { hasError: true, error }; + } + + componentDidCatch(error: Error, info: React.ErrorInfo) { + // Логируем ошибку CodeMirror для диагностики + console.error('[DiffErrorBoundary] CodeMirror crash:', error, info.componentStack); + } + + render() { + if (this.state.hasError) { + return ( +
+
+ + + Ошибка отображения diff для {this.props.filePath} + +
+

+ {this.state.error?.message ?? 'Unknown error'} +

+ {this.props.onRetry && ( + + )} + {/* Raw text fallback — пользователь всё равно видит изменения */} + {(this.props.oldString || this.props.newString) && ( +
+ + Показать raw diff + +
+
+
— Original
+ 
+                    {this.props.oldString || '(empty)'}
+
+
+
+
+ Modified
+ 
+                    {this.props.newString || '(empty)'}
+
+
+
+
+ )} +
+ ); + } + return this.props.children; + } +} +``` + +**~80 LOC**. Class component (обязательно для ErrorBoundary — React не поддерживает getDerivedStateFromError в функциональных компонентах). + ### 9. Модификация существующих компонентов #### `ChangeReviewDialog.tsx` (MODIFY — замена Phase 1 ReviewDiffContent) -Phase 1 использовал простой HTML-рендер. Phase 2 заменяет на `CodeMirrorDiffView`: +Phase 1 использовал простой HTML-рендер. Phase 2 заменяет на `CodeMirrorDiffView`, обёрнутый в `DiffErrorBoundary`: ```typescript // Phase 1 (удалить) -// Phase 2 (заменить на) - setHunkDecision(selectedFile.filePath, idx, 'accepted')} - onHunkRejected={(idx) => setHunkDecision(selectedFile.filePath, idx, 'rejected')} -/> +// Phase 2 (заменить на — CodeMirror обёрнут в ErrorBoundary) + refetchFileContent(selectedFile.filePath)} +> + setHunkDecision(selectedFile.filePath, idx, 'accepted')} + onHunkRejected={(idx) => setHunkDecision(selectedFile.filePath, idx, 'rejected')} + /> + ``` +**Важно**: `DiffErrorBoundary` оборачивает ТОЛЬКО `CodeMirrorDiffView`, а не весь dialog. Если CodeMirror упадёт — остальной UI (file tree, toolbar, timeline) продолжает работать. При ошибке пользователь видит raw diff text и может нажать "Попробовать снова" для пересоздания CodeMirror instance. + **Lazy loading file content:** ```typescript // При выборе файла — загрузить полное содержимое (если ещё не загружено) @@ -1289,11 +1532,12 @@ function getFileStatusIcon(filePath: string, hunkDecisions: Record; + try { + entry = JSON.parse(trimmed) as Record; + } catch { + continue; // Пропускаем повреждённые строки + } + + const timestamp = typeof entry.timestamp === 'string' ? entry.timestamp : ''; + + // Извлекаем content (тот же паттерн что в parseBoundaries — extractContent) + const message = entry.message as Record | undefined; + let content: unknown[] | null = null; + if (message && Array.isArray(message.content)) { + content = message.content; + } else if (Array.isArray(entry.content)) { + content = entry.content; + } + if (!content) continue; + + // Ищем tool_use блоки с file-modifying tools + for (const block of content) { + if (!block || typeof block !== 'object') continue; + const b = block as Record; + if (b.type !== 'tool_use') continue; + + const rawName = typeof b.name === 'string' ? b.name : ''; + const toolName = rawName.replace(/^proxy_/, ''); + + // Только file-modifying tools + if (!['Edit', 'Write', 'MultiEdit'].includes(toolName)) continue; + + const toolUseId = typeof b.id === 'string' ? b.id : ''; + + // Фильтрация по allowedToolUseIds (если scope задан) + if (shouldFilter && !allowedToolUseIds.has(toolUseId)) continue; + + const input = b.input as Record | undefined; + if (!input) continue; + + const filePath = typeof input.file_path === 'string' ? input.file_path : ''; + if (!filePath) continue; + + // Инициализируем FileChangeSummary если ещё нет + if (!fileMap.has(filePath)) { + fileMap.set(filePath, { + filePath, + relativePath: filePath.split('/').slice(-3).join('/'), // Последние 3 сегмента + snippets: [], + linesAdded: 0, + linesRemoved: 0, + }); + } + const summary = fileMap.get(filePath)!; + + if (toolName === 'Edit') { + const oldString = typeof input.old_string === 'string' ? input.old_string : ''; + const newString = typeof input.new_string === 'string' ? input.new_string : ''; + const replaceAll = input.replace_all === true; + + summary.snippets.push({ + toolUseId, + toolName: 'Edit', + oldString, + newString, + type: 'edit', + timestamp, + replaceAll, + }); + + // Подсчёт строк + const addedLines = newString.split('\n').length; + const removedLines = oldString.split('\n').length; + summary.linesAdded += Math.max(0, addedLines - removedLines); + summary.linesRemoved += Math.max(0, removedLines - addedLines); + } else if (toolName === 'Write') { + const content = typeof input.content === 'string' ? input.content : ''; + // Write: если файл уже встречался в fileMap — это update, иначе new + const isNew = summary.snippets.length === 0; + + summary.snippets.push({ + toolUseId, + toolName: 'Write', + oldString: '', + newString: content, + type: isNew ? 'write-new' : 'write-update', + timestamp, + replaceAll: false, + }); + + summary.linesAdded += content.split('\n').length; + } else if (toolName === 'MultiEdit') { + const edits = Array.isArray(input.edits) ? input.edits : []; + + for (const edit of edits) { + if (!edit || typeof edit !== 'object') continue; + const e = edit as Record; + const oldString = typeof e.old_string === 'string' ? e.old_string : ''; + const newString = typeof e.new_string === 'string' ? e.new_string : ''; + + summary.snippets.push({ + toolUseId, + toolName: 'MultiEdit', + oldString, + newString, + type: 'multi-edit', + timestamp, + replaceAll: false, + }); + + const addedLines = newString.split('\n').length; + const removedLines = oldString.split('\n').length; + summary.linesAdded += Math.max(0, addedLines - removedLines); + summary.linesRemoved += Math.max(0, removedLines - addedLines); + } + } + } } } finally { rl.close(); diff --git a/docs/iterations/diff-view/phase-4-enhanced-features.md b/docs/iterations/diff-view/phase-4-enhanced-features.md index 2acb8b36..637c1bbb 100644 --- a/docs/iterations/diff-view/phase-4-enhanced-features.md +++ b/docs/iterations/diff-view/phase-4-enhanced-features.md @@ -234,8 +234,23 @@ function parseEntry(raw: string | null): ViewedStorageEntry | null { } } +// ВАЖНО: Все localStorage операции обёрнуты в try-catch. +// QuotaExceededError возможен при переполнении (~5MB limit). +// При ошибке: логируем warning, операция no-op (viewed state теряется, не критично). + function saveEntry(teamName: string, scopeKey: string, entry: ViewedStorageEntry): void { - localStorage.setItem(getStorageKey(teamName, scopeKey), JSON.stringify(entry)); + try { + localStorage.setItem(getStorageKey(teamName, scopeKey), JSON.stringify(entry)); + } catch (error) { + console.warn('[diffViewedStorage] localStorage write failed:', error); + // QuotaExceededError — попробуем очистить старые entries и retry + try { + cleanupOldViewedEntries(); + localStorage.setItem(getStorageKey(teamName, scopeKey), JSON.stringify(entry)); + } catch { + // Полный отказ — молча проглатываем, viewed state не критичен + } + } } /** M2 fix: Cleanup старых entries при переполнении */ @@ -552,6 +567,9 @@ import { promisify } from 'util'; const execFileAsync = promisify(execFile); export class GitDiffFallback { + // Все git операции имеют timeout 10s — на больших repo git может зависнуть. + // При timeout execFile выбрасывает error с signal='SIGTERM', catch → return null/false/[]. + // M3 fix: кеш isGitRepo результатов — один exec per projectPath за сессию private gitRepoCache = new Map(); @@ -571,6 +589,7 @@ export class GitDiffFallback { ], { cwd: projectPath, maxBuffer: 10 * 1024 * 1024, // 10MB + timeout: 10_000, }); return stdout; } catch { @@ -592,7 +611,7 @@ export class GitDiffFallback { const { stdout } = await execFileAsync('git', [ 'log', '--format=%H', '--before', timestamp, '-1', '--', relativePath - ], { cwd: projectPath }); + ], { cwd: projectPath, timeout: 10_000 }); return stdout.trim() || null; } catch { return null; @@ -613,7 +632,7 @@ export class GitDiffFallback { const relativePath = filePath.replace(projectPath + '/', ''); const { stdout } = await execFileAsync('git', [ 'diff', fromCommit, toCommit, '--', relativePath - ], { cwd: projectPath }); + ], { cwd: projectPath, timeout: 10_000 }); return stdout || null; } catch { return null; @@ -634,7 +653,7 @@ export class GitDiffFallback { 'log', `--max-count=${maxCount}`, '--format=%H|%aI|%s', '--', relativePath - ], { cwd: projectPath }); + ], { cwd: projectPath, timeout: 10_000 }); return stdout.trim().split('\n') .filter(line => line.includes('|')) @@ -658,6 +677,7 @@ export class GitDiffFallback { try { await execFileAsync('git', ['rev-parse', '--is-inside-work-tree'], { cwd: projectPath, + timeout: 10_000, }); this.gitRepoCache.set(projectPath, true); return true; @@ -677,6 +697,13 @@ export class GitDiffFallback { // Оба используют execFile('git', ...) — одинаковый паттерн. ``` +**Loading states для git operations:** + +Git fallback может быть медленным (особенно на больших repo). В UI: +- `FileContentResolver.resolveFileContent()` уже возвращает Promise -- компонент показывает loading spinner +- Добавить `source` field в ответ, чтобы UI мог показать badge "Git fallback" / "JSONL" / "Disk" +- При timeout (10s) показать toast: "Git operation timed out. Showing current file version." + #### Модификация: `FileContentResolver.ts` (MODIFY — Phase 2 + Phase 4) Добавляем git fallback как третий уровень: