TimZander / TimZander/claude

standards: the commit that applies review findings is the highest-risk commit on the branch

Open Beginner friendly
#231 0 comments 0 reactions 0 assignees View on GitHub

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:

  1. The fix commit is written under "nearly done" pressure, after the hard thinking is finished and while the branch feels complete.
  2. 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.
  3. 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.md under 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-review is considered as a second delivery point — a re-review invoked on <last-reviewed-sha>..HEAD rather 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.