amirbena / amirbena/code-review-skill
Strengthen finding placement accuracy and locality
- Dominant language
- Python
- Stars
- 7
- Forks
- 2
- Avg merge
- 27m
- Merged PRs (30d)
- 188
Description
## Type
Feature
## Area
Review Quality
## Priority
P1 — High
## Problem
Once a finding is accepted, nothing canonically owns *how its fix/action
location is determined* when evidence, causal reasoning, or context
expansion touch more than one place. A finding can drift to arbitrary
nearby code, a downstream symptom, a merely related caller/callee, a
precedent implementation, a supporting test, or an architectural neighbor
visited only for evidence — none of which necessarily owns the claim.
Core principle:
```text
Where evidence is found ≠ where the symptom appears ≠ where the finding belongs
```
Evidence can move. The finding must not drift with it.
## Goal
Make causal/contract-owning finding placement explicit, canonically
owned, benchmark-protected, and documented — starting only after a
finding is already accepted, and without redeciding validity, severity,
or verdict/publication behavior.
## This epic explicitly establishes
- The finding's location must follow its claim: the causal/contract-owning
site, not wherever evidence happened to surface.
- No mechanical caller/callee preference — ownership of the violated
responsibility decides.
- Context expansion (callers, callees, siblings, tests, utilities,
precedent, downstream consumers) may supply evidence; it never by
itself relocates the finding.
- The narrowest *semantically honest* location wins — false precision at
a convenient nearby line is worse than a truthful broader placement.
- This work reuses the existing bounded caller/callee model and the
existing evidence-location/fix-location distinction; it does not
redefine either.
- This concerns where an already-accepted finding is anchored — not
whether it is valid, its severity, its verdict, or GitHub publication
mechanics.
## Out of scope
Candidate-finding validation, severity derivation, verdict derivation,
passive/semi/active semantics, rendering redesign, GitHub transport
plumbing, symbol/post-image propagation, and the anchor-selection/
fallback mechanics already owned by
`skills/github-pr-review/policies/finding-placement.md` (#164) — this
epic feeds the location that policy anchors, it does not redo it.
## Children
- Implementation: #386 — Canonicalize causal/contract-owning fix/action
location selection
- Benchmark / Corpus: #387 — Add benchmark coverage for finding-placement
accuracy and locality
- Wiki / Documentation: #388 — Document the finding-placement mental
model in the Wiki
## Dependency model
```text
#386 Implementation
├── #387 Benchmark (Depends on: #386)
└── #388 Wiki (Depends on: #386)
```
Benchmark and Wiki both depend on Implementation and may proceed in
parallel once its contract is stable; neither depends on the other.
## Acceptance criteria
- [ ] #386 — the canonical causal/contract-owning placement contract is
implemented.
- [ ] #387 — semantic benchmark/corpus coverage protects the behavior.
- [ ] #388 — the Wiki explains the mental model and examples.
## Dependencies
Children: #386, #387, #388
Relates: #381 (candidate-finding validation epic — the preceding,
independent stage in the pipeline: `candidate validation → accepted
finding → finding-placement selection → rendering/publication`).
Contributor guide
Assessment
This issue has not been assessed yet.