mirror of
https://github.com/obra/superpowers.git
synced 2026-04-16 02:02:41 +00:00
Reapply "Replace subagent review loops with lightweight inline self-review"
This reverts commit b045fa3950.
This commit is contained in:
@@ -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-<topic>-design.md` and commit
|
||||
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)
|
||||
7. **Spec self-review** — quick inline check for placeholders, contradictions, ambiguity, scope (see below)
|
||||
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,8 +43,7 @@ digraph brainstorming {
|
||||
"Present design sections" [shape=box];
|
||||
"User approves design?" [shape=diamond];
|
||||
"Write design doc" [shape=box];
|
||||
"Spec review loop" [shape=box];
|
||||
"Spec review passed?" [shape=diamond];
|
||||
"Spec self-review\n(fix inline)" [shape=box];
|
||||
"User reviews spec?" [shape=diamond];
|
||||
"Invoke writing-plans skill" [shape=doublecircle];
|
||||
|
||||
@@ -57,10 +56,8 @@ 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 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"];
|
||||
"Write design doc" -> "Spec self-review\n(fix inline)";
|
||||
"Spec self-review\n(fix inline)" -> "User reviews spec?";
|
||||
"User reviews spec?" -> "Write design doc" [label="changes requested"];
|
||||
"User reviews spec?" -> "Invoke writing-plans skill" [label="approved"];
|
||||
}
|
||||
@@ -116,12 +113,15 @@ digraph brainstorming {
|
||||
- Use elements-of-style:writing-clearly-and-concisely skill if available
|
||||
- Commit the design document to git
|
||||
|
||||
**Spec Review Loop:**
|
||||
After writing the spec document:
|
||||
**Spec Self-Review:**
|
||||
After writing the spec document, look at it with fresh eyes:
|
||||
|
||||
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
|
||||
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.
|
||||
|
||||
**User Review Gate:**
|
||||
After the spec review loop passes, ask the user to review the written spec before proceeding:
|
||||
|
||||
@@ -103,26 +103,33 @@ 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 plan (not "add validation")
|
||||
- Complete code in every step — if a step changes code, show the code
|
||||
- Exact commands with expected output
|
||||
- Reference relevant skills with @ syntax
|
||||
- DRY, YAGNI, TDD, frequent commits
|
||||
|
||||
## Plan Review Loop
|
||||
## Self-Review
|
||||
|
||||
After writing the complete plan:
|
||||
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.
|
||||
|
||||
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
|
||||
**1. Spec coverage:** Skim each section/requirement in the spec. Can you point to a task that implements it? List any gaps.
|
||||
|
||||
**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
|
||||
**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.
|
||||
|
||||
## Execution Handoff
|
||||
|
||||
|
||||
Reference in New Issue
Block a user