# Review notes protocol

Date: 2026-10-02
Status: approved

## Purpose

Record code-review results as git notes under `refs/notes/review` so that the
next reviewer — human or AI — can read the current verdict, see open findings
with stable IDs, and continue from where the last review stopped. This
replaces a CI reviewer: reviews happen locally, results travel with the
commits they judged.

## Storage model

- One note per reviewed commit, in the `review` notes ref.
- The tip of a reviewed branch always carries the latest verdict.
- Re-reviewing after fixes writes a **new** note on the **new** tip commit.
  Old notes stay attached to old commits as history.
- Each note links to the previously reviewed commit via `previous`, forming a
  walkable chain of review rounds per branch.
- Merging to main needs no special handling: the `approve` note on the final
  tip is the record.

## Note format

Line-based header, then a blank line, then free markdown body:

    branch: <branch name>
    round: <integer, starting at 1>
    verdict: approve | request-changes
    previous: none | <sha of previously reviewed tip>
    findings:
      - id: F<N>
        severity: blocker | major | minor
        file: <path>:<line>
        title: <one line>
        status: open | resolved | wontfix
    ## Details

    <markdown prose>

Rules:

- Finding IDs are unique within a note and carry forward across rounds: a
  finding resolved since the last note keeps its ID and moves to
  `status: resolved`; an unfixed finding keeps its ID and `status: open`.
  New findings get fresh IDs that do not collide with carried-forward ones.
- The body is free markdown; the `## Details` heading in the example is
  illustrative, not required.
- Headers are plain `key: value` lines so the helper script can parse them
  with sed/awk.

## Helper script: `scripts/review-note`

Bash, git plumbing only. Subcommands:

- `review-note show [commit]` — print the note for a commit (default: current
  tip). Exit 1 with a message on stderr if there is no note.
- `review-note write <file>` — attach the note in `<file>` to the current tip
  commit. Refuses to replace an existing note unless `--force` is given; a
  proper re-review targets a new commit, so overwriting is always deliberate.
  Validates that `branch`, `round`, and `verdict` are present and that
  `verdict` uses the vocabulary above. No other validation.
- `review-note latest [branch]` — print the note with the highest `round` for
  a branch (default: current branch), scanning the notes ref and matching
  notes on their `branch:` header. Exit 1 if there is no note for the branch.
  (A `previous`-chain walk cannot bootstrap from an unreviewed tip, so the
  scan uses the `branch:` header as the index instead.)
- `review-note push` — `git push origin refs/notes/review`. Plain `git push`
  ignores notes, so this is the only way results travel.

`push` pushes without force; if the remote rejects, report the failure and let
the user decide.

## Lifecycle

1. Reviewer reads `review-note latest <branch>` (exit 1 means first review).
2. Reviewer reviews the branch tip per `REVIEW.md`.
3. Reviewer writes the note with `review-note write` and runs
   `review-note push`.
4. If the verdict is `request-changes`, the author pushes fixes as new
   commits. The next review round starts at the new tip with `round`+1,
   `previous` set to the last reviewed tip, and carried-forward findings.
5. When the verdict is `approve` and the branch merges, the notes remain on
   the branch commits as the review record.

## Documentation integration

- `REVIEW.md` gains a "Recording reviews" section describing the protocol and
  the four script commands. This is the single source of truth for the
  protocol.
- `AGENTS.md` "Reviewing" section gains one line pointing at the section, so
  any AI session picks up the flow automatically.

## Seeding the experiment

The failing review of `home_assistant_controller_cleanup` (commit `ba6e0e8`)
becomes note round 1, with these findings:

- F1 (blocker, src/AFRP.hs:273): hlint's `\_ -> x` → `const x` rewrites break
  the rank-2 `Mealy` type; library does not compile.
- F2 (blocker, test/AFRPLawsSpec.hs:98): seven law tests reduced to
  tautologies by hlint rewrites; see :103, :212, :221, :228, :235, :256.

## Verification

The script is verified by round-trip checks in a throwaway temp clone: write,
show, latest (including chain walk across two rounds), force-overwrite
refusal and acceptance, and push. No Haskell or cabal changes, so no
cabal2nix regeneration.