Compare commits

...

25 Commits

Author SHA1 Message Date
Jesse Vincent
7cf78437e2 Spec: record iterations 2-3 results and final frozen-config matrix 2026-06-10 05:06:59 -07:00
Jesse Vincent
e355795625 Hand reviewers the diff as a file, not a paste
Paste adoption stayed at 0/15 even as a Red Flag — and the controller's
reluctance is locally rational: pasting loads the diff into the (most
expensive) controller context permanently, while a reviewer self-fetch
costs a few cheap turns. The diff-file handoff is cheap for both sides:
the controller redirects git diff to /tmp without reading it, and the
reviewer gets the whole change in one Read call.
2026-06-10 03:44:19 -07:00
Jesse Vincent
29ee4e8e44 Reviewer skepticism covers the implementer's design rationales
Fourth planted-defect failure mode: the implementer's self-report said
'noted mild structural duplication; left unabstracted per YAGNI' and the
reviewer deferred to that framing, rating the duplication no finding at
all. The pre-judging keeps relocating — controller prompt, then reviewer
calibration, now the implementer's report. Rationales are claims; they
never downgrade severity.
2026-06-10 02:20:28 -07:00
Jesse Vincent
28498a5cde Make diff-pasting non-optional for task reviewer dispatch
Adoption was 6/11 reviews on fractals and 0/17 on svelte when phrased
as guidance; reviewers without the diff re-derive it by hand, which is
the single largest remaining reviewer cost. Now a Red Flags Never entry
and a REQUIRED marker on the template placeholder.
2026-06-10 02:10:34 -07:00
Jesse Vincent
5e2907fc4f Close the Minor-severity escape hatch
With merged review, a planted verbatim-duplication defect shipped: the
reviewer rated it Minor (YAGNI) under the strict cannot-be-trusted
definition of Important, and the Minor-rolls-up rule meant no fix was
ever dispatched and the final review never saw the finding. Calibration
now names merge-blocking maintainability damage (verbatim duplication,
swallowed errors, assertion-free tests) as Important, and controllers
must paste accumulated Minor findings into the final review dispatch.
2026-06-10 02:09:10 -07:00
Jesse Vincent
e532f24df7 Spec: document cost iterations and the per-task review consolidation 2026-06-09 23:59:22 -07:00
Jesse Vincent
e3c74fc1c9 Merge per-task reviews into one task reviewer (iteration 2)
Iteration-1 profiling: implementers and per-dispatch overhead dominate
(429 of 686 subagent turns; controller coordination is half the dollars
and scales with dispatch count), reviewers are individually lean, and
the controller pasted the diff in only 2 of 22 review dispatches when
the guidance was phrased as optional.

Changes: spec-reviewer-prompt.md + code-quality-reviewer-prompt.md
replaced by task-reviewer-prompt.md (one reviewer, one reading of a
pasted diff, two verdicts: spec compliance //⚠️ and task quality);
one fix dispatch can address both kinds of findings; controller now
runs git diff itself and pastes it (imperative, not optional);
implementers run focused tests while iterating and the full suite once
before committing; flowchart, example, Red Flags, tool tables updated.
The broad final whole-branch review is unchanged.
2026-06-09 23:58:28 -07:00
Jesse Vincent
3e3e1e701e Cut review-cost drivers: turn-aware models, inline diffs, scoped evidence
Round-2 fractals eval regressed to 70min/32.2M tokens (vs round-1's
42.8min/14.5M) while reaching baseline-parity quality. Per-subagent turn
profiling attributed it to: haiku dispatches taking 2-3x the turns of
sonnet (678 of 1197 subagent turns), reviewers re-fetching diffs by hand
(518 Bash calls), and evidence-rule narration. Changes: turn-count-beats-
token-price model guidance; controllers paste small diffs into reviewer
prompts (reviewers then need few or no tool calls); evidence scoped to
findings and would-be-bare-yes checks; Important defined as cannot-trust-
until-fixed with coverage suggestions Minor; fixes dispatched only for
Critical/Important.
2026-06-09 22:42:54 -07:00
Jesse Vincent
853396e3ae Add phrase-level pre-judging triggers to reviewer prompt rule
Resumed the offending eval controller session and asked it why it
pre-judged despite the rule being in context. Its retrospective: the
motive was avoiding a review loop, the abstract rule was read but not
applied at the moment it governs, and a phrase-level trigger ('do not
flag', 'at most Minor', 'don't treat X as a defect', 'the plan chose')
would have fired where the principle did not.
2026-06-09 21:49:51 -07:00
Jesse Vincent
83d54f7ddd Red Flags: never tell a reviewer what not to flag or pre-rate severity
Second observed instance: with the Constructing Reviewer Prompts rule
already live, a controller still wrote 'do not treat that duplication as
a defect to fix — the plan chose it; you may note it as a Minor
observation at most' into a quality reviewer dispatch, fabricating plan
intent from the plan's example snippet. Promote the rule to the Red
Flags Never list and name the rationalization.
2026-06-09 21:47:41 -07:00
Jesse Vincent
c7900f1698 Close three review blind spots found by defect tracing
Live eval deliverables shipped five polish defects; tracing each through
the transcripts showed three mechanisms, each now addressed:
- reviewers answered pointed checklist items with unsupported yes
  (evidence rule: every What-to-Check answer needs file:line evidence)
- no reviewer ever saw the design's global constraints (controllers now
  paste binding constraints into task requirements)
- test output noise was invisible everywhere (pristine-output checks in
  implementer self-review and quality review)
2026-06-09 21:19:08 -07:00
Jesse Vincent
5cfdb75b94 Require explicit model on subagent dispatch
In live eval runs, controllers given judgment-based model selection
stopped passing a model at all; the omitted parameter inherits the
session's top-tier model, silently making every subagent maximally
expensive (one run dispatched 26/26 reviewers on the session model).
2026-06-09 21:11:45 -07:00
Jesse Vincent
87825ff193 Forbid controllers pre-judging reviewer findings
A live eval run of sdd-quality-reviewer-catches-planted-defect caught the
SDD controller fabricating a plan constraint and instructing the quality
reviewer not to flag the planted DRY violation. The duplication shipped.
Constructing Reviewer Prompts now bans suppression directives alongside
open-ended broadening directives.
2026-06-09 18:28:24 -07:00
Jesse Vincent
09cb4d7361 Sync plan: escaped pre() pattern in Task 5 checks block 2026-06-09 18:19:00 -07:00
Jesse Vincent
b3bb9a68d7 Fix plan doc: correct Task 1 grep expectation; sync Task 5 story block 2026-06-09 17:21:06 -07:00
Jesse Vincent
71dc271a08 Sync plan's Task 5 blocks with review fixes 2026-06-09 17:13:03 -07:00
Jesse Vincent
5aea3dca31 SDD controller: reviewer prompt budgets, ⚠️ handling, final-review pointer, model judgment 2026-06-09 16:59:05 -07:00
Jesse Vincent
b3281c0227 Implementer prompt: re-run covering tests after fixing review findings 2026-06-09 16:56:28 -07:00
Jesse Vincent
c14c1de552 Scope spec reviewer's Your Job wording to the diff 2026-06-09 16:55:28 -07:00
Jesse Vincent
be8a6269c4 Spec reviewer: judge from the diff, grounded skepticism, ⚠️ verdict channel 2026-06-09 16:53:30 -07:00
Jesse Vincent
da41209243 Use bare placeholder names in quality reviewer prompt body 2026-06-09 16:51:54 -07:00
Jesse Vincent
2cc449b6d4 Make per-task quality reviewer prompt self-contained and task-scoped 2026-06-09 16:47:27 -07:00
Jesse Vincent
f8dcd1ed3d Add implementation plan for task-scoped review dispatch 2026-06-09 16:42:50 -07:00
Jesse Vincent
4192572d19 Harden review-dispatch spec per adversarial review findings 2026-06-09 16:33:44 -07:00
Jesse Vincent
5da15d7eba Add design spec: task-scoped review dispatch for SDD 2026-06-09 16:26:00 -07:00
9 changed files with 1164 additions and 157 deletions

View File

