diff --git a/docs/research/diff-view-round3-research.md b/docs/research/diff-view-round3-research.md index 920beb2e..c43ed206 100644 --- a/docs/research/diff-view-round3-research.md +++ b/docs/research/diff-view-round3-research.md @@ -22,36 +22,58 @@ Source: 3 parallel research agents (~260k tokens total) ### A1. FALSE NEGATIVES (критичнее) -**Root cause**: Различие whitespace между snippet и hunk. +**Root cause (уточнён Round 3.1)**: НЕ whitespace (предыдущий анализ был неверен — Edit tool хранит точный текст с indentation). Реальная причина: **context lines** в hunk отбрасываются при matching. -- `ChangeExtractorService` хранит `oldString`/`newString` как есть из Edit tool_use — **без leading whitespace** -- `structuredPatch()` генерирует hunk lines с indentation из файла: `" const x = 1;"` -- `.slice(1)` убирает только `+`/`-` prefix, оставляя indentation -- `.includes()` сравнивает `"const x"` vs `" const x"` → **не находит** +**Механизм**: +- `HunkSnippetMatcher` берёт только `+` и `-` строки из хунка, отбрасывая context (` ` prefix) +- `removedContent` = join только `-` строк → контекстные строки МЕЖДУ изменёнными строками теряются +- Snippet `oldString` содержит ВСЕ строки (включая context), т.к. это точная подстрока файла +- `includes()` в обе стороны фейлится: ни `removedContent ⊂ oldString`, ни наоборот -**Конкретный пример**: +**Concrete proof** (из ресёрча): ```typescript -// Файл с 3 Edit-ами в строках, разделённых < 3 строками: -const [state, setState] = useState(0); // ← Edit 1 -const [data, setData] = useState(null); // ← Edit 2 -const [loading, setLoading] = useState(false); // ← Edit 3 +// Claude's Edit: +// old_string = "interface UserConfig {\n name: string;\n age: number;\n email: string;\n active: boolean;\n premium: boolean;\n}" +// new_string = "interface UserSettings {\n name: string;\n age: number;\n email: string;\n active: boolean;\n isPremium: boolean;\n}" + +// structuredPatch() hunk: +// -interface UserConfig { ← removed +// name: string; ← CONTEXT (discarded!) +// age: number; ← CONTEXT (discarded!) +// email: string; ← CONTEXT (discarded!) +// active: boolean; ← CONTEXT (discarded!) +// - premium: boolean; ← removed +// +interface UserSettings { ← added +// + isPremium: boolean; ← added + +// removedContent = "interface UserConfig {\n premium: boolean;" +// oldString = "interface UserConfig {\n name: string;\n age: number;\n email: string;\n active: boolean;\n premium: boolean;\n}" +// removedContent.includes(oldString) → NO +// oldString.includes(removedContent) → NO (context lines break contiguity) +// ❌ FALSE NEGATIVE — snippet не матчится к своему хунку ``` -`structuredPatch()` сливает их в ОДИН хунк (context window = 3 строки). +**`structuredPatch()` merge threshold**: `context * 2` = 8 строк (default context=4). Хунки мержатся если gap ≤ 8 строк. -- Snippet: `oldString = "const [state, setState] = useState(0);"` -- Hunk removed: `" const [state, setState] = useState(0);"` (с отступом) -- `includes()` → false +**Частота**: ВЫСОКАЯ. Любой Edit где Claude захватывает блок с неизменёнными строками внутри: +- Переименование interface/class + изменение полей +- Смена параметров функции + изменение body +- Конфигурационные объекты (часть полей меняется, часть нет) -**Решение**: Нормализация whitespace при matching: +**Решение**: Реконструировать "old side" и "new side" хунка включая context lines: ```typescript -const normalize = (s: string) => - s.split('\n').map(l => l.trim()).filter(l => l).join('\n'); +// Вместо только +/- строк: +const oldSideContent = hunk.lines + .filter(l => l.startsWith(' ') || l.startsWith('-')) + .map(l => l.slice(1)).join('\n'); +const newSideContent = hunk.lines + .filter(l => l.startsWith(' ') || l.startsWith('+')) + .map(l => l.slice(1)).join('\n'); +// oldSideContent.includes(snippet.oldString) → TRUE ✓ +// newSideContent.includes(snippet.newString) → TRUE ✓ ``` -**Риск**: минимальный — whitespace-only diff в Edit почти невозможен (Claude всегда меняет контент). - -**Оценка**: фиксит ~90% false negatives от merged hunks. +**Уверенность**: 9.5/10 что реальный баг. 9/10 что fix через old/new side reconstruction работает. ### A2. FALSE POSITIVES diff --git a/src/main/services/team/HunkSnippetMatcher.ts b/src/main/services/team/HunkSnippetMatcher.ts index c2a28cfc..5aa9d04c 100644 --- a/src/main/services/team/HunkSnippetMatcher.ts +++ b/src/main/services/team/HunkSnippetMatcher.ts @@ -37,17 +37,22 @@ export class HunkSnippetMatcher { const hunk = patch.hunks[hunkIdx]; const snippetSet = new Set(); - // 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'); + // Reconstruct old/new side of hunk INCLUDING context lines. + // Context lines (` ` prefix) are critical — without them, snippets whose + // oldString spans unchanged lines between changed lines can't be matched. + const oldSideContent = hunk.lines + .filter((l) => !l.startsWith('+')) + .map((l) => l.slice(1)) + .join('\n'); + const newSideContent = hunk.lines + .filter((l) => !l.startsWith('-')) + .map((l) => l.slice(1)) + .join('\n'); for (let sIdx = 0; sIdx < snippets.length; sIdx++) { const snippet = snippets[sIdx]; - // Content overlap: check if snippet's strings appear in hunk's diff content - if (this.hasContentOverlap(snippet, addedContent, removedContent)) { + if (this.hasContentOverlap(snippet, oldSideContent, newSideContent)) { snippetSet.add(sIdx); } } @@ -116,40 +121,30 @@ export class HunkSnippetMatcher { // ── Private helpers ── /** - * Check if a snippet's content overlaps with hunk's added/removed content. + * Check if a snippet's content overlaps with a hunk's reconstructed file ranges. + * + * @param hunkOldSide — reconstructed original file text within hunk range (context + removed lines) + * @param hunkNewSide — reconstructed modified file text within hunk range (context + added lines) */ private hasContentOverlap( snippet: SnippetDiff, - hunkAddedContent: string, - hunkRemovedContent: string + hunkOldSide: string, + hunkNewSide: string ): boolean { - // Skip empty snippets if (!snippet.newString && !snippet.oldString) return false; if (snippet.type === 'write-new' || snippet.type === 'write-update') { - // For Write: check if hunk's added content is a substring of snippet's newString - if (snippet.newString && hunkAddedContent) { - return snippet.newString.includes(hunkAddedContent); + // For Write: snippet.newString is the full file content — check if hunk's new side is within it + if (snippet.newString && hunkNewSide) { + return snippet.newString.includes(hunkNewSide); } return false; } - // For Edit/MultiEdit: check bidirectional overlap - const matchesNew = snippet.newString - ? hunkAddedContent.includes(snippet.newString) || snippet.newString.includes(hunkAddedContent) - : false; + // For Edit/MultiEdit: check if snippet falls within hunk's file range + const matchesOld = snippet.oldString ? hunkOldSide.includes(snippet.oldString) : false; + const matchesNew = snippet.newString ? hunkNewSide.includes(snippet.newString) : false; - const matchesOld = snippet.oldString - ? hunkRemovedContent.includes(snippet.oldString) || - snippet.oldString.includes(hunkRemovedContent) - : false; - - // Both directions match = high confidence - if (matchesNew && matchesOld) return true; - - // Single direction match = acceptable for Edit - if (matchesNew || matchesOld) return true; - - return false; + return matchesOld || matchesNew; } } diff --git a/src/renderer/components/dashboard/CliStatusBanner.tsx b/src/renderer/components/dashboard/CliStatusBanner.tsx index 0a731c48..96f2b22e 100644 --- a/src/renderer/components/dashboard/CliStatusBanner.tsx +++ b/src/renderer/components/dashboard/CliStatusBanner.tsx @@ -161,9 +161,18 @@ export const CliStatusBanner = (): React.JSX.Element | null => { const [showLoginTerminal, setShowLoginTerminal] = useState(false); useEffect(() => { - if (isElectron) { - void fetchCliStatus(); - } + if (!isElectron) return; + + void fetchCliStatus(); + + const interval = setInterval( + () => { + void fetchCliStatus(); + }, + 10 * 60 * 1000 + ); + + return () => clearInterval(interval); }, [isElectron, fetchCliStatus]); const handleInstall = useCallback(() => { @@ -447,6 +456,11 @@ export const CliStatusBanner = (): React.JSX.Element | null => { Claude CLI v{cliStatus.installedVersion ?? 'unknown'} + {cliStatus.authLoggedIn && ( + + Authenticated + + )} {cliStatus.updateAvailable && cliStatus.latestVersion && ( → v{cliStatus.latestVersion} diff --git a/src/renderer/store/slices/changeReviewSlice.ts b/src/renderer/store/slices/changeReviewSlice.ts index 6831b4c4..ac1262fb 100644 --- a/src/renderer/store/slices/changeReviewSlice.ts +++ b/src/renderer/store/slices/changeReviewSlice.ts @@ -540,7 +540,7 @@ export const createChangeReviewSlice: StateCreator = {}; - for (let i = 0; i < file.snippets.length; i++) { + const count = getFileHunkCount(file.filePath, file.snippets.length, fileChunkCounts); + for (let i = 0; i < count; i++) { const key = `${file.filePath}:${i}`; hunkDecs[i] = hunkDecisions[key] ?? 'pending'; } @@ -599,7 +600,7 @@ export const createChangeReviewSlice: StateCreator { - const { hunkDecisions, fileDecisions, activeChangeSet } = get(); + const { hunkDecisions, fileDecisions, fileChunkCounts, activeChangeSet } = get(); if (!activeChangeSet) return; const file = activeChangeSet.files.find((f) => f.filePath === filePath); @@ -607,7 +608,8 @@ export const createChangeReviewSlice: StateCreator = {}; - for (let i = 0; i < file.snippets.length; i++) { + const count = getFileHunkCount(filePath, file.snippets.length, fileChunkCounts); + for (let i = 0; i < count; i++) { hunkDecs[i] = hunkDecisions[`${filePath}:${i}`] ?? 'pending'; }