Work through review findings across pushes

Merged
JS
3d ago
+900-97

An interactive review of a long-lived change loses its findings when the branch moves, because a finding is recorded as a line number and the number goes stale after a push. This adds a plain Markdown ledger (Pending, Completed, Questions, Notes) to the review skill, and review-ledger.py, which places each Pending finding again by the quoted text of its line: exact when the recorded line still holds the quote, moved to the nearest line that does, stale when none does. --write rewrites moved bullets and stamps the head; stale, unchecked and unparsed bullets are reported and left for a person to read, so the counts add up. The format and the reading order (touched tests first, then the code no test reaches) are adapted from externals/codereview and written up in interactive.md.

The same change tightens three neighbouring documents: the tests lens now reads removed, skipped and loosened tests first, the UI review rule asks for a document.elementFromPoint hit test before trusting a pointer, and codetrial-change says to update the document that states a behavior in the same commit. The workbench doc gains the sections that decide whether a large-change explanation applies and how readiness is tracked, and SKILL.md drops the text that moved there.

tests/test_review_ledger.py has 34 cases and is wired into scripts/test.sh as review-ledger-tests; it passes under python3.8 and python3.12 through scripts/run-python-tests.py. The branch went through a local interactive review and its explanation was recorded for this head. I did not run the full scripts/test.sh, scripts/check-skill-routing.py (it needs a logged-in claude) or remote CI. Known gaps, left as questions: a quote found on several lines is moved to the nearest one without a marker, a symlinked ledger is replaced by a regular file on --write, and a code fence opener indented four or more spaces is still treated as a fence.

Summary by cubic

Makes codetrial-review findings survive pushes and turns review into a long-lived branch: a plain Markdown ledger (Pending, Completed, Questions, Notes) now carries the interactive review, and review-ledger.py places each Pending finding again at a new head by the quoted text of its line. --write rewrites moved bullets and stamps the head; stale, unchecked, and unparsed items are reported and left for a person to read.

The same change tightens three neighboring docs: the tests lens reads removed, skipped, or loosened tests first; the UI review rule asks for a document.elementFromPoint hit test instead of trusting an element's rect; and codetrial-change requires updating the document that states a changed behavior in the same commit, with its reference table recording what it adopted from the codereview tool.

  • The large-change explanation steps move from SKILL.md into workbench.md, which gains sections on when the requirement applies and how readiness is tracked.
  • Adds 34 test cases for the ledger script, wired into scripts/test.sh.
  • The PR-description workflow opts into actions/checkout@v7's fork-pr checkout so interactive review stops failing on every pull request from a fork.
  • Placement matches a quote keeping its indentation and writes through a symlinked ledger so the link stays.
  • Known gaps are left as questions: a quote found on several lines goes to the nearest without a marker, and an indented code fence opener is still treated as a fence.