amirbena / amirbena/code-review-skill
Add benchmark coverage for finding-placement accuracy and locality
- Dominant language
- Python
- Stars
- 7
- Forks
- 2
- Avg merge
- 27m
- Merged PRs (30d)
- 188
Description
## Type
Quality
## Area
Review Quality
## Priority
P2 — Medium
## Problem
Finding-location selection is a semantic, model-facing judgment (causal
vs. symptom, caller vs. callee ownership, precedent vs. defect site), and
no existing corpus validates that the **correct primary location** is
chosen — only that an already-resolved location is anchored/published
correctly (#164's existing fixtures test the transport/anchor-selection
layer given a known fix location, not how that location is derived).
## Goal
Add a focused `docs/benchmark/corpus/` sub-corpus proving correct primary
finding-location selection for the reasoning the implementation issue
introduces — it consumes the canonical contract, it does not redefine it.
## Scope
- New corpus directory (e.g.
`docs/benchmark/corpus/finding-placement/`) with a README and the
smallest representative fixture set, following sibling sub-corpus
conventions (e.g. `analogue-placement-pattern/`, `consolidation/`).
- Scenario coverage, validating the correct primary location rather than
merely that a finding exists:
- upstream cause vs. downstream manifestation → location follows the
claim;
- a downstream-local handling defect → location stays downstream, not
mechanically moved upstream;
- caller precondition violation vs. callee contract violation vs. a
pre-existing callee bug exposed by a changed caller → no mechanical
caller/callee preference;
- context-expansion drift (evidence from a caller, sibling, test, or
utility) → finding stays at the defect-owning site;
- precedent trap (a correct sibling implementation used only as
evidence) → finding stays on the defective implementation;
- nearest-changed-line trap / false precision → a broader, truthful
placement is chosen over a misleading precise line;
- multi-file primary-location selection → one primary causal/
contract-owning file, not an arbitrary touched file;
- best semantic location unavailable inline → reuses `finding-placement.md`'s
existing fallback rather than a misleading changed line (confirm no
duplication of #164's existing fixtures for this case; reuse/extend
instead);
- test reveals a production defect vs. a defective test itself →
correct side selected.
- Record dependency on the implementation issue; include expected primary
location per fixture per `fixture-format.md`.
## Non-Goals
- Redefining policy — this consumes the canonical contract only.
- Duplicating #164's existing anchor-selection/fallback regression
fixtures — reuse or extend them where a scenario already exists there.
## Acceptance Criteria
- [ ] Required scenarios above are represented or explicitly mapped to
existing coverage (including #164's fixtures), with no duplication.
- [ ] Cause-vs-symptom, caller/callee non-mechanical ownership,
context-expansion-drift resistance, precedent-trap resistance, and
false-precision resistance are each represented.
- [ ] Corpus/reference metadata and validation pass; repository-prescribed
benchmark tests pass.
## Dependencies
Parent: #385
Depends on: #386 (implementation contract)
Relates: `docs/benchmark/fixture-format.md`, `docs/benchmark/corpus/README.md`,
sibling sub-corpora (`analogue-placement-pattern/`, `consolidation/`) for
schema/style precedent; #164's existing anchor-selection regression
fixtures (transport layer — reuse, do not duplicate).
## Validation
- New fixtures run clean through `run_benchmark.py` / the canonical corpus
runner.
- Existing `tests/unit/benchmark/` and `tests/policy/benchmark/` suites
remain green.
Contributor guide
Assessment
This issue has not been assessed yet.