From 5da15d7eba60e69efb6171306841bffb3fb68653 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 16:26:00 -0700 Subject: [PATCH 01/43] Add design spec: task-scoped review dispatch for SDD --- ...-sdd-task-scoped-review-dispatch-design.md | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md diff --git a/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md b/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md new file mode 100644 index 0000000000..ab64961fb8 --- /dev/null +++ b/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md @@ -0,0 +1,75 @@ +# 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 real sessions on this machine: + +- In one SDD session, 7/8 quality reviewers ran repo-wide greps; the most expensive ran 50+ Bash commands over ~200 seconds. 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.) +- **The two review stages stay separate.** Spec compliance and code quality remain independent subagents, serially gated. No merging. +- **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. +- Review ordering (spec before quality), the fix-and-re-review loops, and the requirement to fix Critical/Important findings are unchanged. + +## 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. After a fix, the implementer re-runs tests on the amended code; the re-reviewer does not repeat that run. This principle appears in both reviewer prompts and in 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. +- **Read-only rule** kept (no mutating the working tree, index, HEAD, or branch state). +- **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. +- 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. +- **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; per-task reviews are task-scoped gates — the broad review happens once, at the final whole-branch review. +- **Final review stays broad, explicitly:** the final whole-branch reviewer dispatch uses `requesting-code-review/code-reviewer.md`. 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. +- Flowchart topology is unchanged. + +## 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 in the `evals/` submodule before and after the change. Reviews must still catch seeded issues; per-task reviewers should show less repo-wide exploration and fewer redundant test runs. From 4192572d19db0226f7a9da702c4f5f550a7323c1 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 16:33:44 -0700 Subject: [PATCH 02/43] Harden review-dispatch spec per adversarial review findings --- ...-sdd-task-scoped-review-dispatch-design.md | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md b/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md index ab64961fb8..a562f9cf79 100644 --- a/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md +++ b/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md @@ -4,9 +4,9 @@ Make subagent-driven-development's per-task reviews cheaper and faster without w ## Problem -Per-task code quality reviewers in SDD routinely do branch-review-scale work on single-task diffs. Evidence from real sessions on this machine: +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 one SDD session, 7/8 quality reviewers ran repo-wide greps; the most expensive ran 50+ Bash commands over ~200 seconds. Quality reviewers cost 4-8× what spec reviewers cost on the same tasks. +- 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()`?"). @@ -27,7 +27,7 @@ A field report (`~/2026-06-09-code-quality-reviewer-scope-budget-issue.md`) firs ## 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.) +- **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. - **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. @@ -37,7 +37,11 @@ A field report (`~/2026-06-09-code-quality-reviewer-scope-budget-issue.md`) firs ### 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. After a fix, the implementer re-runs tests on the amended code; the re-reviewer does not repeat that run. This principle appears in both reviewer prompts and in the controller guidance. +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 @@ -48,22 +52,24 @@ Stop delegating to `requesting-code-review/code-reviewer.md`. The per-task quali - **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. -- **Read-only rule** kept (no mutating the working tree, index, HEAD, or branch state). +- **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. +- 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. +- **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; per-task reviews are task-scoped gates — the broad review happens once, at the final whole-branch review. -- **Final review stays broad, explicitly:** the final whole-branch reviewer dispatch uses `requesting-code-review/code-reviewer.md`. 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. -- Flowchart topology is unchanged. +- **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) @@ -72,4 +78,5 @@ The spec reviewer judges against task text the controller pasted; it cannot catc ## Verification - Plugin infrastructure tests (`tests/`) still pass. -- Run the SDD skill-behavior evals in the `evals/` submodule before and after the change. Reviews must still catch seeded issues; per-task reviewers should show less repo-wide exploration and fewer redundant test runs. +- 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. From f8dcd1ed3d19d23ed45bc6e86a7dc82eb541580d Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 16:42:50 -0700 Subject: [PATCH 03/43] Add implementation plan for task-scoped review dispatch --- ...6-06-09-sdd-task-scoped-review-dispatch.md | 772 ++++++++++++++++++ 1 file changed, 772 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md diff --git a/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md b/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md new file mode 100644 index 0000000000..432ae09717 --- /dev/null +++ b/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md @@ -0,0 +1,772 @@ +# 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 summary, from implementer's report] + + ## Task Requirements (context only) + + [Task N text or plan reference. Context for judging the code, not for + re-checking compliance.] + + ## 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] + ``` + + ## 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 N text or plan reference]` — the task's requirements, 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: two matches (output format + placeholder note) + +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 <>`, then the banner again, then + `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 ` 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: ` 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 ` 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). 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' +} + +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`. From 2cc449b6d4c4fb9e9ff7cf9d00aee3f4e3be1b8a Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 16:47:27 -0700 Subject: [PATCH 04/43] Make per-task quality reviewer prompt self-contained and task-scoped --- .../code-quality-reviewer-prompt.md | 125 ++++++++++++++++-- 1 file changed, 113 insertions(+), 12 deletions(-) diff --git a/skills/subagent-driven-development/code-quality-reviewer-prompt.md b/skills/subagent-driven-development/code-quality-reviewer-prompt.md index 3c1e5696ae..67f950309e 100644 --- a/skills/subagent-driven-development/code-quality-reviewer-prompt.md +++ b/skills/subagent-driven-development/code-quality-reviewer-prompt.md @@ -2,24 +2,125 @@ Use this template when dispatching a code quality reviewer subagent. -**Purpose:** Verify implementation is well-built (clean, tested, maintainable) +**Purpose:** Verify one task's 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: "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. - DESCRIPTION: [task summary, from implementer's report] - PLAN_OR_REQUIREMENTS: Task N from [plan-file] - BASE_SHA: [commit before task] - HEAD_SHA: [current commit] + ## What Was Implemented + + [DESCRIPTION — task summary, from implementer's report] + + ## Task Requirements (context only) + + [Task N text or plan reference. Context for judging the code, not for + re-checking compliance.] + + ## 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] + ``` + + ## 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] ``` -**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.) +**Placeholders:** +- `[DESCRIPTION]` — task summary, from implementer's report +- `[Task N text or plan reference]` — the task's requirements, for context +- `[BASE_SHA]` — commit before this task +- `[HEAD_SHA]` — current commit -**Code reviewer returns:** Strengths, Issues (Critical/Important/Minor), Assessment +**Reviewer returns:** Strengths, Issues (Critical/Important/Minor), Task quality verdict From da412092437bff65ce14c78968a8690eb743aa21 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 16:51:54 -0700 Subject: [PATCH 05/43] Use bare placeholder names in quality reviewer prompt body --- .../2026-06-09-sdd-task-scoped-review-dispatch.md | 11 +++++------ .../code-quality-reviewer-prompt.md | 11 +++++------ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md b/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md index 432ae09717..fb5993d6ad 100644 --- a/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md +++ b/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md @@ -42,17 +42,16 @@ Subagent (general-purpose): ## What Was Implemented - [DESCRIPTION — task summary, from implementer's report] + [DESCRIPTION] ## Task Requirements (context only) - [Task N text or plan reference. Context for judging the code, not for - re-checking compliance.] + [TASK_TEXT] ## Git Range to Review - **Base:** [BASE_SHA — commit before this task] - **Head:** [HEAD_SHA — current commit] + **Base:** [BASE_SHA] + **Head:** [HEAD_SHA] ```bash git diff --stat [BASE_SHA]..[HEAD_SHA] @@ -145,7 +144,7 @@ Subagent (general-purpose): **Placeholders:** - `[DESCRIPTION]` — task summary, from implementer's report -- `[Task N text or plan reference]` — the task's requirements, for context +- `[TASK_TEXT]` — the task's requirements text or plan reference, for context - `[BASE_SHA]` — commit before this task - `[HEAD_SHA]` — current commit diff --git a/skills/subagent-driven-development/code-quality-reviewer-prompt.md b/skills/subagent-driven-development/code-quality-reviewer-prompt.md index 67f950309e..e61107397c 100644 --- a/skills/subagent-driven-development/code-quality-reviewer-prompt.md +++ b/skills/subagent-driven-development/code-quality-reviewer-prompt.md @@ -16,17 +16,16 @@ Subagent (general-purpose): ## What Was Implemented - [DESCRIPTION — task summary, from implementer's report] + [DESCRIPTION] ## Task Requirements (context only) - [Task N text or plan reference. Context for judging the code, not for - re-checking compliance.] + [TASK_TEXT] ## Git Range to Review - **Base:** [BASE_SHA — commit before this task] - **Head:** [HEAD_SHA — current commit] + **Base:** [BASE_SHA] + **Head:** [HEAD_SHA] ```bash git diff --stat [BASE_SHA]..[HEAD_SHA] @@ -119,7 +118,7 @@ Subagent (general-purpose): **Placeholders:** - `[DESCRIPTION]` — task summary, from implementer's report -- `[Task N text or plan reference]` — the task's requirements, for context +- `[TASK_TEXT]` — the task's requirements text or plan reference, for context - `[BASE_SHA]` — commit before this task - `[HEAD_SHA]` — current commit From be8a6269c4f41ce8872f6f477a11899e46e44218 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 16:53:30 -0700 Subject: [PATCH 06/43] =?UTF-8?q?Spec=20reviewer:=20judge=20from=20the=20d?= =?UTF-8?q?iff,=20grounded=20skepticism,=20=E2=9A=A0=EF=B8=8F=20verdict=20?= =?UTF-8?q?channel?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../spec-reviewer-prompt.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/skills/subagent-driven-development/spec-reviewer-prompt.md b/skills/subagent-driven-development/spec-reviewer-prompt.md index 1cc84a76a7..ab4a781b7b 100644 --- a/skills/subagent-driven-development/spec-reviewer-prompt.md +++ b/skills/subagent-driven-development/spec-reviewer-prompt.md @@ -30,14 +30,20 @@ Subagent (general-purpose): Only read files in this diff. Do not crawl the broader codebase. + 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. + ## 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. + 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. ## CRITICAL: Do Not Trust the Report - The implementer finished suspiciously quickly. Their report may be incomplete, - inaccurate, or optimistic. You MUST verify everything independently. + Treat the implementer's report as unverified claims about the code. It may + be incomplete, inaccurate, or optimistic. Verify the claims against the diff. **DO NOT:** - Take their word for what they implemented @@ -74,4 +80,7 @@ Subagent (general-purpose): 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] ``` From c14c1de55269ec0cd6d5d18625defca5854dfd71 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 16:55:28 -0700 Subject: [PATCH 07/43] Scope spec reviewer's Your Job wording to the diff --- skills/subagent-driven-development/spec-reviewer-prompt.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/skills/subagent-driven-development/spec-reviewer-prompt.md b/skills/subagent-driven-development/spec-reviewer-prompt.md index ab4a781b7b..8e6d7f37d2 100644 --- a/skills/subagent-driven-development/spec-reviewer-prompt.md +++ b/skills/subagent-driven-development/spec-reviewer-prompt.md @@ -58,7 +58,7 @@ Subagent (general-purpose): ## Your Job - Read the implementation code and verify: + Read the diff and verify: **Missing requirements:** - Did they implement everything that was requested? @@ -75,7 +75,7 @@ Subagent (general-purpose): - Did they solve the wrong problem? - Did they implement the right feature but wrong way? - **Verify by reading code, not by trusting report.** + **Verify by reading the diff, not by trusting the report.** Report: - ✅ Spec compliant (if everything matches after code inspection) From b3281c0227537faf271e68184faa28018f25d912 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 16:56:28 -0700 Subject: [PATCH 08/43] Implementer prompt: re-run covering tests after fixing review findings --- skills/subagent-driven-development/implementer-prompt.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/skills/subagent-driven-development/implementer-prompt.md b/skills/subagent-driven-development/implementer-prompt.md index d9a32f7024..7c50ae99ab 100644 --- a/skills/subagent-driven-development/implementer-prompt.md +++ b/skills/subagent-driven-development/implementer-prompt.md @@ -97,6 +97,12 @@ Subagent (general-purpose): 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: From 5aea3dca3149c82fed67a987f72b7331fefded44 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 16:59:05 -0700 Subject: [PATCH 09/43] =?UTF-8?q?SDD=20controller:=20reviewer=20prompt=20b?= =?UTF-8?q?udgets,=20=E2=9A=A0=EF=B8=8F=20handling,=20final-review=20point?= =?UTF-8?q?er,=20model=20judgment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- skills/subagent-driven-development/SKILL.md | 40 ++++++++++++++++----- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index d44f91bbcd..72a1740fef 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -62,7 +62,7 @@ digraph process { "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)"; @@ -81,8 +81,8 @@ digraph process { "Code quality reviewer subagent approves?" -> "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 +94,13 @@ 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. + +**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 @@ -119,11 +123,31 @@ 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 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 + ## 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 +- Final whole-branch review: use superpowers:requesting-code-review's [code-reviewer.md](../requesting-code-review/code-reviewer.md) ## Example Workflow @@ -154,7 +178,7 @@ Implementer: "Got it. Implementing now..." 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. +Code reviewer: Strengths: Good test coverage, clean. Issues: None. Task quality: Approved. [Mark Task 1 complete] @@ -188,7 +212,7 @@ Code reviewer: Strengths: Solid. Issues (Important): Magic number (100) Implementer: Extracted PROGRESS_INTERVAL constant [Code reviewer reviews again] -Code reviewer: ✅ Approved +Code reviewer: ✅ Task quality: Approved [Mark Task 2 complete] @@ -269,7 +293,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:** From 71dc271a08ddbdb7303ec16da67e5f1727f47599 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 17:13:03 -0700 Subject: [PATCH 10/43] Sync plan's Task 5 blocks with review fixes --- .../plans/2026-06-09-sdd-task-scoped-review-dispatch.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md b/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md index fb5993d6ad..564da68f84 100644 --- a/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md +++ b/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md @@ -626,7 +626,9 @@ 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). Once the agent reports the plan is complete (both functions +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. @@ -671,6 +673,7 @@ pre() { 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() { From b3bb9a68d7acb0a39189bee213cc946aabc54f47 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 17:21:06 -0700 Subject: [PATCH 11/43] Fix plan doc: correct Task 1 grep expectation; sync Task 5 story block --- ...6-06-09-sdd-task-scoped-review-dispatch.md | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md b/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md index 564da68f84..1422d57788 100644 --- a/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md +++ b/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md @@ -157,7 +157,7 @@ Run: `grep -c "requesting-code-review" skills/subagent-driven-development/code-q Expected: `ABSENT` (no more delegation) Run: `grep -n "Task quality:" skills/subagent-driven-development/code-quality-reviewer-prompt.md | head -2` -Expected: two matches (output format + placeholder note) +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` @@ -637,21 +637,21 @@ you are done. - 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. +- 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. + 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`:** From 09cb4d73617e049dd6def173315b90453fd607e9 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 18:19:00 -0700 Subject: [PATCH 12/43] Sync plan: escaped pre() pattern in Task 5 checks block --- .../plans/2026-06-09-sdd-task-scoped-review-dispatch.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md b/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md index 1422d57788..57cc5fc076 100644 --- a/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md +++ b/docs/superpowers/plans/2026-06-09-sdd-task-scoped-review-dispatch.md @@ -673,7 +673,7 @@ pre() { 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)' + file-contains 'docs/superpowers/plans/report-plan.md' 'repeat\(40\)' } post() { From 87825ff1935b6cc1011cc6dbc9099f6e6b2d36cf Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 18:28:24 -0700 Subject: [PATCH 13/43] 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-sdd-task-scoped-review-dispatch-design.md | 2 +- skills/subagent-driven-development/SKILL.md | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md b/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md index a562f9cf79..685cb14453 100644 --- a/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md +++ b/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md @@ -65,7 +65,7 @@ Stop delegating to `requesting-code-review/code-reviewer.md`. The per-task quali ### 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; per-task reviews are task-scoped gates — the broad review happens once, at the final whole-branch review. +- **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.) - **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. diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index 72a1740fef..cc0972fcb1 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -141,6 +141,10 @@ final whole-branch review. When you fill a reviewer template: 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. ## Prompt Templates From 5cfdb75b9448ed083bbca95e87eb29c3470c6703 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 21:11:45 -0700 Subject: [PATCH 14/43] 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). --- skills/subagent-driven-development/SKILL.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index cc0972fcb1..de4eca3387 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -100,6 +100,10 @@ Use the least powerful model that can handle each role to conserve cost and incr 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. + **Task complexity signals (implementation tasks):** - Touches 1-2 files with a complete spec → cheap model - Touches multiple files with integration concerns → standard model From c7900f169897cc75e310383464f524dff9a9a845 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 21:19:08 -0700 Subject: [PATCH 15/43] 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-sdd-task-scoped-review-dispatch-design.md | 5 +++-- skills/subagent-driven-development/SKILL.md | 3 +++ .../code-quality-reviewer-prompt.md | 6 ++++++ skills/subagent-driven-development/implementer-prompt.md | 1 + 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md b/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md index 685cb14453..6e30ba0320 100644 --- a/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md +++ b/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md @@ -51,7 +51,8 @@ Stop delegating to `requesting-code-review/code-reviewer.md`. The per-task quali - **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. +- **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." @@ -65,7 +66,7 @@ Stop delegating to `requesting-code-review/code-reviewer.md`. The per-task quali ### 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.) +- **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. diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index de4eca3387..7741a53a9c 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -149,6 +149,9 @@ final whole-branch review. When you fill a reviewer template: 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. +- 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. ## Prompt Templates diff --git a/skills/subagent-driven-development/code-quality-reviewer-prompt.md b/skills/subagent-driven-development/code-quality-reviewer-prompt.md index e61107397c..cb9dbc64f9 100644 --- a/skills/subagent-driven-development/code-quality-reviewer-prompt.md +++ b/skills/subagent-driven-development/code-quality-reviewer-prompt.md @@ -61,6 +61,9 @@ Subagent (general-purpose): 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. + ## What to Check **Code quality:** @@ -81,6 +84,9 @@ Subagent (general-purpose): significantly grow existing files? (Don't flag pre-existing file sizes — focus on what this change contributed.) + Answer each item above with file:line evidence, not a bare yes or no. + An unsupported "yes" is not a review. + ## Calibration Categorize issues by actual severity. Not everything is Critical. diff --git a/skills/subagent-driven-development/implementer-prompt.md b/skills/subagent-driven-development/implementer-prompt.md index 7c50ae99ab..9456c0a0a3 100644 --- a/skills/subagent-driven-development/implementer-prompt.md +++ b/skills/subagent-driven-development/implementer-prompt.md @@ -94,6 +94,7 @@ 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. From 83d54f7ddd1828c9c506ea4c616eb34c650d598b Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 21:47:41 -0700 Subject: [PATCH 16/43] Red Flags: never tell a reviewer what not to flag or pre-rate severity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- skills/subagent-driven-development/SKILL.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index 7741a53a9c..60a8ad96c0 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -281,6 +281,9 @@ Done! - Accept "close enough" on spec compliance (spec reviewer found issues = not done) - Skip review loops (reviewer found issues = implementer fixes = review again) - Let implementer self-review replace actual review (both are needed) +- 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 - **Start code quality review before spec compliance is ✅** (wrong order) - Move to next task while either review has open issues From 853396e3ae5aeab43e2f3c5a399ae8f6a3cf2dec Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 21:49:51 -0700 Subject: [PATCH 17/43] 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. --- skills/subagent-driven-development/SKILL.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index 60a8ad96c0..ae801025c5 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -148,7 +148,9 @@ final whole-branch review. When you fill a reviewer template: - 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. + 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. From 3e3e1e701e0f76fff3cc3f2803badd1b549dc50a Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 22:42:54 -0700 Subject: [PATCH 18/43] 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. --- skills/subagent-driven-development/SKILL.md | 11 +++++++++++ .../code-quality-reviewer-prompt.md | 18 ++++++++++++++++-- .../spec-reviewer-prompt.md | 7 +++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index ae801025c5..9a0f066083 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -104,6 +104,12 @@ most capable model; a subtle concurrency change does. 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 @@ -154,6 +160,11 @@ final whole-branch review. When you fill a reviewer template: - 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. +- Paste the task's diff (`git diff BASE..HEAD` output) into the reviewer + prompt when it fits comfortably (up to a few hundred lines). A reviewer + with the diff in hand needs few or no tool calls. +- Dispatch fix subagents for Critical and Important findings. Record Minor + findings and move on — they roll up to the final whole-branch review. ## Prompt Templates diff --git a/skills/subagent-driven-development/code-quality-reviewer-prompt.md b/skills/subagent-driven-development/code-quality-reviewer-prompt.md index cb9dbc64f9..6fb27e3f44 100644 --- a/skills/subagent-driven-development/code-quality-reviewer-prompt.md +++ b/skills/subagent-driven-development/code-quality-reviewer-prompt.md @@ -32,6 +32,14 @@ Subagent (general-purpose): git diff [BASE_SHA]..[HEAD_SHA] ``` + ## Diff + + [DIFF] + + If the diff is provided above, review from it directly — do not re-run + the git commands or re-read the files it already shows. Fetch anything + further only for a named concrete risk. + ## Read-Only Review Your review is read-only on this checkout. Do not mutate the working tree, @@ -84,12 +92,15 @@ Subagent (general-purpose): significantly grow existing files? (Don't flag pre-existing file sizes — focus on what this change contributed.) - Answer each item above with file:line evidence, not a bare yes or no. - An unsupported "yes" is not a review. + 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; + "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. @@ -127,5 +138,8 @@ Subagent (general-purpose): - `[TASK_TEXT]` — the task's requirements text or plan reference, for context - `[BASE_SHA]` — commit before this task - `[HEAD_SHA]` — current commit +- `[DIFF]` — paste `git diff BASE..HEAD` output when it fits comfortably + (up to a few hundred lines); otherwise replace with "(not provided — run + the git commands above)" **Reviewer returns:** Strengths, Issues (Critical/Important/Minor), Task quality verdict diff --git a/skills/subagent-driven-development/spec-reviewer-prompt.md b/skills/subagent-driven-development/spec-reviewer-prompt.md index 8e6d7f37d2..43e8d3d31b 100644 --- a/skills/subagent-driven-development/spec-reviewer-prompt.md +++ b/skills/subagent-driven-development/spec-reviewer-prompt.md @@ -30,6 +30,13 @@ Subagent (general-purpose): Only read files in this diff. Do not crawl the broader codebase. + ## Diff + + [DIFF] + + If the diff is provided above, review from it directly — do not re-run + the git commands or re-read the files it already shows. + 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 From e3c74fc1c979a7b14252a157d4f9ae6b15038dc5 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 23:58:28 -0700 Subject: [PATCH 19/43] Merge per-task reviews into one task reviewer (iteration 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- skills/subagent-driven-development/SKILL.md | 92 +++++------ .../code-quality-reviewer-prompt.md | 145 ---------------- .../implementer-prompt.md | 3 + .../spec-reviewer-prompt.md | 93 ----------- .../task-reviewer-prompt.md | 156 ++++++++++++++++++ .../references/antigravity-tools.md | 2 +- .../references/gemini-tools.md | 2 +- 7 files changed, 198 insertions(+), 295 deletions(-) delete mode 100644 skills/subagent-driven-development/code-quality-reviewer-prompt.md delete mode 100644 skills/subagent-driven-development/spec-reviewer-prompt.md create mode 100644 skills/subagent-driven-development/task-reviewer-prompt.md diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index 9a0f066083..ee06395739 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -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,12 +51,9 @@ 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]; } @@ -70,15 +67,11 @@ 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 (../requesting-code-review/code-reviewer.md)" [label="no"]; @@ -119,7 +112,7 @@ single-file mechanical fixes. 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. @@ -133,12 +126,12 @@ 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 Spec Reviewer ⚠️ Items +## Handling 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 +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. @@ -160,17 +153,17 @@ final whole-branch review. When you fill a reviewer template: - 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. -- Paste the task's diff (`git diff BASE..HEAD` output) into the reviewer - prompt when it fits comfortably (up to a few hundred lines). A reviewer - with the diff in hand needs few or no tool calls. +- Run `git diff BASE..HEAD` yourself and paste the output into the reviewer + prompt (`--stat` plus the relevant hunks if it exceeds a few hundred + lines). A reviewer with the diff in hand needs few or no tool calls; do + not make reviewers re-derive the diff. - Dispatch fix subagents for Critical and Important findings. Record Minor findings and move on — they roll up to the final whole-branch review. ## 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 @@ -198,11 +191,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. Task quality: 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] @@ -218,25 +209,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 - -[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 +[Dispatch fix subagent with all findings] +Fixer: Removed --json flag, added progress reporting, extracted PROGRESS_INTERVAL constant -[Code reviewer reviews again] -Code reviewer: ✅ Task quality: Approved +[Task reviewer reviews again] +Task reviewer: Spec ✅. Task quality: Approved. [Mark Task 2 complete] @@ -270,13 +253,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) @@ -285,20 +268,19 @@ 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) - 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 -- **Start code quality review before spec compliance is ✅** (wrong order) -- Move to next task while either review has open issues +- Move to next task while the review has open Critical/Important issues **If subagent asks questions:** - Answer clearly and completely diff --git a/skills/subagent-driven-development/code-quality-reviewer-prompt.md b/skills/subagent-driven-development/code-quality-reviewer-prompt.md deleted file mode 100644 index 6fb27e3f44..0000000000 --- a/skills/subagent-driven-development/code-quality-reviewer-prompt.md +++ /dev/null @@ -1,145 +0,0 @@ -# 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] - ``` - - ## Diff - - [DIFF] - - If the diff is provided above, review from it directly — do not re-run - the git commands or re-read the files it already shows. Fetch anything - further only for a named concrete risk. - - ## 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. - - Warnings or other noise in the implementer's reported test output are - findings — test output should be pristine. - - ## 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.) - - 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; - "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 - - ### 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 -- `[DIFF]` — paste `git diff BASE..HEAD` output when it fits comfortably - (up to a few hundred lines); otherwise replace with "(not provided — run - the git commands above)" - -**Reviewer returns:** Strengths, Issues (Critical/Important/Minor), Task quality verdict diff --git a/skills/subagent-driven-development/implementer-prompt.md b/skills/subagent-driven-development/implementer-prompt.md index 9456c0a0a3..d5f4600da5 100644 --- a/skills/subagent-driven-development/implementer-prompt.md +++ b/skills/subagent-driven-development/implementer-prompt.md @@ -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 diff --git a/skills/subagent-driven-development/spec-reviewer-prompt.md b/skills/subagent-driven-development/spec-reviewer-prompt.md deleted file mode 100644 index 43e8d3d31b..0000000000 --- a/skills/subagent-driven-development/spec-reviewer-prompt.md +++ /dev/null @@ -1,93 +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. - - ## Diff - - [DIFF] - - If the diff is provided above, review from it directly — do not re-run - the git commands or re-read the files it already shows. - - 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. - - ## 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. - - ## CRITICAL: 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. - - **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 diff 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 the diff, not by trusting the report.** - - 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] -``` diff --git a/skills/subagent-driven-development/task-reviewer-prompt.md b/skills/subagent-driven-development/task-reviewer-prompt.md new file mode 100644 index 0000000000..744cfe638f --- /dev/null +++ b/skills/subagent-driven-development/task-reviewer-prompt.md @@ -0,0 +1,156 @@ +# 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] + + Review from the diff above — do not re-run git commands or re-read the + files it already shows. If no diff was provided, fetch it 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. + + ## 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; + "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]` — paste `git diff BASE..HEAD` output (use `--stat` plus the + relevant hunks if it exceeds a few hundred lines); a reviewer with the + diff in hand needs few or no tool calls + +**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. diff --git a/skills/using-superpowers/references/antigravity-tools.md b/skills/using-superpowers/references/antigravity-tools.md index 723048b7dd..b0d4fa1fe5 100644 --- a/skills/using-superpowers/references/antigravity-tools.md +++ b/skills/using-superpowers/references/antigravity-tools.md @@ -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 diff --git a/skills/using-superpowers/references/gemini-tools.md b/skills/using-superpowers/references/gemini-tools.md index 095df29e20..b01b65238a 100644 --- a/skills/using-superpowers/references/gemini-tools.md +++ b/skills/using-superpowers/references/gemini-tools.md @@ -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 | From e532f24df72fd498e4e104fb83be773d7ec5d205 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 23:59:22 -0700 Subject: [PATCH 20/43] Spec: document cost iterations and the per-task review consolidation --- ...-sdd-task-scoped-review-dispatch-design.md | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md b/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md index 6e30ba0320..0f6e6b98a3 100644 --- a/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md +++ b/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md @@ -28,10 +28,34 @@ A field report (`~/2026-06-09-code-quality-reviewer-scope-budget-issue.md`) firs ## 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. +- ~~**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. -- Review ordering (spec before quality), the fix-and-re-review loops, and the requirement to fix Critical/Important findings are unchanged. +- 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 a pasted diff, two + verdicts; one fix dispatch addresses both kinds of findings); diff-pasting + made imperative (controller runs `git diff` itself); implementers run the + focused test while iterating, full suite once before commit. ## Design From 5e2907fc4f86cd3d9785f07b36c9834cbc4f548d Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 02:09:10 -0700 Subject: [PATCH 21/43] 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. --- skills/subagent-driven-development/SKILL.md | 4 +++- skills/subagent-driven-development/task-reviewer-prompt.md | 7 +++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index ee06395739..67257e186d 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -158,7 +158,9 @@ final whole-branch review. When you fill a reviewer template: lines). A reviewer with the diff in hand needs few or no tool calls; do not make reviewers re-derive the diff. - Dispatch fix subagents for Critical and Important findings. Record Minor - findings and move on — they roll up to the final whole-branch review. + 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 diff --git a/skills/subagent-driven-development/task-reviewer-prompt.md b/skills/subagent-driven-development/task-reviewer-prompt.md index 744cfe638f..12c6da5159 100644 --- a/skills/subagent-driven-development/task-reviewer-prompt.md +++ b/skills/subagent-driven-development/task-reviewer-prompt.md @@ -104,8 +104,11 @@ Subagent (general-purpose): ## Calibration Categorize issues by actual severity. Not everything is Critical. - Important means this task cannot be trusted until it is fixed; - "coverage could be broader" and polish suggestions are Minor. + 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. From 28498a5cde8c8d6f459289b0cbb8ae37ece24d44 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 02:10:34 -0700 Subject: [PATCH 22/43] 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. --- skills/subagent-driven-development/SKILL.md | 3 +++ .../task-reviewer-prompt.md | 10 ++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index 67257e186d..c21dce502d 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -282,6 +282,9 @@ Done! - 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 pasting the diff into the prompt — run + `git diff BASE..HEAD` yourself first (`--stat` plus relevant hunks if it + exceeds a few hundred lines) - Move to next task while the review has open Critical/Important issues **If subagent asks questions:** diff --git a/skills/subagent-driven-development/task-reviewer-prompt.md b/skills/subagent-driven-development/task-reviewer-prompt.md index 12c6da5159..c3ead9cbff 100644 --- a/skills/subagent-driven-development/task-reviewer-prompt.md +++ b/skills/subagent-driven-development/task-reviewer-prompt.md @@ -28,7 +28,9 @@ Subagent (general-purpose): **Base:** [BASE_SHA] **Head:** [HEAD_SHA] - [DIFF] + [DIFF — REQUIRED: the controller pastes `git diff BASE..HEAD` output + here before dispatching. Dispatching with this placeholder unfilled is + a dispatch error.] Review from the diff above — do not re-run git commands or re-read the files it already shows. If no diff was provided, fetch it yourself: @@ -148,9 +150,9 @@ Subagent (general-purpose): - `[DESCRIPTION]` — what the implementer reports they built - `[BASE_SHA]` — commit before this task - `[HEAD_SHA]` — current commit -- `[DIFF]` — paste `git diff BASE..HEAD` output (use `--stat` plus the - relevant hunks if it exceeds a few hundred lines); a reviewer with the - diff in hand needs few or no tool calls +- `[DIFF]` — REQUIRED: paste `git diff BASE..HEAD` output (use `--stat` + plus the relevant hunks if it exceeds a few hundred lines); a reviewer + with the diff in hand needs few or no tool calls **Reviewer returns:** Spec Compliance verdict (✅/❌/⚠️), Strengths, Issues (Critical/Important/Minor), Task quality verdict From 29ee4e8e442a5b4fe19832c2ebc98de682c15512 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 02:20:28 -0700 Subject: [PATCH 23/43] Reviewer skepticism covers the implementer's design rationales MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- skills/subagent-driven-development/task-reviewer-prompt.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/skills/subagent-driven-development/task-reviewer-prompt.md b/skills/subagent-driven-development/task-reviewer-prompt.md index c3ead9cbff..f6b98bd691 100644 --- a/skills/subagent-driven-development/task-reviewer-prompt.md +++ b/skills/subagent-driven-development/task-reviewer-prompt.md @@ -48,7 +48,10 @@ Subagent (general-purpose): Treat the implementer's report as unverified claims about the code. It may be incomplete, inaccurate, or optimistic. Verify the claims against - the diff. + 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 From e355795625800027f45e9d81246e2c67d9d0f95e Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 03:44:19 -0700 Subject: [PATCH 24/43] Hand reviewers the diff as a file, not a paste MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- skills/subagent-driven-development/SKILL.md | 15 ++++++++------- .../task-reviewer-prompt.md | 16 +++++++--------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index c21dce502d..ca5c4795c7 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -153,10 +153,11 @@ final whole-branch review. When you fill a reviewer template: - 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. -- Run `git diff BASE..HEAD` yourself and paste the output into the reviewer - prompt (`--stat` plus the relevant hunks if it exceeds a few hundred - lines). A reviewer with the diff in hand needs few or no tool calls; do - not make reviewers re-derive the diff. +- 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 @@ -282,9 +283,9 @@ Done! - 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 pasting the diff into the prompt — run - `git diff BASE..HEAD` yourself first (`--stat` plus relevant hunks if it - exceeds a few hundred lines) +- 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:** diff --git a/skills/subagent-driven-development/task-reviewer-prompt.md b/skills/subagent-driven-development/task-reviewer-prompt.md index f6b98bd691..37b4dd6779 100644 --- a/skills/subagent-driven-development/task-reviewer-prompt.md +++ b/skills/subagent-driven-development/task-reviewer-prompt.md @@ -27,13 +27,11 @@ Subagent (general-purpose): **Base:** [BASE_SHA] **Head:** [HEAD_SHA] + **Diff file:** [DIFF_FILE] - [DIFF — REQUIRED: the controller pastes `git diff BASE..HEAD` output - here before dispatching. Dispatching with this placeholder unfilled is - a dispatch error.] - - Review from the diff above — do not re-run git commands or re-read the - files it already shows. If no diff was provided, fetch it yourself: + 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 @@ -153,9 +151,9 @@ Subagent (general-purpose): - `[DESCRIPTION]` — what the implementer reports they built - `[BASE_SHA]` — commit before this task - `[HEAD_SHA]` — current commit -- `[DIFF]` — REQUIRED: paste `git diff BASE..HEAD` output (use `--stat` - plus the relevant hunks if it exceeds a few hundred lines); a reviewer - with the diff in hand needs few or no tool calls +- `[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 From 7cf78437e2c77baae60b5f4759a8939f76f79219 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 05:06:59 -0700 Subject: [PATCH 25/43] Spec: record iterations 2-3 results and final frozen-config matrix --- ...-sdd-task-scoped-review-dispatch-design.md | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md b/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md index 0f6e6b98a3..bd60bc6656 100644 --- a/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md +++ b/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md @@ -52,10 +52,27 @@ evidence-rule narration. 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 a pasted diff, two - verdicts; one fix dispatch addresses both kinds of findings); diff-pasting - made imperative (controller runs `git diff` itself); implementers run the - focused test while iterating, full suite once before commit. + `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 From 2434ef7f35ec2662e55d5e9927a0124461b8cc28 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 08:28:28 -0700 Subject: [PATCH 26/43] Describe the review design as current state, not as a delta The skill read as a changelog: 'combined task review,' 'one reviewer, one reading,' 'one dispatch,' and an example still showing diffs pasted into prompts. A reader who never saw the two-reviewer design has no referent for 'combined.' Prose now states the design directly, and the flowchart/example reflect the diff-file handoff. --- skills/subagent-driven-development/SKILL.md | 22 +++++++++---------- .../task-reviewer-prompt.md | 9 ++++---- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index ca5c4795c7..4340f9d817 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -5,11 +5,11 @@ description: Use when executing implementation plans with independent tasks in t # Subagent-Driven Development -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. +Execute plan by dispatching a fresh implementer subagent per task, a task review (spec compliance + code quality) 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 + one task review (spec + quality verdicts) + broad final review = high quality, fast iteration +**Core principle:** Fresh subagent per task + task review (spec + quality) + 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) -- Combined review after each task (spec compliance + code quality verdicts), broad review at the end +- Review after each task (spec compliance + code quality), broad review at the end - Faster iteration (no human-in-loop between tasks) ## The Process @@ -51,7 +51,7 @@ digraph process { "Implementer subagent asks questions?" [shape=diamond]; "Answer questions, provide context" [shape=box]; "Implementer subagent implements, tests, commits, self-reviews" [shape=box]; - "Run git diff, dispatch task reviewer subagent (./task-reviewer-prompt.md)" [shape=box]; + "Write diff file, 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]; @@ -67,10 +67,10 @@ 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" -> "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?"; + "Implementer subagent implements, tests, commits, self-reviews" -> "Write diff file, dispatch task reviewer subagent (./task-reviewer-prompt.md)"; + "Write diff file, 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"]; + "Dispatch fix subagent for Critical/Important findings" -> "Write diff file, 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"]; @@ -112,7 +112,7 @@ single-file mechanical fixes. Implementer subagents report one of four statuses. Handle each appropriately: -**DONE:** Run `git diff BASE..HEAD`, then dispatch the task reviewer. +**DONE:** Write the task's diff to a file (`git diff BASE..HEAD > /tmp/sdd-task-N.diff`), then dispatch the task reviewer with that path. **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. @@ -166,7 +166,7 @@ final whole-branch review. When you fill a reviewer template: ## Prompt Templates - [implementer-prompt.md](implementer-prompt.md) - Dispatch implementer subagent -- [task-reviewer-prompt.md](task-reviewer-prompt.md) - Dispatch task reviewer subagent (spec compliance + code quality, one dispatch) +- [task-reviewer-prompt.md](task-reviewer-prompt.md) - Dispatch task reviewer subagent (spec compliance + code quality) - Final whole-branch review: use superpowers:requesting-code-review's [code-reviewer.md](../requesting-code-review/code-reviewer.md) ## Example Workflow @@ -194,7 +194,7 @@ Implementer: "Got it. Implementing now..." - Self-review: Found I missed --force flag, added it - Committed -[Run git diff, dispatch task reviewer with the diff pasted in] +[Write diff to /tmp/sdd-task-N.diff, dispatch task reviewer with the path] Task reviewer: Spec ✅ - all requirements met, nothing extra. Strengths: Good test coverage, clean. Issues: None. Task quality: Approved. @@ -212,7 +212,7 @@ Implementer: - Self-review: All good - Committed -[Run git diff, dispatch task reviewer with the diff pasted in] +[Write diff to /tmp/sdd-task-N.diff, dispatch task reviewer with the path] Task reviewer: Spec ❌: - Missing: Progress reporting (spec says "report every 100 items") - Extra: Added --json flag (not requested) diff --git a/skills/subagent-driven-development/task-reviewer-prompt.md b/skills/subagent-driven-development/task-reviewer-prompt.md index 37b4dd6779..630ec99195 100644 --- a/skills/subagent-driven-development/task-reviewer-prompt.md +++ b/skills/subagent-driven-development/task-reviewer-prompt.md @@ -1,7 +1,8 @@ # 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. +Use this template when dispatching a task reviewer subagent. The reviewer +reads the task's diff once and returns 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) @@ -158,5 +159,5 @@ Subagent (general-purpose): **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. +A fix dispatch can address spec gaps and quality findings together; +re-review after fixes covers both verdicts. From d4dbf44162f2740b0d216a52540a4469107ecdd1 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 08:51:16 -0700 Subject: [PATCH 27/43] Add review-package script; close fix-dispatch test gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scripts/review-package generates the reviewer's input deterministically: commit list, stat summary, and net diff with -U10 context, written to a file from an explicit BASE. Live runs showed controllers improvising 'git diff HEAD~1..HEAD', which silently truncates multi-commit tasks, and svelte's five fix dispatches shipped without re-running any tests — fix dispatches now explicitly carry the implementer's re-run-and-report contract. --- skills/subagent-driven-development/SKILL.md | 25 ++++++++----- .../scripts/review-package | 37 +++++++++++++++++++ .../task-reviewer-prompt.md | 13 ++++--- 3 files changed, 60 insertions(+), 15 deletions(-) create mode 100755 skills/subagent-driven-development/scripts/review-package diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index 4340f9d817..6c8d83423c 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -112,7 +112,7 @@ single-file mechanical fixes. Implementer subagents report one of four statuses. Handle each appropriately: -**DONE:** Write the task's diff to a file (`git diff BASE..HEAD > /tmp/sdd-task-N.diff`), then dispatch the task reviewer with that path. +**DONE:** Generate the review package (`scripts/review-package BASE HEAD /tmp/sdd-task-N.diff`, from this skill's directory; BASE is the commit you recorded before dispatching the implementer — never `HEAD~1`, which silently drops all but the last commit of a multi-commit task), then dispatch the task reviewer with that path. **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. @@ -153,15 +153,22 @@ final whole-branch review. When you fill a reviewer template: - 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. +- Hand the reviewer its diff as a file: run this skill's + `scripts/review-package BASE HEAD /tmp/sdd-task-N.diff` (or, without + bash: `git log --oneline`, `git diff --stat`, and `git diff -U10` for + the range, redirected to the file). The output never enters your own + context, and the reviewer sees the commit list, stat summary, and full + diff with context in one Read call. Use the BASE you recorded before + dispatching the implementer — never `HEAD~1`, which silently truncates + multi-commit tasks. - 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. +- Every fix dispatch carries the implementer contract: the fix subagent + re-runs the tests covering its change and reports the results. A fix + report without test evidence is incomplete — do not re-review on top of + it. ## Prompt Templates @@ -283,9 +290,9 @@ Done! - 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 +- Dispatch a task reviewer without a diff file — generate it first + (`scripts/review-package BASE HEAD /tmp/sdd-task-N.diff`) and name that + path in the prompt - Move to next task while the review has open Critical/Important issues **If subagent asks questions:** diff --git a/skills/subagent-driven-development/scripts/review-package b/skills/subagent-driven-development/scripts/review-package new file mode 100755 index 0000000000..90970f1b3b --- /dev/null +++ b/skills/subagent-driven-development/scripts/review-package @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +# Generate a task review package: commit list, stat summary, and the net +# diff with extended context, written to a file the reviewer reads in one +# call. Using the recorded per-task BASE (not HEAD~1) keeps multi-commit +# tasks intact. +# +# Usage: review-package BASE HEAD OUTFILE +# Example: review-package a1b2c3d HEAD /tmp/sdd-task-3.diff +set -euo pipefail + +if [ $# -ne 3 ]; then + echo "usage: review-package BASE HEAD OUTFILE" >&2 + exit 2 +fi + +base=$1 +head=$2 +out=$3 + +git rev-parse --verify --quiet "$base" >/dev/null || { echo "bad BASE: $base" >&2; exit 2; } +git rev-parse --verify --quiet "$head" >/dev/null || { echo "bad HEAD: $head" >&2; exit 2; } + +{ + echo "# Review package: ${base}..${head}" + echo + echo "## Commits" + git log --oneline "${base}..${head}" + echo + echo "## Files changed" + git diff --stat "${base}..${head}" + echo + echo "## Diff" + git diff -U10 "${base}..${head}" +} > "$out" + +commits=$(git rev-list --count "${base}..${head}") +echo "wrote ${out}: ${commits} commit(s), $(wc -c < "$out" | tr -d ' ') bytes" diff --git a/skills/subagent-driven-development/task-reviewer-prompt.md b/skills/subagent-driven-development/task-reviewer-prompt.md index 630ec99195..e92c2c52c7 100644 --- a/skills/subagent-driven-development/task-reviewer-prompt.md +++ b/skills/subagent-driven-development/task-reviewer-prompt.md @@ -30,9 +30,10 @@ Subagent (general-purpose): **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: + Read the diff file once — it contains the commit list, a stat summary, + and the full diff with surrounding context, and it 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 @@ -152,9 +153,9 @@ Subagent (general-purpose): - `[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) +- `[DIFF_FILE]` — REQUIRED: the path the controller wrote the review + package to (`scripts/review-package 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 From a995af2e240629ee8dc83ff6339daed70ca755a5 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 09:39:21 -0700 Subject: [PATCH 28/43] Shared: unique review-package collateral names --- skills/subagent-driven-development/SKILL.md | 24 +++++++++---------- .../scripts/review-package | 22 ++++++++++++----- .../task-reviewer-prompt.md | 4 ++-- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index 6c8d83423c..4976d00271 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -112,7 +112,7 @@ single-file mechanical fixes. Implementer subagents report one of four statuses. Handle each appropriately: -**DONE:** Generate the review package (`scripts/review-package BASE HEAD /tmp/sdd-task-N.diff`, from this skill's directory; BASE is the commit you recorded before dispatching the implementer — never `HEAD~1`, which silently drops all but the last commit of a multi-commit task), then dispatch the task reviewer with that path. +**DONE:** Generate the review package (`scripts/review-package BASE HEAD`, from this skill's directory — it prints the unique file path it wrote; BASE is the commit you recorded before dispatching the implementer — never `HEAD~1`, which silently drops all but the last commit of a multi-commit task), then dispatch the task reviewer with the printed path. **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. @@ -154,13 +154,13 @@ final whole-branch review. When you fill a reviewer template: 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 this skill's - `scripts/review-package BASE HEAD /tmp/sdd-task-N.diff` (or, without - bash: `git log --oneline`, `git diff --stat`, and `git diff -U10` for - the range, redirected to the file). The output never enters your own - context, and the reviewer sees the commit list, stat summary, and full - diff with context in one Read call. Use the BASE you recorded before - dispatching the implementer — never `HEAD~1`, which silently truncates - multi-commit tasks. + `scripts/review-package BASE HEAD` and pass the reviewer the file path + it prints (or, without bash: `git log --oneline`, `git diff --stat`, + and `git diff -U10` for the range, redirected to one uniquely named + file). The output never enters your own context, and the reviewer sees + the commit list, stat summary, and full diff with context in one Read + call. Use the BASE you recorded before dispatching the implementer — + never `HEAD~1`, which silently truncates multi-commit tasks. - 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 @@ -201,7 +201,7 @@ Implementer: "Got it. Implementing now..." - Self-review: Found I missed --force flag, added it - Committed -[Write diff to /tmp/sdd-task-N.diff, dispatch task reviewer with the path] +[Run review-package, dispatch task reviewer with the printed path] Task reviewer: Spec ✅ - all requirements met, nothing extra. Strengths: Good test coverage, clean. Issues: None. Task quality: Approved. @@ -219,7 +219,7 @@ Implementer: - Self-review: All good - Committed -[Write diff to /tmp/sdd-task-N.diff, dispatch task reviewer with the path] +[Run review-package, dispatch task reviewer with the printed path] Task reviewer: Spec ❌: - Missing: Progress reporting (spec says "report every 100 items") - Extra: Added --json flag (not requested) @@ -291,8 +291,8 @@ Done! 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 — generate it first - (`scripts/review-package BASE HEAD /tmp/sdd-task-N.diff`) and name that - path in the prompt + (`scripts/review-package BASE HEAD`) and name the printed path in the + prompt - Move to next task while the review has open Critical/Important issues **If subagent asks questions:** diff --git a/skills/subagent-driven-development/scripts/review-package b/skills/subagent-driven-development/scripts/review-package index 90970f1b3b..88a00224dc 100755 --- a/skills/subagent-driven-development/scripts/review-package +++ b/skills/subagent-driven-development/scripts/review-package @@ -1,25 +1,35 @@ #!/usr/bin/env bash -# Generate a task review package: commit list, stat summary, and the net +# Generate a review package: commit list, stat summary, and the net # diff with extended context, written to a file the reviewer reads in one # call. Using the recorded per-task BASE (not HEAD~1) keeps multi-commit # tasks intact. # -# Usage: review-package BASE HEAD OUTFILE -# Example: review-package a1b2c3d HEAD /tmp/sdd-task-3.diff +# Usage: review-package BASE HEAD [OUTFILE] +# Default OUTFILE: /sdd/review-...diff — unique per +# repo instance and per range, so concurrent sessions cannot collide and a +# re-review after fixes always gets a distinctly named fresh file. set -euo pipefail -if [ $# -ne 3 ]; then - echo "usage: review-package BASE HEAD OUTFILE" >&2 +if [ $# -lt 2 ] || [ $# -gt 3 ]; then + echo "usage: review-package BASE HEAD [OUTFILE]" >&2 exit 2 fi base=$1 head=$2 -out=$3 git rev-parse --verify --quiet "$base" >/dev/null || { echo "bad BASE: $base" >&2; exit 2; } git rev-parse --verify --quiet "$head" >/dev/null || { echo "bad HEAD: $head" >&2; exit 2; } +if [ $# -eq 3 ]; then + out=$3 +else + dir=$(git rev-parse --git-path sdd) + mkdir -p "$dir" + dir=$(cd "$dir" && pwd) + out="$dir/review-$(git rev-parse --short "$base")..$(git rev-parse --short "$head").diff" +fi + { echo "# Review package: ${base}..${head}" echo diff --git a/skills/subagent-driven-development/task-reviewer-prompt.md b/skills/subagent-driven-development/task-reviewer-prompt.md index e92c2c52c7..465a620534 100644 --- a/skills/subagent-driven-development/task-reviewer-prompt.md +++ b/skills/subagent-driven-development/task-reviewer-prompt.md @@ -154,8 +154,8 @@ Subagent (general-purpose): - `[BASE_SHA]` — commit before this task - `[HEAD_SHA]` — current commit - `[DIFF_FILE]` — REQUIRED: the path the controller wrote the review - package to (`scripts/review-package BASE HEAD /tmp/sdd-task-N.diff`, - redirected so it never enters the controller's context) + package to (`scripts/review-package BASE HEAD` prints the unique path it + wrote; the package never enters the controller's context) **Reviewer returns:** Spec Compliance verdict (✅/❌/⚠️), Strengths, Issues (Critical/Important/Minor), Task quality verdict From 926096a1d75e6133b2f20f2c052b43120448ae36 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 12:32:06 -0700 Subject: [PATCH 29/43] =?UTF-8?q?Spec:=20positive-instruction=20redesign?= =?UTF-8?q?=20=E2=80=94=20audit=20results,=20micro-test=20method,=20writin?= =?UTF-8?q?g-plans=20variants?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...10-positive-instruction-redesign-design.md | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-10-positive-instruction-redesign-design.md diff --git a/docs/superpowers/specs/2026-06-10-positive-instruction-redesign-design.md b/docs/superpowers/specs/2026-06-10-positive-instruction-redesign-design.md new file mode 100644 index 0000000000..a19c938cae --- /dev/null +++ b/docs/superpowers/specs/2026-06-10-positive-instruction-redesign-design.md @@ -0,0 +1,164 @@ +# Positive-Instruction Redesign of Skill Guidance — Design Spec + +**Status:** Proposed (follow-up to the 2026-06-09 SDD review-dispatch work; separate PR per the one-problem-per-PR rule) +**Driver:** Measured evidence (2026-06-10) that some negative instructions in skill prose backfire, while others work — and that the difference is predictable. + +## The measured finding this spec generalizes + +Micro-tests on 2026-06-10 (opus, 5 reps per phrasing, programmatic scoring; +harness described below) measured how guidance phrasing changes what a +controller composes: + +| Case | Phrasing | Result | +|---|---|---| +| Dispatch composition ("don't restate the brief") | prohibition | **4.4** spec values re-typed — *worse than no guidance* (3.6) | +| Dispatch composition | positive recipe ("your dispatch should contain: (1)…(5)") | **3.0, zero variance** — adopted | +| Dispatch composition | recipe + nuance clause ("quote only the fragment…") | 3.8, noisy — nuance dilutes recipes | +| Test-rerun directive ("do not ask reviewer to re-run tests") | prohibition | **0/5 violations** — works fine (control: 3/5) | +| Test-rerun directive | positive recipe | 0/5 — equal, but longer | + +**The doctrine** (use this to classify any negative instruction): + +1. **Tripwires work.** Phrase-level self-checks on concrete tokens ("if the + prompt you are writing contains 'do not flag' … stop") fire reliably. +2. **Recognition tables work.** Red-Flags/rationalization tables read at + decision time, not composition time. +3. **Discrete-directive prohibitions work.** "Do not ask X to do Y" holds + when the model has no competing incentive to do Y. +4. **Composition prohibitions backfire** when the model has its own agenda + for the output (e.g., restating specs feels like helpful curation). + Only a positive composition recipe moves these — and adding nuance + clauses to a winning recipe makes it worse, not better. +5. **Ties go to the shorter phrasing.** Codex re-reads SKILL.md ~500× per + long session (measured 2026-06-10); prose length is a real cost. + +## Audit results (2026-06-10, all ~30 skills + prompt templates) + +Counts: 3 tripwires (keep), 14 recognition tables (keep), ~20 policy gates +(keep — "never push without permission" is policy, not composition +shaping), 5 composition-prohibitions: + +| # | Location | Disposition | +|---|---|---| +| 1 | `subagent-driven-development/task-reviewer-prompt.md` — "Cite, don't narrate" | **Queued in PR #1717 batch**: lead with the positive half ("Your report should point at evidence: file:line for every finding…"), drop the prohibition half (dead weight — the positive half already exists and carries the load) | +| 2 | `subagent-driven-development/SKILL.md` — "Do not add open-ended directives" | **Keep as-is**: micro-test could not elicit the failure in 15 samples; no evidence either way; shorter wins | +| 3 | `subagent-driven-development/SKILL.md` — "Do not ask a reviewer to re-run tests" | **Keep as-is**: measured 0/5 violations; the prohibition also usefully propagates itself into dispatches | +| 4 | `subagent-driven-development/SKILL.md` — "do not re-review on top of it" | **Queued in PR #1717 batch**: replace with the three-element checklist ("Before re-dispatching the reviewer, confirm the fix report contains: the covering tests, the command run, and the output") | +| 5 | `writing-plans/SKILL.md` — the "No Placeholders" banned-patterns list | **This spec's main subject** — see below | + +Borderline, deferred with #5: `task-reviewer-prompt.md` "Don't flag +pre-existing file sizes — focus on what this change contributed" (positive +half present and load-bearing; low impact; test alongside #5 if convenient). + +## The writing-plans change (deferred item #5) + +### Current state + +`skills/writing-plans/SKILL.md`, "No Placeholders": one positive sentence +("Every step must contain the actual content an engineer needs") followed +by a six-bullet banned-patterns list ("never write them: 'TBD', 'TODO', +'Add appropriate error handling', 'Write tests for the above', 'Similar to +Task N', …"). + +### Why it matters and why it is genuinely uncertain + +- Plans are the **largest generated artifact** in the workflow, and the + model has a real competing incentive to emit placeholders (they are the + path of least effort under length pressure) — the incentive structure of + the case where prohibition measurably backfired. +- But the banned items are **discrete, recognizable tokens** — the shape + of the case where prohibition measurably held. +- **The list is load-bearing elsewhere:** the skill's Self-Review section + references it ("Placeholder scan: search your plan for red flags — any + of the patterns from the 'No Placeholders' section above"). The tokens + double as the review-time scan inventory, and review-time recognition is + the category that works. A naive swap to a positive checklist breaks + that reference and discards good tripwire tokens. + +### Variants to test + +- **V0 (current):** positive sentence + banned list at composition time; + Self-Review references the list. +- **V1 (auditor's checklist):** composition-time positive recipe only — + "Before finalizing a step, confirm it has: the literal code to write, a + runnable command with expected output, types and method names defined + within this plan, error handling shown explicitly. A step is complete + when an engineer could implement it without asking any follow-up + questions." Self-Review keeps a generic placeholder scan. +- **V2 (restructure by mechanism — predicted winner):** composition time + gets only V1's positive recipe; the named patterns move wholesale into + the Self-Review placeholder-scan step, reframed as recognition ("when + you scan, look for: 'TBD', 'TODO', 'Similar to Task N', …"). Same + tokens, relocated from the category that primes to the category that + detects. +- **V3 (control):** positive sentence only, no list anywhere. + +### Micro-test design + +- **Task:** opus writes a 2-3 task implementation plan from a deliberately + under-specified spec (under-specification is what tempts placeholders). + Use a fixture spec with: one well-specified task, one task whose error + handling the spec hand-waves, one task similar to the first (tempting + "Similar to Task 1"). +- **Sampling:** 5+ reps per variant, default temperature, model + `claude-opus-4-8` (the model that writes plans in practice). +- **Programmatic scoring** (lower is better unless noted): + - banned-token count: `TBD|TODO|implement later|fill in details|appropriate error handling|handle edge cases|Similar to Task|Write tests for the above` + - steps lacking a fenced code block where the step changes code + - references to types/functions not defined anywhere in the plan output + - (higher is better) runnable commands with expected output per task +- **Two-stage scoring for V2:** also test the Self-Review half — feed each + generated plan back with the variant's Self-Review section and measure + whether the scan actually catches seeded placeholders (insert 2 known + placeholders into a fixture plan; detection rate is the metric). +- **Acceptance:** adopt a variant only if it beats V0 on banned-token count + without losing code-block coverage or self-review detection rate. + Expected cost: ~$6-10 total. + +### PR scoping + +Separate PR (writing-plans is a different skill; its "No Placeholders" +list is tuned content where the contributor guidelines demand eval +evidence). The PR must include: the micro-test harness + results table, +before/after text, and the V2 relocation rationale. + +## The micro-test harness (method, so it isn't lost) + +`/tmp/sdd-exp/micro/run-micro.py` and `/tmp/sdd-exp/micro2/run-micro2.py` +(2026-06-10; to be committed to superpowers-evals as +`docs/superpowers/skills/micro-testing-prompt-guidance.md` + scripts): + +- One API call per sample: system prompt = the skill-guidance variant in + realistic surrounding context; user = a realistic mid-workflow scenario; + output = the composed artifact (dispatch prompt, plan, report). +- Programmatic scoring with greps for unambiguous markers; **manually + inspect every match before trusting a verdict** — one of tonight's + "violations" was the controller correctly quoting the prohibition, and + automated negation detection mislabeled another. +- ~$0.15-0.30/sample, seconds per iteration vs $12/50-min full eval runs. + Iterate phrasings here; confirm winners in full runs only when the + change is structural. +- Always include a no-guidance control — tonight it revealed both a + backfire (restating: prohibition worse than nothing) and a working + prohibition (test-reruns: 3/5 control failures vs 0/5 with either + phrasing). + +## Also explicitly not-dropped (tested-and-declined, with data) + +Recorded so nobody re-proposes them without new evidence — full numbers in +the 2026-06-09 SDD design spec's Cost-iterations section: + +- **Controller turn batching / parallel tool calls in one message:** the + controller emits exactly one tool call per message (0 multi-tool + messages across every measured run, with and without guidance). 46% of + controller turns are thinking/narration with no tool call — a + prompt-immune floor. +- **Pipelined reviews via parallel calls:** dead for the same reason. +- **Pipelined reviews via `run_in_background`:** mechanism adopted when + offered (7/28 dispatches) but benefit below the run-to-run noise floor + on 45-min scenarios (reviews are only ~30-60s each); adds dual + result-stream coordination. Worth revisiting only for plans whose + reviews are individually long. +- **Nuance clauses appended to winning recipes:** measurably degrade them + (C2: 3.8 noisy vs C: 3.0 consistent). Iterate by re-deriving the recipe, + not by appending caveats. From b81f35bb1e4f7f6e545fce4b18720f0faf7f8f83 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 13:08:06 -0700 Subject: [PATCH 30/43] Land eval-tuned combo: file handoffs, progress ledger, final-review package, REQUIRED model lines, reviewer risk budget Validated 2026-06-10 (all gates pass): go-fractals 54.1-54.7 min / $12.81-14.31 (baseline 64.9 / $16.07); svelte-todo 55.0 min / 19.3M / $14.99 (baseline 79.7 / 27.3M / $20.98); planted-defect pass $2.77. Dispatch-model discipline 3/3 runs after moving model: into the templates as a REQUIRED line. Full experiment log: evals docs/experiments/2026-06-10-sdd-cost-experiments.md --- skills/subagent-driven-development/SKILL.md | 96 +++++++++++++++---- .../implementer-prompt.md | 21 +++- .../scripts/task-brief | 42 ++++++++ .../task-reviewer-prompt.md | 37 ++++--- 4 files changed, 161 insertions(+), 35 deletions(-) create mode 100755 skills/subagent-driven-development/scripts/task-brief diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index 4976d00271..64b9e7232b 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -54,15 +54,15 @@ digraph process { "Write diff file, 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]; + "Mark task complete in todo list and progress ledger" [shape=box]; } - "Read plan, extract all tasks with full text, note context, create todos" [shape=box]; + "Read plan, note context and global constraints, create todos" [shape=box]; "More tasks remain?" [shape=diamond]; "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)"; + "Read plan, note context and global constraints, create todos" -> "Dispatch implementer subagent (./implementer-prompt.md)"; "Dispatch implementer subagent (./implementer-prompt.md)" -> "Implementer subagent asks questions?"; "Implementer subagent asks questions?" -> "Answer questions, provide context" [label="yes"]; "Answer questions, provide context" -> "Dispatch implementer subagent (./implementer-prompt.md)"; @@ -71,8 +71,8 @@ digraph process { "Write diff file, 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" -> "Write diff file, 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?"; + "Task reviewer reports spec ✅ and quality approved?" -> "Mark task complete in todo list and progress ledger" [label="yes"]; + "Mark task complete in todo list and progress ledger" -> "More tasks remain?"; "More tasks remain?" -> "Dispatch implementer subagent (./implementer-prompt.md)" [label="yes"]; "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"; @@ -161,14 +161,74 @@ final whole-branch review. When you fill a reviewer template: the commit list, stat summary, and full diff with context in one Read call. Use the BASE you recorded before dispatching the implementer — never `HEAD~1`, which silently truncates multi-commit tasks. +- A dispatch prompt describes one task, not the session's history. Do not + paste accumulated prior-task summaries ("state after Tasks 1-3") into + later dispatches — a real session's dispatch hit 42k chars of which 99% + was pasted history. A fresh subagent needs its task, the interfaces it + touches, and the global constraints. Nothing else. - 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 + findings in the progress ledger as you go, and point the final + whole-branch review at that list so it can triage which must be fixed before merge. A roll-up nobody reads is a silent discard. +- The final whole-branch review gets a package too: run + `scripts/review-package MERGE_BASE HEAD` (MERGE_BASE = the commit the + branch started from, e.g. `git merge-base main HEAD`) and include the + printed path in the final review dispatch, so the final reviewer reads + one file instead of re-deriving the branch diff with git commands. - Every fix dispatch carries the implementer contract: the fix subagent - re-runs the tests covering its change and reports the results. A fix - report without test evidence is incomplete — do not re-review on top of - it. + re-runs the tests covering its change and reports the results. Name the + covering test files in the dispatch — a one-line fix does not need the + whole suite. A fix report without test evidence is incomplete — do not + re-review on top of it. +- If the final whole-branch review returns findings, dispatch ONE fix + subagent with the complete findings list — not one fixer per finding. + Per-finding fixers each rebuild context and re-run suites; a real + session's final-review fix wave cost more than all its tasks combined. + +## File Handoffs + +Everything you paste into a dispatch prompt — and everything a subagent +prints back — stays resident in your context for the rest of the session +and is re-read on every later turn. Hand artifacts over as files: + +- **Task brief:** before dispatching an implementer, run this skill's + `scripts/task-brief PLAN_FILE N` — it extracts the task's full text to a + uniquely named file and prints the path. Compose the dispatch so the + brief stays the single source of requirements. Your dispatch should + contain: (1) one line on where this task fits in the project; (2) the + brief path, introduced as "read this first — it is your requirements, + with the exact values to use verbatim"; (3) interfaces and decisions + from earlier tasks that the brief cannot know; (4) your resolution of + any ambiguity you noticed in the brief; (5) the report-file path and + report contract. Exact values (numbers, magic strings, signatures, test + cases) appear only in the brief. +- **Report file:** name the implementer's report file after the brief + (brief `…/task-N-brief.md` → report `…/task-N-report.md`) and put it in + the dispatch prompt. The implementer writes the full report there and + returns only status, commits, a one-line test summary, and concerns. +- **Reviewer inputs:** the task reviewer gets three paths — the same brief + file, the report file, and the review package — plus the global + constraints that bind the task. +- Fix dispatches append their fix report (with test results) to the same + report file and return a short summary; re-reviews read the updated file. + +## Durable Progress + +Conversation memory does not survive compaction. In real sessions, +controllers that lost their place have re-dispatched entire completed task +sequences — the single most expensive failure observed. Track progress in +a ledger file, not only in todos. + +- At skill start, check for a ledger: + `cat "$(git rev-parse --git-path sdd)/progress.md"`. Tasks listed there + as complete are DONE — do not re-dispatch them; resume at the first task + not marked complete. +- When a task's review comes back clean, append one line to the ledger in + the same message as your other bookkeeping: + `Task N: complete (commits .., review clean)`. +- The ledger is your recovery map: the commits it names exist in git even + when your context no longer remembers creating them. After compaction, + trust the ledger and `git log` over your own recollection. ## Prompt Templates @@ -182,13 +242,11 @@ final whole-branch review. When you fill a reviewer template: You: I'm using Subagent-Driven Development to execute this plan. [Read plan file once: docs/superpowers/plans/feature-plan.md] -[Extract all 5 tasks with full text and context] [Create todos for all tasks] Task 1: Hook installation script -[Get Task 1 text and context (already extracted)] -[Dispatch implementation subagent with full task text + context] +[Run task-brief for Task 1; dispatch implementer with brief + report paths + context] Implementer: "Before I begin - should the hook be installed at user or system level?" @@ -209,8 +267,7 @@ Task reviewer: Spec ✅ - all requirements met, nothing extra. Task 2: Recovery modes -[Get Task 2 text and context (already extracted)] -[Dispatch implementation subagent with full task text + context] +[Run task-brief for Task 2; dispatch implementer with brief + report paths + context] Implementer: [No questions, proceeds] Implementer: @@ -256,8 +313,8 @@ Done! - Review checkpoints automatic **Efficiency gains:** -- No file reading overhead (controller provides full text) -- Controller curates exactly what context is needed +- Controller curates exactly what context is needed; bulk artifacts move + as files, not pasted text - Subagent gets complete information upfront - Questions surfaced before work begins (not after) @@ -281,7 +338,8 @@ Done! - 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) +- Make a subagent read the whole plan file (hand it its task brief — + `scripts/task-brief` — 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 (reviewer found spec issues = not done) @@ -294,6 +352,8 @@ Done! (`scripts/review-package BASE HEAD`) and name the printed path in the prompt - Move to next task while the review has open Critical/Important issues +- Re-dispatch a task the progress ledger already marks complete — check + the ledger (and `git log`) after any compaction or resume **If subagent asks questions:** - Answer clearly and completely diff --git a/skills/subagent-driven-development/implementer-prompt.md b/skills/subagent-driven-development/implementer-prompt.md index d5f4600da5..218fcfeb57 100644 --- a/skills/subagent-driven-development/implementer-prompt.md +++ b/skills/subagent-driven-development/implementer-prompt.md @@ -5,12 +5,15 @@ Use this template when dispatching an implementer subagent. ``` Subagent (general-purpose): description: "Implement Task N: [task name]" + model: [MODEL — REQUIRED: choose per SKILL.md Model Selection; an omitted + model silently inherits the session's most expensive one] prompt: | You are implementing Task N: [task name] ## Task Description - [FULL TEXT of task from plan - paste it here, don't make subagent read file] + Read your task brief first: [BRIEF_FILE] + It contains the full task text from the plan. ## Context @@ -104,13 +107,12 @@ Subagent (general-purpose): ## 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 + the amended code and append the results to your report file. Reviewers will not re-run tests for you — your report is the test evidence. ## Report Format - When done, report: - - **Status:** DONE | DONE_WITH_CONCERNS | BLOCKED | NEEDS_CONTEXT + Write your full report to [REPORT_FILE]: - What you implemented (or what you attempted, if blocked) - What you tested and test results - **TDD Evidence** (if TDD was required for this task): @@ -120,6 +122,17 @@ Subagent (general-purpose): - Self-review findings (if any) - Any issues or concerns + Then report back with ONLY (under 15 lines — the detail lives in the + report file): + - **Status:** DONE | DONE_WITH_CONCERNS | BLOCKED | NEEDS_CONTEXT + - Commits created (short SHA + subject) + - One-line test summary (e.g. "14/14 passing, output pristine") + - Your concerns, if any + - The report file path + + If BLOCKED or NEEDS_CONTEXT, put the specifics in the final message + itself — the controller acts on it directly. + Use DONE_WITH_CONCERNS if you completed the work but have doubts about correctness. Use BLOCKED if you cannot complete the task. Use NEEDS_CONTEXT if you need information that wasn't provided. Never silently produce work you're unsure about. diff --git a/skills/subagent-driven-development/scripts/task-brief b/skills/subagent-driven-development/scripts/task-brief new file mode 100755 index 0000000000..b046a2bb1c --- /dev/null +++ b/skills/subagent-driven-development/scripts/task-brief @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +# Extract one task's full text from an implementation plan into a file the +# implementer reads in one call, so the task text never has to be pasted +# through the controller's context. +# +# Usage: task-brief PLAN_FILE TASK_NUMBER [OUTFILE] +# Default OUTFILE: /sdd/task--brief.md — unique per repo +# instance, so concurrent sessions cannot collide. +set -euo pipefail + +if [ $# -lt 2 ] || [ $# -gt 3 ]; then + echo "usage: task-brief PLAN_FILE TASK_NUMBER [OUTFILE]" >&2 + exit 2 +fi + +plan=$1 +n=$2 +[ -f "$plan" ] || { echo "no such plan file: $plan" >&2; exit 2; } + +if [ $# -eq 3 ]; then + out=$3 +else + dir=$(git rev-parse --git-path sdd) + mkdir -p "$dir" + dir=$(cd "$dir" && pwd) + out="$dir/task-${n}-brief.md" +fi + +awk -v n="$n" ' + /^```/ { infence = !infence } + !infence && /^#+[ \t]+Task[ \t]+[0-9]+/ { + intask = ($0 ~ ("^#+[ \t]+Task[ \t]+" n "([^0-9]|$)")) + } + intask { print } +' "$plan" > "$out" + +if [ ! -s "$out" ]; then + echo "task ${n} not found in ${plan} (no heading matching 'Task ${n}')" >&2 + exit 3 +fi + +echo "wrote ${out}: $(wc -l < "$out" | tr -d ' ') lines" diff --git a/skills/subagent-driven-development/task-reviewer-prompt.md b/skills/subagent-driven-development/task-reviewer-prompt.md index 465a620534..19f56b9a47 100644 --- a/skills/subagent-driven-development/task-reviewer-prompt.md +++ b/skills/subagent-driven-development/task-reviewer-prompt.md @@ -10,6 +10,8 @@ more, nothing less) and is well-built (clean, tested, maintainable) ``` Subagent (general-purpose): description: "Review Task N (spec + quality)" + model: [MODEL — REQUIRED: choose per SKILL.md Model Selection; an omitted + model silently inherits the session's most expensive one] 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, @@ -18,11 +20,14 @@ Subagent (general-purpose): ## What Was Requested - [TASK_REQUIREMENTS] + Read the task brief: [BRIEF_FILE] + + Global constraints from the spec/design that bind this task: + [GLOBAL_CONSTRAINTS] ## What the Implementer Claims They Built - [DESCRIPTION] + Read the implementer's report: [REPORT_FILE] ## Diff Under Review @@ -32,14 +37,17 @@ Subagent (general-purpose): Read the diff file once — it contains the commit list, a stat summary, and the full diff with surrounding context, and it 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: + change. The diff's context lines ARE the changed files: do not Read a + changed file separately unless a hunk you must judge is cut off + mid-function — and say so in your report. Do not re-run git commands. + 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. + Do not crawl the broader codebase. Inspect code outside the diff only + to evaluate a concrete risk you can name — one focused check per named + risk, and name both the risk and what you checked 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. @@ -147,10 +155,13 @@ Subagent (general-purpose): ``` **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 +- `[MODEL]` — REQUIRED: reviewer model per SKILL.md Model Selection +- `[BRIEF_FILE]` — REQUIRED: the task brief file (`scripts/task-brief PLAN N` + prints the path; same file the implementer worked from) +- `[GLOBAL_CONSTRAINTS]` — the spec/design's global constraints that bind + this task (version floors, naming and copy rules, platform requirements) +- `[REPORT_FILE]` — REQUIRED: the file the implementer wrote its detailed + report to - `[BASE_SHA]` — commit before this task - `[HEAD_SHA]` — current commit - `[DIFF_FILE]` — REQUIRED: the path the controller wrote the review From fe90d6c46946c46b47adc677eefe3aa9b5804683 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 13:08:19 -0700 Subject: [PATCH 31/43] Adopt audited positive phrasings: evidence rule leads positive; fix-report completeness as checklist --- skills/subagent-driven-development/SKILL.md | 5 +++-- skills/subagent-driven-development/task-reviewer-prompt.md | 7 ++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index 64b9e7232b..55c8e3d1b4 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -178,8 +178,9 @@ final whole-branch review. When you fill a reviewer template: - Every fix dispatch carries the implementer contract: the fix subagent re-runs the tests covering its change and reports the results. Name the covering test files in the dispatch — a one-line fix does not need the - whole suite. A fix report without test evidence is incomplete — do not - re-review on top of it. + whole suite. Before re-dispatching the reviewer, confirm the fix report + contains the covering tests, the command run, and the output; dispatch + the re-review once all three are present. - If the final whole-branch review returns findings, dispatch ONE fix subagent with the complete findings list — not one fixer per finding. Per-finding fixers each rebuild context and re-run suites; a real diff --git a/skills/subagent-driven-development/task-reviewer-prompt.md b/skills/subagent-driven-development/task-reviewer-prompt.md index 19f56b9a47..3bc019a025 100644 --- a/skills/subagent-driven-development/task-reviewer-prompt.md +++ b/skills/subagent-driven-development/task-reviewer-prompt.md @@ -110,9 +110,10 @@ Subagent (general-purpose): 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. + Your report should point at evidence: file:line references for every + finding and for any check you would otherwise answer with a bare + "yes." A tight report that cites lines gives the controller everything + it needs. ## Calibration From 43a6ee23f78dcc50cb0aaf960e28dce6875c10a8 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 13:08:40 -0700 Subject: [PATCH 32/43] Spec: record iterations 4-5 (variance honesty, structural fixes, final validated ranges) --- ...-sdd-task-scoped-review-dispatch-design.md | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md b/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md index bd60bc6656..23cdd4a7cc 100644 --- a/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md +++ b/docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md @@ -74,6 +74,42 @@ evidence-rule narration. calls, must-fix bar for a test whose name promises verification it never performs) passes with the defect caught and fixed. +### Iterations 4-5 (2026-06-10): variance honesty, structural fixes, positive recipes + +A same-config re-run exposed run-to-run variance (44.4→57.1 min on +identical prompts; reviewer escape-hatch appetite swung 1.0→6.3 tool +calls/review), so all subsequent claims use ranges. Five parallel +experiment variants on go-fractals plus transcript mining of real local +sessions (full log with negative results: +`evals/docs/experiments/2026-06-10-sdd-cost-experiments.md`) produced the +final config: + +- **Adopted:** final-review package (final reviewer 33→6 turns at + controller-model prices); REQUIRED `model:` line in both templates + (prose guidance decayed mid-session once, inheriting opus for 17 + dispatches, +$5); task-brief + report files (`scripts/task-brief`; + fidelity anchor, modest context savings); progress ledger in + `/sdd/progress.md` (real sessions re-dispatched entire + completed task sequences after compaction — 269 dispatches for ~22 + tasks); omnibus final fixer (a real session's per-finding fix wave cost + more than all its tasks); scoped fix tests; unique SHA-range collateral + names (worktree/submodule-safe); dispatch-composition recipe and + reviewer named-risk budget (micro-tested: positive recipe 3.0 + transcribed values vs prohibition 4.4 vs control 3.6 — prohibitions can + backfire; see `2026-06-10-positive-instruction-redesign-design.md`). +- **Tested and declined:** controller turn batching and parallel-call + pipelining (controller emits exactly one tool call per message — 0 + multi-tool messages in every run; 46% of its turns are + thinking/narration, a prompt-immune floor); background-dispatch + pipelining (mechanism adopted 7/28 but benefit below the ±6 min noise + floor on these scenarios). +- **Final validated config (b81f35b family), all gates pass:** go-fractals + 54.1-54.7 min / 14.4-16.6M / $12.81-14.31 (baseline 64.9 / 21.2M / + $16.07); svelte-todo 55.0 min / 19.3M / $14.99 (baseline 79.7 / 27.3M / + $20.98); planted-defect pass / $2.77. Across all 8 same-design fractals + runs: 44.4-57.1 min / 13.4-20.0M / $11.67-14.84 — the worst draw beats + baseline on every axis; typical mid-band savings ~20-25%. + ## Design ### Shared principle: don't re-run tests on code that hasn't changed From 60fa4f6fc4d57a3862a2bdb7dc889fa4d0895938 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 14:31:50 -0700 Subject: [PATCH 33/43] Record writing-plans micro-test result: resolved, no change needed --- ...6-06-10-positive-instruction-redesign-design.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/superpowers/specs/2026-06-10-positive-instruction-redesign-design.md b/docs/superpowers/specs/2026-06-10-positive-instruction-redesign-design.md index a19c938cae..1ec77cf902 100644 --- a/docs/superpowers/specs/2026-06-10-positive-instruction-redesign-design.md +++ b/docs/superpowers/specs/2026-06-10-positive-instruction-redesign-design.md @@ -143,6 +143,20 @@ before/after text, and the V2 relocation rationale. prohibition (test-reruns: 3/5 control failures vs 0/5 with either phrasing). +## Result: writing-plans micro-test (run 2026-06-10, after this spec was written) + +**Resolved — no change needed.** Stage 1 (3-task spec, no pressure): 0 +placeholders in all 20 plans across all four variants including the +no-guidance control. Stage 1b (10-task spec, five near-identical commands +tempting "Similar to Task N", explicit ~2,500-word economy target): 40/40 +clean — the single regex hit was a V2 self-review *attesting* "no +TBD/TODO ✓". Current-generation opus does not produce plan placeholders +even under deliberate pressure, with or without the banned-patterns list. +Disposition: leave the No Placeholders section exactly as it is (it costs +little and the counterfactual is unmeasurable); do NOT open the follow-up +PR. The V2 relocation design remains on file here should a future model +generation regress. + ## Also explicitly not-dropped (tested-and-declined, with data) Recorded so nobody re-proposes them without new evidence — full numbers in From 9a25a75bacc9b269e233c51f22c0a9e185cf8995 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 14:35:00 -0700 Subject: [PATCH 34/43] =?UTF-8?q?Spec:=20strict-cost=20SDD=20experiment=20?= =?UTF-8?q?ladder=20=E2=80=94=20judgment=20as=20co-invariant,=20plan-side?= =?UTF-8?q?=20crispness=20first?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2026-06-10-strict-cost-sdd-design.md | 186 ++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md diff --git a/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md b/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md new file mode 100644 index 0000000000..9f800f0a72 --- /dev/null +++ b/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md @@ -0,0 +1,186 @@ +# Strict-Cost SDD — Design Spec + +**Status:** Proposed experiment ladder (not implementation). Each rung ships +only with its gate evidence; abort any rung whose gates fail. +**Objective:** minimize dollars per plan-execution. Wall-clock is +unconstrained; token count matters only as a cost driver. +**Hard invariant:** quality. Concretely: `sdd-quality-reviewer-catches- +planted-defect` pass rate over **N=5 runs** (not 1 — single-run gates were +this campaign's weakest methodology), `sdd-rejects-extra-features` pass, +all end-to-end scenarios pass, blind A/B deliverable parity with the +current config. Any quality regression kills the rung, full stop. + +## Where the dollars are (final 2026-06-10 config, go-fractals, ~$13/run) + +| Component | $ | Driver | +|---|---|---| +| Controller (session model, opus) | ~6-7 | ~150 turns × resident context; prompt-immune turn floor (46% thinking/narration) | +| Implementers (sonnet, 10-13 dispatches) | ~5-6 | the actual work; ~25 turns each; ~13 pre-edit exploration calls each | +| Task reviewers (sonnet, 10) | ~1-1.5 | 3-9 turns each with package | +| Final review + fixes | ~1 | 6 turns with branch package | + +Review-loop count (2-4 per run) is the biggest run-to-run cost variance; +loops are mostly caused by plan ambiguity the implementer resolved wrongly. + +## Judgment guardrail (co-invariant with quality) + +**Cheapen mechanics, never judgment.** Every rung must enumerate which +decisions it moves to a cheaper model and show each is *mechanical* — +deterministic, scriptable, or cheaply verifiable after the fact. Judgment +stays at the highest tier or with the human. The judgment points in SDD, +explicitly: + +- **BLOCKED / NEEDS_CONTEXT handling** — diagnosing why a subagent is stuck + and choosing the remedy +- **⚠️ "cannot verify from diff" resolution** — the controller adjudicating + with cross-task context +- **Dispatch curation** — ambiguity resolution and task-boundary drawing + (measured load-bearing: the Task 5 gradient-direction note prevented a + wrong implementation) +- **Review verdicts and severity calibration** — what is Important vs Minor +- **Review-loop adjudication** — deciding a finding is a false positive +- **Escalate-to-human recognition** — knowing the plan itself is wrong + +A rung that would move any of these to a cheaper model must either (a) +restructure so the decision is made once by the expensive model at plan +time, (b) add an explicit escalation rule routing it back up at execution +time, or (c) die. "The cheap model usually gets it right" is not +acceptance evidence — judgment failures are rare-event, high-blast-radius, +and largely invisible to pass/fail gates, which is why every tier change +below carries a judgment audit (session-resume interrogation of each +judgment point in the gate runs, compared against the expensive-controller +baseline) in addition to the N=5 scenario gates. + +## Thesis guardrail + +SDD's thesis: **a fresh subagent per task with precisely curated context, +gated per task.** Rungs below must preserve it. Dispatch-time task batching +(one implementer dispatch handling several plan tasks) is **counter-thesis** +— it pollutes the fresh-context property and coarsens the gates — and is +deliberately NOT on the ladder. The thesis-compatible route to the same +dispatch economics is plan-time task right-sizing (L1): if the plan defines +fewer, better-sized tasks, SDD still runs one fresh subagent per task. + +## The ladder (in expected $/leverage order) + +### L1 — Plan-side crispness (writing-plans changes; est. −$1.5-3/run, plus variance reduction) + +The plan is upstream of every cost: task count sets dispatch count; plan +ambiguity sets review-loop count; plan completeness sets implementer +exploration. Current writing-plans optimizes for implementer success, not +execution economics. Changes to test: + +1. **Task right-sizing guidance.** Today's plans produce tasks as small as + "create .gitignore" — each costing a full dispatch + review cycle + (~$0.60-1.00 fixed overhead). Add: "A task is the smallest unit that + carries its own test cycle and is worth a fresh reviewer's gate. Merge + setup/config steps into the task that needs them; split only at + boundaries where a reviewer could meaningfully reject." Fractals' plan + would drop from 10 tasks to ~7. Validate: dispatch count falls, gates + hold, review granularity still catches the planted defect. +2. **Structured `## Global Constraints` section** in the plan header + (version floors, naming/copy rules, platform requirements). Today these + live in design.md prose and reach reviewers only if the controller + remembers to paste them (a `go 1.26.1` floor violation shipped because + none did). A fixed heading makes them mechanically extractable — + `task-brief` can append them to every brief automatically (small script + change), removing a controller responsibility entirely. +3. **Per-task `Interfaces:` line** (consumes/produces, exact signatures). + The controller currently re-derives cross-task interfaces per dispatch + (its main legitimate "restating"), and implementers spend ~13 tool calls + re-discovering context. The planner already knows the interfaces; one + line per task moves the work to where it is done once. +4. **Per-task model-tier recommendation** from the planner ("mechanical / + standard / judgment"). The planner has the best information for the + Model Selection decision the controller currently re-makes per dispatch; + the controller keeps override authority. + +Validation: micro-test the planner output shape (recipe-style, per the +instruction-design doctrine), then full runs. Note the 2026-06-10 result: +plan *placeholders* cannot be elicited from current opus — these changes +target economics and ambiguity, not placeholder hygiene. + +### L2 — Controller tier (est. −$4-5/run; the biggest single lever, gated hardest) + +The controller is half the dollars solely because it inherits the session +model. Its turn floor is prompt-immune, so the lever is the rate per turn — +but the controller is also where most judgment points live, so this rung is +designed judgment-first: + +1. **Primary form — judgment moved up front, mechanics cheapened:** the + expensive model does the judgment-dense work at plan time (L1's + Interfaces lines, ambiguity resolutions, per-task constraints — i.e. + the dispatch curation is pre-written into the plan). The mid-tier + execution session then runs a loop that is genuinely mechanical: + extract brief, dispatch, run script, route verdicts. Explicit + escalation rules in the skill: on BLOCKED, on any ⚠️ item, on a + suspected false positive, or on anything the plan does not already + answer, the cheap controller STOPS and escalates (to the human, or to + a fresh expensive-model consultation dispatch) — it never resolves + judgment alone. +2. **Gates beyond the standard N=5:** a judgment audit — every + BLOCKED/⚠️/adjudication event in the gate runs interrogated via + session-resume and scored against how the opus-controller baseline + handled the same class of event; any silently-absorbed judgment call + (cheap controller resolving what it should have escalated) fails the + rung regardless of scenario verdicts. +3. **User authority preserved:** the skill recommends, never enforces, the + execution-session tier. + +Caveat from this campaign: cheap-model turn inflation was measured on +multi-step *work*, not dispatch loops; whether a mid-tier controller holds +~150 turns is part of what the experiment determines. + +### L3 — Reviewer tier (est. −$0.7-1/run; most likely rung to die on the judgment guardrail) + +The package reviewer is near-single-step mechanically (3 turns / 1 Read +when calm), which invalidates the original turn-inflation rationale for the +mid-tier floor — but reviewing is judgment through and through: severity +calibration, spec verdicts, knowing what not to flag. Mechanical cheapness +does not make the decisions mechanical. Test haiku-with-package only with +the full judgment battery: planted-defect ×5, a severity-calibration check +(seeded Minor-vs-Important pairs; miscalibration fails the rung), and the +escape-hatch variance re-measured at that tier. Prior expectation: this +rung dies, and that is a fine outcome — it converts "we suspect cheap +reviewers are bad" into recorded evidence. + +### L4 — Resident-context diet (est. −$0.5-1/run) + +- `task-brief --list` mode: controller reads task headings + Global + Constraints, never the full plan (the plan body is already delivered via + briefs). +- Reports trim 15 → 8 lines. +- SKILL.md minification pass (every section added this week re-justified + at composition-recipe density; Codex pays ~10k chars × ~500 re-reads per + long session). + +### L5 — Re-litigations (explicitly flagged, maintainer-vetoed or counter-thesis) + +Recorded for completeness; each requires Jesse's explicit reversal before +any experiment: +- **Scoped re-reviews** (verify fix + regression scan instead of full + re-review): vetoed 2026-06-09; worth ~$0.50/run at most. +- **Dispatch-time task batching**: counter-thesis (see guardrail). L1.1 + is the sanctioned form. + +## Budget and sequencing + +L1 and L2.1 are independent — run both first (~$80: micro-tests + 2×5-run +gates + A/B). L3 after L2 settles the controller (reviewer behavior depends +on dispatch quality; ~$25 — planted-defect runs are $2-3 each). L4 last +(cheap, but re-gate once after the stack; ~$30). Total ≲ $150 for the full +ladder with honest N=5 gates. Expected end state if every rung survives its gates: **$5-7/run on +fractals (from $12-15)**; if the judgment-sensitive rungs (L2 beyond its +primary form, L3) die as expected, **$8-10/run** — the honest target, since +the guardrail prices judgment above dollars by construction. + +## Relationship to existing work + +Builds on the 2026-06-09 task-scoped review dispatch design (PR #1717) and +the 2026-06-10 experiment campaign (evals +`docs/experiments/2026-06-10-sdd-cost-experiments.md` — consult the +negative-results section before adding rungs; turn-discipline and +parallel-call mechanisms are dead). Instruction wording for any new prose +follows the positive-instruction doctrine spec and gets micro-tested before +full runs. L1 is a writing-plans change → its own PR with eval evidence; +L2-L4 are SDD changes → separate PR(s). From 27788fdef9bb481cf97dffa1fcb63b6e496fa673 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 16:59:43 -0700 Subject: [PATCH 35/43] Strict-cost spec: record batch A-E rung verdicts (L1 validated, L2 recon positive, L3 dead) --- .../2026-06-10-strict-cost-sdd-design.md | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md b/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md index 9f800f0a72..44f6595f0a 100644 --- a/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md +++ b/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md @@ -65,6 +65,14 @@ fewer, better-sized tasks, SDD still runs one fresh subagent per task. ### L1 — Plan-side crispness (writing-plans changes; est. −$1.5-3/run, plus variance reduction) +**Status 2026-06-11: validated in effect.** A hand-crisped fractals plan +(10 → 7 tasks, `## Global Constraints` header, per-task `Interfaces:` +lines — scenario `sdd-go-fractals-crisp`) ran 3/3 green at $9.51-12.65 +(mean $11.60 vs combo band $11.67-14.84), 20-24 dispatches vs 28, fix +waves flat. What remains is elicitation: getting writing-plans guidance +to *produce* such plans (micro-test per the doctrine, then the follow-up +PR). See the experiments log, Batch A-E. + The plan is upstream of every cost: task count sets dispatch count; plan ambiguity sets review-loop count; plan completeness sets implementer exploration. Current writing-plans optimizes for implementer success, not @@ -102,6 +110,17 @@ target economics and ambiguity, not placeholder hygiene. ### L2 — Controller tier (est. −$4-5/run; the biggest single lever, gated hardest) +**Status 2026-06-11: recon positive, gates still owed.** Sonnet-controller +run 1 (claude-sonnet coding-agent): all gates green at **$6.68** / 31 min +(combo band $11.67-14.84), 26/26 dispatches model-explicit, review loops +and omnibus-fixer rules followed, and the controller caught a fixer +side-effect (`go mod tidy` removed cobra) before re-review — real +adjudication, not silent absorption. But the run surfaced zero +BLOCKED/⚠️ events (the escalation points were never stressed) and the +final review ran on sonnet rather than the most capable tier. The N=5 +quality gates + full judgment audit below remain mandatory before any +skill change. + The controller is half the dollars solely because it inherits the session model. Its turn floor is prompt-immune, so the lever is the rate per turn — but the controller is also where most judgment points live, so this rung is @@ -133,6 +152,16 @@ multi-step *work*, not dispatch loops; whether a mid-tier controller holds ### L3 — Reviewer tier (est. −$0.7-1/run; most likely rung to die on the judgment guardrail) +**Status 2026-06-11: DEAD, as pre-registered.** Planted-defect ×5 with +forced-haiku task reviewers: 2 pass / 1 indeterminate / 2 fail (baseline +5/5); per-task haiku cleanly flagged 0 of 10 planted defects at correct +severity — 1 found-but-downgraded with the exact prohibited rationale, +9 missed or rationalized (DRY praised as YAGNI; assert-nothing test +called plan-compliant). Cheap reviewers fail by *advocating* for +defects; passing runs survived only on controller redundancy or the +final review. Recorded in the experiments log, Batch A-E. Do not +re-propose without a structurally different design. + The package reviewer is near-single-step mechanically (3 turns / 1 Read when calm), which invalidates the original turn-inflation rationale for the mid-tier floor — but reviewing is judgment through and through: severity From eba16f6b9115d355a2d8823026a28f6109f5b4de Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 17:11:26 -0700 Subject: [PATCH 36/43] Strict-cost spec: L2 recon n=2 (sonnet controller $6.68/$8.05, judgment clean, escalation points unstressed) --- .../2026-06-10-strict-cost-sdd-design.md | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md b/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md index 44f6595f0a..dd212160ae 100644 --- a/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md +++ b/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md @@ -110,16 +110,19 @@ target economics and ambiguity, not placeholder hygiene. ### L2 — Controller tier (est. −$4-5/run; the biggest single lever, gated hardest) -**Status 2026-06-11: recon positive, gates still owed.** Sonnet-controller -run 1 (claude-sonnet coding-agent): all gates green at **$6.68** / 31 min -(combo band $11.67-14.84), 26/26 dispatches model-explicit, review loops -and omnibus-fixer rules followed, and the controller caught a fixer -side-effect (`go mod tidy` removed cobra) before re-review — real -adjudication, not silent absorption. But the run surfaced zero -BLOCKED/⚠️ events (the escalation points were never stressed) and the -final review ran on sonnet rather than the most capable tier. The N=5 -quality gates + full judgment audit below remain mandatory before any -skill change. +**Status 2026-06-11: recon positive (n=2), gates still owed.** +Sonnet-controller runs (claude-sonnet coding-agent): all gates green at +**$6.68 and $8.05** / 31-41 min (combo band $11.67-14.84), tokens inside +the combo band — no cheap-controller turn inflation. 26/26 and 31/31 +dispatches model-explicit, with heavier (and sane) haiku tiering than +opus controllers showed; review loops, per-task Important→fix→re-review, +and omnibus-fixer rules followed in both runs; the run-1 controller +caught a fixer side-effect (`go mod tidy` removed cobra) before +re-review — real adjudication, not silent absorption. But neither run +surfaced a BLOCKED/⚠️ event (the escalation points were never stressed) +and final reviews ran on sonnet rather than the most capable tier. The +N=5 quality gates + full judgment audit below remain mandatory before +any skill change. The controller is half the dollars solely because it inherits the session model. Its turn floor is prompt-immune, so the lever is the rate per turn — From ec014e7a7f1cc0febf4aba96d70e9ee0bc7b0ef0 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 19:39:02 -0700 Subject: [PATCH 37/43] Bump evals submodule to merged superpowers-evals main (ac264b1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Carries the planted-defect + crisp scenarios, batch A-E experiment logs, claude-sonnet model-variant target, and method docs — rebased onto the obol migration and pushed to superpowers-evals main. --- evals | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evals b/evals index ff3ee83f94..ac264b104c 160000 --- a/evals +++ b/evals @@ -1 +1 @@ -Subproject commit ff3ee83f94860226443948a092f46d65589fb460 +Subproject commit ac264b104ced80c199844be13ff19cd47eef40db From de1d35e5e7da94dbb6ac5091ef23020975ed9b47 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 21:44:23 -0700 Subject: [PATCH 38/43] =?UTF-8?q?Strict-cost=20spec:=20L1=20final=20?= =?UTF-8?q?=E2=80=94=20cost=20win=20re-attributed=20to=20complete-code=20p?= =?UTF-8?q?lans;=20guidance=20owns=20fidelity/variance?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2026-06-10-strict-cost-sdd-design.md | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md b/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md index dd212160ae..5506cf8943 100644 --- a/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md +++ b/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md @@ -65,13 +65,21 @@ fewer, better-sized tasks, SDD still runs one fresh subagent per task. ### L1 — Plan-side crispness (writing-plans changes; est. −$1.5-3/run, plus variance reduction) -**Status 2026-06-11: validated in effect.** A hand-crisped fractals plan -(10 → 7 tasks, `## Global Constraints` header, per-task `Interfaces:` -lines — scenario `sdd-go-fractals-crisp`) ran 3/3 green at $9.51-12.65 -(mean $11.60 vs combo band $11.67-14.84), 20-24 dispatches vs 28, fix -waves flat. What remains is elicitation: getting writing-plans guidance -to *produce* such plans (micro-test per the doctrine, then the follow-up -PR). See the experiments log, Batch A-E. +**Status 2026-06-11 (final): elicitation tested end-to-end; claims +re-attributed.** Micro-tests: constraints header and Interfaces blocks +elicit deterministically (0→5/5, 0→100% of tasks, exact values); +right-sizing is modest and scale-dependent (9.4→8.4 tasks at svelte +scale, nothing to move at fractals scale). Full runs: an elicited plan +executed at $6.34/$8.49 — but the no-guidance control (opus plan, +complete code) hit $7.59/$7.73, inside that range. **The cost win +belongs to opus-written complete-code plans; the hand-written prose +fixture plans all prior numbers used are unrepresentative and ~2× +costlier to execute.** The guidance owns fidelity and variance instead: +deterministic constraints propagation (the one elicited-run fix was a +version-floor catch), exact cross-task interfaces, fix waves 1 vs 2-4 +(the control plan shipped a real Sierpinski bug both runs had to fix). +The writing-plans PR claims those grounds, not dollars. Draft at +/tmp/sdd-exp/writing-plans-l1 (branch writing-plans-crisp). The plan is upstream of every cost: task count sets dispatch count; plan ambiguity sets review-loop count; plan completeness sets implementer From 72cb21b82cf7125f858c4e9d2100bb8f8d88dc72 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Thu, 11 Jun 2026 10:31:48 -0700 Subject: [PATCH 39/43] Constraints block is the reviewer's attention lens: copy spec verbatim, never improvise process rules E30 replay: the planted-DRY catch is causally determined by the controller-composed constraints block (0/6 with process-shaped vs 5/6 with the spec's own wording). E31 micro: this recipe doubles the rate at which composed blocks carry the spec's cross-component relationship (6/6 vs 3/6). Affects dev and the redesign equally (E29: both 4/5). --- skills/subagent-driven-development/SKILL.md | 10 +++++++--- .../task-reviewer-prompt.md | 6 ++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index 55c8e3d1b4..eb4623ab7f 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -150,9 +150,13 @@ final whole-branch review. When you fill a reviewer template: 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. +- The global-constraints block you hand the reviewer is its attention + lens. Copy the binding requirements verbatim from the plan's Global + Constraints section or the spec: exact values, exact formats, and the + stated relationships between components ("same layout as X", "matches + Y"). The reviewer's template already carries the process rules (YAGNI, + test hygiene, review method) — the constraints block is for what THIS + project's spec demands. - Hand the reviewer its diff as a file: run this skill's `scripts/review-package BASE HEAD` and pass the reviewer the file path it prints (or, without bash: `git log --oneline`, `git diff --stat`, diff --git a/skills/subagent-driven-development/task-reviewer-prompt.md b/skills/subagent-driven-development/task-reviewer-prompt.md index 3bc019a025..d2177f0f54 100644 --- a/skills/subagent-driven-development/task-reviewer-prompt.md +++ b/skills/subagent-driven-development/task-reviewer-prompt.md @@ -159,8 +159,10 @@ Subagent (general-purpose): - `[MODEL]` — REQUIRED: reviewer model per SKILL.md Model Selection - `[BRIEF_FILE]` — REQUIRED: the task brief file (`scripts/task-brief PLAN N` prints the path; same file the implementer worked from) -- `[GLOBAL_CONSTRAINTS]` — the spec/design's global constraints that bind - this task (version floors, naming and copy rules, platform requirements) +- `[GLOBAL_CONSTRAINTS]` — the binding requirements copied verbatim from + the plan's Global Constraints section or the spec: exact values, formats, + and stated relationships between components (not process rules — those + are already in this template) - `[REPORT_FILE]` — REQUIRED: the file the implementer wrote its detailed report to - `[BASE_SHA]` — commit before this task From 710f031ad0685cd7fc5600af526deea060312b54 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 10 Jun 2026 20:44:48 -0700 Subject: [PATCH 40/43] writing-plans: task right-sizing, Global Constraints header, per-task Interfaces blocks Claims are fidelity and variance, not dollars (full attribution in the superpowers-evals experiment log, 2026-06-11 L1 entry): - Global Constraints header: 0/5 -> 5/5 adoption in micro-tests, exact values verbatim; makes constraints mechanically propagatable to briefs and reviewers (a version-floor violation class shipped because they weren't). The one fix wave in the elicited full runs was a version-floor catch this header enabled. - Per-task Interfaces blocks: 0 -> 100% of tasks, exact signatures, within-plan consistent; removes the controller's per-dispatch interface re-derivation. - Task right-sizing: 9.4 -> 8.4 mean tasks at svelte scale (kills standalone Types/README micro-tasks); no effect at small scale. - End-to-end (opus-written plan executed under SDD): guidance plan ran 1 fix wave vs control's 2-4 (control plan shipped a real Sierpinski bug); execution cost equal within noise. --- skills/writing-plans/SKILL.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/skills/writing-plans/SKILL.md b/skills/writing-plans/SKILL.md index 847412ece1..b1613eb057 100644 --- a/skills/writing-plans/SKILL.md +++ b/skills/writing-plans/SKILL.md @@ -33,6 +33,15 @@ Before defining tasks, map out which files will be created or modified and what This structure informs the task decomposition. Each task should produce self-contained changes that make sense independently. +## Task Right-Sizing + +A task is the smallest unit that carries its own test cycle and is worth a +fresh reviewer's gate. When drawing task boundaries: fold setup, +configuration, scaffolding, and documentation steps into the task whose +deliverable needs them; split only where a reviewer could meaningfully +reject one task while approving its neighbor. Each task ends with an +independently testable deliverable. + ## Bite-Sized Task Granularity **Each step is one action (2-5 minutes):** @@ -57,6 +66,13 @@ This structure informs the task decomposition. Each task should produce self-con **Tech Stack:** [Key technologies/libraries] +## Global Constraints + +[The spec's project-wide requirements — version floors, dependency limits, +naming and copy rules, platform requirements — one line each, with exact +values copied verbatim from the spec. Every task's requirements implicitly +include this section.] + --- ``` @@ -70,6 +86,12 @@ This structure informs the task decomposition. Each task should produce self-con - Modify: `exact/path/to/existing.py:123-145` - Test: `tests/exact/path/to/test.py` +**Interfaces:** +- Consumes: [what this task uses from earlier tasks — exact signatures] +- Produces: [what later tasks rely on — exact function names, parameter + and return types. A task's implementer sees only their own task; this + block is how they learn the names and types neighboring tasks use.] + - [ ] **Step 1: Write the failing test** ```python From ac11700642c102bb5de56c989bc23da81b680a6d Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Thu, 11 Jun 2026 11:37:41 -0700 Subject: [PATCH 41/43] Bump evals submodule: L1 elicitation + autoresearch scenarios and logs (649b1f8) --- evals | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evals b/evals index ac264b104c..649b1f825e 160000 --- a/evals +++ b/evals @@ -1 +1 @@ -Subproject commit ac264b104ced80c199844be13ff19cd47eef40db +Subproject commit 649b1f825ee3d16248e29c99277c1a55cb140fc8 From d1fcc9889adfd32d1d36424b09023188b14a61d0 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Thu, 11 Jun 2026 13:11:32 -0700 Subject: [PATCH 42/43] =?UTF-8?q?Strict-cost=20spec:=20L2=20final=20?= =?UTF-8?q?=E2=80=94=20died=20at=20gates;=20explicit=20escalation=20holds?= =?UTF-8?q?=20at=20sonnet,=20implicit=20adjudication=20does=20not?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2026-06-10-strict-cost-sdd-design.md | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md b/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md index 5506cf8943..ab51c43e4c 100644 --- a/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md +++ b/docs/superpowers/specs/2026-06-10-strict-cost-sdd-design.md @@ -118,7 +118,25 @@ target economics and ambiguity, not placeholder hygiene. ### L2 — Controller tier (est. −$4-5/run; the biggest single lever, gated hardest) -**Status 2026-06-11: recon positive (n=2), gates still owed.** +**Status 2026-06-11 (final): DIED AT THE GATES, as pre-registered — with +useful anatomy.** Recon was positive ($6.68/$8.05, n=2, mechanics clean). +The full battery split the judgment surface: the new +`sdd-escalates-broken-plan` scenario (explicit plan self-contradiction; +the human never volunteers it) passed **5/5 at sonnet** ($1.02-1.37/run; +opus baseline 2/2) — explicit conflicts get escalated. But the +planted-defect battery failed decisively: under a sonnet controller the +per-task quality gate collapsed into plan-compliance advocacy ("no +assertion, as required" listed under Strengths), the defect shipped in +4/5 runs (deterministic check), and only the tier-pinned opus final +reviewer ever caught it — while the same sonnet-tier reviewers under an +opus controller flagged it 5/5. Cheap controllers handle explicit +escalation; they absorb implicit authority-vs-quality adjudication. +A possible L2b (discrete rule: "a reviewer finding that conflicts with +the plan's text is the human's decision — escalate it") would route the +failing judgment through the escalation behavior that held; untested. +Original recon notes follow. + +**Recon (superseded):** Sonnet-controller runs (claude-sonnet coding-agent): all gates green at **$6.68 and $8.05** / 31-41 min (combo band $11.67-14.84), tokens inside the combo band — no cheap-controller turn inflation. 26/26 and 31/31 From 420c234a2c23232a2cc10487d5ba082db18b2b5c Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Thu, 11 Jun 2026 13:17:09 -0700 Subject: [PATCH 43/43] Bump evals submodule: E29-E34 quality investigation + L2 gate results (af05326) --- evals | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evals b/evals index 649b1f825e..af05326467 160000 --- a/evals +++ b/evals @@ -1 +1 @@ -Subproject commit 649b1f825ee3d16248e29c99277c1a55cb140fc8 +Subproject commit af0532646754b0a1d164800c609ddfc16e6f91dc