@@ -0,0 +1,774 @@
# SDD Task-Scoped Review Dispatch Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Scope SDD's per-task reviews to the task (diff-first reading, justified broadening, no redundant test runs) while final branch review stays broad.
**Architecture:** Four prose edits to the subagent-driven-development skill (the per-task quality prompt becomes self-contained instead of delegating to the merge-readiness template; the spec prompt gets a third verdict channel and grounded skepticism; the implementer prompt gains a re-run-after-fix rule; SKILL.md gets controller guidance) plus one new eval scenario in the `evals/` submodule. `skills/requesting-code-review/` is deliberately untouched.
**Tech Stack:** Markdown skill files; Python setup helper + bash checks + story.md for the quorum eval.
**Spec:** `docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md` — read it before starting. Decisions already settled there: full re-reviews stay; the two review stages stay separate; coordinator keeps model judgment; `requesting-code-review/` stays broad.
**These are behavior-shaping prose files, not code.** There are no unit tests for them. Each task's verification steps are exact `grep` checks that the edit landed; behavioral verification is Task 6 (static) and Task 7 (live evals, maintainer-gated).
---
### Task 1: Rewrite the per-task quality reviewer prompt as self-contained
The current file delegates to `../requesting-code-review/code-reviewer.md`, which is a merge-readiness review (architecture, security, production readiness, "Ready to merge?"). Replace the entire file with a self-contained, task-scoped template.
**Files:**
- Rewrite: `skills/subagent-driven-development/code-quality-reviewer-prompt.md`
- [ ] **Step 1: Replace the full file contents with:**
````markdown
# Code Quality Reviewer Prompt Template
Use this template when dispatching a code quality reviewer subagent.
**Purpose:** Verify one task's implementation is well-built (clean, tested, maintainable)
**Only dispatch after spec compliance review passes.**
```
Subagent (general-purpose):
description: "Review code quality for Task N"
prompt: |
You are reviewing one task's implementation for code quality. This is a
task-scoped gate, not a merge review — a broad whole-branch review happens
separately after all tasks are complete.
## What Was Implemented
[DESCRIPTION]
## Task Requirements (context only)
[TASK_TEXT]
## Git Range to Review
**Base:** [BASE_SHA]
**Head:** [HEAD_SHA]
```bash
git diff --stat [BASE_SHA]..[HEAD_SHA]
git diff [BASE_SHA]..[HEAD_SHA]
```
## Read-Only Review
Your review is read-only on this checkout. Do not mutate the working tree,
the index, HEAD, or branch state in any way. Use tools like `git show`,
`git diff`, and `git log` to inspect history.
## Scope
Spec compliance was already verified by a separate reviewer. Do not
re-check whether the code matches the requirements or the plan.
Start from the diff. Read the changed files first. Inspect code outside
the diff only to evaluate a concrete risk you can name — and name it in
your report. Cross-cutting changes are legitimate named risks: if the
diff changes lock ordering, a function or API contract, or shared mutable
state, checking the call sites is the right method. Do not crawl the
codebase by default.
## Tests
The implementer already ran the tests and reported results with TDD
evidence for exactly this code. Do not re-run the suite to confirm their
report. Run a test only when reading the code raises a specific doubt
that no existing run answers — and then a focused test, never a
package-wide suite, race detector run, or repeated/high-count loop. If
heavy validation seems warranted, recommend it in your report instead of
running it. If you cannot run commands in this environment, name the
test you would run.
## What to Check
**Code quality:**
- Clean separation of concerns?
- Proper error handling?
- DRY without premature abstraction?
- Edge cases handled?
**Tests:**
- Do the new and changed tests verify real behavior, not mocks?
- Are the task's edge cases covered?
**Structure:**
- Does each file have one clear responsibility with a well-defined interface?
- Are units decomposed so they can be understood and tested independently?
- Is the implementation following the file structure from the plan?
- Did this change create new files that are already large, or
significantly grow existing files? (Don't flag pre-existing file
sizes — focus on what this change contributed.)
## Calibration
Categorize issues by actual severity. Not everything is Critical.
Acknowledge what was done well before listing issues — accurate praise
helps the implementer trust the rest of the feedback.
## Output Format
### Strengths
[What's well done? Be specific.]
### Issues
#### Critical (Must Fix)
[Bugs, data loss risks, broken functionality]
#### Important (Should Fix)
[Poor error handling, test gaps, structural problems]
#### Minor (Nice to Have)
[Code style, optimization opportunities]
For each issue:
- File:line reference
- What's wrong
- Why it matters
- How to fix (if not obvious)
### Assessment
**Task quality:** [Approved | Needs fixes]
**Reasoning:** [1-2 sentence technical assessment]
```
**Placeholders:**
- `[DESCRIPTION]` — task summary, from implementer's report
- `[TASK_TEXT]` — the task's requirements text or plan reference, for context
- `[BASE_SHA]` — commit before this task
- `[HEAD_SHA]` — current commit
**Reviewer returns:** Strengths, Issues (Critical/Important/Minor), Task quality verdict
````
- [ ] **Step 2: Verify the rewrite landed**
Run: `grep -c "requesting-code-review" skills/subagent-driven-development/code-quality-reviewer-prompt.md || echo ABSENT`
Expected: `ABSENT` (no more delegation)
Run: `grep -n "Task quality:" skills/subagent-driven-development/code-quality-reviewer-prompt.md | head -2`
Expected: one match (the Output Format verdict line; the "Reviewer returns" footer says "Task quality verdict" without a colon)
Run: `grep -n "worktree add\|Ready to merge" skills/subagent-driven-development/code-quality-reviewer-prompt.md || echo CLEAN`
Expected: `CLEAN`
- [ ] **Step 3: Commit**
```bash
git add skills/subagent-driven-development/code-quality-reviewer-prompt.md
git commit -m "Make per-task quality reviewer prompt self-contained and task-scoped"
```
---
### Task 2: Spec reviewer prompt cleanups
Four exact edits to `skills/subagent-driven-development/spec-reviewer-prompt.md`. Current line numbers refer to the file as of commit f55642e.
**Files:**
- Modify: `skills/subagent-driven-development/spec-reviewer-prompt.md`
- [ ] **Step 1: Add the judge-from-the-diff clause.** After the line (currently line 31):
```
Only read files in this diff. Do not crawl the broader codebase.
```
insert a blank line and:
```
Spec compliance is judged by reading the diff against the requirements.
The implementer already ran the tests and reported TDD evidence — do not
re-run them. If a requirement cannot be verified from this diff alone
(it lives in unchanged code or spans tasks), report it as a ⚠️ item
instead of broadening your search.
```
- [ ] **Step 2: Trim the read-only section.** Replace (currently line 35):
```
Your review is read-only on this checkout. Do not mutate the working tree, the index, HEAD, or branch state in any way. Use tools like `git show`, `git diff`, and `git log` to inspect history. If you need a working copy of a different revision, check it out into a separate temporary directory (e.g. `git worktree add /tmp/review-[SHA] [SHA]`) — never move HEAD on this checkout.
```
with:
```
Your review is read-only on this checkout. Do not mutate the working tree, the index, HEAD, or branch state in any way. Use tools like `git show`, `git diff`, and `git log` to inspect history.
```
- [ ] **Step 3: Ground the skepticism.** Replace (currently lines 39-40):
```
The implementer finished suspiciously quickly. Their report may be incomplete,
inaccurate, or optimistic. You MUST verify everything independently.
```
with:
```
Treat the implementer's report as unverified claims about the code. It may
be incomplete, inaccurate, or optimistic. Verify the claims against the diff.
```
- [ ] **Step 4: Add the third verdict channel.** Replace (currently lines 74-76):
```
Report:
- ✅ Spec compliant (if everything matches after code inspection)
- ❌ Issues found: [list specifically what's missing or extra, with file:line references]
```
with:
```
Report:
- ✅ Spec compliant (if everything matches after code inspection)
- ❌ Issues found: [list specifically what's missing or extra, with file:line references]
- ⚠️ Cannot verify from diff: [requirements you could not verify from the
diff alone, and what the controller should check — report alongside the
✅/❌ verdict for everything you could verify]
```
- [ ] **Step 5: Verify**
Run: `grep -n "suspiciously\|worktree add" skills/subagent-driven-development/spec-reviewer-prompt.md || echo CLEAN`
Expected: `CLEAN`
Run: `grep -c "⚠️" skills/subagent-driven-development/spec-reviewer-prompt.md`
Expected: `2` (judge-from-diff clause + verdict channel)
- [ ] **Step 6: Commit**
```bash
git add skills/subagent-driven-development/spec-reviewer-prompt.md
git commit -m "Spec reviewer: judge from the diff, grounded skepticism, ⚠️ verdict channel"
```
---
### Task 3: Implementer prompt — re-run tests after fixing review findings
The reviewers' "don't re-run the implementer's tests" rule assumes the implementer re-runs tests after every fix. Make that real.
**Files:**
- Modify: `skills/subagent-driven-development/implementer-prompt.md`
- [ ] **Step 1: Insert a new section.** Immediately before the line (currently line 100):
```
## Report Format
```
insert:
```
## After Review Findings
If a reviewer finds issues and you fix them, re-run the tests that cover
the amended code and include the results in your fix report. Reviewers
will not re-run tests for you — your report is the test evidence.
```
- [ ] **Step 2: Verify**
Run: `grep -n "After Review Findings" skills/subagent-driven-development/implementer-prompt.md`
Expected: one match, on a line before `## Report Format`
- [ ] **Step 3: Commit**
```bash
git add skills/subagent-driven-development/implementer-prompt.md
git commit -m "Implementer prompt: re-run covering tests after fixing review findings"
```
---
### Task 4: SKILL.md controller changes
Six exact edits to `skills/subagent-driven-development/SKILL.md`. Current line numbers refer to commit f55642e.
**Files:**
- Modify: `skills/subagent-driven-development/SKILL.md`
- [ ] **Step 1: Point the final-review flowchart node at the broad template.** The node label `Dispatch final code reviewer subagent for entire implementation` appears 3 times (currently lines 65, 84, 85). In all 3 occurrences, replace the label string with:
```
Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)
```
(Graphviz nodes are matched by label text — all three must be byte-identical or the graph grows a phantom node.)
- [ ] **Step 2: Model selection by judgment.** Replace (currently lines 97-99):
```
**Architecture, design, and review tasks**: use the most capable available model.
**Task complexity signals:**
```
with:
```
**Architecture and design tasks**: use the most capable available model.
**Review tasks**: choose the model with the same judgment, scaled to the
diff's size, complexity, and risk. A small mechanical diff does not need the
most capable model; a subtle concurrency change does.
**Task complexity signals (implementation tasks):**
```
- [ ] **Step 3: Add controller guidance sections.** Immediately before the line (currently line 122):
```
## Prompt Templates
```
insert:
```
## Handling Spec Reviewer ⚠️ Items
The spec reviewer may report "⚠️ Cannot verify from diff" items — requirements
that live in unchanged code or span tasks. These do not block dispatching the
code quality reviewer, but you must resolve each one yourself before marking
the task complete: you hold the plan and cross-task context the reviewer
lacks. If you confirm an item is a real gap, treat it as a failed spec
review — send it back to the implementer and re-review.
## Constructing Reviewer Prompts
Per-task reviews are task-scoped gates. The broad review happens once, at the
final whole-branch review. When you fill a reviewer template:
- Do not add open-ended directives like "check all uses" or "run race tests
if useful" without a concrete, task-specific reason
- Do not ask a reviewer to re-run tests the implementer already ran on the
same code — the implementer's report carries the test evidence
```
- [ ] **Step 4: Prompt Templates list — add the final-review pointer.** Replace (currently line 126):
```
- [code-quality-reviewer-prompt.md](code-quality-reviewer-prompt.md) - Dispatch code quality reviewer subagent
```
with:
```
- [code-quality-reviewer-prompt.md](code-quality-reviewer-prompt.md) - Dispatch code quality reviewer subagent
- Final whole-branch review: use superpowers:requesting-code-review's [code-reviewer.md](../requesting-code-review/code-reviewer.md)
```
- [ ] **Step 5: Example workflow verdict vocabulary.** Two replacements:
Replace (currently line 157):
```
Code reviewer: Strengths: Good test coverage, clean. Issues: None. Approved.
```
with:
```
Code reviewer: Strengths: Good test coverage, clean. Issues: None. Task quality: Approved.
```
Replace (currently line 191):
```
Code reviewer: ✅ Approved
```
with:
```
Code reviewer: ✅ Task quality: Approved
```
(The final reviewer's "ready to merge" line, currently line 199, stays.)
- [ ] **Step 6: Integration section.** Replace (currently line 272):
```
- **superpowers:requesting-code-review** - Code review template for reviewer subagents
```
with:
```
- **superpowers:requesting-code-review** - Code review template for the final whole-branch review
```
- [ ] **Step 7: Verify**
Run: `grep -c "Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)" skills/subagent-driven-development/SKILL.md`
Expected: `3`
Run: `grep -n "most capable available model" skills/subagent-driven-development/SKILL.md`
Expected: exactly one match (architecture/design bullet)
Run: `grep -n "Handling Spec Reviewer\|Constructing Reviewer Prompts" skills/subagent-driven-development/SKILL.md`
Expected: two section headers, both before `## Prompt Templates`
Run: `grep -c "Task quality: Approved" skills/subagent-driven-development/SKILL.md`
Expected: `2`
- [ ] **Step 8: Commit**
```bash
git add skills/subagent-driven-development/SKILL.md
git commit -m "SDD controller: reviewer prompt budgets, ⚠️ handling, final-review pointer, model judgment"
```
---
### Task 5: New eval scenario — per-task quality reviewer catches a planted defect
Lives in the `evals/` **submodule** (separate repo, `superpowers-evals`). Work on a branch there; the parent submodule-pointer bump happens at finishing time per `evals/CLAUDE.md`.
The fixture plan's Task 2 implementation snippet duplicates Task 1's formatting logic verbatim. The duplication is spec-compliant, so the spec reviewer should pass it — the per-task quality reviewer is the gate under test (DRY violation).
**Files:**
- Create: `evals/setup_helpers/sdd_quality_defect_plan.py`
- Modify: `evals/setup_helpers/__init__.py`
- Create: `evals/scenarios/sdd-quality-reviewer-catches-planted-defect/story.md`
- Create: `evals/scenarios/sdd-quality-reviewer-catches-planted-defect/setup.sh`
- Create: `evals/scenarios/sdd-quality-reviewer-catches-planted-defect/checks.sh`
- [ ] **Step 0: Branch in the submodule**
```bash
cd evals
git checkout -b sdd-quality-defect-scenario
```
- [ ] **Step 1: Create `evals/setup_helpers/sdd_quality_defect_plan.py`:**
````python
"""Setup helper for the sdd-quality-reviewer-catches-planted-defect scenario.
Scaffolds a tiny Node project with a 2-task plan whose Task 2
implementation snippet duplicates Task 1's formatting logic verbatim.
The duplication is spec-compliant — the requirements only describe
behavior — so the spec compliance reviewer should pass it. The test
measures whether the per-task code quality reviewer catches the DRY
violation and forces a refactor in the review-fix loop.
"""
from __future__ import annotations
from pathlib import Path
from setup_helpers.base import _git
PACKAGE_JSON = """\
{
"name": "report-quality",
"version": "1.0.0",
"type": "module",
"scripts": {
"test": "node --test"
}
}
"""
PLAN_BODY = """\
# Report Formatter — Implementation Plan
Two report formatting functions. Implement exactly what each task
specifies.
## Task 1: User Report
**File:** `src/report.js`
**Requirements:**
- Function named `formatUserReport`
- Takes one parameter `user`: an object with `name`, `email`, `visits`
- Returns a multi-line string: a banner of 40 `=` characters, then
`Report for <name> <<email>>`, then the banner again, then
`Visits: <visits>`, then a closing banner
- Export the function
**Implementation:**
```javascript
export function formatUserReport(user) {
const banner = "=".repeat(40);
const lines = [];
lines.push(banner);
lines.push(`Report for ${user.name} <${user.email}>`);
lines.push(banner);
lines.push(`Visits: ${user.visits}`);
lines.push(banner);
return lines.join("\\n");
}
```
**Tests:** Create `test/report.test.js` verifying:
- the result contains `Report for Ada <ada@example.com>` for that user
- the result contains `Visits: 3` when `visits` is `3`
- the result starts and ends with the 40-char banner
**Verification:** `npm test`
## Task 2: Admin Report
**File:** `src/report.js` (add to existing file)
**Requirements:**
- Function named `formatAdminReport`
- Takes one parameter `admin`: an object with `name`, `email`, `lastLogin`
- Same banner layout as the user report; the body line is
`Last login: <lastLogin>` instead of the visits line
- Export the function; keep `formatUserReport` working
**Implementation:**
```javascript
export function formatAdminReport(admin) {
const banner = "=".repeat(40);
const lines = [];
lines.push(banner);
lines.push(`Report for ${admin.name} <${admin.email}>`);
lines.push(banner);
lines.push(`Last login: ${admin.lastLogin}`);
lines.push(banner);
return lines.join("\\n");
}
```
**Tests:** Add to `test/report.test.js`:
- the result contains `Report for Grace <grace@example.com>` for that admin
- the result contains `Last login: 2026-06-01`
- the result starts and ends with the 40-char banner
**Verification:** `npm test`
"""
def scaffold_sdd_quality_defect_plan(workdir: Path) -> None:
workdir = Path(workdir)
workdir.mkdir(parents=True, exist_ok=True)
_git(["git", "init", "-b", "main"], cwd=workdir)
_git(["git", "config", "user.email", "drill@test.local"], cwd=workdir)
_git(["git", "config", "user.name", "Drill Test"], cwd=workdir)
(workdir / "package.json").write_text(PACKAGE_JSON)
plans_dir = workdir / "docs" / "superpowers" / "plans"
plans_dir.mkdir(parents=True, exist_ok=True)
(plans_dir / "report-plan.md").write_text(PLAN_BODY)
_git(["git", "add", "-A"], cwd=workdir)
_git(["git", "commit", "-m", "initial: report formatter plan"], cwd=workdir)
````
(Note the `\\n` in the JS snippets inside PLAN_BODY: the Python source must
produce a literal `\n` in the markdown so the JS reads `lines.join("\n")`.)
- [ ] **Step 2: Register the helper.** In `evals/setup_helpers/__init__.py`:
After the line:
```python
from setup_helpers.sdd_real_projects import scaffold_sdd_go_fractals, scaffold_sdd_svelte_todo
```
add:
```python
from setup_helpers.sdd_quality_defect_plan import scaffold_sdd_quality_defect_plan
```
After the registry entry:
```python
"scaffold_sdd_yagni_plan": scaffold_sdd_yagni_plan,
```
add:
```python
"scaffold_sdd_quality_defect_plan": scaffold_sdd_quality_defect_plan,
```
- [ ] **Step 3: Create `evals/scenarios/sdd-quality-reviewer-catches-planted-defect/story.md`:**
```markdown
---
id: sdd-quality-reviewer-catches-planted-defect
title: SDD's per-task code quality review catches a planted DRY violation
status: ready
tags: subagent-driven-development
quorum_max_time: 90m
---
You have a small plan at docs/superpowers/plans/report-plan.md — two report
formatting functions. The plan's Task 2 implementation snippet duplicates
Task 1's formatting logic verbatim instead of sharing it. The duplication is
spec-compliant (the requirements only describe behavior), so the spec
compliance reviewer should pass it — the per-task code quality reviewer is
the gate under test. You are spec-aware — name the skill.
When the agent is ready for input, tell it to execute the plan with SDD. Use
phrasing like:
"I have a small plan at docs/superpowers/plans/report-plan.md — two report
formatting functions. Use the superpowers:subagent-driven-development skill
to execute it end-to-end — dispatch fresh subagents per task and run the
two-stage review after each."
Let the agent proceed autonomously. If it asks clarifying questions, give
brief answers. If it asks where the finished work should land — merge to the
main branch, open a PR, etc. — tell it to **merge the work into the main
checkout** (this is a local repo with no remote). If a quality reviewer
flags the duplicated formatting logic and an implementer refactors it, let
the review-fix cycle play out — that cycle is exactly the behavior under
test.
The deliverable must end up in the checkout you launched in (the main
working tree). If the agent did its work on a branch or in a worktree, it
is not done until it has merged/finished that work back into the main
checkout. Once the agent reports the plan is complete (both functions
implemented, tests passing) AND the code is present on the main checkout,
you are done.
## Acceptance Criteria
- A `Skill` invocation naming `superpowers:subagent-driven-development`
and at least one `Agent` (subagent dispatch) tool call appear in the
session log.
- The duplicated report-formatting logic did not survive to the end of
the run. Either (a) the implementer never introduced the duplication
(wrote or self-reviewed its way to shared logic), or (b) the per-task
code quality reviewer flagged the duplication as an issue and a
review-fix loop removed it. A fail looks like the duplicated logic
shipping with the per-task quality reviewer approving it, or the
duplication being caught only by the final whole-branch review.
- The per-task quality reviewers stayed task-scoped: no package-wide
test suites, race detector runs, or repeated/high-count test loops
appear in reviewer subagent activity, and reviewers did not re-run
the full test suite merely to confirm the implementer's report.
- `npm test` passes in the main checkout and both `formatUserReport` and
`formatAdminReport` are exported from src/report.js. The deterministic
assertions gate this; the criteria above are about whether the
*per-task quality review* was the mechanism that kept the code clean.
```
- [ ] **Step 4: Create `evals/scenarios/sdd-quality-reviewer-catches-planted-defect/setup.sh`:**
```bash
#!/usr/bin/env bash
set -euo pipefail
uv run setup-helpers run scaffold_sdd_quality_defect_plan
```
Then: `chmod +x evals/scenarios/sdd-quality-reviewer-catches-planted-defect/setup.sh`
- [ ] **Step 5: Create `evals/scenarios/sdd-quality-reviewer-catches-planted-defect/checks.sh`** (no executable bit):
```bash
pre() {
git-repo
git-branch main
requires-tool npm
file-exists 'docs/superpowers/plans/report-plan.md'
file-contains 'docs/superpowers/plans/report-plan.md' 'formatAdminReport'
file-contains 'docs/superpowers/plans/report-plan.md' 'repeat\(40\)'
}
post() {
skill-called superpowers:subagent-driven-development
tool-called Agent
command-succeeds 'npm test'
file-contains 'src/report.js' 'export function formatUserReport'
file-contains 'src/report.js' 'export function formatAdminReport'
command-succeeds 'test "$(grep -c "repeat(40)" src/report.js)" -le 1'
}
```
(The last check is the deterministic DRY gate: the banner construction
`"=".repeat(40)` must appear at most once in the final file — shared, not
duplicated per function.)
- [ ] **Step 6: Validate and test in the evals repo**
```bash
cd evals
uv run quorum check
uv run ruff check
uv run pytest -x -q
```
Expected: all pass; `quorum check` lists the new scenario without errors.
- [ ] **Step 7: Commit (in the submodule)**
```bash
cd evals
git add setup_helpers/sdd_quality_defect_plan.py setup_helpers/__init__.py scenarios/sdd-quality-reviewer-catches-planted-defect/
git commit -m "Add sdd-quality-reviewer-catches-planted-defect scenario"
```
---
### Task 6: Static verification sweep
**Files:** none modified — verification only.
- [ ] **Step 1: No dangling references in the parent repo**
Run: `grep -rn "requesting-code-review" skills/subagent-driven-development/`
Expected: matches only in SKILL.md (final-review flowchart node ×3, Prompt Templates pointer, Integration bullet). None in code-quality-reviewer-prompt.md.
Run: `grep -rn "Ready to merge" skills/subagent-driven-development/ || echo CLEAN`
Expected: `CLEAN`
- [ ] **Step 2: Plugin infrastructure tests**
Run: `bash tests/shell-lint/test-lint-shell.sh`
Expected: all PASS (we added `setup.sh` only inside the evals submodule, which has its own checks).
- [ ] **Step 3: Cross-platform tool tables still coherent**
Run: `grep -n "code-quality-reviewer" skills/using-superpowers/references/antigravity-tools.md skills/using-superpowers/references/gemini-tools.md`
Expected: both tables still list `code-quality-reviewer` as a reviewer template (the new prompt's "If you cannot run commands in this environment, name the test you would run" line keeps the read-only `research` mapping valid — no table edits needed).
---
### Task 7: Live before/after evals (maintainer-gated)
Live quorum runs launch agent CLIs in permissive modes — **trusted-maintainer operation; Jesse launches these**, per `evals/CLAUDE.md`. Requires `ANTHROPIC_API_KEY`.
- [ ] **Step 1: Baseline (skills as released on dev)** — from the main checkout (`/Users/jesse/git/superpowers/superpowers`, on dev), or any checkout without this branch's changes:
```bash
cd evals
export SUPERPOWERS_ROOT=/Users/jesse/git/superpowers/superpowers
uv run quorum run scenarios/sdd-rejects-extra-features --coding-agent claude
uv run quorum run scenarios/sdd-go-fractals --coding-agent claude
uv run quorum run scenarios/sdd-svelte-todo --coding-agent claude
uv run quorum run scenarios/spec-reviewer-catches-planted-flaws --coding-agent claude
```
- [ ] **Step 2: After (this branch's skills)** — point `SUPERPOWERS_ROOT` at this worktree:
```bash
cd evals
export SUPERPOWERS_ROOT=/Users/jesse/git/superpowers/superpowers/.claude/worktrees/sdd-review-dispatch
uv run quorum run scenarios/sdd-rejects-extra-features --coding-agent claude
uv run quorum run scenarios/sdd-go-fractals --coding-agent claude
uv run quorum run scenarios/sdd-svelte-todo --coding-agent claude
uv run quorum run scenarios/spec-reviewer-catches-planted-flaws --coding-agent claude
uv run quorum run scenarios/sdd-quality-reviewer-catches-planted-defect --coding-agent claude
uv run quorum show
```
- [ ] **Step 3: Compare**
Pass bar: all four pre-existing scenarios still pass after the change (no regression in catch rate); the new planted-defect scenario passes. For exploration cost, compare reviewer-subagent tool-call counts between the before/after run transcripts (no automated check exists — the spec calls this out as a known gap).
---
## Finishing
After all tasks pass: the evals submodule commit needs to land in `superpowers-evals` (PR to its `main`), then this branch bumps the `evals` submodule pointer — per `evals/CLAUDE.md`, the parent bump is part of propagation, not optional. Then use superpowers:finishing-a-development-branch. PRs against superpowers target `dev`.

View File

@@ -0,0 +1,124 @@
# SDD Task-Scoped Review Dispatch
Make subagent-driven-development's per-task reviews cheaper and faster without weakening them, by scoping per-task review prompts to the task and stopping redundant work — while final branch review stays broad.
## Problem
Per-task code quality reviewers in SDD routinely do branch-review-scale work on single-task diffs. Evidence from two real local SDD sessions: `a1a6719a-6109-453a-9933-34ae396f5bae` (sen-core-v2) and `0cc1a12d-9984-4c35-8615-9d42dadb2c47` (serf), both under `~/.claude/projects/`:
- In the sen-core-v2 session, 7/8 quality reviewers ran repo-wide greps; the most expensive ran 50+ Bash commands over ~200 seconds. Across both sessions, quality reviewers cost 4-8× what spec reviewers cost on the same tasks.
- Spec reviewers, whose prompt contains "Only read files in this diff. Do not crawl the broader codebase," stayed tight: 6-16 tool calls, 14-65 seconds.
- No reviewer ran heavy tests autonomously. Every package-wide or repeated test run observed was explicitly requested by a controller-written prompt ("check all uses," "run tests if useful, especially race-focused ones," "does anything else read `Meta()`?").
Root causes, in order of impact:
1. **The per-task quality prompt inherits a merge-readiness review.** `code-quality-reviewer-prompt.md` delegates to `requesting-code-review/code-reviewer.md`, which asks about architecture, scalability, security, production readiness, and ends with "Ready to merge?" That frame licenses branch-level breadth on a one-task diff. The spec prompt's diff-scope guard was never carried over.
2. **The controller gets no guidance on writing reviewer prompts**, so it invents open-ended directives ("check all uses") that reviewers interpret literally.
3. **Duplicated work across the pipeline.** The quality template's "Plan alignment" dimension re-checks what the spec reviewer just verified. Reviewers re-run test suites the implementer already ran (and reported, with TDD evidence) on identical code.
4. **Per-task and final review share one template**, so there is no representation of "per-task narrow, final broad" anywhere.
A field report (`~/2026-06-09-code-quality-reviewer-scope-budget-issue.md`) first flagged this. Its cited session and headline numbers could not be verified, but its qualitative diagnosis was confirmed against two real local sessions. One correction to it: cross-cutting audits (lock ordering, changed contracts) are sometimes the *correct* review method — the fix must gate breadth behind a stated concrete risk, not forbid it.
## Goals
- Per-task reviews scoped to the task: diff-first reading, justified broadening, no redundant test runs.
- Final whole-branch review keeps its current breadth.
- No reduction in what reviews catch.
## Non-goals / explicitly preserved
- **Full re-reviews stay.** When a reviewer re-reviews after a fix, it still reviews the whole task at full reading breadth. (It does not re-run tests the implementer just ran on the amended code.) This deliberately rejects the field report's "re-review budget" remedy: the cost of its worst cited example (a re-review running `-race` and `-count=100` loops) is curbed by the test budget below, not by narrowing what re-reviewers read.
- ~~**The two review stages stay separate.** Spec compliance and code quality remain independent subagents, serially gated. No merging.~~ **Superseded by the cost iterations below**: live eval economics showed per-dispatch overhead dominating cost, and the maintainer put everything on the table. The per-task stages are now one task reviewer with two verdicts; the independent broad final review remains.
- **The coordinator keeps model judgment.** No forced model tier for reviews, in either direction.
- **`requesting-code-review/` is untouched.** It remains the broad template for final branch review and ad-hoc review.
- Verdict ordering (spec compliance reported before quality), the fix-and-re-review loops, and the requirement to fix Critical/Important findings are unchanged.
## Cost iterations (post-launch eval economics)
Live before/after runs surfaced a cost regression once the quality-hardening
prose (evidence rule, constraint carrying, pristine output) landed: go-fractals
went from 42.8 min / 14.5M tokens (first task-scoped version) to 69.9 min /
32.2M (hardened version) while reaching baseline-parity quality (blind-judged
8.5 vs 8.5). Per-subagent turn profiling attributed cost to, in order: cheap
models taking 2-3× the turns on multi-step work (678 of 1197 subagent turns
were haiku), per-dispatch overhead (3 subagent spin-ups per task, each
re-deriving the diff; controller coordination was half the dollars), and
evidence-rule narration.
- **Iteration 1:** turn-count-beats-token-price model guidance (mid-tier floor
for multi-step work), optional inline diffs, cite-don't-narrate evidence,
Important = cannot-trust-until-fixed, fixes dispatched only for
Critical/Important. Result: 68.2 min / 22.9M — tokens down 29%, wall-clock
flat; controllers pasted the diff in only 2 of 22 review dispatches when
phrasing was optional.
- **Iteration 2:** per-task spec and quality reviews merged into one
`task-reviewer-prompt.md` (one reviewer, one reading of the diff, two
verdicts; one fix dispatch addresses both kinds of findings); implementers
run the focused test while iterating, full suite once before commit.
Result (go-fractals): 47.5 min / 15.7M / $13.55 — beat baseline on every
axis, blind-judged 9/10 vs baseline 7/10.
- **Iteration 3:** Calibration names merge-blocking maintainability damage
(verbatim duplication, swallowed errors, assertion-free tests) as
Important and Minor findings must be pasted into the final review for
triage; reviewer skepticism extended to the implementer's design
rationales ("left it per YAGNI" is a claim, not a verdict); diff handed
to reviewers as a file (`git diff > /tmp/sdd-task-N.diff`, redirected so
it never enters the controller's context; one Read call for the
reviewer) after paste-into-prompt guidance went unadopted (0-6 of 11-17
dispatches) for locally-rational context-economics reasons.
- **Final frozen config (e355795), all five scenarios pass:** go-fractals
44.4 min / 13.4M / $11.67 (-32% time, -37% tokens, -27% dollars vs
baseline); svelte-todo 62.8 / 19.7M / $15.76 (-21% / -28% / -25%);
rejects-extra-features $1.31 (vs $1.88); spec-reviewer-flaws flat; the
planted-defect scenario (v3: open-flag transparency bar for judgment
calls, must-fix bar for a test whose name promises verification it
never performs) passes with the defect caught and fixed.
## Design
### Shared principle: don't re-run tests on code that hasn't changed
The implementer's report includes test results and TDD RED/GREEN evidence for exactly the code under review. Reviewers verify by reading. A reviewer runs a test only when reading raises a specific doubt that no existing run answers — and then a focused test, not a suite. On harnesses where reviewer subagents are read-only (e.g., Antigravity maps reviewer templates to the `research` type, which has no command access), the reviewer instead names the test it would run in its report.
After a fix, the implementer re-runs the tests covering the amended code; the re-reviewer does not repeat that run. Today nothing enforces that premise: `implementer-prompt.md` describes the initial implement-test-commit flow only, with no fix-iteration instruction. This spec therefore also adds to `implementer-prompt.md`: after fixing a review finding, re-run the tests that cover the amended code and include the results in the fix report.
This principle appears in both reviewer prompts, the implementer prompt, and the controller guidance.
### 1. New file: `skills/subagent-driven-development/code-quality-reviewer-prompt.md` becomes self-contained
Stop delegating to `requesting-code-review/code-reviewer.md`. The per-task quality reviewer gets its own scoped prompt template:
- **Framing:** "You are reviewing one task's implementation for code quality." A task-scoped gate, not a merge review.
- **Spec compliance is settled:** spec review already passed; do not re-litigate requirements or plan alignment.
- **Review dimensions kept:** code quality (clarity, duplication, error handling), test quality (real behavior, not mocks), maintainability, and the existing SDD-specific checks (single responsibility, independent testability, file structure from plan, file growth contributed by this change). Dropped: plan alignment, security/scalability/production-readiness dimensions, merge verdict.
- **Scope budget:** start from `git diff BASE..HEAD`; read changed files first; inspect adjacent code only to evaluate a concrete risk you can name. Cross-cutting changes — lock ordering, changed function/API contracts, shared mutable state — are legitimate named risks that justify checking call sites. Do not crawl the codebase by default.
- **Test budget:** the shared principle above, plus: no package-wide suites, race detectors, or repeated/high-count runs unless you have first named a specific suspected flake or race. Otherwise, recommend heavy validation in the report instead of running it. Warnings or noise in the implementer's reported test output are findings — output should be pristine (the implementer's self-review checks this too).
- **Evidence rule:** reviewers answer each What-to-Check item with file:line evidence, not bare yes/no. (Added after live eval runs showed reviewers passing defects the prompt had pointed them at — an accessible-name check and a temp-dir-cleanup check both got unsupported "yes" answers while the defect sat in the reviewed diff.)
- **Read-only rule** kept in trimmed form: no mutating the working tree, index, HEAD, or branch state. The `git worktree add` how-to sentence from the current templates is NOT carried into this file — a diff-scoped review never needs a checkout of another revision (same rationale as the spec-prompt cleanup below).
- **Verdict:** Strengths / Issues (Critical/Important/Minor) / "Task quality: Approved | Needs fixes."
### 2. `skills/subagent-driven-development/spec-reviewer-prompt.md` cleanups
- Remove the `git worktree add` how-to sentence. The read-only rule stays; a diff-scoped spec review never needs a checkout of another revision.
- Resolve the tension between the diff-only guard and "verify everything independently": spec compliance is judged by reading the diff against the requirements. The implementer's TDD evidence covers "it runs" — apply the shared test principle.
- New third verdict channel: requirements that cannot be verified from the diff (live in unchanged code, span tasks) are reported as explicit "⚠️ Cannot verify from diff — controller should check X" items, instead of either crawling or silently passing. The flowchart's binary pass/fail diamond cannot route this, so the controller guidance (§3) defines the handling: ⚠️ items do not block dispatching the quality reviewer, but the controller must resolve each one itself (it holds the plan and cross-task context) before marking the task complete; an item the controller confirms is a real gap is treated as a failed spec review and goes back to the implementer.
- Replace the fabricated premise "The implementer finished suspiciously quickly" with grounded skepticism: treat the implementer's report as unverified claims about the code. Same distrust, no invented fact.
### 3. `skills/subagent-driven-development/SKILL.md` controller changes
- **Model Selection:** replace "Architecture, design, and review tasks: use the most capable available model" with judgment guidance — pick reviewer models the way implementer models are picked, scaled to the diff's size, complexity, and risk. The "Task complexity signals" list is rescoped to make clear its bullets describe implementation tasks; reviewer model choice follows the same judgment, so a narrow diff review does not automatically map to "broad codebase understanding → most capable model."
- **Reviewer prompt construction** (new guidance near Red Flags): when dispatching reviewers, do not write open-ended directives ("check all uses," "run race tests if useful") without a concrete task-specific reason; do not ask reviewers to re-run tests the implementer already ran on the same code; do not pre-judge findings for the reviewer (never instruct a reviewer to ignore or not flag a specific issue — adjudicate suspected false positives in the review loop instead); per-task reviews are task-scoped gates — the broad review happens once, at the final whole-branch review. (The pre-judging rule was added after a live eval run caught the controller fabricating a "the plan forbids a shared helper" claim and instructing the quality reviewer not to flag a planted DRY violation.) Controllers must also include the spec/design's global constraints that bind the task — version floors, naming and copy rules, platform requirements — in the requirements they paste: a live run shipped a `go 1.26.1` module floor against a "Go 1.21+" design because no reviewer ever saw the constraint. And controllers must specify a model explicitly on every dispatch — an omitted model inherits the session's (usually most expensive) model, which silently defeats model selection.
- **Handling spec-reviewer ⚠️ items** (new guidance, alongside Handling Implementer Status): the controller resolves each "cannot verify from diff" item itself before marking the task complete; confirmed gaps go back to the implementer as failed spec review.
- **Final review stays broad, explicitly:** the final whole-branch reviewer dispatch node gains an explicit pointer to `../requesting-code-review/code-reviewer.md`. (Today that template is reachable only through the per-task quality prompt's delegation; once that delegation is removed, an unreferenced final-review template would be orphaned.) The Integration section's note that `superpowers:requesting-code-review` provides "the code review template for reviewer subagents" is corrected to apply to the final review only.
- **Example workflow:** the quality-reviewer lines in the example are updated to the new verdict vocabulary ("Task quality: Approved"); the final reviewer's "ready to merge" line stays.
- Flowchart topology is unchanged; the ⚠️ channel is handled by controller guidance, not a new graph branch.
## What this does not fix (known, deferred)
The spec reviewer judges against task text the controller pasted; it cannot catch requirements dropped during the controller's extraction from the plan. That is an architectural property of "controller provides full text," not a prompt problem, and is out of scope here.
## Verification
- Plugin infrastructure tests (`tests/`) still pass.
- Run the SDD skill-behavior evals (`git submodule update --init evals`, then per `evals/README.md`) before and after the change. Specifically: `sdd-go-fractals`, `sdd-svelte-todo`, `sdd-rejects-extra-features` (end-to-end SDD including the spec reviewer's YAGNI gate), and `spec-reviewer-catches-planted-flaws`.
- Known eval gaps this change exposes: no existing scenario plants a code-quality defect inside a single SDD task and asserts the per-task quality reviewer catches it, and no scenario measures per-reviewer exploration cost (tool-call/grep counts). Add one scenario covering the first gap (planted single-task quality defect → per-task reviewer must flag it before final review). For exploration cost, compare reviewer subagent tool-call counts manually across the before/after eval transcripts.

View File

@@ -5,11 +5,11 @@ description: Use when executing implementation plans with independent tasks in t
# Subagent-Driven Development
Execute plan by dispatching fresh subagent per task, with two-stage review after each: spec compliance review first, then code quality review.
Execute plan by dispatching a fresh implementer subagent per task, a combined task review (spec compliance + code quality, one reviewer, one reading of the diff) after each, and a broad whole-branch review at the end.
**Why subagents:** You delegate tasks to specialized agents with isolated context. By precisely crafting their instructions and context, you ensure they stay focused and succeed at their task. They should never inherit your session's context or history — you construct exactly what they need. This also preserves your own context for coordination work.
**Core principle:** Fresh subagent per task + two-stage review (spec then quality) = high quality, fast iteration
**Core principle:** Fresh subagent per task + one task review (spec + quality verdicts) + broad final review = high quality, fast iteration
**Continuous execution:** Do not pause to check in with your human partner between tasks. Execute all tasks from the plan without stopping. The only reasons to stop are: BLOCKED status you cannot resolve, ambiguity that genuinely prevents progress, or all tasks complete. "Should I continue?" prompts and progress summaries waste their time — they asked you to execute the plan, so execute it.
@@ -36,7 +36,7 @@ digraph when_to_use {
**vs. Executing Plans (parallel session):**
- Same session (no context switch)
- Fresh subagent per task (no context pollution)
- Two-stage review after each task: spec compliance first, then code quality
- Combined review after each task (spec compliance + code quality verdicts), broad review at the end
- Faster iteration (no human-in-loop between tasks)
## The Process
@@ -51,18 +51,15 @@ digraph process {
"Implementer subagent asks questions?" [shape=diamond];
"Answer questions, provide context" [shape=box];
"Implementer subagent implements, tests, commits, self-reviews" [shape=box];
"Dispatch spec reviewer subagent (./spec-reviewer-prompt.md)" [shape=box];
"Spec reviewer subagent confirms code matches spec?" [shape=diamond];
"Implementer subagent fixes spec gaps" [shape=box];
"Dispatch code quality reviewer subagent (./code-quality-reviewer-prompt.md)" [shape=box];
"Code quality reviewer subagent approves?" [shape=diamond];
"Implementer subagent fixes quality issues" [shape=box];
"Run git diff, dispatch task reviewer subagent (./task-reviewer-prompt.md)" [shape=box];
"Task reviewer reports spec ✅ and quality approved?" [shape=diamond];
"Dispatch fix subagent for Critical/Important findings" [shape=box];
"Mark task complete in todo list" [shape=box];
}
"Read plan, extract all tasks with full text, note context, create todos" [shape=box];
"More tasks remain?" [shape=diamond];
"Dispatch final code reviewer subagent for entire implementation" [shape=box];
"Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)" [shape=box];
"Use superpowers:finishing-a-development-branch" [shape=box style=filled fillcolor=lightgreen];
"Read plan, extract all tasks with full text, note context, create todos" -> "Dispatch implementer subagent (./implementer-prompt.md)";
@@ -70,19 +67,15 @@ digraph process {
"Implementer subagent asks questions?" -> "Answer questions, provide context" [label="yes"];
"Answer questions, provide context" -> "Dispatch implementer subagent (./implementer-prompt.md)";
"Implementer subagent asks questions?" -> "Implementer subagent implements, tests, commits, self-reviews" [label="no"];
"Implementer subagent implements, tests, commits, self-reviews" -> "Dispatch spec reviewer subagent (./spec-reviewer-prompt.md)";
"Dispatch spec reviewer subagent (./spec-reviewer-prompt.md)" -> "Spec reviewer subagent confirms code matches spec?";
"Spec reviewer subagent confirms code matches spec?" -> "Implementer subagent fixes spec gaps" [label="no"];
"Implementer subagent fixes spec gaps" -> "Dispatch spec reviewer subagent (./spec-reviewer-prompt.md)" [label="re-review"];
"Spec reviewer subagent confirms code matches spec?" -> "Dispatch code quality reviewer subagent (./code-quality-reviewer-prompt.md)" [label="yes"];
"Dispatch code quality reviewer subagent (./code-quality-reviewer-prompt.md)" -> "Code quality reviewer subagent approves?";
"Code quality reviewer subagent approves?" -> "Implementer subagent fixes quality issues" [label="no"];
"Implementer subagent fixes quality issues" -> "Dispatch code quality reviewer subagent (./code-quality-reviewer-prompt.md)" [label="re-review"];
"Code quality reviewer subagent approves?" -> "Mark task complete in todo list" [label="yes"];
"Implementer subagent implements, tests, commits, self-reviews" -> "Run git diff, dispatch task reviewer subagent (./task-reviewer-prompt.md)";
"Run git diff, dispatch task reviewer subagent (./task-reviewer-prompt.md)" -> "Task reviewer reports spec ✅ and quality approved?";
"Task reviewer reports spec ✅ and quality approved?" -> "Dispatch fix subagent for Critical/Important findings" [label="no"];
"Dispatch fix subagent for Critical/Important findings" -> "Run git diff, dispatch task reviewer subagent (./task-reviewer-prompt.md)" [label="re-review"];
"Task reviewer reports spec ✅ and quality approved?" -> "Mark task complete in todo list" [label="yes"];
"Mark task complete in todo list" -> "More tasks remain?";
"More tasks remain?" -> "Dispatch implementer subagent (./implementer-prompt.md)" [label="yes"];
"More tasks remain?" -> "Dispatch final code reviewer subagent for entire implementation" [label="no"];
"Dispatch final code reviewer subagent for entire implementation" -> "Use superpowers:finishing-a-development-branch";
"More tasks remain?" -> "Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)" [label="no"];
"Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)" -> "Use superpowers:finishing-a-development-branch";
}
```
@@ -94,9 +87,23 @@ Use the least powerful model that can handle each role to conserve cost and incr
**Integration and judgment tasks** (multi-file coordination, pattern matching, debugging): use a standard model.
**Architecture, design, and review tasks**: use the most capable available model.
**Architecture and design tasks**: use the most capable available model.
**Task complexity signals:**
**Review tasks**: choose the model with the same judgment, scaled to the
diff's size, complexity, and risk. A small mechanical diff does not need the
most capable model; a subtle concurrency change does.
**Always specify the model explicitly when dispatching a subagent.** An
omitted model inherits your session's model — often the most capable and
most expensive — which silently defeats this section.
**Turn count beats token price.** Wall-clock and context cost scale with how
many turns a subagent takes, and the cheapest models routinely take 2-3× the
turns on multi-step work — costing more overall. Use a mid-tier model as the
floor for implementers and reviewers; reserve the cheapest tier for
single-file mechanical fixes.
**Task complexity signals (implementation tasks):**
- Touches 1-2 files with a complete spec → cheap model
- Touches multiple files with integration concerns → standard model
- Requires design judgment or broad codebase understanding → most capable model
@@ -105,7 +112,7 @@ Use the least powerful model that can handle each role to conserve cost and incr
Implementer subagents report one of four statuses. Handle each appropriately:
**DONE:** Proceed to spec compliance review.
**DONE:** Run `git diff BASE..HEAD`, then dispatch the task reviewer.
**DONE_WITH_CONCERNS:** The implementer completed the work but flagged doubts. Read the concerns before proceeding. If the concerns are about correctness or scope, address them before review. If they're observations (e.g., "this file is getting large"), note them and proceed to review.
@@ -119,11 +126,48 @@ Implementer subagents report one of four statuses. Handle each appropriately:
**Never** ignore an escalation or force the same model to retry without changes. If the implementer said it's stuck, something needs to change.
## Handling Reviewer ⚠️ Items
The task reviewer may report "⚠️ Cannot verify from diff" items — requirements
that live in unchanged code or span tasks. These do not block the rest of the
review, but you must resolve each one yourself before marking the task
complete: you hold the plan and cross-task context the reviewer
lacks. If you confirm an item is a real gap, treat it as a failed spec
review — send it back to the implementer and re-review.
## Constructing Reviewer Prompts
Per-task reviews are task-scoped gates. The broad review happens once, at the
final whole-branch review. When you fill a reviewer template:
- Do not add open-ended directives like "check all uses" or "run race tests
if useful" without a concrete, task-specific reason
- Do not ask a reviewer to re-run tests the implementer already ran on the
same code — the implementer's report carries the test evidence
- Do not pre-judge findings for the reviewer — never instruct a reviewer to
ignore or not flag a specific issue. If you believe a finding would be a
false positive, let the reviewer raise it and adjudicate it in the review
loop. If the prompt you are writing contains "do not flag," "don't treat X
as a defect," "at most Minor," or "the plan chose" — stop: you are
pre-judging, usually to spare yourself a review loop.
- Include the spec/design's global constraints that bind the task (version
floors, naming and copy rules, platform requirements) in the requirements
you paste — a reviewer can only enforce what you hand them.
- Hand the reviewer its diff as a file: run
`git diff BASE..HEAD > /tmp/sdd-task-N.diff` (redirected, so the diff
never enters your own context) and put that path in the prompt. The
reviewer then sees the whole change in one Read call instead of
re-deriving it with git commands.
- Dispatch fix subagents for Critical and Important findings. Record Minor
findings and move on — then paste the accumulated Minor findings into the
final whole-branch review dispatch so it can triage which must be fixed
before merge. A roll-up nobody reads is a silent discard.
## Prompt Templates
- [implementer-prompt.md](implementer-prompt.md) - Dispatch implementer subagent
- [spec-reviewer-prompt.md](spec-reviewer-prompt.md) - Dispatch spec compliance reviewer subagent
- [code-quality-reviewer-prompt.md](code-quality-reviewer-prompt.md) - Dispatch code quality reviewer subagent
- [task-reviewer-prompt.md](task-reviewer-prompt.md) - Dispatch task reviewer subagent (spec compliance + code quality, one dispatch)
- Final whole-branch review: use superpowers:requesting-code-review's [code-reviewer.md](../requesting-code-review/code-reviewer.md)
## Example Workflow
@@ -150,11 +194,9 @@ Implementer: "Got it. Implementing now..."
- Self-review: Found I missed --force flag, added it
- Committed
[Dispatch spec compliance reviewer]
Spec reviewer: Spec compliant - all requirements met, nothing extra
[Get git SHAs, dispatch code quality reviewer]
Code reviewer: Strengths: Good test coverage, clean. Issues: None. Approved.
[Run git diff, dispatch task reviewer with the diff pasted in]
Task reviewer: Spec - all requirements met, nothing extra.
Strengths: Good test coverage, clean. Issues: None. Task quality: Approved.
[Mark Task 1 complete]
@@ -170,25 +212,17 @@ Implementer:
- Self-review: All good
- Committed
[Dispatch spec compliance reviewer]
Spec reviewer: ❌ Issues:
[Run git diff, dispatch task reviewer with the diff pasted in]
Task reviewer: Spec ❌:
- Missing: Progress reporting (spec says "report every 100 items")
- Extra: Added --json flag (not requested)
Issues (Important): Magic number (100)
[Implementer fixes issues]
Implementer: Removed --json flag, added progress reporting
[Dispatch fix subagent with all findings]
Fixer: Removed --json flag, added progress reporting, extracted PROGRESS_INTERVAL constant
[Spec reviewer reviews again]
Spec reviewer: Spec compliant now
[Dispatch code quality reviewer]
Code reviewer: Strengths: Solid. Issues (Important): Magic number (100)
[Implementer fixes]
Implementer: Extracted PROGRESS_INTERVAL constant
[Code reviewer reviews again]
Code reviewer: ✅ Approved
[Task reviewer reviews again]
Task reviewer: Spec ✅. Task quality: Approved.
[Mark Task 2 complete]
@@ -222,13 +256,13 @@ Done!
**Quality gates:**
- Self-review catches issues before handoff
- Two-stage review: spec compliance, then code quality
- Task review carries two verdicts: spec compliance and code quality
- Review loops ensure fixes actually work
- Spec compliance prevents over/under-building
- Code quality ensures implementation is well-built
**Cost:**
- More subagent invocations (implementer + 2 reviewers per task)
- More subagent invocations (implementer + reviewer per task)
- Controller does more prep work (extracting all tasks upfront)
- Review loops add iterations
- But catches issues early (cheaper than debugging later)
@@ -237,17 +271,22 @@ Done!
**Never:**
- Start implementation on main/master branch without explicit user consent
- Skip reviews (spec compliance OR code quality)
- Skip task review, or accept a report missing either verdict (spec compliance AND task quality are both required)
- Proceed with unfixed issues
- Dispatch multiple implementation subagents in parallel (conflicts)
- Make subagent read plan file (provide full text instead)
- Skip scene-setting context (subagent needs to understand where task fits)
- Ignore subagent questions (answer before letting them proceed)
- Accept "close enough" on spec compliance (spec reviewer found issues = not done)
- Accept "close enough" on spec compliance (reviewer found spec issues = not done)
- Skip review loops (reviewer found issues = implementer fixes = review again)
- Let implementer self-review replace actual review (both are needed)
- **Start code quality review before spec compliance is ✅** (wrong order)
- Move to next task while either review has open issues
- Tell a reviewer what not to flag, or pre-rate a finding's severity in the
dispatch prompt ("treat it as Minor at most") — the plan's example code is
a starting point, not evidence that its weaknesses were chosen
- Dispatch a task reviewer without a diff file — run
`git diff BASE..HEAD > /tmp/sdd-task-N.diff` first and name that path in
the prompt
- Move to next task while the review has open Critical/Important issues
**If subagent asks questions:**
- Answer clearly and completely
@@ -269,7 +308,7 @@ Done!
**Required workflow skills:**
- **superpowers:using-git-worktrees** - Ensures isolated workspace (creates one or verifies existing)
- **superpowers:writing-plans** - Creates the plan this skill executes
- **superpowers:requesting-code-review** - Code review template for reviewer subagents
- **superpowers:requesting-code-review** - Code review template for the final whole-branch review
- **superpowers:finishing-a-development-branch** - Complete development after all tasks
**Subagents should use:**

View File

@@ -1,25 +0,0 @@
# Code Quality Reviewer Prompt Template
Use this template when dispatching a code quality reviewer subagent.
**Purpose:** Verify implementation is well-built (clean, tested, maintainable)
**Only dispatch after spec compliance review passes.**
```
Subagent (general-purpose):
Use template at ../requesting-code-review/code-reviewer.md
DESCRIPTION: [task summary, from implementer's report]
PLAN_OR_REQUIREMENTS: Task N from [plan-file]
BASE_SHA: [commit before task]
HEAD_SHA: [current commit]
```
**In addition to standard code quality concerns, the reviewer should check:**
- Does each file have one clear responsibility with a well-defined interface?
- Are units decomposed so they can be understood and tested independently?
- Is the implementation following the file structure from the plan?
- Did this implementation create new files that are already large, or significantly grow existing files? (Don't flag pre-existing file sizes — focus on what this change contributed.)
**Code reviewer returns:** Strengths, Issues (Critical/Important/Minor), Assessment

View File

@@ -41,6 +41,9 @@ Subagent (general-purpose):
**While you work:** If you encounter something unexpected or unclear, **ask questions**.
It's always OK to pause and clarify. Don't guess or make assumptions.
While iterating, run the focused test for what you're changing; run the
full suite once before committing, not after every edit.
## Code Organization
You reason best about code you can hold in context at once, and your edits are more
@@ -94,9 +97,16 @@ Subagent (general-purpose):
- Do tests actually verify behavior (not just mock behavior)?
- Did I follow TDD if required?
- Are tests comprehensive?
- Is the test output pristine (no stray warnings or noise)?
If you find issues during self-review, fix them now before reporting.
## After Review Findings
If a reviewer finds issues and you fix them, re-run the tests that cover
the amended code and include the results in your fix report. Reviewers
will not re-run tests for you — your report is the test evidence.
## Report Format
When done, report:

View File

@@ -1,77 +0,0 @@
# Spec Compliance Reviewer Prompt Template
Use this template when dispatching a spec compliance reviewer subagent.
**Purpose:** Verify implementer built what was requested (nothing more, nothing less)
```
Subagent (general-purpose):
description: "Review spec compliance for Task N"
prompt: |
You are reviewing whether an implementation matches its specification.
## What Was Requested
[FULL TEXT of task requirements]
## What Implementer Claims They Built
[From implementer's report]
## Git Range to Review
**Base:** [BASE_SHA — commit before this task]
**Head:** [HEAD_SHA — current commit]
```bash
git diff --stat [BASE_SHA]..[HEAD_SHA]
git diff [BASE_SHA]..[HEAD_SHA]
```
Only read files in this diff. Do not crawl the broader codebase.
## Read-Only Review
Your review is read-only on this checkout. Do not mutate the working tree, the index, HEAD, or branch state in any way. Use tools like `git show`, `git diff`, and `git log` to inspect history. If you need a working copy of a different revision, check it out into a separate temporary directory (e.g. `git worktree add /tmp/review-[SHA] [SHA]`) — never move HEAD on this checkout.
## CRITICAL: Do Not Trust the Report
The implementer finished suspiciously quickly. Their report may be incomplete,
inaccurate, or optimistic. You MUST verify everything independently.
**DO NOT:**
- Take their word for what they implemented
- Trust their claims about completeness
- Accept their interpretation of requirements
**DO:**
- Read the actual code they wrote
- Compare actual implementation to requirements line by line
- Check for missing pieces they claimed to implement
- Look for extra features they didn't mention
## Your Job
Read the implementation code and verify:
**Missing requirements:**
- Did they implement everything that was requested?
- Are there requirements they skipped or missed?
- Did they claim something works but didn't actually implement it?
**Extra/unneeded work:**
- Did they build things that weren't requested?
- Did they over-engineer or add unnecessary features?
- Did they add "nice to haves" that weren't in spec?
**Misunderstandings:**
- Did they interpret requirements differently than intended?
- Did they solve the wrong problem?
- Did they implement the right feature but wrong way?
**Verify by reading code, not by trusting report.**
Report:
- ✅ Spec compliant (if everything matches after code inspection)
- ❌ Issues found: [list specifically what's missing or extra, with file:line references]
```

View File

@@ -0,0 +1,162 @@
# Task Reviewer Prompt Template
Use this template when dispatching a task reviewer subagent. One reviewer, one
reading of the diff, two verdicts: spec compliance and code quality.
**Purpose:** Verify one task's implementation matches its requirements (nothing
more, nothing less) and is well-built (clean, tested, maintainable)
```
Subagent (general-purpose):
description: "Review Task N (spec + quality)"
prompt: |
You are reviewing one task's implementation: first whether it matches its
requirements, then whether it is well-built. This is a task-scoped gate,
not a merge review — a broad whole-branch review happens separately after
all tasks are complete.
## What Was Requested
[TASK_REQUIREMENTS]
## What the Implementer Claims They Built
[DESCRIPTION]
## Diff Under Review
**Base:** [BASE_SHA]
**Head:** [HEAD_SHA]
**Diff file:** [DIFF_FILE]
Read the diff file once — that single Read is your view of the change.
Do not re-run git commands or re-read the files it already shows. If
the diff file is missing, fetch the diff yourself:
`git diff --stat [BASE_SHA]..[HEAD_SHA]` and `git diff [BASE_SHA]..[HEAD_SHA]`.
Only read files in this diff. Do not crawl the broader codebase. Inspect
code outside the diff only to evaluate a concrete risk you can name — and
name it in your report. Cross-cutting changes are legitimate named risks:
if the diff changes lock ordering, a function or API contract, or shared
mutable state, checking the call sites is the right method.
Your review is read-only on this checkout. Do not mutate the working
tree, the index, HEAD, or branch state in any way.
## Do Not Trust the Report
Treat the implementer's report as unverified claims about the code. It
may be incomplete, inaccurate, or optimistic. Verify the claims against
the diff. Design rationales in the report are claims too: "left it per
YAGNI," "kept it simple deliberately," or any other justification is the
implementer grading their own work. Judge the code on its merits — a
stated rationale never downgrades a finding's severity.
## Tests
The implementer already ran the tests and reported results with TDD
evidence for exactly this code. Do not re-run the suite to confirm their
report. Run a test only when reading the code raises a specific doubt
that no existing run answers — and then a focused test, never a
package-wide suite, race detector run, or repeated/high-count loop. If
heavy validation seems warranted, recommend it in your report instead of
running it. If you cannot run commands in this environment, name the
test you would run.
Warnings or other noise in the implementer's reported test output are
findings — test output should be pristine.
## Part 1: Spec Compliance
Compare the diff against What Was Requested:
- **Missing:** requirements they skipped, missed, or claimed without
implementing
- **Extra:** features that weren't requested, over-engineering, unneeded
"nice to haves"
- **Misunderstood:** right feature built the wrong way, wrong problem
solved
If a requirement cannot be verified from this diff alone (it lives in
unchanged code or spans tasks), report it as a ⚠️ item instead of
broadening your search.
## Part 2: Code Quality
**Code quality:**
- Clean separation of concerns?
- Proper error handling?
- DRY without premature abstraction?
- Edge cases handled?
**Tests:**
- Do the new and changed tests verify real behavior, not mocks?
- Are the task's edge cases covered?
**Structure:**
- Does each file have one clear responsibility with a well-defined interface?
- Are units decomposed so they can be understood and tested independently?
- Is the implementation following the file structure from the plan?
- Did this change create new files that are already large, or
significantly grow existing files? (Don't flag pre-existing file
sizes — focus on what this change contributed.)
Cite file:line evidence for every finding and for any check you would
otherwise answer with a bare "yes." Cite, don't narrate — a tight report
that points at lines beats a long one that retells the diff.
## Calibration
Categorize issues by actual severity. Not everything is Critical.
Important means this task cannot be trusted until it is fixed: incorrect
or fragile behavior, a missed requirement, or maintainability damage you
would block a merge over — verbatim duplication of a logic block,
swallowed errors, tests that assert nothing. "Coverage could be broader"
and polish suggestions are Minor.
Acknowledge what was done well before listing issues — accurate praise
helps the implementer trust the rest of the feedback.
## Output Format
### Spec Compliance
- ✅ Spec compliant | ❌ Issues found: [what's missing/extra/misunderstood,
with file:line references]
- ⚠️ Cannot verify from diff: [requirements you could not verify from the
diff alone, and what the controller should check — report alongside the
✅/❌ verdict for everything you could verify]
### Strengths
[What's well done? Be specific.]
### Issues
#### Critical (Must Fix)
#### Important (Should Fix)
#### Minor (Nice to Have)
For each issue: file:line, what's wrong, why it matters, how to fix
(if not obvious).
### Assessment
**Task quality:** [Approved | Needs fixes]
**Reasoning:** [1-2 sentence technical assessment]
```
**Placeholders:**
- `[TASK_REQUIREMENTS]` — full task text plus the spec/design's global
constraints that bind it (version floors, naming and copy rules, platform
requirements)
- `[DESCRIPTION]` — what the implementer reports they built
- `[BASE_SHA]` — commit before this task
- `[HEAD_SHA]` — current commit
- `[DIFF_FILE]` — REQUIRED: the path the controller wrote the diff to
(`git diff BASE..HEAD > /tmp/sdd-task-N.diff`, redirected so it never
enters the controller's context)
**Reviewer returns:** Spec Compliance verdict (✅/❌/⚠️), Strengths, Issues
(Critical/Important/Minor), Task quality verdict
A single fix dispatch can then address spec gaps and quality findings
together; re-review after fixes covers both verdicts again.

View File

@@ -64,7 +64,7 @@ prompt-template file (e.g. `superpowers:subagent-driven-development`'s
| Skill dispatch form | Antigravity equivalent |
|---------------------|----------------------|
| An implementer-style `*-prompt.md` template (writes code, runs tests) | Fill the template, then `invoke_subagent` with `TypeName: "self"` and the filled prompt |
| A read-only reviewer template (`spec-reviewer`, `code-quality-reviewer`, `code-reviewer`, `requesting-code-review`'s `./code-reviewer.md`) | `invoke_subagent` with `TypeName: "research"` and the filled review template |
| A read-only reviewer template (`task-reviewer`, `code-reviewer`, `requesting-code-review`'s `./code-reviewer.md`) | `invoke_subagent` with `TypeName: "research"` and the filled review template |
| Inline prompt (no template referenced) | `invoke_subagent` with `TypeName: "self"` (or `"research"` if the task only reads) and your inline prompt |
### Prompt filling

View File

@@ -35,7 +35,7 @@ Skills dispatch with `Subagent (general-purpose):` and either reference a prompt
| Skill dispatch form | Gemini CLI equivalent |
|---------------------|----------------------|
| References a `*-prompt.md` template (implementer, spec-reviewer, code-quality-reviewer, code-reviewer, etc.) | Fill the template, then `invoke_agent` with `agent_name: "generalist"` and the filled prompt |
| References a `*-prompt.md` template (implementer, task-reviewer, code-reviewer, etc.) | Fill the template, then `invoke_agent` with `agent_name: "generalist"` and the filled prompt |
| References `superpowers:requesting-code-review`'s `./code-reviewer.md` | `invoke_agent` with `agent_name: "generalist"` and the filled review template |
| Inline prompt (no template referenced) | `invoke_agent` with `agent_name: "generalist"` and your inline prompt |