amirbena / amirbena/code-review-skill

Add benchmark coverage for finding-placement accuracy and locality

Open
#387 0 comments 0 reactions 1 assignee Claimed by @amirbena View on GitHub
area:review-quality maintainer-led priority:P2 type:quality
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.