standards: the commit that applies review findings is the highest-risk commit on the branch
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 0
- Forks
- 1
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 7
Description
The pattern
Across two consecutive branches in AudioClassifier (#696 → PR #700, and #632 → PR #703), the worst defect on the branch lived in the commit that applied the previous review's findings — six times.
The workflow that produced it is the standard one, and that is the problem: run a deep review, fix what it found, push, merge. The fix commit is the one piece of the branch nobody reviews, because the review that motivated it necessarily predates it.
Evidence
| Branch | The review-fix commit contained |
|---|---|
| #696 | Focus moved to a control destroyed by the very repaint that fires — the "fix" was measurably dead |
| #696 | Pointer and focus sharing one nullable timer, so either exit re-armed the clock while the other was engaged |
| #696 | A guard rewritten to be vacuous again — the assertion moved after a second action that also satisfied it, so flush: hide passed 13 unit + 36 e2e tests |
| #632 | A false user-visible string: "Compares the filter only", untrue whenever the gain is boosted |
| #632 | An orphaned sentence — three lines inserted mid-sentence left its tail stranded after a full stop |
| #632 | A comment asserting the opposite of the trade-off it was written to document |
Two of the six were strings shown to users, and one of those survived a device pass.
There is a compounding version too: on #632, a guard written during a review fix to catch the previous vacuous guard was itself vacuous — it matched a substring that also appeared in an unrelated dependency-wiring line, so deleting the real call left it green.
Why it happens
Three plausible mechanisms, all observed:
- The fix commit is written under "nearly done" pressure, after the hard thinking is finished and while the branch feels complete.
- Attention is on the finding, not the fix. The reviewer's item is treated as a spec; whether the edit that satisfies it is itself correct gets no second look.
- Review-fix edits are disproportionately prose and copy — comments, hints, labels — which no test covers and no linter checks, so the usual safety nets are absent exactly where the risk concentrates.
Proposal
Add to standards/CLAUDE.md, under the code-review section:
"The commit that applies a review's findings is the highest-risk commit on the branch, not the lowest. It is written after the hard thinking is done, its edits are disproportionately prose and copy that no test covers, and by construction it is the one commit the review that motivated it could not have seen. Review it as its own unit before pushing — diff it alone, not as part of the branch — and pay particular attention to any user-visible string it touches. 'The review passed' is never evidence about a commit that came after the review."
Acceptance criteria
- The rule is in
standards/CLAUDE.mdunder code-review standards - It states the mechanism (post-review edits are untested prose), not just the instruction, so it survives being skimmed
- It names the concrete action: diff the fix commit alone before pushing
-
deep-reviewis considered as a second delivery point — a re-review invoked on<last-reviewed-sha>..HEADrather than the whole branch (relates to #222, which proposes delta re-review; this issue is the reason that feature matters)
Relates to
- #222 — delta re-review against the last reviewed SHA is the mechanical form of this rule
- #188 — when a comment is found wrong, delete it (a review-fix edit that qualifies rather than deletes is the same failure)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the code-review section of standards/CLAUDE.md and read related issue #222 for the proposed delta re-review context. Add the rule's mechanism, the instruction to diff the review-fix commit alone, and the deep-review delivery point. Done means all acceptance criteria are represented in the standards and the wording remains consistent with the related guidance.
Written by the indexing model from the issue text.
Assessment
- Domain
- documentation
- Issue type
- Documentation
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100