Skip to main content

A Review Loop Is a Design Signal

In one line: repeated findings of the same shape are a signal to examine policy ownership and missed consumers, not proof that every long review loop requires redesign.

Evidence and limits. The pilot partner reports one production project over one day: three pull requests took 10, 12 and 11 external review rounds, roughly eight hours each; other PRs reportedly closed in 1–2 rounds. Two changes introduced states interpreted by multiple readers: an unestablished assessment in a four-eyes gate (2,949 added lines in one module) and an effective case context in a measure register (4 callers across 9 application files). Authors reported repeatedly fixing one uncovered reader before consolidating policy in one function with caller-enumeration tests. Another change reportedly stopped successive model-supplied field leaks after introducing an identifier-only projection and property-enumeration tests.

These are supplied observations, not independently reproduced measurements in this kit. Their convergence is consistent with duplicated policy contributing to the loop. It does not establish equal task difficulty, that the final design was known at round one, or that restructuring alone caused the improvement. No comparative pricing establishes a review bot as the most expensive enumeration tool. One project and selected converged PRs do not establish the prevalence of this failure mode.

The nearest existing canon guidance is §3.2 Phase 2: categorize failures before choosing a fix. This section adds an explicit response to repeated review findings; it does not attribute a “three failed fixes means the architecture is wrong” rule to the canon.

R170 — The second same-shape finding. On the second confirmed instance of a previously fixed failure shape, enumerate affected consumers/producers within the approved change boundary before treating the family as resolved. Record searched roots, supported access forms, exclusions and unresolved discovery gaps. Use source search or an AST walk where appropriate, and add a regression that discovers an unregistered in-scope consumer. A list without a discovery mechanism cannot catch a newly added site. Test allowed registered consumers too.

Inspect whether policy is duplicated or whether consumers legitimately apply distinct rules. Address confirmed affected sites together where safe and authorized. Cross-boundary changes require the relevant owner; an urgent permitted containment fix can proceed while the broader family remains open. A single-site fix does not establish family-wide resolution.

R171 — Round four requires reassessment. Count rounds per predicate family, not total rounds per PR. At round four, before requesting another routine review, record the evidence and obtain a disposition from the authority responsible for the affected scope: structural correction, a justified bounded correction, or permitted deferral with an owner and tracking issue. Required design scrutiny still applies to a redesign. Reassessment is mandatory; centralizing policy is not a predetermined answer.

A justified bounded correction explains why it is safe, its independent tests, remaining discovery gaps and the next reassessment point. Existing incident authority may permit urgent containment before this review decision; record that action and retain unresolved structural work. A tracking issue does not waive a blocking defect, required check or non-waivable obligation. Deferral requires explicit authority and any applicable expiry/release consequences. Neither a low count nor a completed reassessment means the candidate is ready to merge.

R172 — Readers named before review. When a change introduces or reinterprets a state, identify its in-scope readers in the PR as a checkable list with discovery evidence and coverage limits. Where policy is genuinely shared, prefer one owning resolver and converted callers within one reviewed compatible change. “Resolver first” describes implementation ordering, not permission to deploy an intermediate inconsistent system. Retain independent behavioral tests for how callers use the answer.

Counting contract and pre-request self-pass.

  • A predicate family is a stable identifier for an invariant/failure shape, with linked findings and affected subjects. Assigning families requires judgment; a label is not objective proof.
  • A round is a completed review of an identified candidate revision assessing that family. Duplicate bot messages are not extra rounds. Record review/run/head references; unknown counts stay unknown.
  • Preserve family history across new workers, renamed PRs and split PRs. Do not reset the count to evade reassessment. Total PR rounds may be reported for cost, but never trigger R171 on their own.
  • Before re-review, enumerate newly affected sites, verify the changed revision and update the family's round count, findings, gaps and disposition. Four unrelated first-round families do not constitute a fourth round for any one family.

Detector and receipt. The skill and operating card prescribe a procedural review obligation; this kit does not ship a round collector or enforcement gate. Counts require attributable review records and family classification, not a coordinator's assertion alone. Source-level tests must state roots, languages, aliases and dynamic access they do or do not cover; they establish only the structural property actually asserted. Behavioral tests are separate.

Four is a chosen reference threshold, not an empirically optimal value. An adopter may set a different reviewed threshold from its workload, frozen before comparing results. Evaluate repeated versus distinct findings, review latency, correction effort, escaped defects and unfinished/deferred work—not fewer review rounds alone. See skill:s4u-code-review and appendix A §14.2.