From 4ae1a3d6a6e1e2171ae9de0eb7daf3e192c2b792 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 24 Mar 2026 10:43:58 -0700 Subject: [PATCH] Revert "Replace subagent review loops with lightweight inline self-review" This reverts commit bf8f7572eb85d793b73a44913e8039f9a2e83c1e. --- skills/brainstorming/SKILL.md | 24 ++++++++++++------------ skills/writing-plans/SKILL.md | 31 ++++++++++++------------------- 2 files changed, 24 insertions(+), 31 deletions(-) diff --git a/skills/brainstorming/SKILL.md b/skills/brainstorming/SKILL.md index 06cd0a21..edbc2b57 100644 --- a/skills/brainstorming/SKILL.md +++ b/skills/brainstorming/SKILL.md @@ -27,7 +27,7 @@ You MUST create a task for each of these items and complete them in order: 4. **Propose 2-3 approaches** — with trade-offs and your recommendation 5. **Present design** — in sections scaled to their complexity, get user approval after each section 6. **Write design doc** — save to `docs/superpowers/specs/YYYY-MM-DD--design.md` and commit -7. **Spec self-review** — quick inline check for placeholders, contradictions, ambiguity, scope (see below) +7. **Spec review loop** — dispatch spec-document-reviewer subagent with precisely crafted review context (never your session history); fix issues and re-dispatch until approved (max 3 iterations, then surface to human) 8. **User reviews written spec** — ask user to review the spec file before proceeding 9. **Transition to implementation** — invoke writing-plans skill to create implementation plan @@ -43,7 +43,8 @@ digraph brainstorming { "Present design sections" [shape=box]; "User approves design?" [shape=diamond]; "Write design doc" [shape=box]; - "Spec self-review\n(fix inline)" [shape=box]; + "Spec review loop" [shape=box]; + "Spec review passed?" [shape=diamond]; "User reviews spec?" [shape=diamond]; "Invoke writing-plans skill" [shape=doublecircle]; @@ -56,8 +57,10 @@ digraph brainstorming { "Present design sections" -> "User approves design?"; "User approves design?" -> "Present design sections" [label="no, revise"]; "User approves design?" -> "Write design doc" [label="yes"]; - "Write design doc" -> "Spec self-review\n(fix inline)"; - "Spec self-review\n(fix inline)" -> "User reviews spec?"; + "Write design doc" -> "Spec review loop"; + "Spec review loop" -> "Spec review passed?"; + "Spec review passed?" -> "Spec review loop" [label="issues found,\nfix and re-dispatch"]; + "Spec review passed?" -> "User reviews spec?" [label="approved"]; "User reviews spec?" -> "Write design doc" [label="changes requested"]; "User reviews spec?" -> "Invoke writing-plans skill" [label="approved"]; } @@ -113,15 +116,12 @@ digraph brainstorming { - Use elements-of-style:writing-clearly-and-concisely skill if available - Commit the design document to git -**Spec Self-Review:** -After writing the spec document, look at it with fresh eyes: +**Spec Review Loop:** +After writing the spec document: -1. **Placeholder scan:** Any "TBD", "TODO", incomplete sections, or vague requirements? Fix them. -2. **Internal consistency:** Do any sections contradict each other? Does the architecture match the feature descriptions? -3. **Scope check:** Is this focused enough for a single implementation plan, or does it need decomposition? -4. **Ambiguity check:** Could any requirement be interpreted two different ways? If so, pick one and make it explicit. - -Fix any issues inline. No need to re-review — just fix and move on. +1. Dispatch spec-document-reviewer subagent (see spec-document-reviewer-prompt.md) +2. If Issues Found: fix, re-dispatch, repeat until Approved +3. If loop exceeds 3 iterations, surface to human for guidance **User Review Gate:** After the spec review loop passes, ask the user to review the written spec before proceeding: diff --git a/skills/writing-plans/SKILL.md b/skills/writing-plans/SKILL.md index 0d9c00ba..60f9834f 100644 --- a/skills/writing-plans/SKILL.md +++ b/skills/writing-plans/SKILL.md @@ -103,33 +103,26 @@ git commit -m "feat: add specific feature" ``` ```` -## No Placeholders - -Every step must contain the actual content an engineer needs. These are **plan failures** — never write them: -- "TBD", "TODO", "implement later", "fill in details" -- "Add appropriate error handling" / "add validation" / "handle edge cases" -- "Write tests for the above" (without actual test code) -- "Similar to Task N" (repeat the code — the engineer may be reading tasks out of order) -- Steps that describe what to do without showing how (code blocks required for code steps) -- References to types, functions, or methods not defined in any task - ## Remember - Exact file paths always -- Complete code in every step — if a step changes code, show the code +- Complete code in plan (not "add validation") - Exact commands with expected output +- Reference relevant skills with @ syntax - DRY, YAGNI, TDD, frequent commits -## Self-Review +## Plan Review Loop -After writing the complete plan, look at the spec with fresh eyes and check the plan against it. This is a checklist you run yourself — not a subagent dispatch. +After writing the complete plan: -**1. Spec coverage:** Skim each section/requirement in the spec. Can you point to a task that implements it? List any gaps. +1. Dispatch a single plan-document-reviewer subagent (see plan-document-reviewer-prompt.md) with precisely crafted review context — never your session history. This keeps the reviewer focused on the plan, not your thought process. + - Provide: path to the plan document, path to spec document +2. If ❌ Issues Found: fix the issues, re-dispatch reviewer for the whole plan +3. If ✅ Approved: proceed to execution handoff -**2. Placeholder scan:** Search your plan for red flags — any of the patterns from the "No Placeholders" section above. Fix them. - -**3. Type consistency:** Do the types, method signatures, and property names you used in later tasks match what you defined in earlier tasks? A function called `clearLayers()` in Task 3 but `clearFullLayers()` in Task 7 is a bug. - -If you find issues, fix them inline. No need to re-review — just fix and move on. If you find a spec requirement with no task, add the task. +**Review loop guidance:** +- Same agent that wrote the plan fixes it (preserves context) +- If loop exceeds 3 iterations, surface to human for guidance +- Reviewers are advisory — explain disagreements if you believe feedback is incorrect ## Execution Handoff