Skip to content

A Re-Review's Base Is the Last Fully Covered Pass

Problem

SPEC-216 lets a review pass after the first select its reviewers from the diff since a base, the head of an earlier pass, instead of from the whole branch. The roster gets smaller and cheaper. The risk is the reverse of what it looks like: if the base moves past code that a reviewer was owed and never saw, a later pass selects that reviewer from a diff that no longer contains the code. That reviewer then never sees the code at all, and every gate passes.

Review passes 3 to 7 each found one new High of this kind, and each was a different way to move the base too early:

Pass How the base moved too early
5–6 A pass counted once the complete event existed, although one dispatched reviewer had not returned (P6-SEC-1)
6 A narrowing complete for an older pass was written after a later pass had already used it as its base (P6-SEC-6, P7-SEC-4)
7 A reviewer was selected but omitted from the dispatch, and the base only checked the dispatched set (P7-DIG-1)
7 check-gate tied reviewers_returned in one direction only, so a gate could claim reviewers the ledger never completed (P7-SEC-3)

Solution

The base is the head of the last pass whose complete names the whole selection and the whole dispatch. Nothing less counts:

if sel is not None and disp and set(sel['selected']) | set(disp[0]['dispatched']) <= returned[p]:
    base = sel['head']

The rules around that test:

  • A partial return does not move the base. The pass stays usable as a review, but the next pass selects from the old base.
  • An omission does not move the base either. A selected reviewer the pass did not dispatch is owed the same code as one that did not return.
  • History is append-only in time. A complete for pass N is refused once pass N+1 is dispatched, including a narrowing one, because pass N+1 may already have used pass N as its base.
  • The gate and the ledger agree in both directions. reviewers equals the dispatch event's list exactly, and reviewers_returned equals what the complete names. A pass without a complete counts as nothing returned, not as everything.

Prevention

  • When a step narrows who looks at what, state the invariant as coverage: every reviewer who is owed a range of code sees it in some pass. Then test each way that range can be lost (a missing return, an omission, a late correction, a gate over-claim) as its own mutant. One fixture per rule was not enough here: each fix closed one route and the next pass found another.
  • Fall back to the broad selection whenever no pass qualifies. A wider roster costs money. A narrower one can hide a defect permanently.

Related: docs/solutions/patterns/verdict-needs-a-denominator.md, and the SPEC-216 decisions (specs/216-scoped-reviewer-execution-and-re-review-selection/decisions.md, Decisions 18 and 20).