fix(composer): key docs fingerprints to the file, not the hunk

[klaidliadon]

Push a fix to a doc, and codegenie comments on the same problem again in a new thread. It happened twice in a row on one pull request here, for a finding whose text never changed between the pushes. The cause is that a prose finding has no stable identity, so it gets a new one on every push.

Why it happens

The fingerprint answers one question: is this the same finding I already reported? For code it answers by asking which function the finding sits in, and that survives edits elsewhere in the file. Prose has no functions. So it falls through to the hunk id, which is a hash of line numbers and offsets, and those move the moment anyone edits anything above. The fallback below that, the packet id, is built from hunk ids, so it moves too. There was no stable rung on the ladder.

Duplicate detection then has nothing to match on. The exact-fingerprint arm misses, and the fuzzy arm only catches a prior comment within 5 lines on the same path and side. The two comments here were 7 lines apart.

The fix

One guard in fingerprintLocationIdentity. When the finding sits on a docs path, the location component becomes a fixed <file> scope instead of diff geometry.

if (isDocsPath(finding.path)) {
  return FILE_SCOPE_IDENTITY;
}

The symbol path and the hunk-id path are untouched, so no code finding changes fingerprint. Nothing on an open pull request gets re-keyed by this.

What it costs

Two findings in the same docs file, same category and same lens, now share a fingerprint and merge onto one thread instead of two. That is because fingerprintFinding is the grouping key inside a run as well as the dedup key across runs, so coarsening it for stability also coarsens grouping. With location fixed per file, category and lensId are the only discriminators left.

A test pins that so it stays visible. I took it over the current behavior, which opens a fresh thread for every unfixed docs finding on every push.

Transition

Comments already posted carry old fingerprints. On the first run after this lands, a docs finding on an in-flight pull request will not match its own existing comment and will open one more thread. From the run after that, the fingerprint holds still. Code findings are unaffected in either direction.

Detail: evidence, the approach I backed out, tests, and #26

Evidence

Two review comments on the same design document, in consecutive runs, both reporting that one value named in a Markdown table is not a member of the enum the surrounding prose claims already carries it.

Run	Anchor
first	line 81, the prose bullet making the claim
second	line 76, the table row naming the value

The two runs saw different commits. The bullet and the table row are byte-identical in both. The diff between those commits adds 14 lines and removes 23 elsewhere in the file, which moved this section up by 7 lines. The finding did not change. Its geometry did.

Note what that rules out. The two runs anchored the same defect to two different pieces of text, so no line-level or content-level key would have matched them either. Only a file-level key can.

How the ids are built

fingerprintFinding in src/pipeline/composer.ts hashes path, location identity, category and lens. The hunk id is sha256(path, oldStart, newStart, header, added and deleted line numbers) (src/git/diff-parser.ts:416). The packet id is a hash of the path plus its sorted hunk ids and kind.

The approach I backed out

I first tried the general form: drop geometry for every symbol-less finding, and generalize multi-symbol hunks to a sorted symbol set. It broke final finding fingerprints are stable across model rewording and does not merge unanchored findings from different hunks in one coalesced packet.

Those encode a real invariant rather than the bug, since coarsening the key merges distinct findings inside a single run. A code hunk spanning two functions has no unique symbol and would have collapsed to file scope. Gating on the path role keeps that invariant, and it also avoids keying identity on symbolFacts being empty for a transient reason such as a parse failure or a missing grammar, which would be unstable in its own right.

That leaves one case with the same root cause unfixed: a hunk spanning two or more symbols still falls back to the hunk id and is still unstable across pushes. Fixing it would move fingerprints for code findings on every open pull request, so it is a candidate follow-up rather than part of this change.

Tests

Both are in tests/pipeline-phase5.test.ts and both fail on master.

• keeps docs fingerprints stable when a later push shifts the hunk composes the same docs finding twice with different hunk ids and line numbers and asserts a single fingerprint. On master the two runs produce different hashes.
• merges same-category docs findings from one file onto a single thread pins the trade above. On master it sees two findings instead of one.

pnpm test passes 851 of 853. The two failures are package-build.test.ts and review-command.test.ts, both macOS /private symlink path assertions that fail identically on a clean master checkout. pnpm run check and pnpm run build are clean. Six further tests time out locally under load at the default 5s timeout and pass at --testTimeout=60000; that is unrelated to this change.

Conflict risk with #26

#26 edits src/pipeline/composer.ts directly above fingerprintFinding, replacing compactEvidence and the evidence-block helpers. It does not touch fingerprintLocationIdentity, symbolForHunk, uniquePacketSymbol or inferredHunkId, and its new src/pipeline/finding-location.ts is about anchor clarification, not identity. The overlap is adjacent context lines. #26 also edits tests/pipeline-phase5.test.ts; my additions sit at the end of two existing blocks, so a rebase should be mechanical.

This branch is based on master and does not depend on #26. No version bump here, since #26 already moves the package to 0.6.0.

[pkieltyka]

Thanks @klaidliadon for the contribution, detailed investigation, and regression tests! We used your reproduction and test cases to implement the fix on the current codebase in #35.

The replacement matches unchanged documentation comment content across shifted anchors while keeping distinct findings in the same document separate. It also adds deterministic synthetic evals runnable with make evals and additional regression coverage.

Closing this PR as replaced by #35. Thank you again for finding and documenting the issue!