amirbena / amirbena/code-review-skill
Define Existing Review Evidence schema
- Dominant language
- Python
- Stars
- 7
- Forks
- 2
- Avg merge
- 27m
- Merged PRs (30d)
- 188
Description
## Problem
`github-pr-review`'s Existing Review Evidence model is strongly implemented: `shared/policies/review-evidence.md` defines it, and `tests/reference/review/pr_review_evidence.py` implements `PriorFinding` (with a `reviewed_sha` field), `Reconciliation`, `PriorItemClass`, and `ReviewState`, with explicit head-changed semantics and reconciliation against the current PR HEAD — exercised by `tests/unit/review/test_pr_review_evidence.py`.
The local-side equivalent, `tests/reference/review/pr_context_reconciliation.py` (mirroring `skills/local-code-review/policies/pr-context.md`), has comparable reconciliation logic (`ExistingFinding`, `FindingReconciliation`, `FindingStatus` lifecycle) but its `ExistingFinding` carries no reviewed-state identity field analogous to `reviewed_sha` — findings are tracked only by `id` and `touches`. `pr-context.md` itself has no mention of SHA/fingerprint-based identity, and does not reference the staged-delta fingerprint already defined in `skills/local-code-review/policies/repository-state.md` as a way to detect whether the reviewed local state has changed since a finding was recorded.
## Goal
Give local-side existing-finding reconciliation the same reviewed-state identity rigor the GitHub side has, using the local delta's own existing identity mechanism (the staged-delta fingerprint in `repository-state.md`) rather than inventing a new one.
## Scope
- Add a reviewed-state identity field to `ExistingFinding` (or equivalent), populated from the local staged-delta fingerprint / committed-delta reference already defined in `repository-state.md`.
- Extend `reconcile_finding` to account for whether the reviewed state has changed since the finding was recorded, mirroring the github side's `head_changed` handling.
- Update `pr-context.md` to state this explicitly.
## Non-goals
- Delta computation itself (see #43).
- The identity mechanism generally (see #42) — reuse the existing local fingerprint, don't redesign it.
- Any change to the GitHub-side evidence model, which already meets the bar.
## Acceptance criteria
- [ ] Local `ExistingFinding`/reconciliation carries a reviewed-state identity field.
- [ ] Reconciliation logic distinguishes "state unchanged" from "state changed since finding was recorded," mirroring the github side.
- [ ] `pr-context.md` documents this explicitly.
- [ ] Tests cover both unchanged- and changed-state reconciliation.
## Dependencies
Relates: #42, #43.
## Validation
- Focused unit tests extending `tests/unit/review/test_pr_context_reconciliation.py`.
Contributor guide
Assessment
This issue has not been assessed yet.