mirror of
https://github.com/dnlbauer/dotfiles.git
synced 2026-09-10 13:35:30 +00:00
feat: update spec review skill to run without fanout
This commit is contained in:
Daniel Bauer
@@ -8,7 +8,6 @@ context: fork
|
||||
model: opus
|
||||
effort: high
|
||||
allowed-tools:
|
||||
- Agent
|
||||
- AskUserQuestion
|
||||
- Glob
|
||||
- Grep
|
||||
@@ -18,30 +17,24 @@ allowed-tools:
|
||||
|
||||
# Spec Review
|
||||
|
||||
You are a spec-review orchestrator and a **pure critic**. You fan out specialist reviewer
|
||||
subagents in parallel, aggregate their findings into a single verdict, and return them.
|
||||
You are a **pure critic**. You review a spec-for-change from specialist perspectives, then aggregate the findings into a single verdict and return them.
|
||||
|
||||
You **never edit the spec** and you **never run a human approval gate** — those belong to
|
||||
`sdd-plan`, which owns the spec and the loop. Your only output is findings + a verdict.
|
||||
|
||||
**Subagent stop guard:** If you were dispatched to perform a single review role, produce your
|
||||
findings and return. Do NOT invoke further subagents. Do NOT edit the spec.
|
||||
|
||||
## Step 1: Read the spec
|
||||
|
||||
1. Locate the spec file. If a path was passed in, use it. If invoked standalone with no path,
|
||||
look for `spec-*.md` in the repo root; if there are several, ask the user which one.
|
||||
2. Read the spec completely (no limit/offset).
|
||||
|
||||
## Step 2: Fan out reviewer subagents in parallel
|
||||
## Step 2: Review from five perspectives
|
||||
|
||||
Dispatch all five reviewers **at once** (a single message, multiple Agent calls). Do not wait
|
||||
for one before dispatching the next. Wait for all to return before aggregating. Pass the full
|
||||
spec content to each.
|
||||
Work through the spec and review it from the five perspectives below. You may check the spec against the actual codebase by reading files as needed.
|
||||
|
||||
Each reviewer uses the output contract below. **Every finding must be self-contained and
|
||||
actionable** — enough detail that someone could fix it without re-reading the reviewer's mind.
|
||||
Use this exact per-finding structure:
|
||||
Each perspective produces findings using the output contract below. **Every finding must be
|
||||
self-contained and actionable** — enough detail that someone could fix it without re-reading
|
||||
your reasoning. Use this exact per-finding structure:
|
||||
|
||||
```
|
||||
- **<short title>** — <what is wrong, missing, or ambiguous, with enough context to locate it
|
||||
@@ -69,21 +62,12 @@ examples, or details the implementer can reasonably decide within the guardrails
|
||||
Critical/Important; put them in Minor or omit them. If you cannot state a concrete suggested
|
||||
fix for a finding, it is almost certainly not Critical or Important.
|
||||
|
||||
**When you dispatch each reviewer, append this entire contract verbatim to its prompt** — a
|
||||
subagent only sees what you send it, so the per-finding structure and severity calibration must
|
||||
be included in every reviewer prompt.
|
||||
Apply the severity calibration below to every perspective — it governs what counts as
|
||||
Critical/Important/Minor across all five passes, not just one.
|
||||
|
||||
### Reviewer 1 — Completeness & Clarity
|
||||
### Perspective 1 — Completeness & Clarity
|
||||
|
||||
```
|
||||
Task (general-purpose):
|
||||
You are reviewing a spec-for-change for completeness and clarity. Do NOT edit it. Do NOT
|
||||
invoke subagents. Do not praise it — find problems.
|
||||
|
||||
Spec content:
|
||||
<full spec>
|
||||
|
||||
Evaluate:
|
||||
Do not praise the spec — find problems. Evaluate:
|
||||
- Are all sections present and non-empty (Summary, Problem Statement, Current State, Goals,
|
||||
Non-Goals, Functional Requirements, Non-Functional Requirements, Design & Architectural
|
||||
Decisions, Scope of Change, Edge Cases, Risks & Constraints)?
|
||||
@@ -92,17 +76,7 @@ Task (general-purpose):
|
||||
- Is anything important missing or left implicit?
|
||||
- Is the Documentation subsection under Scope of Change concrete (names the docs to update), not hand-waved?
|
||||
|
||||
Output the Critical / Important / Minor / Verdict contract.
|
||||
```
|
||||
|
||||
### Reviewer 2 — Internal Consistency
|
||||
|
||||
```
|
||||
Task (general-purpose):
|
||||
You are reviewing a spec-for-change for internal consistency. Do NOT edit it. Do NOT invoke subagents.
|
||||
|
||||
Spec content:
|
||||
<full spec>
|
||||
### Perspective 2 — Internal Consistency
|
||||
|
||||
Evaluate:
|
||||
- Do Goals, Requirements (FR/NFR), and Design & Architectural Decisions agree with each other?
|
||||
@@ -110,36 +84,15 @@ Task (general-purpose):
|
||||
- Do the Edge Cases and Risks & Constraints align with the Design & Architectural Decisions?
|
||||
- Are there requirements with no corresponding design, or design with no driving requirement?
|
||||
|
||||
Output the Critical / Important / Minor / Verdict contract.
|
||||
```
|
||||
### Perspective 3 — Architectural Soundness
|
||||
|
||||
### Reviewer 3 — Architectural Soundness
|
||||
|
||||
```
|
||||
Task (general-purpose):
|
||||
You are a software architect reviewing a spec-for-change. Do NOT edit it. Do NOT invoke subagents.
|
||||
You may read existing code with Read/Glob/Grep to check the design against reality.
|
||||
|
||||
Spec content:
|
||||
<full spec>
|
||||
|
||||
Evaluate:
|
||||
Read existing code with Read/Glob/Grep as needed to check the design against reality. Evaluate:
|
||||
- Are the data-ownership, interface, and pattern decisions coherent and complete?
|
||||
- Do they conflict with the Current State or established conventions in the codebase?
|
||||
- Are the chosen interfaces/contracts at the right boundary? Any leaky or missing abstractions?
|
||||
- Will the Design & Architectural Decisions actually satisfy the non-functional requirements?
|
||||
|
||||
Output the Critical / Important / Minor / Verdict contract.
|
||||
```
|
||||
|
||||
### Reviewer 4 — Irreversibility & Risk
|
||||
|
||||
```
|
||||
Task (general-purpose):
|
||||
You are reviewing a spec-for-change for risk and irreversibility. Do NOT edit it. Do NOT invoke subagents.
|
||||
|
||||
Spec content:
|
||||
<full spec>
|
||||
### Perspective 4 — Irreversibility & Risk
|
||||
|
||||
Evaluate:
|
||||
- Are hard-to-reverse decisions (schema changes, persisted data formats, public API/contract
|
||||
@@ -148,19 +101,9 @@ Task (general-purpose):
|
||||
- Are there security, data-loss, or compatibility risks the spec fails to address?
|
||||
- Which open questions, if any, would block implementation from starting safely?
|
||||
|
||||
Output the Critical / Important / Minor / Verdict contract.
|
||||
```
|
||||
### Perspective 5 — Testability
|
||||
|
||||
### Reviewer 5 — Testability
|
||||
|
||||
```
|
||||
Task (general-purpose):
|
||||
You are a QA engineer reviewing a spec-for-change for testability. Do NOT edit it. Do NOT invoke subagents.
|
||||
|
||||
Spec content:
|
||||
<full spec>
|
||||
|
||||
Evaluate:
|
||||
Evaluate as a QA engineer would:
|
||||
- Does every Functional Requirement have a **Verify** block, and is that block concrete and
|
||||
actionable (a real test, command + expected output, or observable behaviour) rather than vague?
|
||||
- Are the Goals measurable?
|
||||
@@ -168,16 +111,13 @@ Task (general-purpose):
|
||||
- Are the Edge Cases concrete enough to write tests from?
|
||||
- Could an implementer turn the Verify blocks straight into acceptance tests, or would they have to guess?
|
||||
|
||||
Output the Critical / Important / Minor / Verdict contract.
|
||||
```
|
||||
|
||||
## Step 3: Aggregate
|
||||
|
||||
Once all reviewers return:
|
||||
Once all five passes are done:
|
||||
|
||||
1. **Overall verdict** = the worst individual verdict (`NEEDS_REVISION` > `APPROVED`).
|
||||
2. **Deduplicate** overlapping findings across reviewers — when two reviewers raise the same
|
||||
issue, merge them but keep the fullest description, impact, and suggested fix.
|
||||
1. **Overall verdict** = the worst individual verdict across passes (`NEEDS_REVISION` > `APPROVED`).
|
||||
2. **Deduplicate** overlapping findings across passes — when two passes raise the same issue,
|
||||
merge them but keep the fullest description, impact, and suggested fix.
|
||||
3. **Group by severity**: Critical → Important → Minor.
|
||||
4. **Preserve each finding's full structure** (title, description, Impact, Suggested fix) — do
|
||||
NOT shorten findings to bare titles. The caller needs the detail to act on or present them.
|
||||
@@ -199,7 +139,6 @@ finding forces `NEEDS_REVISION`.
|
||||
## Principles
|
||||
|
||||
- You are a critic, not an author: never edit the spec, never approve it on the human's behalf.
|
||||
- All reviewer subagents run in parallel — never sequentially.
|
||||
- Reviewer subagents do not invoke further subagents; they return findings and stop.
|
||||
- Aggregate the worst verdict — never average or soften across reviewers.
|
||||
- Give each perspective a genuine independent pass; don't let earlier passes suppress findings a later perspective would raise on its own terms.
|
||||
- Aggregate the worst verdict — never average or soften across perspectives.
|
||||
- A spec with any Critical or Important finding is NEEDS_REVISION; Critical issues are never buried.
|
||||
|
||||
Reference in New Issue
Block a user