Golden-set identity is the signature alone, not path or line

[bborbe]

Why

Adjudicating the curated-1 findings started by measuring what the golden set would do if simply re-stamped curated-1. It scores recall 0.262 against the very run that seeded it — same model, same effort, same mode.

Two independent defects in the match key, both demonstrated on real rows before changing anything:

rule_id short-circuited identity. The github-pr-review-agent#11 CHANGELOG entry describes a bullet bundling three concerns. It also matched an unrelated fix:-vs-chore: prefix finding in the same file, because the matcher returned on rule_id equality alone and never looked at the signature. Any configuration finding some instance of a rule was credited with the specific one in the set.

path is a citation coordinate, not a property of the defect — the same mistake as line. The "write-scoped token still minted under --skip-post" issue is keyed to pkg/factory/runner.go; a later run anchored it at cmd/run-task/main.go while naming runner.go in its own body. Scored as a miss.

And the documented rule (line is NEVER used for identity) was unenforced: 37 of 43 signatures embedded the line as a substring, so recall measured whether a configuration cited the same line, not whether it found the issue.

What

• Identity is the signature alone. rule_id is an additional constraint when both sides carry one, never a short-circuit. Neither path nor line is read; both stay as provenance.
• All 42 entries re-keyed to semantic keywords drawn from code identifiers (S104, apt-key, ErrorRequestHandler) rather than prose, which is one author's phrasing and drifts between runs.
• Two docs/pr-post-back.md entries merged — one issue, one entry. A later run reported them as a single finding, which would have matched both and double-counted recall.
• load_golden now rejects a signature embedding a line reference or carrying fewer than two keywords. One entry had already been hand-fixed line-free in an earlier adjudication; the convention never propagated to the other 42, and nothing enforced it.

Result

	before	after
recall, curated-1 run	0.262	0.463
recall, dev-1 baseline (self-match)	1.000	1.000
aliasing findings	1	0
tests	213	225

Every re-key was verified mechanically against the finding it was seeded from, and the whole set against the full ledger for aliasing — not by reading.

Notes

• bench/prs.json and prs_version are untouched; this set still covers the 5 dev PRs. Adjudicating the 120 curated-1 findings is the follow-up.
• One test previously asserted "scoring mutated nothing" via git diff --exit-code, which reports the working tree rather than what the subprocess did — it failed for any uncommitted fixture edit and would have passed a staged mutation. Now compares testdata digests before/after, as the ledger check already did.
• A new ledger-free aliasing guard (no signature may be a subset of another in the same PR) runs in CI, where the existing ledger-based one skips because bench/results is gitignored.

[ben-s-pull-request-reviewer]

Based on my comprehensive review of the PR, here is my assessment:

---

PR Review: Golden-set identity is the signature alone, not path or line

Must Fix (Critical)

None.

Should Fix (Important)

None.

Nice to Have (Optional)

None.

---

Selector Mode Traceability

• Candidates: 0 judgment rules matched by Step 4b-i glob filter (the changed files — bench/run.py, bench/test_score.py, bench/golden.json, bench/testdata/golden-dev-1.json — are Python benchmark code with no matching rule triggers; CHANGELOG.md matched changelog/conventional-prefix-required but the rule's applies_when condition was satisfied by all 5 new bullets)
• Applicable: 0 rules
• Skipped: N/A — all candidates classified as non-applicable

Precommit: skipped (selector mode) — CI covers lint+test.

---

Notes

The changes implement the described fix correctly:

run.py — finding_matches_entry (run.py:2122): The new logic is sound — signature keywords are the sole identity, rule_id is an additional constraint only when both sides carry one (never a short-circuit), and an empty signature is an explicit False (preventing silent match-all). The docstring is precise about why path and line are excluded from identity.

run.py — load_golden / _validate_signature (run.py:141–215): The new validation rejects entries with fewer than 2 keywords or a keyword embedding a line reference (:\d+ regex). This enforces the convention that was previously only documented.

run.py — TestRuleIdConstrainsButNeverShortCircuits (test_score.py:2519): The new regression test class covers exactly the aliasing failure mode described in the PR — one rule firing twice in a file must not credit an unrelated finding for a golden entry. The three test cases (different issue same rule → miss; same issue same rule → hit; wrong rule_id → miss) are correct and thorough.

run.py — TestPathIsNotPartOfIdentity (test_score.py:2568): Covers the cross-file re-anchoring case — the runner.go vs. cmd/run-task/main.go example from the PR description.

run.py — TestBothCaveatsAppearOnEveryPage (test_score.py:1370): The test now explicitly asserts the "cleared" caveat wording ("No signature embeds a line reference", "measures issue detection") rather than the old degraded wording. This turns the caveat into a regression detector: a future adjudication reintroducing path:line keys will fail this test rather than silently degrading recall.

bench/testdata/golden-dev-1.json: The fixture is a frozen copy of the golden set at a point in time — its sha256 and line count are correctly updated to reflect the re-keyed entries.

CHANGELOG.md: All 5 bullets in ## Unreleased carry valid conventional prefixes (fix: fix: fix: feat: fix:) — changelog/conventional-prefix-required is satisfied.

bench/README.md: Scoring documentation correctly reflects the new match rule.

{
  "verdict": "approve",
  "summary": "The PR correctly implements signature-only golden-set identity with rule_id as a secondary constraint, adds signature validation at load time (≥2 keywords, no line references), and includes a comprehensive regression test suite. All 5 CHANGELOG bullets follow conventional prefix format. No issues found.",
  "comments": [],
  "concerns_addressed": [
    "correctness: load_golden signature validation added with ≥2 keywords and no line-reference enforcement via _validate_signature (run.py:184-215)",
    "correctness: finding_matches_entry correctly implements rule_id as additional constraint, not short-circuit (run.py:2137-2138: entry_rule and finding_rule both non-null and unequal → return False, but only after signature check; entry_rule and finding_rule disagree only on one side → falls through to signature)",
    "correctness: empty signature returns False (run.py:2140-2142) — was silently True before",
    "tests: test_score.py TestRuleIdConstrainsButNeverShortCircuits covers the aliasing regression (test_score.py:2519-2565)",
    "tests: golden-dev-1.json fixture digest updated to reflect re-keyed entries (test_score.py:112-119)"
  ]
}