From 78c055ce1233a270b946e8c416a3d3f98a75daae Mon Sep 17 00:00:00 2001 From: Esme Yi Date: Fri, 14 Aug 2026 15:21:25 +0200 Subject: [PATCH 01/10] Fix unrelated lines getting swept into AI attribution by blanket hunk crediting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit consumeAndShift() decided whether to keep a tracked line's AI attribution at the whole-hunk level: if a hunk's overall old-vs-new text was similar enough ("insignificant"), it credited *every* new line in that hunk's range to the tasklet that owned the one tracked line inside it — not just the specific line that actually matched. That's fine for the common case (a hunk that's just the tracked line itself, tweaked). But git diff has no move detection, so a restructuring can bundle a wide, mostly-unrelated span of lines into one hunk that still scores as "similar overall" (most of the text is still present, just reshuffled or interleaved with something else nearby). When that happens, lines the tracked tasklet never touched — anything else that happened to land in the same hunk — were getting mislabeled as AI-generated. Fixed by matching each tracked line to the specific new line its own content actually corresponds to (exact match first, then best BLEU-similar candidate), instead of crediting the whole hunk indiscriminately. If nothing in the hunk resembles it anymore, the line's attribution is dropped rather than guessed at. Verified with a real git-based repro: an AI-written line and an adjacent, never-AI-attributed comment edited in the same commit (no blank line between them, so git bundles both into one hunk). Before the fix, both lines were credited to the AI tasklet; after, only the one it actually wrote is. --- vscode-extension/src/history/buildHistory.ts | 74 +++++++++++++++---- .../src/test/buildHistory.test.ts | 70 ++++++++++++++++++ vscode-extension/src/utils.ts | 10 ++- 3 files changed, 136 insertions(+), 18 deletions(-) diff --git a/vscode-extension/src/history/buildHistory.ts b/vscode-extension/src/history/buildHistory.ts index bfabe04..b4420b5 100644 --- a/vscode-extension/src/history/buildHistory.ts +++ b/vscode-extension/src/history/buildHistory.ts @@ -1,5 +1,6 @@ import { BuildHistoryFailureReason, BuildHistoryResult, Change, CommitInfo, DiffHunk, History, TaskletMessage, TaskletQuestion } from "./types"; import { + bleuSimilarity, getActiveTracyId, getCommitTree, getDiff, @@ -8,7 +9,8 @@ import { groupChangesByFile, isAiChange, mapLinesToTree, - runGit + runGit, + SIMILARITY_THRESHOLD } from "../utils"; const DELIMITER = "||#--TRACY--#||"; @@ -603,14 +605,50 @@ async function buildUncommittedChanges( return { uncommittedChanges: chainChanges.flat(), lastTracyTip }; } -// Drops lines that fall inside a significant modified or deleted hunk -// Insignificant hunks preserve AI attribution - ALL new lines from the hunk are kept +// Finds which new line in an insignificant hunk this specific old line's +// content actually maps to, instead of crediting every new line in the +// hunk. A restructuring (reorder, extract-and-move) can make git bundle a +// wide, mostly-unrelated span of lines into one hunk that still scores as +// "similar" overall (most of the text is still present, just reshuffled) — +// blanket-crediting the whole hunk would then mislabel lines the tracked +// line never had anything to do with. Exact (whitespace-insensitive) match +// first, then the best BLEU-similar candidate above the same threshold +// used to decide hunk significance. +function findMatchingNewLineIndex(oldLineText: string, hunk: DiffHunk): number | null { + const addedLines = hunk.addedLines ?? []; + if (addedLines.length === 0) { + return null; + } + + const normalize = (s: string) => s.replace(/\s+/g, ''); + const normalizedOld = normalize(oldLineText); + const exactIndex = addedLines.findIndex(l => normalize(l) === normalizedOld); + if (exactIndex !== -1) { + return exactIndex; + } + + let bestIndex = -1; + let bestScore = -1; + addedLines.forEach((candidate, i) => { + const score = bleuSimilarity(oldLineText, candidate); + if (score > bestScore) { + bestScore = score; + bestIndex = i; + } + }); + + return bestScore > SIMILARITY_THRESHOLD ? bestIndex : null; +} + +// Drops lines that fall inside a significant modified or deleted hunk. +// Insignificant hunks preserve AI attribution only for the specific new +// line this old line's content actually matches (see +// findMatchingNewLineIndex) — not every new line in the hunk. // Pure insertions (oldCount = 0) never consume old lines, only shift subsequent ones // Also returns the new-tree positions of lines consumed by significant hunks so the // caller can record them as "ghost" attribution for the previous owner. function consumeAndShift(lines: number[], hunks: DiffHunk[]): { survivors: number[]; consumedNewPositions: number[] } { const survivingLines = new Set(); - const processedInsignificantHunks = new Set(); const consumedHunks = new Set(); const sortedLines = [...lines].sort((a, b) => a - b); @@ -628,19 +666,25 @@ function consumeAndShift(lines: number[], hunks: DiffHunk[]): { survivors: numbe if (containingHunk) { if (containingHunk.isSignificant) { - // Significant hunk: consume the line (user override) + // Significant hunk: consume the line (user override). Ghost-tracked + // across the whole replaced block below, since a real rewrite has + // no specific "successor line" to point to. consumedHunks.add(containingHunk); continue; - } else { - // Insignificant hunk: preserve AI attribution for ALL new lines in the hunk - // Only process each hunk once to avoid duplicates - if (!processedInsignificantHunks.has(containingHunk)) { - for (let i = 0; i < containingHunk.newCount; i++) { - survivingLines.add(containingHunk.newStart + i); - } - processedInsignificantHunks.add(containingHunk); - } - // The current line is "absorbed" - no need to add individually + } + + // Insignificant hunk: only credit the specific new line this old + // line's content matches. If nothing in the hunk resembles it + // anymore, drop it rather than guessing — deliberately not + // ghost-tracked either, to avoid the same blanket-crediting problem + // this function exists to avoid. + const oldLineText = containingHunk.removedLines?.[line - containingHunk.oldStart]; + const matchIndex = oldLineText !== undefined + ? findMatchingNewLineIndex(oldLineText, containingHunk) + : null; + + if (matchIndex !== null) { + survivingLines.add(containingHunk.newStart + matchIndex); } } else { // Line not in any hunk, apply shifts from all hunks before this line diff --git a/vscode-extension/src/test/buildHistory.test.ts b/vscode-extension/src/test/buildHistory.test.ts index 3abe399..a2c1023 100644 --- a/vscode-extension/src/test/buildHistory.test.ts +++ b/vscode-extension/src/test/buildHistory.test.ts @@ -209,3 +209,73 @@ suite('buildHistory significance filtering across a tracy-local chain', () => { ); }); }); + +suite('buildHistory does not blanket-credit a whole hunk to one tasklet', () => { + test('a human commit bundling a tiny AI-line tweak with an unrelated adjacent comment only credits the AI line', async () => { + const dir = makeTempDir(); + execSync('git init -q', { cwd: dir }); + execSync('git config user.email test@example.com', { cwd: dir }); + execSync('git config user.name Test', { cwd: dir }); + + const filePath = path.join(dir, 'app.py'); + fs.writeFileSync(filePath, [ + 'def calculate_total(items):', + ' total = 0', + ' # end of discount check', + ' return total', + '', + ].join('\n')); + execSync('git add app.py && git commit -q -m init', { cwd: dir }); + const baseCommit = execSync('git rev-parse HEAD', { cwd: dir, encoding: 'utf8' }).trim(); + + // tasklet1 (AI): inserts the discount-check line — a pure insertion, + // attributed outright. + fs.writeFileSync(filePath, [ + 'def calculate_total(items):', + ' total = 0', + ' if len(items) > 10:', + ' # end of discount check', + ' return total', + '', + ].join('\n')); + execSync('git add app.py', { cwd: dir }); + const aiCommit = commitAiEdit(dir, baseCommit, 'tasklet-1', 'sess1', 'add a discount check', 1000); + execSync(`git update-ref refs/tracy-local/aaaa1111 ${aiCommit}`, { cwd: dir }); + + // Finalize tasklet1's chain into a real commit, the way the extension + // does on `git commit` — tracy-id note + refs/tracy/ pointing at + // the hidden AI commit. + execSync('git add -A && git commit -q -m "add discount check (AI assisted)"', { cwd: dir }); + execSync('git notes add -m "tracy-id: aaaa1111" HEAD', { cwd: dir }); + execSync(`git update-ref refs/tracy/aaaa1111 ${aiCommit}`, { cwd: dir }); + + // A later, separate human commit tweaks the AI-written line AND makes + // a trivial punctuation edit to the immediately adjacent comment (never + // AI-attributed), with no unchanged line between them — git bundles + // both into ONE hunk that's still similar enough overall to be + // "insignificant". Before the fix, the whole hunk's new lines + // (including the unrelated comment) were blanket-credited to tasklet1. + fs.writeFileSync(filePath, [ + 'def calculate_total(items):', + ' total = 0', + ' if len(items) >= 10:', + ' # end of discount check.', + ' return total', + '', + ].join('\n')); + execSync('git add -A && git commit -q -m "human tweak both lines"', { cwd: dir }); + + const result = await buildHistory(dir); + assert.strictEqual(result.ok, true); + if (!result.ok) { return; } + + const file = result.history.files.find(f => f.path === 'app.py'); + const tasklet = file?.tasklets.find(t => t.taskletId === 'tasklet-1'); + assert.ok(tasklet, 'tasklet-1 should still be attributed to the discount-check line'); + assert.deepStrictEqual( + tasklet!.lines, + [3], + 'only the AI-written line should be credited — the adjacent comment (never AI-authored) must not be swept in' + ); + }); +}); diff --git a/vscode-extension/src/utils.ts b/vscode-extension/src/utils.ts index 577d182..f895b4c 100644 --- a/vscode-extension/src/utils.ts +++ b/vscode-extension/src/utils.ts @@ -285,6 +285,12 @@ export async function getDiff( return fileChanges; } +// Character-level 4-gram BLEU similarity between two arbitrary text blocks +// (single lines or whole hunks). 1.0 = identical, lower = more different. +export function bleuSimilarity(oldText: string, newText: string): number { + return bleu(tokenizeHunk(oldText), tokenizeHunk(newText), 4); +} + // Compute significance of a hunk by comparing old and new content using BLEU function computeHunkSignificance(oldLines: string[], newLines: string[]): { isSignificant: boolean; bleuScore: number | null } { if (oldLines.length === 0 && newLines.length === 0) { @@ -301,9 +307,7 @@ function computeHunkSignificance(oldLines: string[], newLines: string[]): { isSi return { isSignificant: true, bleuScore: null }; } - const oldContent = oldLines.join("\n"); - const newContent = newLines.join("\n"); - const score = bleu(tokenizeHunk(oldContent), tokenizeHunk(newContent), 4); + const score = bleuSimilarity(oldLines.join("\n"), newLines.join("\n")); return { isSignificant: score <= SIMILARITY_THRESHOLD, bleuScore: score }; } From a36b4caea250a700c7a866f81588acfee4bd01d0 Mon Sep 17 00:00:00 2001 From: Esme Yi Date: Fri, 14 Aug 2026 15:35:09 +0200 Subject: [PATCH 02/10] Fix significance check using array-adjacent chain entry instead of real parent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review finding on this PR: extractSnapshot() decided whether a snapshot was diffing against a prior AI edit by checking isAiChange(chain[index - 1]) — the array-adjacent entry. That's only a valid stand-in for "this snapshot's actual parent" on a simple linear chain. getTracyChain() does a BFS over (possibly multiple) parents to support squash-merged chains (see its doc comment), so on a chain with a merge point, array-adjacent entries can be siblings from different branches rather than parent/child. Concretely: a branch A snapshot whose real parent is the base (non-AI) commit could end up array-adjacent to an unrelated branch B AI snapshot after the BFS-then-reverse ordering. The significance check would then wrongly treat branch A's edit as AI-to-AI and skip filtering, letting a trivial edit (that should have been filtered as a User->AI change) get attributed. The diff base itself (diffFromTree) was already resolved correctly via snapshot.parentHash — only the "is this AI-authored" check was using the wrong reference. Fixed by resolving the real parent from snapshot.parentHash and looking it up in the chain by hash, instead of assuming array adjacency. Multi-parent snapshots (the merge commit itself) keep the old array-adjacent fallback, since diffFromTree already silently falls back to it too (getCommitTree rejects the space-joined multi-parent string), so the two stay consistent. Verified with a real git-based repro: a squash-merged two-branch chain where branch A's tiny edit and branch B's substantial edit land array- adjacent to each other. Before the fix, branch A's trivial edit was wrongly attributed; after, only branch B's real edit is. --- vscode-extension/src/history/buildHistory.ts | 43 ++++--- .../src/test/buildHistory.test.ts | 119 ++++++++++++++++++ 2 files changed, 148 insertions(+), 14 deletions(-) diff --git a/vscode-extension/src/history/buildHistory.ts b/vscode-extension/src/history/buildHistory.ts index b4420b5..2ff221a 100644 --- a/vscode-extension/src/history/buildHistory.ts +++ b/vscode-extension/src/history/buildHistory.ts @@ -378,12 +378,31 @@ async function extractSnapshot( agentSource, } = buildTaskletMessages(snapshot.description); let diffFromTree = index > 0 ? chain[index - 1].treeHash : baseTree; + // chain[index-1] (array-adjacent) is only a reliable stand-in for "this + // snapshot's actual parent" on a simple linear chain. getTracyChain() + // does a BFS over possibly-multiple parents to support squash-merged + // chains (see its doc comment above), so on a chain with a merge point, + // array-adjacent entries can be siblings from different branches rather + // than parent/child. Default to the array-adjacent check and only + // override it below once the real single parent is resolved. + let diffBaseIsAiAuthored = index > 0 && isAiChange(chain[index - 1]); if (snapshot.parentHash) { - const parentTree = await getCommitTree(repoPath, snapshot.parentHash); - - if (parentTree) { - diffFromTree = parentTree; + const parentHashes = snapshot.parentHash.split(" ").filter(Boolean); + + // A multi-parent parentHash (merge commit) isn't resolvable by + // getCommitTree (git rejects the space-joined string as a single + // revision), so diffFromTree already silently falls back to the + // array-adjacent tree above for that case — keep diffBaseIsAiAuthored + // consistent with whatever diffFromTree actually ends up being. + if (parentHashes.length === 1) { + const parentTree = await getCommitTree(repoPath, parentHashes[0]); + + if (parentTree) { + diffFromTree = parentTree; + const actualParent = chain.find(c => c.hash === parentHashes[0]); + diffBaseIsAiAuthored = actualParent ? isAiChange(actualParent) : false; + } } } @@ -393,16 +412,12 @@ async function extractSnapshot( const hunks = fileChangesMap.get(filePath) || []; // Only filter by significance when diffing against non-AI content - // (the real User->AI case). chain[0] is the last real, on-branch - // commit (pushed by getTracyChain() before it stops walking), so the - // first AI edit is at index 1, not 0 — "index > 0" alone isn't - // enough. Skipping the filter for AI->AI hops matters because a - // hunk diffed against the AI's OWN prior edit is very often - // textually close to it (small follow-up prompts, or just the - // unchanged surrounding context dominating the score), which isn't - // the "insignificant" case this filter exists to catch. - const diffsAgainstPriorAiEdit = index > 0 && isAiChange(chain[index - 1]); - const significantHunks = diffsAgainstPriorAiEdit ? hunks : hunks.filter(h => h.isSignificant); + // (the real User->AI case). Skipping the filter for AI->AI hops + // matters because a hunk diffed against the AI's OWN prior edit is + // very often textually close to it (small follow-up prompts, or + // just the unchanged surrounding context dominating the score), + // which isn't the "insignificant" case this filter exists to catch. + const significantHunks = diffBaseIsAiAuthored ? hunks : hunks.filter(h => h.isSignificant); const linesAtSnapshot: number[] = []; for (const hunk of significantHunks) { for (let i = 0; i < hunk.newCount; i++) { diff --git a/vscode-extension/src/test/buildHistory.test.ts b/vscode-extension/src/test/buildHistory.test.ts index a2c1023..e8c563f 100644 --- a/vscode-extension/src/test/buildHistory.test.ts +++ b/vscode-extension/src/test/buildHistory.test.ts @@ -279,3 +279,122 @@ suite('buildHistory does not blanket-credit a whole hunk to one tasklet', () => ); }); }); + +suite('buildHistory resolves significance against the real parent, not array adjacency', () => { + test('a squash-merged, two-branch chain still filters a tiny edit whose real parent is the base commit', async () => { + // getTracyChain() does a BFS over both parents of a squash-merge + // commit, so array-adjacent chain entries can be siblings from + // different branches rather than parent/child. Branch A's tiny edit + // (> 10 -> >= 10) has the real base commit as its parent and should be + // filtered as an insignificant User->AI change — even though branch + // B's unrelated AI commit can land array-adjacent to it after the BFS + // traversal reverses. + const dir = makeTempDir(); + execSync('git init -q', { cwd: dir }); + execSync('git config user.email test@example.com', { cwd: dir }); + execSync('git config user.name Test', { cwd: dir }); + + const filePath = path.join(dir, 'app.py'); + fs.writeFileSync(filePath, [ + 'def calculate_total(items):', + ' total = 0', + ' if len(items) > 10:', + ' total *= 0.9', + ' return total', + '', + 'def helper():', + ' z = None', + ' return z', + '', + ].join('\n')); + execSync('git add app.py && git commit -q -m init', { cwd: dir }); + const baseCommit = execSync('git rev-parse HEAD', { cwd: dir, encoding: 'utf8' }).trim(); + + // Branch A: a tiny, near-identical edit off the base commit. + fs.writeFileSync(filePath, [ + 'def calculate_total(items):', + ' total = 0', + ' if len(items) >= 10:', + ' total *= 0.9', + ' return total', + '', + 'def helper():', + ' z = None', + ' return z', + '', + ].join('\n')); + execSync('git add app.py', { cwd: dir }); + const commitA = commitAiEdit(dir, baseCommit, 'tasklet-A1', 'sessA', 'tiny tweak on branch A', 1000); + + // Branch B: a substantial, unrelated edit, ALSO off the base commit. + execSync(`git read-tree ${baseCommit}^{tree}`, { cwd: dir }); + fs.writeFileSync(filePath, [ + 'def calculate_total(items):', + ' total = 0', + ' if len(items) > 10:', + ' total *= 0.9', + ' return total', + '', + 'def helper():', + ' z = compute_something_entirely_different()', + ' return z', + '', + ].join('\n')); + execSync('git add app.py', { cwd: dir }); + const commitB = commitAiEdit(dir, baseCommit, 'tasklet-B1', 'sessB', 'unrelated edit on branch B', 2000); + + // Synthetic merge commit combining both branches, mimicking the + // post-rewrite squash-merge hook described in getTracyChain()'s doc + // comment. + fs.writeFileSync(filePath, [ + 'def calculate_total(items):', + ' total = 0', + ' if len(items) >= 10:', + ' total *= 0.9', + ' return total', + '', + 'def helper():', + ' z = compute_something_entirely_different()', + ' return z', + '', + ].join('\n')); + execSync('git add app.py', { cwd: dir }); + const mergeTree = execSync('git write-tree', { cwd: dir, encoding: 'utf8' }).trim(); + const mergeCommit = execSync(`git commit-tree ${mergeTree} -p ${commitA} -p ${commitB} -m "merge chains"`, { + cwd: dir, + encoding: 'utf8', + env: { ...process.env, GIT_AUTHOR_NAME: 'Tracybot', GIT_AUTHOR_EMAIL: 'tracybot@local', GIT_COMMITTER_NAME: 'Tracybot', GIT_COMMITTER_EMAIL: 'tracybot@local' }, + }).trim(); + + execSync(`git update-ref refs/tracy-local/aaaa1111 ${mergeCommit}`, { cwd: dir }); + execSync('git config tracy.current-id aaaa1111', { cwd: dir }); + execSync(`git reset -q --mixed ${baseCommit}`, { cwd: dir }); + fs.writeFileSync(filePath, [ + 'def calculate_total(items):', + ' total = 0', + ' if len(items) >= 10:', + ' total *= 0.9', + ' return total', + '', + 'def helper():', + ' z = compute_something_entirely_different()', + ' return z', + '', + ].join('\n')); + + const result = await buildHistory(dir); + assert.strictEqual(result.ok, true); + if (!result.ok) { return; } + + const file = result.history.files.find(f => f.path === 'app.py'); + const taskletA = file?.tasklets.find(t => t.taskletId === 'tasklet-A1'); + const taskletB = file?.tasklets.find(t => t.taskletId === 'tasklet-B1'); + + assert.ok( + !taskletA || taskletA.lines.length === 0, + 'branch A\'s tiny edit must be filtered — its real parent is the base commit, not branch B\'s AI snapshot' + ); + assert.ok(taskletB, 'branch B\'s substantial edit should still be attributed'); + assert.ok(taskletB!.lines.length > 0, 'branch B\'s edit should still own a live line'); + }); +}); From f85c35e902f964503a14d9aa2a1ad36b3c7b473b Mon Sep 17 00:00:00 2001 From: Esme Yi Date: Fri, 14 Aug 2026 15:44:36 +0200 Subject: [PATCH 03/10] Fix duplicate lines within a hunk colliding onto the same match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review finding on this PR: findMatchingNewLineIndex matched each tracked old line independently via findIndex, which always resolves to the FIRST matching new line. If an insignificant hunk contains two duplicate AI-attributed old lines (common: `}`, `return;`, identical log statements), both independent lookups landed on the same new position. The Set-based survivor collection then silently dropped the second one — and if the two duplicates belonged to different tasklets (consumeAndShift is called separately per tasklet's own Change), the later call's line could overwrite the earlier tasklet's legitimate claim entirely. Replaced the independent per-line lookup with alignHunkLines(), which aligns a hunk's old and new lines one-to-one and in order for the whole hunk at once: exact (whitespace-insensitive) matches are aligned via LCS, which respects both order and duplicate multiplicity, so two `}` lines in the old text land on two different `}` lines in the new text rather than both on the first one. Anything left over falls back to the best remaining (not-yet-used) BLEU-similar candidate. The alignment is a pure function of the hunk's own content, memoized per hunk within a single consumeAndShift call — and since it doesn't depend on which specific old line is being queried, two separate calls against the same hunk (e.g. for two different tasklets) independently arrive at the same consistent mapping. Verified with two real git-based repros: two duplicate AI-written lines from the SAME tasklet bundled into one hunk (before: the second one vanished; after: both survive at distinct positions), and two duplicate lines from DIFFERENT tasklets bundled together (before: one tasklet's line was silently absorbed into the other's; after: each keeps its own). --- vscode-extension/src/history/buildHistory.ts | 130 ++++++++++----- .../src/test/buildHistory.test.ts | 150 ++++++++++++++++++ 2 files changed, 239 insertions(+), 41 deletions(-) diff --git a/vscode-extension/src/history/buildHistory.ts b/vscode-extension/src/history/buildHistory.ts index 2ff221a..95a6cd9 100644 --- a/vscode-extension/src/history/buildHistory.ts +++ b/vscode-extension/src/history/buildHistory.ts @@ -620,51 +620,99 @@ async function buildUncommittedChanges( return { uncommittedChanges: chainChanges.flat(), lastTracyTip }; } -// Finds which new line in an insignificant hunk this specific old line's -// content actually maps to, instead of crediting every new line in the -// hunk. A restructuring (reorder, extract-and-move) can make git bundle a -// wide, mostly-unrelated span of lines into one hunk that still scores as -// "similar" overall (most of the text is still present, just reshuffled) — -// blanket-crediting the whole hunk would then mislabel lines the tracked -// line never had anything to do with. Exact (whitespace-insensitive) match -// first, then the best BLEU-similar candidate above the same threshold -// used to decide hunk significance. -function findMatchingNewLineIndex(oldLineText: string, hunk: DiffHunk): number | null { +// Aligns a hunk's old lines to its new lines one-to-one and in order, +// instead of matching each tracked line independently — independent +// per-line lookups all resolve a duplicated line (`}`, `return;`, +// identical log lines, common in an "insignificant" hunk) to the SAME +// first occurrence, colliding distinct duplicates onto one new position +// and losing attribution for the rest, or letting a later match overwrite +// an earlier one that legitimately owns a different occurrence. +// +// Exact (whitespace-insensitive) matches are aligned via LCS, which +// respects both order and duplicate multiplicity — two `}` lines in the +// old text land on two different `}` lines in the new text, not both on +// the first one. Whatever's left over falls back to the best remaining +// (not yet used) BLEU-similar candidate, so a restructuring that also +// tweaks content along the way still gets a best-effort match. +function alignHunkLines(hunk: DiffHunk): Map { + const oldLines = hunk.removedLines ?? []; const addedLines = hunk.addedLines ?? []; - if (addedLines.length === 0) { - return null; + const normalize = (s: string) => s.replace(/\s+/g, ''); + const normalizedOld = oldLines.map(normalize); + const normalizedNew = addedLines.map(normalize); + + const n = normalizedOld.length; + const m = normalizedNew.length; + const dp: number[][] = Array.from({ length: n + 1 }, () => new Array(m + 1).fill(0)); + for (let i = n - 1; i >= 0; i--) { + for (let j = m - 1; j >= 0; j--) { + dp[i][j] = normalizedOld[i] === normalizedNew[j] + ? dp[i + 1][j + 1] + 1 + : Math.max(dp[i + 1][j], dp[i][j + 1]); + } } - const normalize = (s: string) => s.replace(/\s+/g, ''); - const normalizedOld = normalize(oldLineText); - const exactIndex = addedLines.findIndex(l => normalize(l) === normalizedOld); - if (exactIndex !== -1) { - return exactIndex; - } - - let bestIndex = -1; - let bestScore = -1; - addedLines.forEach((candidate, i) => { - const score = bleuSimilarity(oldLineText, candidate); - if (score > bestScore) { - bestScore = score; - bestIndex = i; + const alignment = new Map(); + const usedNewIndices = new Set(); + let i = 0; + let j = 0; + while (i < n && j < m) { + if (normalizedOld[i] === normalizedNew[j]) { + alignment.set(i, j); + usedNewIndices.add(j); + i++; + j++; + } else if (dp[i + 1][j] >= dp[i][j + 1]) { + i++; + } else { + j++; + } + } + + for (let oldIndex = 0; oldIndex < n; oldIndex++) { + if (alignment.has(oldIndex)) { + continue; } - }); - return bestScore > SIMILARITY_THRESHOLD ? bestIndex : null; + let bestIndex = -1; + let bestScore = -1; + for (let newIndex = 0; newIndex < m; newIndex++) { + if (usedNewIndices.has(newIndex)) { + continue; + } + const score = bleuSimilarity(oldLines[oldIndex], addedLines[newIndex]); + if (score > bestScore) { + bestScore = score; + bestIndex = newIndex; + } + } + + if (bestIndex !== -1 && bestScore > SIMILARITY_THRESHOLD) { + alignment.set(oldIndex, bestIndex); + usedNewIndices.add(bestIndex); + } + } + + return alignment; } // Drops lines that fall inside a significant modified or deleted hunk. // Insignificant hunks preserve AI attribution only for the specific new -// line this old line's content actually matches (see -// findMatchingNewLineIndex) — not every new line in the hunk. +// line this old line's content aligns to (see alignHunkLines) — not every +// new line in the hunk. // Pure insertions (oldCount = 0) never consume old lines, only shift subsequent ones // Also returns the new-tree positions of lines consumed by significant hunks so the // caller can record them as "ghost" attribution for the previous owner. function consumeAndShift(lines: number[], hunks: DiffHunk[]): { survivors: number[]; consumedNewPositions: number[] } { const survivingLines = new Set(); const consumedHunks = new Set(); + // Memoized per hunk within this call: alignHunkLines is a pure function + // of the hunk's own content, so every tracked line landing in the same + // hunk (whether from this tasklet's own lines, or a separate + // consumeAndShift call for a different tasklet touching the same hunk) + // resolves against the same one-to-one mapping — duplicate lines land on + // distinct occurrences instead of colliding on the first match. + const hunkAlignments = new Map>(); const sortedLines = [...lines].sort((a, b) => a - b); const sortedHunks = [...hunks].sort((a, b) => a.oldStart - b.oldStart); @@ -689,17 +737,17 @@ function consumeAndShift(lines: number[], hunks: DiffHunk[]): { survivors: numbe } // Insignificant hunk: only credit the specific new line this old - // line's content matches. If nothing in the hunk resembles it - // anymore, drop it rather than guessing — deliberately not - // ghost-tracked either, to avoid the same blanket-crediting problem - // this function exists to avoid. - const oldLineText = containingHunk.removedLines?.[line - containingHunk.oldStart]; - const matchIndex = oldLineText !== undefined - ? findMatchingNewLineIndex(oldLineText, containingHunk) - : null; - - if (matchIndex !== null) { - survivingLines.add(containingHunk.newStart + matchIndex); + // line aligns to. If nothing in the hunk resembles it anymore, drop + // it rather than guessing — deliberately not ghost-tracked either, + // to avoid the same blanket-crediting problem this function exists + // to avoid. + if (!hunkAlignments.has(containingHunk)) { + hunkAlignments.set(containingHunk, alignHunkLines(containingHunk)); + } + const newIndex = hunkAlignments.get(containingHunk)!.get(line - containingHunk.oldStart); + + if (newIndex !== undefined) { + survivingLines.add(containingHunk.newStart + newIndex); } } else { // Line not in any hunk, apply shifts from all hunks before this line diff --git a/vscode-extension/src/test/buildHistory.test.ts b/vscode-extension/src/test/buildHistory.test.ts index e8c563f..0808e21 100644 --- a/vscode-extension/src/test/buildHistory.test.ts +++ b/vscode-extension/src/test/buildHistory.test.ts @@ -398,3 +398,153 @@ suite('buildHistory resolves significance against the real parent, not array adj assert.ok(taskletB!.lines.length > 0, 'branch B\'s edit should still own a live line'); }); }); + +suite('buildHistory aligns duplicate lines within a hunk one-to-one', () => { + test('two identical AI-written lines bundled into one insignificant hunk both keep distinct attribution', async () => { + // A naive per-line lookup (findIndex over the hunk's new lines) always + // resolves a duplicated line to the SAME first occurrence — both old + // duplicates collapse onto one new position, and the Set-based + // survivor collection silently drops the second one. A proper + // one-to-one, order-preserving alignment must keep them distinct. + const dir = makeTempDir(); + execSync('git init -q', { cwd: dir }); + execSync('git config user.email test@example.com', { cwd: dir }); + execSync('git config user.name Test', { cwd: dir }); + + const filePath = path.join(dir, 'app.py'); + fs.writeFileSync(filePath, [ + 'def process(items):', + ' return items', + '', + ].join('\n')); + execSync('git add app.py && git commit -q -m init', { cwd: dir }); + const baseCommit = execSync('git rev-parse HEAD', { cwd: dir, encoding: 'utf8' }).trim(); + + // tasklet1 (AI): inserts a loop with two identical log lines — a pure + // insertion, both attributed outright. + fs.writeFileSync(filePath, [ + 'def process(items):', + ' for item in items:', + ' if item.valid:', + ' log.debug("done")', + ' else:', + ' log.debug("done")', + ' return items', + '', + ].join('\n')); + execSync('git add app.py', { cwd: dir }); + const aiCommit = commitAiEdit(dir, baseCommit, 'tasklet-1', 'sess1', 'add a validation loop with debug logging', 1000); + execSync(`git update-ref refs/tracy-local/aaaa1111 ${aiCommit}`, { cwd: dir }); + + execSync('git add -A && git commit -q -m "add validation loop (AI assisted)"', { cwd: dir }); + execSync('git notes add -m "tracy-id: aaaa1111" HEAD', { cwd: dir }); + execSync(`git update-ref refs/tracy/aaaa1111 ${aiCommit}`, { cwd: dir }); + + // A later, separate human commit reindents the whole block AND renames + // a field, forcing git to bundle both duplicate lines into ONE hunk + // that's still similar enough overall to be "insignificant". + fs.writeFileSync(filePath, [ + 'def process(items):', + ' for item in items:', + ' if item.is_valid:', + ' log.debug("done")', + ' else:', + ' log.debug("done")', + ' return items', + '', + ].join('\n')); + execSync('git add -A && git commit -q -m "human reindents and renames"', { cwd: dir }); + + const result = await buildHistory(dir); + assert.strictEqual(result.ok, true); + if (!result.ok) { return; } + + const file = result.history.files.find(f => f.path === 'app.py'); + const tasklet = file?.tasklets.find(t => t.taskletId === 'tasklet-1'); + assert.ok(tasklet, 'tasklet-1 should still be attributed to some of the loop'); + assert.deepStrictEqual( + tasklet!.lines, + [2, 4, 5, 6], + 'both duplicate log lines (4 and 6) must independently survive — neither should collapse onto the other' + ); + }); + + test('two different tasklets each owning one occurrence of a duplicated line keep their own distinct positions', async () => { + // consumeAndShift is called separately per tasklet's own Change + // (propagateChanges maps over each Change independently), so this + // also exercises the cross-call case: two separate invocations against + // the same hunk must still agree on which duplicate maps to which + // tasklet, instead of one tasklet's line overwriting the other's. + const dir = makeTempDir(); + execSync('git init -q', { cwd: dir }); + execSync('git config user.email test@example.com', { cwd: dir }); + execSync('git config user.name Test', { cwd: dir }); + + const filePath = path.join(dir, 'app.py'); + fs.writeFileSync(filePath, [ + 'def process(a, b):', + ' pass', + '', + ].join('\n')); + execSync('git add app.py && git commit -q -m init', { cwd: dir }); + const baseCommit = execSync('git rev-parse HEAD', { cwd: dir, encoding: 'utf8' }).trim(); + + // tasklet1 (AI): adds branch a with a log line. + fs.writeFileSync(filePath, [ + 'def process(a, b):', + ' if a:', + ' log.info(\'processed\')', + ' pass', + '', + ].join('\n')); + execSync('git add app.py', { cwd: dir }); + const aiCommit1 = commitAiEdit(dir, baseCommit, 'tasklet-1', 'sess1', 'log when a is processed', 1000); + execSync(`git update-ref refs/tracy-local/aaaa1111 ${aiCommit1}`, { cwd: dir }); + execSync('git add -A && git commit -q -m "log when a is processed (AI assisted)"', { cwd: dir }); + const commitA = execSync('git rev-parse HEAD', { cwd: dir, encoding: 'utf8' }).trim(); + execSync('git notes add -m "tracy-id: aaaa1111" HEAD', { cwd: dir }); + execSync(`git update-ref refs/tracy/aaaa1111 ${aiCommit1}`, { cwd: dir }); + + // tasklet2 (AI, a SEPARATE later chain): adds an IDENTICAL log line for + // branch b. + fs.writeFileSync(filePath, [ + 'def process(a, b):', + ' if a:', + ' log.info(\'processed\')', + ' if b:', + ' log.info(\'processed\')', + ' pass', + '', + ].join('\n')); + execSync('git add app.py', { cwd: dir }); + const aiCommit2 = commitAiEdit(dir, commitA, 'tasklet-2', 'sess2', 'also log when b is processed', 2000); + execSync(`git update-ref refs/tracy-local/bbbb2222 ${aiCommit2}`, { cwd: dir }); + execSync('git add -A && git commit -q -m "also log when b is processed (AI assisted)"', { cwd: dir }); + execSync('git notes add -m "tracy-id: bbbb2222" HEAD', { cwd: dir }); + execSync(`git update-ref refs/tracy/bbbb2222 ${aiCommit2}`, { cwd: dir }); + + // Human commit: reindents the whole region, bundling both duplicates + // into one hunk. + fs.writeFileSync(filePath, [ + 'def process(a, b):', + ' if a:', + ' log.info(\'processed\')', + ' if b:', + ' log.info(\'processed\')', + ' pass', + '', + ].join('\n')); + execSync('git add -A && git commit -q -m "human reindents"', { cwd: dir }); + + const result = await buildHistory(dir); + assert.strictEqual(result.ok, true); + if (!result.ok) { return; } + + const file = result.history.files.find(f => f.path === 'app.py'); + const tasklet1 = file?.tasklets.find(t => t.taskletId === 'tasklet-1'); + const tasklet2 = file?.tasklets.find(t => t.taskletId === 'tasklet-2'); + + assert.deepStrictEqual(tasklet1?.lines, [2, 3], 'tasklet-1 should keep its own branch-a lines'); + assert.deepStrictEqual(tasklet2?.lines, [4, 5], 'tasklet-2 should keep its own branch-b lines, not tasklet-1\'s'); + }); +}); From 4d6323f38ea56498467fb96c849e65b76edc497a Mon Sep 17 00:00:00 2001 From: Esme Yi Date: Fri, 14 Aug 2026 15:56:33 +0200 Subject: [PATCH 04/10] Fix O(n*m) memory blowup in alignHunkLines on large hunks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review finding on this PR: alignHunkLines() unconditionally built a full (n+1)x(m+1) LCS table to align a hunk's old and new lines. A reformat, reorder, or bulk rename across a large AI-generated file can easily put thousands of lines into a single hunk (whitespace is stripped before the significance check, so a pure reindent stays "insignificant" and lands right in this path) — at that scale the DP table means tens to hundreds of millions of number slots for one hunk alone. Confirmed empirically: at 8,000 lines the old table pushed peak memory to ~620MB for a single hunk, growing quadratically from there. The prior implementation (independent per-line findIndex lookups) was linear, so this was a real regression, not a pre-existing cost. Replaced the LCS table with an O(n+m) hash-based pass for exact (whitespace-insensitive) matches: group new-line indices by content, then consume each group in order as old lines are walked. This still gives each duplicate line (`}`, `return;`, identical log statements) its own distinct occurrence — the property the LCS table existed for — without building a quadratic table. It's not strictly optimal the way LCS is (a pathological case with duplicates AND genuine reordering could pick a less-intuitive pairing), but it never produces a collision or an incorrect cross-tasklet overwrite, which is what actually mattered. The BLEU fuzzy-match fallback stays inherently O(unmatched x unmatched), so it's now bounded by FUZZY_MATCH_SEARCH_CAP (200,000 candidate pairs): past that, leftover lines are left unmatched rather than guessed at, consistent with this function's existing "drop rather than guess" philosophy. In practice this pass has little to do anyway once exact matching (which alone handles ordinary reformatting/reindentation) runs first. Verified all six previously-added repro scenarios (from this PR and the one before it) still produce identical results. Added a performance regression test: a 5,000-line single-hunk reindent now resolves in ~0.5s with ~0MB heap growth; the same scenario against the old LCS implementation took ~195MB of heap growth, confirming the test catches the regression this fix addresses. --- vscode-extension/src/history/buildHistory.ts | 107 ++++++++++-------- .../src/test/buildHistory.test.ts | 76 +++++++++++++ 2 files changed, 136 insertions(+), 47 deletions(-) diff --git a/vscode-extension/src/history/buildHistory.ts b/vscode-extension/src/history/buildHistory.ts index 95a6cd9..4059007 100644 --- a/vscode-extension/src/history/buildHistory.ts +++ b/vscode-extension/src/history/buildHistory.ts @@ -620,6 +620,17 @@ async function buildUncommittedChanges( return { uncommittedChanges: chainChanges.flat(), lastTracyTip }; } +// Bounds the BLEU fallback pass below: it's inherently O(unmatched old x +// unmatched new), and unlike the exact-match pass (an O(n+m) hash lookup, +// safe at any size) that cost doesn't have a cheap linear alternative. A +// large hunk that's mostly reformatting (the common "insignificant" +// case — whitespace is stripped before comparison, so reindentation exact- +// matches almost everything) leaves little for this pass to do regardless +// of hunk size. Past the cap, leftover lines are left unmatched rather +// than guessed at, consistent with this function's existing "drop rather +// than guess" fallback. +const FUZZY_MATCH_SEARCH_CAP = 200_000; + // Aligns a hunk's old lines to its new lines one-to-one and in order, // instead of matching each tracked line independently — independent // per-line lookups all resolve a duplicated line (`}`, `return;`, @@ -628,68 +639,70 @@ async function buildUncommittedChanges( // and losing attribution for the rest, or letting a later match overwrite // an earlier one that legitimately owns a different occurrence. // -// Exact (whitespace-insensitive) matches are aligned via LCS, which -// respects both order and duplicate multiplicity — two `}` lines in the -// old text land on two different `}` lines in the new text, not both on -// the first one. Whatever's left over falls back to the best remaining -// (not yet used) BLEU-similar candidate, so a restructuring that also -// tweaks content along the way still gets a best-effort match. +// Exact (whitespace-insensitive) matches are found via an O(n+m) hash +// lookup — grouping new-line indices by content and consuming each +// group in order as old lines are walked — which respects both order and +// duplicate multiplicity (two `}` lines in the old text land on two +// different `}` lines in the new text, not both on the first one) without +// the O(n*m) time and space a full LCS table would cost. A large hunk +// (reformatting or bulk-renaming an equally large file) can easily reach +// thousands of lines, where an O(n*m) table would mean hundreds of MB to +// GBs of allocation. Whatever's left over falls back to the best +// remaining BLEU-similar candidate, bounded by FUZZY_MATCH_SEARCH_CAP. function alignHunkLines(hunk: DiffHunk): Map { const oldLines = hunk.removedLines ?? []; const addedLines = hunk.addedLines ?? []; const normalize = (s: string) => s.replace(/\s+/g, ''); - const normalizedOld = oldLines.map(normalize); - const normalizedNew = addedLines.map(normalize); - - const n = normalizedOld.length; - const m = normalizedNew.length; - const dp: number[][] = Array.from({ length: n + 1 }, () => new Array(m + 1).fill(0)); - for (let i = n - 1; i >= 0; i--) { - for (let j = m - 1; j >= 0; j--) { - dp[i][j] = normalizedOld[i] === normalizedNew[j] - ? dp[i + 1][j + 1] + 1 - : Math.max(dp[i + 1][j], dp[i][j + 1]); + + const newIndicesByContent = new Map(); + addedLines.forEach((text, newIndex) => { + const key = normalize(text); + const bucket = newIndicesByContent.get(key); + if (bucket) { + bucket.push(newIndex); + } else { + newIndicesByContent.set(key, [newIndex]); } - } + }); const alignment = new Map(); const usedNewIndices = new Set(); - let i = 0; - let j = 0; - while (i < n && j < m) { - if (normalizedOld[i] === normalizedNew[j]) { - alignment.set(i, j); - usedNewIndices.add(j); - i++; - j++; - } else if (dp[i + 1][j] >= dp[i][j + 1]) { - i++; + const unmatchedOldIndices: number[] = []; + oldLines.forEach((text, oldIndex) => { + const bucket = newIndicesByContent.get(normalize(text)); + const newIndex = bucket?.shift(); + if (newIndex !== undefined) { + alignment.set(oldIndex, newIndex); + usedNewIndices.add(newIndex); } else { - j++; + unmatchedOldIndices.push(oldIndex); } - } + }); - for (let oldIndex = 0; oldIndex < n; oldIndex++) { - if (alignment.has(oldIndex)) { - continue; + const unmatchedNewIndices: number[] = []; + for (let newIndex = 0; newIndex < addedLines.length; newIndex++) { + if (!usedNewIndices.has(newIndex)) { + unmatchedNewIndices.push(newIndex); } + } - let bestIndex = -1; - let bestScore = -1; - for (let newIndex = 0; newIndex < m; newIndex++) { - if (usedNewIndices.has(newIndex)) { - continue; - } - const score = bleuSimilarity(oldLines[oldIndex], addedLines[newIndex]); - if (score > bestScore) { - bestScore = score; - bestIndex = newIndex; + if (unmatchedOldIndices.length * unmatchedNewIndices.length <= FUZZY_MATCH_SEARCH_CAP) { + const remainingNewIndices = new Set(unmatchedNewIndices); + for (const oldIndex of unmatchedOldIndices) { + let bestIndex = -1; + let bestScore = -1; + for (const newIndex of remainingNewIndices) { + const score = bleuSimilarity(oldLines[oldIndex], addedLines[newIndex]); + if (score > bestScore) { + bestScore = score; + bestIndex = newIndex; + } } - } - if (bestIndex !== -1 && bestScore > SIMILARITY_THRESHOLD) { - alignment.set(oldIndex, bestIndex); - usedNewIndices.add(bestIndex); + if (bestIndex !== -1 && bestScore > SIMILARITY_THRESHOLD) { + alignment.set(oldIndex, bestIndex); + remainingNewIndices.delete(bestIndex); + } } } diff --git a/vscode-extension/src/test/buildHistory.test.ts b/vscode-extension/src/test/buildHistory.test.ts index 0808e21..baaca4a 100644 --- a/vscode-extension/src/test/buildHistory.test.ts +++ b/vscode-extension/src/test/buildHistory.test.ts @@ -548,3 +548,79 @@ suite('buildHistory aligns duplicate lines within a hunk one-to-one', () => { assert.deepStrictEqual(tasklet2?.lines, [4, 5], 'tasklet-2 should keep its own branch-b lines, not tasklet-1\'s'); }); }); + +suite('buildHistory stays fast and memory-bounded on a large single hunk', () => { + test('reformatting a large AI-generated file (one big insignificant hunk) resolves quickly without excess memory growth', async function () { + // A full-file reformat (e.g. running a formatter over an AI-generated + // file) touches every line, so git can produce ONE hunk spanning the + // whole file — thousands of lines on both sides. An O(n*m) alignment + // (a full LCS table) would allocate on the order of n*m number slots + // for that single hunk: harmless at a few hundred lines, but hundreds + // of MB to GB once a hunk reaches the thousands, which a large + // generated file crosses easily. The alignment must stay roughly + // linear in hunk size instead. + this.timeout(20000); + + const lineCount = 5000; + const dir = makeTempDir(); + execSync('git init -q', { cwd: dir }); + execSync('git config user.email test@example.com', { cwd: dir }); + execSync('git config user.name Test', { cwd: dir }); + + const filePath = path.join(dir, 'app.py'); + fs.writeFileSync(filePath, '# placeholder\n'); + execSync('git add app.py && git commit -q -m init', { cwd: dir }); + const baseCommit = execSync('git rev-parse HEAD', { cwd: dir, encoding: 'utf8' }).trim(); + + const buildLines = (indent: string) => { + const out = ['def process():']; + for (let i = 0; i < lineCount; i++) { + out.push(`${indent}log.debug("line ${i}")`); + } + out.push(`${indent}return None`); + out.push(''); + return out.join('\n'); + }; + + fs.writeFileSync(filePath, buildLines(' ')); + execSync('git add app.py', { cwd: dir }); + const aiCommit = commitAiEdit(dir, baseCommit, 'tasklet-1', 'sess1', 'generate a large function', 1000); + execSync(`git update-ref refs/tracy-local/aaaa1111 ${aiCommit}`, { cwd: dir }); + execSync('git add -A && git commit -q -m "generate large function (AI assisted)"', { cwd: dir }); + execSync('git notes add -m "tracy-id: aaaa1111" HEAD', { cwd: dir }); + execSync(`git update-ref refs/tracy/aaaa1111 ${aiCommit}`, { cwd: dir }); + + // Human commit: reindent the entire block — every line changes, so the + // whole thing lands in one hunk that's still similarity-wise + // "insignificant" (whitespace is stripped before comparison). + fs.writeFileSync(filePath, buildLines(' ')); + execSync('git add -A && git commit -q -m "human reindents the whole file"', { cwd: dir }); + + const heapBefore = process.memoryUsage().heapUsed; + const startedAt = Date.now(); + const result = await buildHistory(dir); + const elapsedMs = Date.now() - startedAt; + const heapGrowthMb = (process.memoryUsage().heapUsed - heapBefore) / (1024 * 1024); + + assert.strictEqual(result.ok, true); + if (!result.ok) { return; } + + const file = result.history.files.find(f => f.path === 'app.py'); + const tasklet = file?.tasklets.find(t => t.taskletId === 'tasklet-1'); + assert.ok(tasklet, 'the large reformatted function should still be attributed'); + assert.strictEqual( + tasklet!.lines.length, + lineCount + 2, + 'every reformatted line (plus the def and return lines) should still resolve to its own distinct position' + ); + + assert.ok( + elapsedMs < 5000, + `expected a ${lineCount}-line single-hunk alignment to resolve in well under 5s, took ${elapsedMs}ms — an O(n*m) alignment would scale quadratically here` + ); + assert.ok( + heapGrowthMb < 150, + `expected heap growth well under 150MB for a ${lineCount}-line hunk, saw ${heapGrowthMb.toFixed(1)}MB — an O(n*m) alignment table would allocate on the order of ${lineCount}^2 number slots` + ); + }); +}); From a1a58e54ad0ab30a76e1453ab2da14a9f9f0718a Mon Sep 17 00:00:00 2001 From: Esme Yi Date: Fri, 14 Aug 2026 16:17:04 +0200 Subject: [PATCH 05/10] Fix O(k^2) bucket consumption via Array.shift() in alignHunkLines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review finding on this PR: the exact-match pass in alignHunkLines() (added to fix the O(n*m) LCS table) grouped new-line indices by content into buckets, then consumed each bucket with Array.prototype.shift(). shift() shifts every remaining element down by one index, so it's O(k) per call — and a hunk with many identical lines (`}`, blank lines, templated log statements) means the SAME bucket gets shifted repeatedly, for a total cost of O(k^2) on that bucket alone. The previous performance test used lines with unique content (each line embeds its own index), so every bucket had exactly one entry and never exercised this path. Confirmed empirically with an isolated microbenchmark of shift() alone: 30k elements ~50ms, 60k ~219ms, 120k ~901ms — a clean quadratic curve. Reproduced in the full pipeline with a 300,000-line single-hunk, all-identical-content reindent: ~11.7s pre-fix vs ~6.1s after. Fixed by tracking a read cursor per content bucket instead of mutating the array — each consumption becomes an O(1) map lookup plus array index, with no shifting. Every previously-passing scenario (six repro tests across this PR and the one before it) still produces identical results. Added a dedicated large-hunk-with-repeated-content performance test, separate from the existing large-unique-content one, since that one doesn't build large buckets and can't catch this specific cost. --- vscode-extension/src/history/buildHistory.ts | 16 +++- .../src/test/buildHistory.test.ts | 74 +++++++++++++++++++ 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/vscode-extension/src/history/buildHistory.ts b/vscode-extension/src/history/buildHistory.ts index 4059007..e6fa026 100644 --- a/vscode-extension/src/history/buildHistory.ts +++ b/vscode-extension/src/history/buildHistory.ts @@ -665,13 +665,23 @@ function alignHunkLines(hunk: DiffHunk): Map { } }); + // Consumed via a per-bucket cursor rather than bucket.shift(): shift() + // shifts every remaining element down by one, so repeatedly shifting the + // SAME bucket (a hunk with many identical lines — `}`, blank lines, + // templated log statements — is exactly when this bucket gets consumed + // over and over) is O(k) per call, O(k^2) total for that bucket alone. + // A cursor makes each consumption O(1) regardless of bucket size. + const bucketCursors = new Map(); const alignment = new Map(); const usedNewIndices = new Set(); const unmatchedOldIndices: number[] = []; oldLines.forEach((text, oldIndex) => { - const bucket = newIndicesByContent.get(normalize(text)); - const newIndex = bucket?.shift(); - if (newIndex !== undefined) { + const key = normalize(text); + const bucket = newIndicesByContent.get(key); + const cursor = bucketCursors.get(key) ?? 0; + if (bucket && cursor < bucket.length) { + const newIndex = bucket[cursor]; + bucketCursors.set(key, cursor + 1); alignment.set(oldIndex, newIndex); usedNewIndices.add(newIndex); } else { diff --git a/vscode-extension/src/test/buildHistory.test.ts b/vscode-extension/src/test/buildHistory.test.ts index baaca4a..ab2c7e6 100644 --- a/vscode-extension/src/test/buildHistory.test.ts +++ b/vscode-extension/src/test/buildHistory.test.ts @@ -623,4 +623,78 @@ suite('buildHistory stays fast and memory-bounded on a large single hunk', () => `expected heap growth well under 150MB for a ${lineCount}-line hunk, saw ${heapGrowthMb.toFixed(1)}MB — an O(n*m) alignment table would allocate on the order of ${lineCount}^2 number slots` ); }); + + test('a large hunk where every line is identical resolves without quadratic slowdown', async function () { + // The previous test's lines are all distinct, so it can't catch a + // different cost: consuming a per-content bucket of matched new-line + // indices via Array.prototype.shift() is O(k) per call — shift() + // shifts every remaining element down by one — so repeatedly shifting + // the SAME bucket (which is exactly what happens when a hunk has many + // identical lines: `}`, blank lines, templated log statements) costs + // O(k^2) for that bucket alone, even though the rest of the alignment + // is linear. This needs a much larger line count than the previous + // test before that quadratic term dominates the (still-linear) cost + // of everything else in the pipeline (diff parsing, file I/O). + this.timeout(60000); + + const lineCount = 300000; + const dir = makeTempDir(); + execSync('git init -q', { cwd: dir }); + execSync('git config user.email test@example.com', { cwd: dir }); + execSync('git config user.name Test', { cwd: dir }); + + const filePath = path.join(dir, 'app.py'); + fs.writeFileSync(filePath, '# placeholder\n'); + execSync('git add app.py && git commit -q -m init', { cwd: dir }); + const baseCommit = execSync('git rev-parse HEAD', { cwd: dir, encoding: 'utf8' }).trim(); + + const buildLines = (indent: string) => { + const out = ['def process():']; + for (let i = 0; i < lineCount; i++) { + out.push(`${indent}log.debug("tick")`); + } + out.push(`${indent}return None`); + out.push(''); + return out.join('\n'); + }; + + fs.writeFileSync(filePath, buildLines(' ')); + execSync('git add app.py', { cwd: dir }); + const aiCommit = commitAiEdit(dir, baseCommit, 'tasklet-1', 'sess1', 'generate a large function with repeated log lines', 1000); + execSync(`git update-ref refs/tracy-local/aaaa1111 ${aiCommit}`, { cwd: dir }); + execSync('git add -A && git commit -q -m "generate large function (AI assisted)"', { cwd: dir }); + execSync('git notes add -m "tracy-id: aaaa1111" HEAD', { cwd: dir }); + execSync(`git update-ref refs/tracy/aaaa1111 ${aiCommit}`, { cwd: dir }); + + // Human commit: reindent the entire block of identical lines — one + // giant hunk, one giant content bucket. + fs.writeFileSync(filePath, buildLines(' ')); + execSync('git add -A && git commit -q -m "human reindents the whole file"', { cwd: dir }); + + const startedAt = Date.now(); + const result = await buildHistory(dir); + const elapsedMs = Date.now() - startedAt; + + assert.strictEqual(result.ok, true); + if (!result.ok) { return; } + + const file = result.history.files.find(f => f.path === 'app.py'); + const tasklet = file?.tasklets.find(t => t.taskletId === 'tasklet-1'); + assert.ok(tasklet, 'the large reformatted function should still be attributed'); + assert.strictEqual( + tasklet!.lines.length, + lineCount + 2, + 'every reformatted line (plus the def and return lines) should still resolve to its own distinct position' + ); + + // Measured on a dev machine: a cursor-based bucket consumption + // resolves this in ~6s; the pre-fix Array.shift()-based consumption + // took ~11.7s for the same input. The bound below leaves generous + // margin over the fixed-code time for slower CI hardware while still + // sitting below the pre-fix time. + assert.ok( + elapsedMs < 10000, + `expected a ${lineCount}-line single-hunk alignment with all-identical content to resolve in well under 10s, took ${elapsedMs}ms — repeatedly Array.shift()-ing the same content bucket is O(k^2) for a bucket of this size` + ); + }); }); From dd2680f9c846a5fd50c64492a7c69bead3f430df Mon Sep 17 00:00:00 2001 From: Esme Yi Date: Sun, 16 Aug 2026 15:30:22 +0200 Subject: [PATCH 06/10] Fix BLEU fallback dropping whole hunks past a total-size cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review finding on this PR: the BLEU fuzzy-match fallback gated the ENTIRE pass on the total unmatched-old x unmatched-new product against a fixed cap (FUZZY_MATCH_SEARCH_CAP = 200,000). Past that cap, the whole fallback was skipped rather than searching less — so a hunk with enough purely-unmatched lines (e.g. a uniform field rename applied throughout a large AI-generated block, ~450 lines on each side with none exact- matching post-rename) lost attribution for the ENTIRE hunk, not just the lines a bounded search couldn't resolve. This is exactly the case the fallback exists to handle, so the cap turned a precision trade-off into a functional regression: confirmed empirically with a 500-line uniform rename (500*500 = 250,000, over the cap) — only the 2 untouched lines (def/return) kept attribution; the 500 renamed lines were dropped entirely. Replaced the total-size gate with a fixed-size search window per old line: a restructuring or in-place rename rarely moves a line far from its proportional position in the hunk, so each unmatched old line searches outward from its expected position (estimated by linear scaling across the hunk) instead of scanning every remaining candidate. This bounds cost per line regardless of hunk size — no all-or-nothing cliff — while still giving every old line a real, local search. The window alone reintroduced a different cost: always scanning the full window even after finding an obviously-correct match at offset 0 (the common case for an in-place rename) made a 5,000-line rename take ~67s. Added an early exit: once a match already above the significance threshold has gone unbeaten for 20 consecutive offsets, stop searching. Brought the same case down to ~3.5s while keeping identical results. Verified: the 500-line rename now correctly attributes all 502 lines (previously only 2). Re-verified all six repro scenarios from this PR and #139 still produce identical results, and the two existing performance tests (large unique-content and large repeated-content hunks) are unaffected. Added a committed regression test for this exact scenario. --- vscode-extension/src/history/buildHistory.ts | 73 +++++++++++++++---- .../src/test/buildHistory.test.ts | 60 +++++++++++++++ 2 files changed, 117 insertions(+), 16 deletions(-) diff --git a/vscode-extension/src/history/buildHistory.ts b/vscode-extension/src/history/buildHistory.ts index e6fa026..aa63199 100644 --- a/vscode-extension/src/history/buildHistory.ts +++ b/vscode-extension/src/history/buildHistory.ts @@ -620,16 +620,23 @@ async function buildUncommittedChanges( return { uncommittedChanges: chainChanges.flat(), lastTracyTip }; } -// Bounds the BLEU fallback pass below: it's inherently O(unmatched old x -// unmatched new), and unlike the exact-match pass (an O(n+m) hash lookup, -// safe at any size) that cost doesn't have a cheap linear alternative. A -// large hunk that's mostly reformatting (the common "insignificant" -// case — whitespace is stripped before comparison, so reindentation exact- -// matches almost everything) leaves little for this pass to do regardless -// of hunk size. Past the cap, leftover lines are left unmatched rather -// than guessed at, consistent with this function's existing "drop rather -// than guess" fallback. -const FUZZY_MATCH_SEARCH_CAP = 200_000; +// Bounds the BLEU fallback pass below to a fixed-size window per old line +// rather than searching every remaining candidate. Gating on the total +// unmatched-old x unmatched-new product (an earlier version of this cap) +// meant that once a hunk had enough purely-unmatched lines — e.g. a +// uniform field rename applied throughout, ~450 lines on each side, none +// of which exact-match — the ENTIRE fallback got skipped, dropping +// attribution for a whole hunk that's exactly the case this fallback +// exists for. A per-line window keeps cost bounded (linear in hunk size) +// without that all-or-nothing cliff: every unmatched old line still gets +// a real, bounded search. +const FUZZY_MATCH_WINDOW = 500; +// Once a match's score has gone unbeaten for this many consecutive +// offsets (and already clears the significance threshold), stop +// searching — the common case (an in-place rename, no reordering) finds +// its match at or near offset 0 and gains nothing from scanning the rest +// of the window. +const FUZZY_MATCH_EARLY_EXIT_PATIENCE = 20; // Aligns a hunk's old lines to its new lines one-to-one and in order, // instead of matching each tracked line independently — independent @@ -696,16 +703,50 @@ function alignHunkLines(hunk: DiffHunk): Map { } } - if (unmatchedOldIndices.length * unmatchedNewIndices.length <= FUZZY_MATCH_SEARCH_CAP) { + if (unmatchedOldIndices.length > 0 && unmatchedNewIndices.length > 0) { const remainingNewIndices = new Set(unmatchedNewIndices); + const oldCount = oldLines.length; + const newCount = addedLines.length; + for (const oldIndex of unmatchedOldIndices) { + // A restructuring or in-place rename rarely moves a line far from + // its proportional position in the hunk, so search outward from + // where it's expected to land instead of scanning every remaining + // candidate. + const estimatedNewIndex = oldCount > 1 + ? Math.round((oldIndex * (newCount - 1)) / (oldCount - 1)) + : 0; + + // Stop early once a confidently-good match has been sitting + // unbeaten for a while — without this, every line would always + // scan the full window even after finding an obviously-correct + // match at offset 0 (the common case for an in-place rename that + // doesn't reorder anything), which is what actually made this loop + // slow in practice, not the window bound itself. let bestIndex = -1; let bestScore = -1; - for (const newIndex of remainingNewIndices) { - const score = bleuSimilarity(oldLines[oldIndex], addedLines[newIndex]); - if (score > bestScore) { - bestScore = score; - bestIndex = newIndex; + let noImprovementStreak = 0; + for (let offset = 0; offset <= FUZZY_MATCH_WINDOW; offset++) { + const candidates = offset === 0 + ? [estimatedNewIndex] + : [estimatedNewIndex - offset, estimatedNewIndex + offset]; + + let improved = false; + for (const candidate of candidates) { + if (candidate < 0 || candidate >= newCount || !remainingNewIndices.has(candidate)) { + continue; + } + const score = bleuSimilarity(oldLines[oldIndex], addedLines[candidate]); + if (score > bestScore) { + bestScore = score; + bestIndex = candidate; + improved = true; + } + } + + noImprovementStreak = improved ? 0 : noImprovementStreak + 1; + if (bestScore > SIMILARITY_THRESHOLD && noImprovementStreak >= FUZZY_MATCH_EARLY_EXIT_PATIENCE) { + break; } } diff --git a/vscode-extension/src/test/buildHistory.test.ts b/vscode-extension/src/test/buildHistory.test.ts index ab2c7e6..532cc4a 100644 --- a/vscode-extension/src/test/buildHistory.test.ts +++ b/vscode-extension/src/test/buildHistory.test.ts @@ -697,4 +697,64 @@ suite('buildHistory stays fast and memory-bounded on a large single hunk', () => `expected a ${lineCount}-line single-hunk alignment with all-identical content to resolve in well under 10s, took ${elapsedMs}ms — repeatedly Array.shift()-ing the same content bucket is O(k^2) for a bucket of this size` ); }); + + test('a large uniform rename with no exact matches still attributes every line, not just the untouched ones', async function () { + // A prior version of the BLEU fallback gated the ENTIRE pass on the + // total unmatched-old x unmatched-new product: past a fixed cap, it + // skipped fuzzy matching altogether rather than searching less. A + // uniform rename applied throughout a large AI-generated block (every + // line references the renamed identifier, so NOTHING exact-matches + // after the rename) crosses that cap easily and lost attribution for + // the whole hunk — not a precision trade-off, a functional regression, + // since this is exactly the case the fallback exists to handle. + this.timeout(30000); + + const lineCount = 500; // 500*500 = 250,000, over the old 200,000 cap + const dir = makeTempDir(); + execSync('git init -q', { cwd: dir }); + execSync('git config user.email test@example.com', { cwd: dir }); + execSync('git config user.name Test', { cwd: dir }); + + const filePath = path.join(dir, 'app.py'); + fs.writeFileSync(filePath, '# placeholder\n'); + execSync('git add app.py && git commit -q -m init', { cwd: dir }); + const baseCommit = execSync('git rev-parse HEAD', { cwd: dir, encoding: 'utf8' }).trim(); + + const buildLines = (fieldName: string) => { + const out = ['def process(items):']; + for (let i = 0; i < lineCount; i++) { + out.push(` record${i} = load(${fieldName}=${i})`); + } + out.push(' return None'); + out.push(''); + return out.join('\n'); + }; + + fs.writeFileSync(filePath, buildLines('user_id')); + execSync('git add app.py', { cwd: dir }); + const aiCommit = commitAiEdit(dir, baseCommit, 'tasklet-1', 'sess1', 'generate a large function using user_id', 1000); + execSync(`git update-ref refs/tracy-local/aaaa1111 ${aiCommit}`, { cwd: dir }); + execSync('git add -A && git commit -q -m "generate large function (AI assisted)"', { cwd: dir }); + execSync('git notes add -m "tracy-id: aaaa1111" HEAD', { cwd: dir }); + execSync(`git update-ref refs/tracy/aaaa1111 ${aiCommit}`, { cwd: dir }); + + // Human commit: rename user_id -> userId everywhere. Every line + // changes (nothing exact-matches), but each is still highly similar + // to its own counterpart. + fs.writeFileSync(filePath, buildLines('userId')); + execSync('git add -A && git commit -q -m "human renames user_id to userId everywhere"', { cwd: dir }); + + const result = await buildHistory(dir); + assert.strictEqual(result.ok, true); + if (!result.ok) { return; } + + const file = result.history.files.find(f => f.path === 'app.py'); + const tasklet = file?.tasklets.find(t => t.taskletId === 'tasklet-1'); + assert.ok(tasklet, 'the large renamed function should still be attributed'); + assert.strictEqual( + tasklet!.lines.length, + lineCount + 2, + 'every renamed line (plus the def and return lines) should still resolve to its own position — not just the two untouched lines' + ); + }); }); From f1265253a9e51014cfb25f9bf05685cfb28e2636 Mon Sep 17 00:00:00 2001 From: Esme Yi Date: Sun, 16 Aug 2026 15:57:19 +0200 Subject: [PATCH 07/10] Fix wrong-position matches in the BLEU fallback near ambiguous content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review finding on this PR: the fixed-size proportional estimate assumes a hunk's line-count change is spread evenly across it. When it's actually concentrated at one end — e.g. a block of lines inserted right before a large renamed section — an old line near that end gets an estimate off by roughly the insertion size. Combined with the early exit, if something else in the hunk happens to be near-duplicate content (templated/generated code often is), the search could settle on that wrong occurrence before ever reaching the real match, which sits further out but is still within the window. Not a missed match — a wrong one. Two changes: 1. Estimate the expected position by interpolating between the nearest exact-match anchors on either side of the gap (already found by the earlier exact-match pass) instead of a single global proportional scale. An anchor's position is exactly correct by construction, so this corrects for insertions/deletions concentrated anywhere in the hunk, not just ones spread evenly. Falls back to the hunk's own boundaries when there are no anchors nearby (a hunk with no exact matches at all, which is exactly the case this whole fallback exists for). 2. Scale the early-exit patience by the local line-count drift for that gap (nextAnchorNew - prevAnchorNew vs nextAnchorOld - prevAnchorOld), so the search can't settle before at least reaching the position the gap's own count change implies — and prefer the later candidate on an exact score tie, so that when a nearby wrong candidate and the position accounting for the drift score identically, the one that reflects the actual shift wins instead of whichever was found first. Verified with a repro that isolates the mechanism: 50 lines inserted right before a 500-line renamed block, reusing indices 0..49 so the inserted lines become byte-identical to the first 50 lines of the real (shifted) block — the sharpest version of "templated lines similar enough to collide". Before this fix, all 50 of those lines were pulled onto the inserted duplicate instead of the real block; after, none are. Re-verified all seven prior repro scenarios and all three performance tests in this PR still produce identical results. --- vscode-extension/src/history/buildHistory.ts | 64 ++++++++++--- .../src/test/buildHistory.test.ts | 89 +++++++++++++++++++ 2 files changed, 143 insertions(+), 10 deletions(-) diff --git a/vscode-extension/src/history/buildHistory.ts b/vscode-extension/src/history/buildHistory.ts index aa63199..8e1bff1 100644 --- a/vscode-extension/src/history/buildHistory.ts +++ b/vscode-extension/src/history/buildHistory.ts @@ -708,21 +708,58 @@ function alignHunkLines(hunk: DiffHunk): Map { const oldCount = oldLines.length; const newCount = addedLines.length; + // Anchors: the exact matches already found above, used to LOCALLY + // estimate an unmatched line's expected position instead of scaling + // it against the hunk as a whole. A single global scale is wrong by + // however many lines were inserted/deleted before this point in the + // hunk — e.g. ~50 lines inserted right before a large renamed block + // shifts every renamed line's true position by ~50, but a global + // estimate only accounts for a fraction of that, landing the search + // window's center nowhere near the real match deep inside the block. + // Anchored to the exact matches immediately surrounding each gap + // instead — which, being exact, already reflect the true offset at + // that point exactly — the very first candidate checked is already + // close to correct. unmatchedOldIndices is walked in ascending order, + // so a single forward-moving pointer through the sorted anchors is + // enough; no need to re-search from the start each time. + const anchorOldIndices = Array.from(alignment.keys()).sort((a, b) => a - b); + let anchorPointer = 0; + for (const oldIndex of unmatchedOldIndices) { - // A restructuring or in-place rename rarely moves a line far from - // its proportional position in the hunk, so search outward from - // where it's expected to land instead of scanning every remaining - // candidate. - const estimatedNewIndex = oldCount > 1 - ? Math.round((oldIndex * (newCount - 1)) / (oldCount - 1)) - : 0; + while (anchorPointer < anchorOldIndices.length && anchorOldIndices[anchorPointer] < oldIndex) { + anchorPointer++; + } + const prevAnchorOld = anchorPointer > 0 ? anchorOldIndices[anchorPointer - 1] : -1; + const prevAnchorNew = anchorPointer > 0 ? alignment.get(prevAnchorOld)! : -1; + const nextAnchorOld = anchorPointer < anchorOldIndices.length ? anchorOldIndices[anchorPointer] : oldCount; + const nextAnchorNew = anchorPointer < anchorOldIndices.length ? alignment.get(nextAnchorOld)! : newCount; + + const span = nextAnchorOld - prevAnchorOld; + const estimatedNewIndex = span > 0 + ? Math.round(prevAnchorNew + ((oldIndex - prevAnchorOld) * (nextAnchorNew - prevAnchorNew)) / span) + : prevAnchorNew + 1; + + // How many net lines were inserted/deleted within this specific + // anchor-bounded gap (the whole hunk, when there are no anchors at + // all — exactly the "big rename hunk, nothing exact-matches" case). + // The estimate above assumes that shift is spread proportionally + // across the gap; if it's actually concentrated at one end (e.g. a + // block of lines inserted right before a renamed section), an old + // line near that end can have templated/near-duplicate content + // elsewhere in the gap score just as well as its real match, which + // — without a bound tied to the shift itself — patience-based + // exit could settle on before the search ever reaches the real one. + const localDrift = Math.abs((nextAnchorNew - prevAnchorNew) - (nextAnchorOld - prevAnchorOld)); + const patience = Math.min(FUZZY_MATCH_WINDOW, Math.max(FUZZY_MATCH_EARLY_EXIT_PATIENCE, localDrift)); // Stop early once a confidently-good match has been sitting // unbeaten for a while — without this, every line would always // scan the full window even after finding an obviously-correct // match at offset 0 (the common case for an in-place rename that // doesn't reorder anything), which is what actually made this loop - // slow in practice, not the window bound itself. + // slow in practice, not the window bound itself. Patience scales + // with localDrift so a search can't settle before at least reaching + // the position the gap's own line-count change implies. let bestIndex = -1; let bestScore = -1; let noImprovementStreak = 0; @@ -737,7 +774,14 @@ function alignHunkLines(hunk: DiffHunk): Map { continue; } const score = bleuSimilarity(oldLines[oldIndex], addedLines[candidate]); - if (score > bestScore) { + // >= rather than >: on an exact tie, prefer the farther-explored + // candidate. Combined with drift-scaled patience, a tie between + // a nearby candidate and one found only after searching out to + // localDrift favors the position that actually accounts for the + // shift — the more likely correct one when the gap's own line + // count changed, rather than an arbitrary "whichever was found + // first" tiebreak. + if (score >= bestScore) { bestScore = score; bestIndex = candidate; improved = true; @@ -745,7 +789,7 @@ function alignHunkLines(hunk: DiffHunk): Map { } noImprovementStreak = improved ? 0 : noImprovementStreak + 1; - if (bestScore > SIMILARITY_THRESHOLD && noImprovementStreak >= FUZZY_MATCH_EARLY_EXIT_PATIENCE) { + if (bestScore > SIMILARITY_THRESHOLD && noImprovementStreak >= patience) { break; } } diff --git a/vscode-extension/src/test/buildHistory.test.ts b/vscode-extension/src/test/buildHistory.test.ts index 532cc4a..f4a99e8 100644 --- a/vscode-extension/src/test/buildHistory.test.ts +++ b/vscode-extension/src/test/buildHistory.test.ts @@ -757,4 +757,93 @@ suite('buildHistory stays fast and memory-bounded on a large single hunk', () => 'every renamed line (plus the def and return lines) should still resolve to its own position — not just the two untouched lines' ); }); + + test('lines inserted before a large rename do not pull AI-attributed lines onto the wrong occurrence', async function () { + // Code review finding: a fixed-size proportional estimate assumes the + // hunk's own line-count change is spread evenly across it. If it's + // actually concentrated at one end — e.g. a block of lines inserted + // right before a large renamed section — an old line near that end + // gets an estimate that's off by roughly the insertion size, and if + // something else in the hunk happens to be near-duplicate content + // (templated code very often is), the search can settle on that wrong + // occurrence via early exit before ever reaching the real one, which + // sits further out but still within the search window. + // + // This reproduces it precisely: 50 lines are inserted right before + // tasklet-1's 500-line renamed block, reusing indices 0..49 with + // content that becomes BYTE-IDENTICAL to the first 50 lines of + // tasklet-1's own (renamed) block — the sharpest version of "templated + // lines that are highly similar to each other." A naive estimate + // lands the search right on the inserted duplicate first. + this.timeout(20000); + + const lineCount = 500; + const insertedCount = 50; + const dir = makeTempDir(); + execSync('git init -q', { cwd: dir }); + execSync('git config user.email test@example.com', { cwd: dir }); + execSync('git config user.name Test', { cwd: dir }); + + const filePath = path.join(dir, 'app.py'); + fs.writeFileSync(filePath, '# placeholder\n'); + execSync('git add app.py && git commit -q -m init', { cwd: dir }); + const baseCommit = execSync('git rev-parse HEAD', { cwd: dir, encoding: 'utf8' }).trim(); + + const buildAiLines = () => { + const out = ['def process(items):']; + for (let i = 0; i < lineCount; i++) { + out.push(` record${i} = load(user_id=${i})`); + } + out.push(' return None'); + out.push(''); + return out.join('\n'); + }; + + fs.writeFileSync(filePath, buildAiLines()); + execSync('git add app.py', { cwd: dir }); + const aiCommit = commitAiEdit(dir, baseCommit, 'tasklet-1', 'sess1', 'generate a large function using user_id', 1000); + execSync(`git update-ref refs/tracy-local/aaaa1111 ${aiCommit}`, { cwd: dir }); + execSync('git add -A && git commit -q -m "generate large function (AI assisted)"', { cwd: dir }); + execSync('git notes add -m "tracy-id: aaaa1111" HEAD', { cwd: dir }); + execSync(`git update-ref refs/tracy/aaaa1111 ${aiCommit}`, { cwd: dir }); + + // Human commit: insert 50 lines reusing indices 0..49 right before the + // block, and rename user_id -> userId across the real block too. After + // the rename, indices 0..49 exist twice in the new file — once as the + // inserted lines, once as the true (shifted) AI content, byte- + // identical to each other. + const buildRenamedLines = () => { + const out = ['def process(items):']; + for (let i = 0; i < insertedCount; i++) { + out.push(` record${i} = load(userId=${i})`); + } + for (let i = 0; i < lineCount; i++) { + out.push(` record${i} = load(userId=${i})`); + } + out.push(' return None'); + out.push(''); + return out.join('\n'); + }; + fs.writeFileSync(filePath, buildRenamedLines()); + execSync('git add -A && git commit -q -m "human inserts duplicate-numbered lines and renames user_id to userId"', { cwd: dir }); + + const result = await buildHistory(dir); + assert.strictEqual(result.ok, true); + if (!result.ok) { return; } + + const file = result.history.files.find(f => f.path === 'app.py'); + const tasklet = file?.tasklets.find(t => t.taskletId === 'tasklet-1'); + assert.ok(tasklet, 'the large renamed function should still be attributed'); + + // The inserted duplicate occupies new lines [2, 1 + insertedCount]. + // None of tasklet-1's own lines should land there. + const wrongRangeStart = 2; + const wrongRangeEnd = 1 + insertedCount; + const misattributed = tasklet!.lines.filter(l => l >= wrongRangeStart && l <= wrongRangeEnd); + assert.deepStrictEqual( + misattributed, + [], + `expected none of tasklet-1's lines to land in the inserted-duplicate range [${wrongRangeStart},${wrongRangeEnd}], got ${JSON.stringify(misattributed)}` + ); + }); }); From 2ee4940ed896d44718bd45da0695f2f4cfbfd575 Mon Sep 17 00:00:00 2001 From: Esme Yi Date: Sun, 16 Aug 2026 16:10:40 +0200 Subject: [PATCH 08/10] Fix ties defeating the fuzzy-match early exit on massively-repeated content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review finding on this PR: the previous commit's tie-break (prefer the farther-explored candidate on an exact score tie, so a real match found after accounting for drift wins over an earlier, wrong one) treated every tie as "progress" and reset the patience counter. A hunk where every old line is identical to every other — a real pattern: the same templated statement repeated verbatim thousands of times — and gets uniformly renamed means every old line's fuzzy search sees thousands of candidates that all score identically. Patience never accumulates, so every single line scans the full search window regardless of the patience setting. Confirmed empirically: ~26s for a 5,000-line uniform rename where every line is identical, versus ~0.4-3.5s for the earlier tests that used unique-per-line content and never actually exercised a long run of ties. Fixed by separating "does this become the new pick" from "does this count as progress": a strict score improvement does both (updates the pick, resets patience). An exact tie still updates the pick — preserving the drift-direction tie-break from the previous commit — but does NOT reset patience, so a long run of tied candidates no longer defeats the exit. Brought the 5,000-line uniform-rename case down to ~2s (from ~26s) while keeping correctness: re-verified the previous commit's drift/tie-break repro (50 lines inserted before a 500-line rename, duplicate indices) still has zero misattribution, and all other repro and performance scenarios in this PR are unaffected. Added a dedicated regression test for this exact case. --- vscode-extension/src/history/buildHistory.ts | 23 ++++-- .../src/test/buildHistory.test.ts | 73 +++++++++++++++++++ 2 files changed, 88 insertions(+), 8 deletions(-) diff --git a/vscode-extension/src/history/buildHistory.ts b/vscode-extension/src/history/buildHistory.ts index 8e1bff1..fbd7053 100644 --- a/vscode-extension/src/history/buildHistory.ts +++ b/vscode-extension/src/history/buildHistory.ts @@ -768,23 +768,30 @@ function alignHunkLines(hunk: DiffHunk): Map { ? [estimatedNewIndex] : [estimatedNewIndex - offset, estimatedNewIndex + offset]; + // A strict improvement both updates the pick AND counts as + // progress (resets the patience counter below). An exact tie + // still updates the pick — preferring the farther-explored + // candidate, so a tie between a nearby candidate and one found + // only after searching out to localDrift favors the position that + // actually accounts for the shift — but does NOT count as + // progress: a hunk where many old lines each have many + // identically-scoring candidates (e.g. a uniform rename repeated + // verbatim across thousands of lines) would otherwise have every + // single one of those ties reset the counter, defeating the + // patience-based exit entirely and forcing a full-window scan for + // every line. let improved = false; for (const candidate of candidates) { if (candidate < 0 || candidate >= newCount || !remainingNewIndices.has(candidate)) { continue; } const score = bleuSimilarity(oldLines[oldIndex], addedLines[candidate]); - // >= rather than >: on an exact tie, prefer the farther-explored - // candidate. Combined with drift-scaled patience, a tie between - // a nearby candidate and one found only after searching out to - // localDrift favors the position that actually accounts for the - // shift — the more likely correct one when the gap's own line - // count changed, rather than an arbitrary "whichever was found - // first" tiebreak. - if (score >= bestScore) { + if (score > bestScore) { bestScore = score; bestIndex = candidate; improved = true; + } else if (score === bestScore) { + bestIndex = candidate; } } diff --git a/vscode-extension/src/test/buildHistory.test.ts b/vscode-extension/src/test/buildHistory.test.ts index f4a99e8..82a81c6 100644 --- a/vscode-extension/src/test/buildHistory.test.ts +++ b/vscode-extension/src/test/buildHistory.test.ts @@ -846,4 +846,77 @@ suite('buildHistory stays fast and memory-bounded on a large single hunk', () => `expected none of tasklet-1's lines to land in the inserted-duplicate range [${wrongRangeStart},${wrongRangeEnd}], got ${JSON.stringify(misattributed)}` ); }); + + test('a large hunk of uniformly-renamed identical lines resolves without the tie-breaking early exit stalling', async function () { + // Code review finding: preferring the farther-explored candidate on + // an exact score tie (added so a real match isn't lost to an earlier, + // wrong-but-equally-scoring one — see the previous test) marked every + // tie as "progress", resetting the patience counter. A hunk where + // every old line is identical to every other (a real pattern: the + // same templated statement repeated verbatim thousands of times) and + // gets uniformly renamed means every old line's fuzzy search sees + // thousands of candidates that ALL score identically — so patience + // never accumulates, and every single line scans the full search + // window regardless of the patience setting. Reproduced a ~13x + // slowdown (~2s fixed vs ~26s before) at 5,000 lines. + this.timeout(20000); + + const lineCount = 5000; + const dir = makeTempDir(); + execSync('git init -q', { cwd: dir }); + execSync('git config user.email test@example.com', { cwd: dir }); + execSync('git config user.name Test', { cwd: dir }); + + const filePath = path.join(dir, 'app.py'); + fs.writeFileSync(filePath, '# placeholder\n'); + execSync('git add app.py && git commit -q -m init', { cwd: dir }); + const baseCommit = execSync('git rev-parse HEAD', { cwd: dir, encoding: 'utf8' }).trim(); + + const buildLines = (fieldName: string) => { + const out = ['def process():']; + for (let i = 0; i < lineCount; i++) { + out.push(` record = load(${fieldName}=value)`); + } + out.push(' return None'); + out.push(''); + return out.join('\n'); + }; + + fs.writeFileSync(filePath, buildLines('user_id')); + execSync('git add app.py', { cwd: dir }); + const aiCommit = commitAiEdit(dir, baseCommit, 'tasklet-1', 'sess1', 'generate a large function using user_id repeatedly', 1000); + execSync(`git update-ref refs/tracy-local/aaaa1111 ${aiCommit}`, { cwd: dir }); + execSync('git add -A && git commit -q -m "generate large function (AI assisted)"', { cwd: dir }); + execSync('git notes add -m "tracy-id: aaaa1111" HEAD', { cwd: dir }); + execSync(`git update-ref refs/tracy/aaaa1111 ${aiCommit}`, { cwd: dir }); + + // Human commit: rename user_id -> userId across every (identical) + // line at once. + fs.writeFileSync(filePath, buildLines('userId')); + execSync('git add -A && git commit -q -m "human renames user_id to userId everywhere"', { cwd: dir }); + + const startedAt = Date.now(); + const result = await buildHistory(dir); + const elapsedMs = Date.now() - startedAt; + + assert.strictEqual(result.ok, true); + if (!result.ok) { return; } + + const file = result.history.files.find(f => f.path === 'app.py'); + const tasklet = file?.tasklets.find(t => t.taskletId === 'tasklet-1'); + assert.ok(tasklet, 'the large renamed function should still be attributed'); + // With every line identical to every other, which specific line + // "belongs" to which is genuinely ambiguous — a handful can be lost + // at the margins without it being a correctness bug. The overwhelming + // majority resolving is what matters here. + assert.ok( + tasklet!.lines.length > lineCount * 0.95, + `expected the vast majority of ${lineCount} identical renamed lines to still resolve, got ${tasklet!.lines.length}` + ); + + assert.ok( + elapsedMs < 8000, + `expected a ${lineCount}-line hunk of identical, uniformly-renamed lines to resolve in well under 8s, took ${elapsedMs}ms — ties resetting the early-exit patience counter would force a full-window scan for every line` + ); + }); }); From 0c9e21c5f799d7803886d6cab0f866538073e2ec Mon Sep 17 00:00:00 2001 From: Esme Yi Date: Sun, 16 Aug 2026 16:25:56 +0200 Subject: [PATCH 09/10] Fix tie-break drifting attribution when nothing actually shifted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review finding on this PR: always preferring the farther-explored candidate on an exact score tie was only correct when the gap actually had a size change to account for. When it didn't (no insertion/deletion — oldCount === newCount for the gap, an in-place rename with no lines added or removed), the estimate is already exactly right, and "prefer farther" has no justification — it just walks every tied pick outward toward the edge of the search window for no reason. Concretely: 1,000 identical lines "record = load(user_id=value)" uniformly renamed to userId — old line i's correct position is new line i, but tie-breaking toward "farthest explored" pushed every pick to roughly i + patience, losing the earliest lines entirely (repro showed the lowest attributed line at 4 instead of 1, and 991/1002 lines instead of 1002). Fixed by making the tie-break target the position implied by the gap's own SIGNED line-count change (positive for a net insertion, negative for a deletion, zero when nothing shifted) instead of unconditionally preferring "farther". On a tie, the candidate whose offset from the estimate is closest to that signed drift wins — closest to the estimate itself when nothing shifted (fixing this bug), and still closest to the drift-corrected position when something did (preserving the previous commit's fix for the insertion case). Ties still don't count as progress for the patience counter, so the earlier fix for long tie-runs stalling the search is unaffected. Verified: the no-drift uniform-rename case now attributes the full, unshifted range [1, 1002] with no loss. Re-verified the drift/tie-break insertion repro from two commits ago still has zero misattribution, and all other repro and performance scenarios in this PR are unaffected. Strengthened the existing large-uniform-rename test to check the attributed line range (not just the count), since a count-only check is exactly what let this regression through undetected. --- vscode-extension/src/history/buildHistory.ts | 68 +++++++++++-------- .../src/test/buildHistory.test.ts | 45 ++++++++---- 2 files changed, 73 insertions(+), 40 deletions(-) diff --git a/vscode-extension/src/history/buildHistory.ts b/vscode-extension/src/history/buildHistory.ts index fbd7053..be83d1f 100644 --- a/vscode-extension/src/history/buildHistory.ts +++ b/vscode-extension/src/history/buildHistory.ts @@ -739,18 +739,19 @@ function alignHunkLines(hunk: DiffHunk): Map { ? Math.round(prevAnchorNew + ((oldIndex - prevAnchorOld) * (nextAnchorNew - prevAnchorNew)) / span) : prevAnchorNew + 1; - // How many net lines were inserted/deleted within this specific - // anchor-bounded gap (the whole hunk, when there are no anchors at - // all — exactly the "big rename hunk, nothing exact-matches" case). - // The estimate above assumes that shift is spread proportionally - // across the gap; if it's actually concentrated at one end (e.g. a - // block of lines inserted right before a renamed section), an old - // line near that end can have templated/near-duplicate content - // elsewhere in the gap score just as well as its real match, which - // — without a bound tied to the shift itself — patience-based - // exit could settle on before the search ever reaches the real one. - const localDrift = Math.abs((nextAnchorNew - prevAnchorNew) - (nextAnchorOld - prevAnchorOld)); - const patience = Math.min(FUZZY_MATCH_WINDOW, Math.max(FUZZY_MATCH_EARLY_EXIT_PATIENCE, localDrift)); + // How many net lines were inserted (positive) or deleted (negative) + // within this specific anchor-bounded gap (the whole hunk, when + // there are no anchors at all — exactly the "big rename hunk, + // nothing exact-matches" case). The estimate above assumes that + // shift is spread proportionally across the gap; if it's actually + // concentrated at one end (e.g. a block of lines inserted right + // before a renamed section), an old line near that end can have + // templated/near-duplicate content elsewhere in the gap score just + // as well as its real match, which — without a bound tied to the + // shift itself — patience-based exit could settle on before the + // search ever reaches the real one. + const signedLocalDrift = (nextAnchorNew - prevAnchorNew) - (nextAnchorOld - prevAnchorOld); + const patience = Math.min(FUZZY_MATCH_WINDOW, Math.max(FUZZY_MATCH_EARLY_EXIT_PATIENCE, Math.abs(signedLocalDrift))); // Stop early once a confidently-good match has been sitting // unbeaten for a while — without this, every line would always @@ -758,40 +759,51 @@ function alignHunkLines(hunk: DiffHunk): Map { // match at offset 0 (the common case for an in-place rename that // doesn't reorder anything), which is what actually made this loop // slow in practice, not the window bound itself. Patience scales - // with localDrift so a search can't settle before at least reaching + // with the drift so a search can't settle before at least reaching // the position the gap's own line-count change implies. let bestIndex = -1; let bestScore = -1; + let bestTieDistance = Infinity; let noImprovementStreak = 0; for (let offset = 0; offset <= FUZZY_MATCH_WINDOW; offset++) { const candidates = offset === 0 - ? [estimatedNewIndex] - : [estimatedNewIndex - offset, estimatedNewIndex + offset]; + ? [{ index: estimatedNewIndex, signedOffset: 0 }] + : [ + { index: estimatedNewIndex - offset, signedOffset: -offset }, + { index: estimatedNewIndex + offset, signedOffset: offset }, + ]; // A strict improvement both updates the pick AND counts as - // progress (resets the patience counter below). An exact tie - // still updates the pick — preferring the farther-explored - // candidate, so a tie between a nearby candidate and one found - // only after searching out to localDrift favors the position that - // actually accounts for the shift — but does NOT count as - // progress: a hunk where many old lines each have many - // identically-scoring candidates (e.g. a uniform rename repeated - // verbatim across thousands of lines) would otherwise have every - // single one of those ties reset the counter, defeating the - // patience-based exit entirely and forcing a full-window scan for - // every line. + // progress (resets the patience counter below). An exact tie only + // updates the pick if it's closer to signedLocalDrift than the + // current pick — i.e. closer to the offset the gap's own + // insertion/deletion actually implies. When nothing shifted + // (signedLocalDrift = 0, an in-place rename with no size change), + // that means preferring whichever tie sits closest to the + // estimate itself, not whichever was found first OR farthest — + // without this, a hunk of many identical, uniformly-renamed lines + // would drift every line's pick toward the edge of the search + // window for no reason, since ties there don't actually reflect + // any real shift. A tie never counts as progress either way: a + // hunk where many old lines each have many identically-scoring + // candidates (e.g. that same uniform rename) would otherwise have + // every tie reset the counter, defeating the patience-based exit + // and forcing a full-window scan for every line. let improved = false; - for (const candidate of candidates) { + for (const { index: candidate, signedOffset } of candidates) { if (candidate < 0 || candidate >= newCount || !remainingNewIndices.has(candidate)) { continue; } const score = bleuSimilarity(oldLines[oldIndex], addedLines[candidate]); + const tieDistance = Math.abs(signedOffset - signedLocalDrift); if (score > bestScore) { bestScore = score; bestIndex = candidate; + bestTieDistance = tieDistance; improved = true; - } else if (score === bestScore) { + } else if (score === bestScore && tieDistance < bestTieDistance) { bestIndex = candidate; + bestTieDistance = tieDistance; } } diff --git a/vscode-extension/src/test/buildHistory.test.ts b/vscode-extension/src/test/buildHistory.test.ts index 82a81c6..780c9af 100644 --- a/vscode-extension/src/test/buildHistory.test.ts +++ b/vscode-extension/src/test/buildHistory.test.ts @@ -847,18 +847,26 @@ suite('buildHistory stays fast and memory-bounded on a large single hunk', () => ); }); - test('a large hunk of uniformly-renamed identical lines resolves without the tie-breaking early exit stalling', async function () { - // Code review finding: preferring the farther-explored candidate on - // an exact score tie (added so a real match isn't lost to an earlier, - // wrong-but-equally-scoring one — see the previous test) marked every - // tie as "progress", resetting the patience counter. A hunk where - // every old line is identical to every other (a real pattern: the - // same templated statement repeated verbatim thousands of times) and - // gets uniformly renamed means every old line's fuzzy search sees - // thousands of candidates that ALL score identically — so patience - // never accumulates, and every single line scans the full search - // window regardless of the patience setting. Reproduced a ~13x - // slowdown (~2s fixed vs ~26s before) at 5,000 lines. + test('a large hunk of uniformly-renamed identical lines resolves without stalling or drifting off position', async function () { + // Two code review findings share this one repro. A hunk where every + // old line is identical to every other (a real pattern: the same + // templated statement repeated verbatim thousands of times) and gets + // uniformly renamed, with no lines inserted or deleted, means every + // old line's fuzzy search sees thousands of candidates that ALL score + // identically: + // + // 1. Preferring the farther-explored candidate on an exact tie (added + // so a real match isn't lost to an earlier, wrong-but-equally- + // scoring one) originally marked every tie as "progress", resetting + // the patience counter — so patience never accumulated and every + // line scanned the full search window. Reproduced a ~13x slowdown + // (~2s fixed vs ~26s before) at 5,000 lines. + // 2. Once ties stopped resetting patience, "always prefer the + // farther-explored tie" was still wrong when nothing actually + // shifted (no size change in this hunk, so the natural position for + // each line is unchanged) — it systematically walked every line's + // pick toward the edge of the search window, losing the earliest + // lines instead of matching them where they already were. this.timeout(20000); const lineCount = 5000; @@ -914,6 +922,19 @@ suite('buildHistory stays fast and memory-bounded on a large single hunk', () => `expected the vast majority of ${lineCount} identical renamed lines to still resolve, got ${tasklet!.lines.length}` ); + // Count alone isn't enough: a systematic bias toward farther-explored + // ties can preserve nearly the full count while shifting every one of + // them off their natural (unshifted) position — losing the earliest + // lines instead of a random scatter. There's no size change here + // (oldCount === newCount), so the natural, unshifted range is exactly + // [1, lineCount + 2] (the def line through the return line). A shift + // shows up as the observed minimum climbing well above 1. + const sortedLines = tasklet!.lines.slice().sort((a, b) => a - b); + assert.ok( + sortedLines[0] <= 3, + `expected attribution to start at or near line 1 (no size change here, so nothing should need to shift) — the lowest attributed line was ${sortedLines[0]}, suggesting matches drifted toward one end of the search window` + ); + assert.ok( elapsedMs < 8000, `expected a ${lineCount}-line hunk of identical, uniformly-renamed lines to resolve in well under 8s, took ${elapsedMs}ms — ties resetting the early-exit patience counter would force a full-window scan for every line` From 46f623ceabafc7c5238fe44648b8828f32e63a6b Mon Sep 17 00:00:00 2001 From: Esme Yi Date: Sun, 16 Aug 2026 16:54:07 +0200 Subject: [PATCH 10/10] Fix drift bias walking uniform content into an unrelated appended tail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review finding on this PR (confirmed as a real, mirror-image regression of the previous commit's own fix): biasing ties toward signedLocalDrift is a single number for the WHOLE gap — it has no way to know WHERE within the gap an insertion or deletion actually happened. The previous commit's own repro put the extra lines BEFORE the AI's block, so biasing toward "+drift" happened to be correct. Mirrored the same construction with the extra lines AFTER the block instead (50 lines, identical to the AI's own content, appended right after it): the AI's 1,000 lines need no shift at all, but the same "+drift" bias walked lines near the end of the block into the appended, non-AI region, since that region is exactly as textually similar to the AI's own lines as the AI's own lines are to each other. Confirmed empirically: 50 lines pulled into the appended tail with the code as of the previous commit. Root cause traced further than the tie-break alone: the anchor- interpolated ESTIMATE itself already assumes a gap's size change is spread proportionally across it, which is also wrong when concentrated at one end — for old lines near that end, no tie-break fix alone could undo an estimate that already overshoots. Fixed by deciding, per old line, whether reaching for the gap's drift is warranted at all — based on how many OTHER old lines share that exact text. A rare old line (occurs only a handful of times) is what a real "moved/renamed block vs. one stray duplicate elsewhere" situation looks like (the previous commit's repro: exactly one decoy per rare index), and both the estimate and the tie-break target account for the full drift for those. Text that instead repeats many times over (uniform/templated content) gets an estimate assuming NO shift (1:1 against the nearest anchor) and ties prefer whichever candidate is closest to THAT — there's no way to tell, from content alone, which of many identical occurrences is this line's own, and assuming a shift risks walking into a same- looking but unrelated region. Verified against all three relevant scenarios: the new append-after repro (0 lines pulled into the tail, previously 50), the original insert-before repro from two commits ago (0 misattribution, unchanged), and the no-drift uniform-rename case (still the full, unshifted [1, 1002] range). Re-verified all seven correctness repro scenarios and all five performance tests in this PR still produce identical results. --- vscode-extension/src/history/buildHistory.ts | 93 ++++++++++++------- .../src/test/buildHistory.test.ts | 85 +++++++++++++++++ 2 files changed, 147 insertions(+), 31 deletions(-) diff --git a/vscode-extension/src/history/buildHistory.ts b/vscode-extension/src/history/buildHistory.ts index be83d1f..929efde 100644 --- a/vscode-extension/src/history/buildHistory.ts +++ b/vscode-extension/src/history/buildHistory.ts @@ -637,6 +637,12 @@ const FUZZY_MATCH_WINDOW = 500; // its match at or near offset 0 and gains nothing from scanning the rest // of the window. const FUZZY_MATCH_EARLY_EXIT_PATIENCE = 20; +// A tied candidate's text occurring at most this many times among the +// hunk's new lines is treated as a rare, discrete duplicate (worth +// reaching across the gap's drift for — see the long comment at its use +// site). More occurrences than this is treated as uniform/templated +// content instead, where reaching for a specific one has no basis. +const FUZZY_MATCH_RARE_DUPLICATE_LIMIT = 4; // Aligns a hunk's old lines to its new lines one-to-one and in order, // instead of matching each tracked line independently — independent @@ -703,6 +709,15 @@ function alignHunkLines(hunk: DiffHunk): Map { } } + // How many other old lines share this exact (whitespace-insensitive) + // text — used below to decide, per old line, whether reaching across + // the gap's drift for a tied candidate is warranted at all. + const oldContentCounts = new Map(); + oldLines.forEach(text => { + const key = normalize(text); + oldContentCounts.set(key, (oldContentCounts.get(key) ?? 0) + 1); + }); + if (unmatchedOldIndices.length > 0 && unmatchedNewIndices.length > 0) { const remainingNewIndices = new Set(unmatchedNewIndices); const oldCount = oldLines.length; @@ -733,34 +748,57 @@ function alignHunkLines(hunk: DiffHunk): Map { const prevAnchorNew = anchorPointer > 0 ? alignment.get(prevAnchorOld)! : -1; const nextAnchorOld = anchorPointer < anchorOldIndices.length ? anchorOldIndices[anchorPointer] : oldCount; const nextAnchorNew = anchorPointer < anchorOldIndices.length ? alignment.get(nextAnchorOld)! : newCount; - const span = nextAnchorOld - prevAnchorOld; - const estimatedNewIndex = span > 0 - ? Math.round(prevAnchorNew + ((oldIndex - prevAnchorOld) * (nextAnchorNew - prevAnchorNew)) / span) - : prevAnchorNew + 1; // How many net lines were inserted (positive) or deleted (negative) // within this specific anchor-bounded gap (the whole hunk, when // there are no anchors at all — exactly the "big rename hunk, - // nothing exact-matches" case). The estimate above assumes that - // shift is spread proportionally across the gap; if it's actually - // concentrated at one end (e.g. a block of lines inserted right - // before a renamed section), an old line near that end can have - // templated/near-duplicate content elsewhere in the gap score just - // as well as its real match, which — without a bound tied to the - // shift itself — patience-based exit could settle on before the - // search ever reaches the real one. + // nothing exact-matches" case). const signedLocalDrift = (nextAnchorNew - prevAnchorNew) - (nextAnchorOld - prevAnchorOld); - const patience = Math.min(FUZZY_MATCH_WINDOW, Math.max(FUZZY_MATCH_EARLY_EXIT_PATIENCE, Math.abs(signedLocalDrift))); + + // This old line's own text tells us whether reaching for that drift + // is warranted at all. If it's rare among the old lines (occurs + // only a handful of times), a real "moved/renamed block vs. one + // stray duplicate elsewhere" situation is what's actually going on + // — see the repro two commits ago: 50 lines inserted before a + // renamed block, reusing indices 0..49 so exactly ONE decoy exists + // per rare index — and it's worth both (a) assuming the gap's whole + // drift already applies by this point (the proportional estimate + // below assumes the shift is spread evenly across the gap; if it's + // actually concentrated at one end, that assumption is wrong for + // lines near that end) and (b) reaching across the gap's drift on a + // tie to find the candidate that accounts for it. + // + // If instead this exact text repeats many times over (a large + // uniform/templated block — many AI-written lines that just happen + // to be identical), assuming or reaching for the gap's drift has no + // basis: there's no way to tell, from content alone, which + // occurrence is the line's own, and guessing risks walking straight + // into a same-looking but unrelated region (e.g. lines appended + // after the AI's own block, which is exactly as "textually similar" + // to this line as the AI's own text is to itself). The estimate + // then assumes NO shift at all (matching this line's own position + // 1:1 against the nearest anchor), and ties prefer whichever + // candidate is closest to that estimate instead of reaching toward + // the drift. + const isRareContent = (oldContentCounts.get(normalize(oldLines[oldIndex])) ?? 1) <= FUZZY_MATCH_RARE_DUPLICATE_LIMIT; + const estimatedNewIndex = isRareContent + ? (span > 0 ? Math.round(prevAnchorNew + ((oldIndex - prevAnchorOld) * (nextAnchorNew - prevAnchorNew)) / span) : prevAnchorNew + 1) + : prevAnchorNew + (oldIndex - prevAnchorOld); + const tieTarget = isRareContent ? signedLocalDrift : 0; + const patience = isRareContent + ? Math.min(FUZZY_MATCH_WINDOW, Math.max(FUZZY_MATCH_EARLY_EXIT_PATIENCE, Math.abs(signedLocalDrift))) + : FUZZY_MATCH_EARLY_EXIT_PATIENCE; // Stop early once a confidently-good match has been sitting // unbeaten for a while — without this, every line would always // scan the full window even after finding an obviously-correct // match at offset 0 (the common case for an in-place rename that // doesn't reorder anything), which is what actually made this loop - // slow in practice, not the window bound itself. Patience scales - // with the drift so a search can't settle before at least reaching - // the position the gap's own line-count change implies. + // slow in practice, not the window bound itself. For rare content, + // patience scales with the drift so a search can't settle before at + // least reaching the position the gap's own line-count change + // implies. let bestIndex = -1; let bestScore = -1; let bestTieDistance = Infinity; @@ -775,27 +813,20 @@ function alignHunkLines(hunk: DiffHunk): Map { // A strict improvement both updates the pick AND counts as // progress (resets the patience counter below). An exact tie only - // updates the pick if it's closer to signedLocalDrift than the - // current pick — i.e. closer to the offset the gap's own - // insertion/deletion actually implies. When nothing shifted - // (signedLocalDrift = 0, an in-place rename with no size change), - // that means preferring whichever tie sits closest to the - // estimate itself, not whichever was found first OR farthest — - // without this, a hunk of many identical, uniformly-renamed lines - // would drift every line's pick toward the edge of the search - // window for no reason, since ties there don't actually reflect - // any real shift. A tie never counts as progress either way: a - // hunk where many old lines each have many identically-scoring - // candidates (e.g. that same uniform rename) would otherwise have - // every tie reset the counter, defeating the patience-based exit - // and forcing a full-window scan for every line. + // updates the pick if it's closer to tieTarget than the current + // pick. A tie never counts as progress either way: a hunk where + // many old lines each have many identically-scoring candidates + // (e.g. a uniform rename repeated verbatim across thousands of + // lines) would otherwise have every tie reset the counter, + // defeating the patience-based exit and forcing a full-window + // scan for every line. let improved = false; for (const { index: candidate, signedOffset } of candidates) { if (candidate < 0 || candidate >= newCount || !remainingNewIndices.has(candidate)) { continue; } const score = bleuSimilarity(oldLines[oldIndex], addedLines[candidate]); - const tieDistance = Math.abs(signedOffset - signedLocalDrift); + const tieDistance = Math.abs(signedOffset - tieTarget); if (score > bestScore) { bestScore = score; bestIndex = candidate; diff --git a/vscode-extension/src/test/buildHistory.test.ts b/vscode-extension/src/test/buildHistory.test.ts index 780c9af..61007f3 100644 --- a/vscode-extension/src/test/buildHistory.test.ts +++ b/vscode-extension/src/test/buildHistory.test.ts @@ -940,4 +940,89 @@ suite('buildHistory stays fast and memory-bounded on a large single hunk', () => `expected a ${lineCount}-line hunk of identical, uniformly-renamed lines to resolve in well under 8s, took ${elapsedMs}ms — ties resetting the early-exit patience counter would force a full-window scan for every line` ); }); + + test('uniformly-renamed identical lines do not drift into unrelated lines appended right after the block', async function () { + // Code review finding, and a mirror of the previous test's own + // reasoning turned back on it: fixing the "drift toward the edge of + // the window" bug meant biasing ties toward the offset the gap's + // line-count change implies. But that bias is a single number for + // the WHOLE gap — it has no way to know WHERE within the gap an + // insertion actually happened. Here, 50 non-AI lines (identical to + // the AI's own, entirely uninformative content — no unique index + // anywhere) are appended AFTER the AI's block instead of inserted + // before it. The AI's own 1,000 lines need no shift at all, but a + // drift target that assumes the gap's whole +50 change applies + // uniformly to every line would still walk lines near the end of the + // block into the appended (non-AI) region, since that region is + // exactly as similar to the AI's own text as the AI's own text is to + // itself. + this.timeout(20000); + + const lineCount = 1000; + const appendedCount = 50; + const dir = makeTempDir(); + execSync('git init -q', { cwd: dir }); + execSync('git config user.email test@example.com', { cwd: dir }); + execSync('git config user.name Test', { cwd: dir }); + + const filePath = path.join(dir, 'app.py'); + fs.writeFileSync(filePath, '# placeholder\n'); + execSync('git add app.py && git commit -q -m init', { cwd: dir }); + const baseCommit = execSync('git rev-parse HEAD', { cwd: dir, encoding: 'utf8' }).trim(); + + const buildAiLines = (fieldName: string) => { + const out = ['def process():']; + for (let i = 0; i < lineCount; i++) { + out.push(` record = load(${fieldName}=value)`); + } + out.push(' return None'); + out.push(''); + return out.join('\n'); + }; + + fs.writeFileSync(filePath, buildAiLines('user_id')); + execSync('git add app.py', { cwd: dir }); + const aiCommit = commitAiEdit(dir, baseCommit, 'tasklet-1', 'sess1', 'generate a large function using user_id repeatedly', 1000); + execSync(`git update-ref refs/tracy-local/aaaa1111 ${aiCommit}`, { cwd: dir }); + execSync('git add -A && git commit -q -m "generate large function (AI assisted)"', { cwd: dir }); + execSync('git notes add -m "tracy-id: aaaa1111" HEAD', { cwd: dir }); + execSync(`git update-ref refs/tracy/aaaa1111 ${aiCommit}`, { cwd: dir }); + + // Human commit: rename user_id -> userId across the AI's block AND + // append 50 more identical (non-AI) lines right after it. + const buildRenamedWithAppend = () => { + const out = ['def process():']; + for (let i = 0; i < lineCount; i++) { + out.push(' record = load(userId=value)'); + } + for (let i = 0; i < appendedCount; i++) { + out.push(' record = load(userId=value)'); + } + out.push(' return None'); + out.push(''); + return out.join('\n'); + }; + fs.writeFileSync(filePath, buildRenamedWithAppend()); + execSync('git add -A && git commit -q -m "human renames and appends more identical lines after (non-AI)"', { cwd: dir }); + + const result = await buildHistory(dir); + assert.strictEqual(result.ok, true); + if (!result.ok) { return; } + + const file = result.history.files.find(f => f.path === 'app.py'); + const tasklet = file?.tasklets.find(t => t.taskletId === 'tasklet-1'); + assert.ok(tasklet, 'the AI block should still be attributed'); + + // The AI's block occupies new lines [2, 1 + lineCount]. The appended, + // non-AI tail occupies [2 + lineCount, 1 + lineCount + appendedCount]. + // None of tasklet-1's lines should land in the tail. + const tailStart = 2 + lineCount; + const tailEnd = 1 + lineCount + appendedCount; + const misattributed = tasklet!.lines.filter(l => l >= tailStart && l <= tailEnd); + assert.deepStrictEqual( + misattributed, + [], + `expected none of tasklet-1's lines to land in the appended, non-AI tail [${tailStart},${tailEnd}], got ${JSON.stringify(misattributed)}` + ); + }); });