Use this skill to review ONE system-design spec for readiness before it is Approved / handed to planning — template conformance, Success-Criteria quality, scope, alignment with the fixed product invariants and prior decisions, reuse pointers, and open questions. Read-only; produces a SPEC-* flagged report with a readiness verdict and direct clarification questions for the author. A soft sensor, not a commit gate.
Use this skill to review ONE system-design spec for readiness before it is Approved / handed to planning — template conformance, Success-Criteria quality, scope, alignment with the fixed product invariants and prior decisions, reuse pointers, and open questions. Read-only; produces a SPEC-* flagged report with a readiness verdict and direct clarification questions for the author. A soft sensor, not a commit gate.
review-spec
A qualitative readiness gate on one filled-in system-design spec
(docs/<feature>_system_design.md), run before it is flipped to Status: Approved / handed to
planning. It answers three questions:
Is this spec enough to build from (Success Criteria a tester can pass/fail without asking the author)?
Does it fit our format and our fixed product invariants?
What must the author clarify first?
Read-only — it produces a report, never rewrites the spec (the fix decision is the author's, like
docs-review). Format and genre are not duplicated here: the template is
docs/templates/system_design_template.md, the genre + Status lifecycle is docs/README.md, and the
product invariants live in AGENTS.md + the product spec (docs/ui-ux.md). This skill checks a spec
against them.
Place in the family
create-spec — creates / maintains a spec (the lifecycle: product spec → system design → Approve → plan).
review-spec — reviews a filled spec for readiness (this skill).
docs-review (check-docs) — rot + bloat across the whole docs corpus.
It runs in two places (the generator–evaluator split):
inside spec-writer as the inner self-review loop (draft → review → fix → re-review). The
writer has Grep, so it verifies its own seam pointers — but it must not be the sole judge of its own
work, so its clean verdict is provisional.
as the independent gate: the spec-reviewer agent (or the orchestrator) runs this skill on the
finished draft. That grep-backed pass is what actually clears a spec for Approval.
When to run
After a spec is filled (Status: Draft), before it is Approved / handed to planning.
Returning to a stalled draft, to see what is still missing.
Not auto-loaded, not a commit gate — a soft sensor whose verdict drives Approval.
What it reads
The target spec + everything it must be consistent with:
the product specdocs/ui-ux.md (budgets, scenarios, fixed decisions);
the cited docs/decisions/* records and any sibling docs/*_system_design.md it depends on;
the template + docs/README.md + the AGENTS.md invariants + docs/agent_rules/*.
Open a cited decision record / sibling spec to spot-check load-bearing references and numeric values
(SPEC-XREF), not only seam paths. Delegate the noisy grep (seam/path resolution) to an explore
sub-agent when the spec is large.
Checks — by flag
Severity: block (can't write a good plan / contradicts a fixed invariant) · warn (friction) ·
info (polish).
No extraneous / out-of-genre section (market sizing, press release, and similar PRD apparatus —
unless the repo genuinely runs on PRDs) → warn (bloat / wrong genre).
Status ∈ {Draft, Approved, Implemented}; Meta present (Author/Date). → warn.
## Tests table present, with an acceptance-intent row per Success Criterion. → warn.
No vacuous section — present but filled with restatements/platitudes instead of decisions. →
block (worse than an honestly-empty one).
SPEC-SC — Success Criteria
Every Success Criterion observable + verifiable: a tester writes pass/fail without asking
the author. → block. But distinguish unit-testable from measure-in-use / benchmark (a
hit-rate target, a p90 latency) — route the latter to the ## Tests table as a benchmark target,
don't call it unverifiable.
Atomic: one obligation per criterion. Roll a compound criterion into one finding listing
the ids, not per-line. Warn only when the bundled obligations need separate tests. → warn.
Failure modes covered, not only the happy path: user-facing → edge + error; infra/runtime →
empty-input / accumulation / contention / overflow. → warn.
A Success Criterion ≠ Definition-of-Done: no «tests pass / lint clean / reviewed» as a criterion. → warn.
Each criterion maps to a ## Tests row (traceable forward). → warn.
SPEC-VAGUE — greppable language audit
Scan Success-Criteria + requirement lines (in whatever language the spec and the product spec are written):
Vague qualifier w/o threshold: fast, robust, seamless, intuitive, clean, efficient, scalable.
→ warn (give a number, or cite the product-spec budget).
Escape clause: where possible, if necessary, as appropriate. → warn.
Open-ended list: etc., and so on, including but not limited to. → warn.
TBD / placeholder inside an in-scope Success Criterion. → block.
SPEC-SCOPE — scope & non-goals
Goal names a verifiable outcome, not only a mechanism. → warn if pure mechanism, no outcome.
Out-of-scope / Non-Goals names concrete adjacent capabilities, not a generic «phase 2». → warn.
No scope creep: every Success Criterion traces to a Goal; nothing contradicts Out-of-scope. → warn.
Internal consistency: no two criteria (or a criterion vs the design text) impose contradictory
behavior on the same case. → block on a real contradiction.
No dangling reference: every surface / entity / port / criterion-id a line points to is defined
elsewhere in the spec. → warn.
Check the spec against the fixed product invariants (AGENTS.md, the product spec's fixed
decisions) and the cited decision records. A contradiction here is a block — these are the
contract. Replace this checklist with your repo's invariants, e.g.:
[main-path budget — a step that blows the budget on the primary flow is rejected by default] → block.
[data-provenance rule — where authoritative numbers/facts must come from; any path that fabricates them] → block.
[cost/latency ordering rule — e.g. the cheap path must run before the expensive one on the common case] → block.
[privacy / visibility boundary — what must never leak across users/tenants] → block.
[architecture boundary — e.g. one core, thin clients; external providers only via the designated layer] → block.
[frozen contract — the shared shape every input path must converge on] → warn/block per severity.
Consistent with the cited decision records (docs/decisions/) — don't re-propose a rejected
option. → block on contradiction.
SPEC-REUSE — don't reinvent
The spec carries a Reuse / Seams pointer (paths + symbols) to existing modules, ports, or
contracts the implementer should build on. Absent in a non-trivial spec → warn (highest leverage
for an LLM implementer).
SPEC-PATH — seam rot (greppable)
Every existing-code pointer (Reuse / Seams, "what exists today") resolves: grep/ls the
path + symbol. Missing → warn.
Pointer resolves and the claim about it is accurate: «wired-but-stub» ≠ «doesn't exist». A
wrong "what exists" claim mis-scopes the plan → warn.
Do NOT flag net-new paths — a Draft spec describes unbuilt code.
SPEC-OPEN — open questions & decisions
Open questions tiered[blocking] vs [deferrable] (template convention). A
clearly-blocking unresolved question → block regardless of tag; an untagged open-Q → info
(nudge to tag).
An inherited open question from a parent spec has a stated implementation default, or is
explicitly left [deferrable]. → warn.
SPEC-TRACE — traceability
Each Success Criterion traceable back to a product-spec section/scenario and forward to a
## Tests row. → info.
Cross-spec dependency declared (what this spec depends on / blocks). → warn.
SPEC-XREF — reference & value accuracy
Cross-check the cited product-spec sections, decision records, and sibling specs. Spot-check
load-bearing items — don't audit every token.
Cited section/scenario references accurate: the section actually covers the claim, and one
concept isn't anchored to two different sections across the spec. → warn.
Load-bearing numbers consistent: a quantity has one value everywhere (a budget, a threshold, a
default), matches the product spec, and is internally sensible. → warn; block if it breaks the
design (e.g. an asserted budget that exceeds the product-spec ceiling).
Domain-plausible values: a load-bearing coefficient/default is physically/domain-sane even
when internally consistent. → warn; block if a wrong value would corrupt the product's core output.
SPEC-DATA — runtime/DB specs only (conditional)
Apply only when the spec touches the runtime or the DB; otherwise skip silently (not a finding).
Chained-artifact linearization acknowledged: a new migration chains onto the live single
head (read at build time, not the spec's possibly-stale value) and keeps one head. A spec that
hardcodes the head as load-bearing → warn (note it will be re-derived at build).
The external-provider / data-provenance boundary is explicit for any provider-touching row (the
runtime analog of SPEC-ALIGN). → warn.
What it deliberately does NOT check (genre fit)
The spec is a lean engineering system-design — not a PRD. Do not import PRD apparatus as checks,
and flag it if it appears (SPEC-FORMAT, out-of-genre): press-release/PRFAQ, market sizing,
pricing/retention funnels, RFC-2119 MUST/SHOULD/MAY modal tiers (Success Criteria are observable
checkboxes). Security / PII review is prompted only when the spec touches auth, shared data, or an
external contract — not by default.
Calibrations
Implementation-free applies to Success-Criteria lines, not the whole spec. Design sections
(Architecture, Key Decisions, Data / Contracts) are legitimate — don't flag them as «how in a spec».
SPEC-ALIGN code-rules bite only where the spec fixes a contract/signature; a spec need not
restate every invariant to pass.
Conditional SPEC-DATA findings apply to runtime/DB specs only.
Compound-criterion is a warn, not a block; roll many into one finding.
«Benchmark» criteria (hit-rate %, p90 latency) are verifiable — route to ## Tests, don't call
them unverifiable.
Output
A Markdown report, never a silent rewrite.
## review-spec — docs/<feature>_system_design.md — <date>**Verdict:** not-ready | ready-with-fixes | ready-to-approve
### Blockers (fix before Approve / planning)- [SPEC-SC] SC3 "[vague criterion]" — unverifiable: no threshold. → give a number or cite the budget.
- [SPEC-ALIGN] the §4 flow violates [fixed invariant]. → [concrete fix].
### Warnings- [SPEC-REUSE] no "Reuse / Seams" — the implementer may duplicate [existing seam].
- [SPEC-DATA] the migration hardcodes a head that will be stale at build — mark "re-derive at build".
### Questions for the author1. SC3: what threshold do we accept (or do we take the product-spec budget)?
2. Open question "[inherited open question]" — implementation default, or leave [deferrable]?
Verdict rule (mechanical, not a mood): any unresolved block OR any live [blocking] open question
→ not-ready; only warn/info → ready-with-fixes; clean → ready-to-approve. Every blocking
SPEC-OPEN becomes a numbered question for the author.
What it doesn't do
Doesn't rewrite the spec (report only; the fix is the author's call — or the spec-writer's in its loop).
Doesn't flip Status or commit — a soft sensor; Approval is the orchestrator's/author's action.
Doesn't replace check-docs (corpus rot/bloat) or create-spec (authoring).
