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.
التثبيت
التثبيت باستخدام Codex أو Claude انسخ هذا Prompt والصقه في Codex أو Claude أو مساعد آخر ليراجع صفحة Skill ويثبّتها لك.
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).
