fix(diff): stop emitting comments whose quote matches no reviewed file

[nitishagar]

Problem

ocr review can emit a comment anchored to the wrong file: the reported cases are Java toString() advice rendered on pom.xml and a Java @RequestMapping security finding rendered on .gitignore, both with start_line: 0, end_line: 0 (#746).

Root cause

When the model files a comment against path P while quoting code from another file, the resolution chain declines at every stage:

1. in-file resolution — the quote is not in P's diff;
2. cross-file relocation — declines when the quote matches no other reviewed file exactly (or matches more than one);
3. LLM re-location — sees only P's diff, so for a quote that is not in P it can at best answer with the closest-looking block, fabricating a located-but-wrong anchor.

After all three, the comment was still collected and emitted verbatim under the claimed path at 0/0 (internal/llmloop/loop.go, the unconditional CommentCollector.Add). Nothing ever flagged or dropped it. Quote matching is also exact per line after trimming, so a quote drifting only in internal spacing (id ) vs id)) defeats even the stages that could have saved it.

Fix

Three targeted changes in the comment resolution chain:

• Whitespace-eliding second-chance tier for deterministic matching (internal/diff/resolver.go): after the exact passes fail on both sides of a hunk (and in the full-content scan), lines are compared with all whitespace elided. Exact matches always outrank loose ones, so existing anchors are unchanged; only quotes that differ purely in internal spacing gain resolution.
• Quote-plausibility gate on LLM re-location: the rescue step now runs only when the comment's quote shares at least one non-trivial (≥ 8 normalized chars) line with the claimed file's diff — under exact or elided comparison, across hunks and full content. A quote from the right file still shares lines with it even when drifted; a Java quote against pom.xml shares none, and the step that could only fabricate a location is skipped.
• Emission gate in review runs: a comment that ends resolution unlocated while carrying an anchorable quote is dropped with a comment_unresolved run warning (same channel as the existing comment_refiled) instead of being emitted under a path it does not belong to. Path-less comments silently bound to a grouped review's comma-joined key are dropped the same way. Comments with no anchorable quote on a reviewed file keep today's behavior (general advice), and scan keeps its collection behavior unchanged (the gate is scoped to review wiring).

Trade-off

A comment whose quote matches nothing in the run is now dropped rather than emitted mis-anchored. The dropped path/summary is visible in the run's warnings, so nothing disappears silently — but the finding itself no longer appears in the report. I took the view that a wrong-file comment is worse than a dropped one (it was reported as a bug twice); if you prefer emission with an explicit unresolved marker instead, the gate is one branch to change.

Tests

• Loop-level reproducers for the issue scenario (wrong-file quote → drop + warn; drifted quote → re-filed onto the right file; LLM rescue preserved for plausible quotes, skipped for zero-overlap ones).
• Resolver tests for the eliding tier (hunk + full content) and for materially-different lines still failing.
• Ambiguous snippet (two files) still declines to guess; the async comment pool applies the same gate; scan wiring keeps unconditional re-location and unchanged collection.
• Full suite green under -race; touched packages at 89.3% / 95.1% coverage.

Fixes #746

[github-actions]

🔍 OpenCodeReview found 1 issue(s) in this PR.

• ✅ Successfully posted inline: 1 comment(s)

[github-actions]

When matchConsecutiveWith is called from resolveFromHunk, the same targetLines are transformed by xform on every invocation (once per hunk × per side × per xform pass). Since targetLines is invariant within each xform pass, the transformation is redundant after the first call.

Consider hoisting the target transformation out of matchConsecutiveWith and accepting pre-transformed targets, or computing them once in resolveFromHunk before the inner hunk loop. For diffs with many hunks, this avoids repeated allocations and transformations of the same slice.