Review notes protocol design

2026-10-02 14:22:24 +03:00
parent 4f1277aa45
commit 7ec491d383
1 changed files with 110 additions and 0 deletions
@@ -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: <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 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 <branch>` (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.