subagent-review-cycles git:20260624.681fa44A

Immutable. This exact content is served forever at /api/v1/blob/448e2c9578279772.

---
name: subagent-review-cycles
description: The Leader / Developer / Code-Reviewer triad pattern — Leader dispatches Developer for proposal → Code Reviewer for adversarial review (with WebSearch, no CLI) → Leader accepts/rejects with specific reasons → iterate up to `limit(N)` rounds. Round-1 cosmetic-grade fixes get inline-applied by Leader instead of consuming a round. Invoke when planning multi-round review on a document, dispatching a Code Reviewer subagent, or when asked "how many review rounds / what skills should the Code Reviewer carry".
---

# Subagent Review Cycles

## When to invoke

- About to dispatch a sub-agent to draft a technical document / design section.
- The first version of a document is ready and a Code Reviewer should audit technical correctness.
- User asks "how many review rounds", "should Code Reviewer run CLI", "what counts as rejection".

## The triad

| Role | Job |
|---|---|
| **Leader** (main agent) | Dispatch, integrate review results, accept / reject, communicate with the user |
| **Developer** (subagent) | Draft / revise document sections; works after Leader replies with rejection. (The Developer role is often dispatched as a Software Architect sub-agent type when the task is architectural in nature, but the role name remains "Developer".) |
| **Code Reviewer** (subagent) | Audits Developer output for technical / API correctness and logic gaps; **CLI forbidden**, WebSearch allowed |

The Leader **never** writes the implementation / drafts a section directly — that's the Developer's job. The Leader **never** does the review either — that's the Code Reviewer's job.

## Round structure

```
round N:
  Leader → Developer: dispatch (scope + skills + inputs + return format + criteria)
  Developer → Leader: draft
  Leader → Code Reviewer: dispatch (review criteria + must use WebSearch, no CLI)
  Code Reviewer → Leader: BLOCKER / MAJOR / MINOR list
  Leader: ACCEPT / REJECT each item with explicit reason
  if accepted_count == total: done
  if N == limit: pause, report to user
  else: round N+1 with feedback
```

`limit(N)` is typically 3–5. **If the limit is reached without convergence**: pause, report to the user, wait for direction.

## The "round-1 cosmetic" pragmatic rule

If a round-1 result has **only spelling / formatting / paragraph order / string typo** cosmetic-grade fixes left:
- **Leader applies inline edits directly** instead of dispatching round 2 to the Developer
- Still record these fixes in the meeting log
- Reasoning: "limit is an upper bound, not a requirement"; burning a whole round just for typos is uneconomical

Criteria: the fix can be completed by the Leader in ≤ 5 minutes and involves **no new decisions**.

## Dispatch contract for Code Reviewer

Every Code Reviewer dispatch prompt must include:

1. **Target file / section scope** (explicit file + section)
2. **Forbidden tools**: CLI / Bash trial-and-error
3. **Allowed tools**: WebSearch / WebFetch (for verifying Apple APIs, library behaviour)
4. **Review criteria** (4 dimensions + domain-specific checklist):
   - Technical correctness (API name, behaviour, version)
   - Logical consistency (internal contradictions, cross-section conflicts)
   - Completeness (missing edge case, error handling, prerequisite)
   - Efficiency (algorithm, CI / build / runtime cost)
5. **Return format**: BLOCKER / MAJOR / MINOR three-level classification; each item with location (file + section) + suggestion

## Accept / Reject reply style

For each review finding the Leader gives:

- **ACCEPT + reason**: accepted; specify who fixes it this round
- **REJECT + reason**: rejected with a technical reason (not just "no")
- **DEFER**: acknowledged but deferred (goes to backlog / open items)

REJECT must cite specific evidence (API doc, prior decision, design constraint); pure preference is not acceptable.

## Anti-patterns

- **Using CLI to probe Apple API behaviour**: forbidden. Use official docs / WebSearch instead.
- **Repeatedly rejecting the same point in the same section**: more than 2 identical rejections counts as a communication failure; pause and clarify with the user.
- **ACCEPT without a reason**: every ACCEPT should still have a one-line note of why it adds value.
- **Leader drafting sections themselves**: violates the role separation; only allowed for cosmetic-grade fixes.

## Verification checklist

- Each round has an explicit dispatch prompt (all 5 elements present).
- Each review finding has an explicit accept / reject label + reason.
- When limit(N) is reached without convergence, pause; don't keep iterating indefinitely.
- Cosmetic fixes are inline-edited by the Leader; don't burn a round on them.
- The round-summary is recorded in the meeting log (not a verbatim copy of review content).

## Phase TODO sweep checklist

A separate close-the-loop activity that fires **once per phase** (not once per review round): before the Leader signs off on a phase-completion PR, run a sweep against the phase's diff scope to catch deferred-and-forgotten debt.

**Command** (Leader-run; against the phase's diff scope, not the full repo):

```
rg -n --no-heading -e 'TODO|FIXME|XXX|HACK|stub|placeholder|Phase [0-9]+ Part' <source-root>/
```

Replace `<source-root>/` with the actual source directory for this project (e.g. `Packages/<target>/Sources/`, `Sources/`, `src/`). The path must match the repo's layout — if it doesn't, the sweep silently finds nothing and the phase incorrectly appears clean.

**Disposition rule** — every match must fall into exactly one bucket; otherwise the phase is not complete:

1. **Resolved this phase** — fixed or implemented before merge.
2. **Moved to §Backlog** — routed to the topic-appropriate document's §Backlog section (product → `design.md`, engineering → `foundations.md`, implementation step → `plan.md`, collaboration → `methodology.md`), and the §Backlog entry **cites the source `file:line`** so the debt is traceable.
3. **Intentionally left** — documented in the phase meeting log as "intentionally left, see <follow-up issue / phase reference>".

**Stub / placeholder code without a literal TODO comment still counts.** Identifier names like `xxxPlaceholder`, `xxxStub`, or scaffolding values that are not real implementations must be flagged in the phase log even if the regex didn't catch them via comment text.

**Ownership**: the Leader runs the sweep. Subagents may flag debt during implementation (in their impl-notes `§未決`), but closing the loop before phase merge is non-delegable. A PR reviewer subagent dispatched for the phase-completion PR should also run the sweep against the diff and report findings as MAJOR (unless already documented per the disposition rule above).

**When to invoke**:
- Before declaring a phase complete.
- When dispatching a Code Reviewer subagent on a phase-completion PR — include this command in the review brief's checklist.
- When auditing whether a previously-declared "complete" phase actually was.

**Anti-pattern**: marking a regex match as "obviously fine, ignore" without writing the disposition down. The point of the sweep is the paper trail; an undocumented justification is indistinguishable from forgetting.

## Related skills

- `leader-developer-handoff-contract`: details the 5 elements of each sub-agent dispatch.
- `spec-phase-orchestration`: review cycles are usually embedded in the spec phase.
- `methodology-pattern-extractor`: "round-1 cosmetic inline edit" is a codifiable pattern.