mirror of
https://github.com/obra/superpowers.git
synced 2026-04-23 09:59:05 +08:00
Tone down review loops: single-pass plan review, raise issue bar
- Remove chunk-based plan review in favor of single whole-plan review - Add Calibration sections to both reviewer prompts so only serious issues block approval - Reduce max review iterations from 5 to 3 - Streamline reviewer checklists (spec: 7→5, plan: 7→4 categories)
This commit is contained in:
@@ -112,22 +112,17 @@ git commit -m "feat: add specific feature"
|
||||
|
||||
## Plan Review Loop
|
||||
|
||||
After completing each chunk of the plan:
|
||||
After writing the complete plan:
|
||||
|
||||
1. Dispatch 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: chunk content, path to spec document
|
||||
2. If ❌ Issues Found:
|
||||
- Fix the issues in the chunk
|
||||
- Re-dispatch reviewer for that chunk
|
||||
- Repeat until ✅ Approved
|
||||
3. If ✅ Approved: proceed to next chunk (or execution handoff if last chunk)
|
||||
|
||||
**Chunk boundaries:** Use `## Chunk N: <name>` headings to delimit chunks. Each chunk should be ≤1000 lines and logically self-contained.
|
||||
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
|
||||
|
||||
**Review loop guidance:**
|
||||
- Same agent that wrote the plan fixes it (preserves context)
|
||||
- If loop exceeds 5 iterations, surface to human for guidance
|
||||
- Reviewers are advisory - explain disagreements if you believe feedback is incorrect
|
||||
- If loop exceeds 3 iterations, surface to human for guidance
|
||||
- Reviewers are advisory — explain disagreements if you believe feedback is incorrect
|
||||
|
||||
## Execution Handoff
|
||||
|
||||
|
||||
@@ -2,17 +2,17 @@
|
||||
|
||||
Use this template when dispatching a plan document reviewer subagent.
|
||||
|
||||
**Purpose:** Verify the plan chunk is complete, matches the spec, and has proper task decomposition.
|
||||
**Purpose:** Verify the plan is complete, matches the spec, and has proper task decomposition.
|
||||
|
||||
**Dispatch after:** Each plan chunk is written
|
||||
**Dispatch after:** The complete plan is written.
|
||||
|
||||
```
|
||||
Task tool (general-purpose):
|
||||
description: "Review plan chunk N"
|
||||
description: "Review plan document"
|
||||
prompt: |
|
||||
You are a plan document reviewer. Verify this plan chunk is complete and ready for implementation.
|
||||
You are a plan document reviewer. Verify this plan is complete and ready for implementation.
|
||||
|
||||
**Plan chunk to review:** [PLAN_FILE_PATH] - Chunk N only
|
||||
**Plan to review:** [PLAN_FILE_PATH]
|
||||
**Spec for reference:** [SPEC_FILE_PATH]
|
||||
|
||||
## What to Check
|
||||
@@ -20,33 +20,30 @@ Task tool (general-purpose):
|
||||
| Category | What to Look For |
|
||||
|----------|------------------|
|
||||
| Completeness | TODOs, placeholders, incomplete tasks, missing steps |
|
||||
| Spec Alignment | Chunk covers relevant spec requirements, no scope creep |
|
||||
| Task Decomposition | Tasks atomic, clear boundaries, steps actionable |
|
||||
| File Structure | Files have clear single responsibilities, split by responsibility not layer |
|
||||
| File Size | Would any new or modified file likely grow large enough to be hard to reason about as a whole? |
|
||||
| Task Syntax | Checkbox syntax (`- [ ]`) on steps for tracking |
|
||||
| Chunk Size | Each chunk under 1000 lines |
|
||||
| Spec Alignment | Plan covers spec requirements, no major scope creep |
|
||||
| Task Decomposition | Tasks have clear boundaries, steps are actionable |
|
||||
| Buildability | Could an engineer follow this plan without getting stuck? |
|
||||
|
||||
## CRITICAL
|
||||
## Calibration
|
||||
|
||||
Look especially hard for:
|
||||
- Any TODO markers or placeholder text
|
||||
- Steps that say "similar to X" without actual content
|
||||
- Incomplete task definitions
|
||||
- Missing verification steps or expected outputs
|
||||
- Files planned to hold multiple responsibilities or likely to grow unwieldy
|
||||
**Only flag issues that would cause real problems during implementation.**
|
||||
An implementer building the wrong thing or getting stuck is an issue.
|
||||
Minor wording, stylistic preferences, and "nice to have" suggestions are not.
|
||||
|
||||
Approve unless there are serious gaps — missing requirements from the spec,
|
||||
contradictory steps, placeholder content, or tasks so vague they can't be acted on.
|
||||
|
||||
## Output Format
|
||||
|
||||
## Plan Review - Chunk N
|
||||
## Plan Review
|
||||
|
||||
**Status:** Approved | Issues Found
|
||||
|
||||
**Issues (if any):**
|
||||
- [Task X, Step Y]: [specific issue] - [why it matters]
|
||||
- [Task X, Step Y]: [specific issue] - [why it matters for implementation]
|
||||
|
||||
**Recommendations (advisory):**
|
||||
- [suggestions that don't block approval]
|
||||
**Recommendations (advisory, do not block approval):**
|
||||
- [suggestions for improvement]
|
||||
```
|
||||
|
||||
**Reviewer returns:** Status, Issues (if any), Recommendations
|
||||
|
||||
Reference in New Issue
Block a user