diff --git a/docs/specs/2026-10-02-review-notes-design.md b/docs/specs/2026-10-02-review-notes-design.md new file mode 100644 index 0000000..4cadb15 --- /dev/null +++ b/docs/specs/2026-10-02-review-notes-design.md @@ -0,0 +1,110 @@ +# 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: + round: + verdict: approve | request-changes + previous: none | + findings: + - id: F + severity: blocker | major | minor + file: : + title: + status: open | resolved | wontfix + ## Details + + + +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 ` — attach the note in `` 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 newest note for a branch: start at + the branch tip (default: current branch), and walk back through `previous` + until a note is found. Exit 1 if the chain is exhausted without a note. +- `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 ` (nothing printed 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. \ No newline at end of file