Review and close

Where the review runs decides what it costs

Run the adversarial review at the close and every defect it finds is close work by definition. It arrives after the sprint is nominally over, is repaired fast and late by whoever is still holding the context, and lands in the most load-bearing code in the project.

One measured run here delivered in five hours and took six and a half hours to close. Gate and test time across the whole close was about 70 minutes - roughly 18%. The rest was repair generated by a review that ran too late.

There was a compounding effect too: a review of the repairs then found nine more surviving mutants, because the repairs had been written fast, late and self-reviewed. Repairs made under close pressure are the least-reviewed code in a sprint, and they sit in the most load-bearing paths.

So the review runs inside delivery, one unit at a time, as each unit lands. Reviewed there, a finding is delivery work in the batch that caused it, priced against that batch, and fixed by a context that still holds it.

Reviewing a unit

One independent reviewer per unit. The brief comes from the tool, and the verdict goes into one ledger.

# brief the reviewer from the unit itself - never a hand-written prompt
critic.py brief --unit US0560 --seat qa

# record the verdict the close, the signature and the report all read
critic.py record --unit US0560 --verdict APPROVE --brief <fingerprint> \
  --reviewer "Sam (QA seat, a fresh context)" \
  --author   "Dani (Engineering seat)" \
  --issues   "[pre-existing] BG0402 the task list has no pagination (tasks.ts:12)"

Every finding carries its origin, decided by execution rather than impression, and findings are separated by ; :

Tag	Means	Blocks?
`[regression]`	this diff broke something that worked at the base ref	yes
`[new]`	this diff introduced a defect that did not exist	yes
`[pre-existing]`	already true of the tree, or already recorded in an open bug or change request	no - reported with its id

An untagged finding is refused, because it is the one a close cannot price against the batch that caused it. A clean pass records --issues none. The --brief value is the fingerprint critic.py brief printed, so the verdict names the prompt its reviewer was given.

The brief carries the seat’s charter, the unit’s acceptance criteria as law, its diff scope - the declared Affects against the run’s base ref - and the history of the files it touches. A hand-written prompt carries none of that, and quietly swaps a bounded review for an unbounded one.

Reviewer and author must differ. A self-review is the context that wrote the code agreeing with itself, and it clears nothing. A REJECT is recorded - the unit was reviewed - but it clears no gate.

Two rounds, then carry

Round one reviews. Round two re-checks the fixes, and it comes from the reviewer who rejected, because it answers that reviewer’s findings. critic.py record refuses a third round (review.max_rounds, default 2).

A unit still rejected at the cap is carried: its findings are filed as a bug, the report names it as a known issue, and it is delivered again in a later run. It is not argued over a third time.

A review judges the unit’s own diff

Scope is the unit’s declared Affects against the run’s base ref. That is what the origin tags above are judged against - git log -S, or a re-probe at the base ref - and only a regression or a newly introduced defect blocks. Anything already true of the tree, or already recorded in an open bug or change request, is reported with its id and does not hold the gate.

This is what makes reviews discriminating rather than uniformly negative. An unjustified REJECT is as much a failure as an unjustified APPROVE, and only a bounded review can tell the two apart.

Preview the close

sprint.py close --retro RETRO0001 --dry-run

It reports every refusal the close would raise, in one pass, and writes nothing. The action steps run against a scratch copy of the workspace, so the retro’s content is judged before you commit to anything. The close runs the same pre-flight first; this is the preview, not an extra step.

Close once

/sdlc-studio sprint close                     # scaffolds the retro, then stops
/sdlc-studio sprint close --retro RETRO0001   # the close, as one chain

Run it with no --retro the first time and it scaffolds the retro for you - allocated id, template, index row, batch and goal pre-filled from the run - then stops so you fill it. Never hand-author a retro; the scaffold is the one path that also wires its index row.

The chain is deterministic: goal verdict, retro validate and extract, lessons summary, the close gate, reconcile. A failing step is printed with its remedy, recorded as a known issue, and the chain runs on. It ends by filing the one-page sprint report you are about to sign. An unanswered item, a failing gate lane or a unit no independent verdict covers is a row on that report, not a reason to run the close again.

Four things still stop it: no Sprint Goal, no goal verdict, no retro, and an uncommitted change to a file a batch unit declares.

Sign once

sprint.py sign --report RPT0001 --principal "You"

You read the report the close filed, and you sign the run. That one command moves each unit to its terminal status - Done for a story, Fixed for a bug, each gated on its criteria having passed - cascades to the epic and the request above it, writes the run’s one signature, and stops. Nothing runs after it: a fact that moves after a signature is a fact the signature did not cover.

It refuses a principal who authored a unit or is recorded as its reviewer, before anything is written, and it never runs without an explicit --principal. It stops at the first unit whose Done gate is red, leaving the units already moved where they are, and a re-run resumes from there.

The compulsory checklist

The checklist is part of the sprint report, not a second document. It carries one row per stage of the cycle, from the pre-plan reconcile to the lessons, plus the figures a close otherwise re-derives by hand: planned against delivered, what was dropped or held or carried and why, scope creep as a count and a ratio, who reviewed what under which seat, and cost.

Every row but one is derived from the tree, because a checklist that asks you to retype what the files already hold gets filled in from memory. A row the run gave nothing to measure reads not measured, with its reason.

The exception is a judgement, and it is the honest part:

Carried finding	Ruling	Ruled by	Date
BG0402	not-stop-ship	you	…
CR0510	deferred	you	…

Every open finding takes a ruling - stop-ship, not-stop-ship, accepted-risk or deferred - in the retro’s ## Known issues carried table. An open finding with no row is reported UNRULED, because “we carried it” and “nobody looked” must never read the same. A stop-ship ruling does not refuse the close or the signature - the signer decides - but the report lists every stop-ship issue first, and sprint.py sign prints each one before it seals.

If an item genuinely does not apply, waive it on the record rather than fudging it:

decisions.py waive --subject rule:sprint-checklist:<item> --rationale "<why>" --authorised-by "<who>"

A finding at close is filed, never repaired inline

The close writes an account of the batch and then stamps the ledger saying the batch is accounted for. Anything reaching a terminal status after that stamp is, by construction, unaccounted for.

So a finding surfaced during a close is filed and deferred. Repairing inline produces a close that appears never to finish: every mechanical check passes, the run reads closed, and the ledger still says a close is owed. One run hit that twice in a single close.

Rolling cycles

/sdlc-studio sprint plan --cycles 3 --goal done --sprint-goal "<goal>"   # a standing policy: roll three cycles
/sdlc-studio sprint boundary --retro RETRO0001   # close this cycle, open the next from the live backlog

Each cycle regenerates its plan against the backlog as it stands, rather than executing a plan drawn before the last two sprints changed it.

Where to next

Delivery - the per-unit loop the review sits inside.
The retrospective - why it is checked on content and not existence.
Lessons - where a retro’s findings go, and how the next plan reads them.
Charters - queueing the next run before this one closes.