amirbena / amirbena/code-review-skill

Define Existing Review Evidence schema

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

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.