# Design Review > Use when about to approve a design document — a spec, plan, design doc, RFC, or ADR — and you want an adversarial pre-approval review that surfaces gaps before sign-off. Hunts recurring gap categories (bootstrap & ownership, naming honesty, scale, hidden assumptions, consistency-with-shipped-code, idempotency/failure, security, necessity, completeness) plus, for a plan, a plan lens (task granularity, dependency-DAG, coverage-vs-spec, exit-criteria testability). Verifies claims against the codebase (file:line, never fabricating); returns findings + a ready-for-approval / has-blockers verdict. Review-only: never edits and never approves — the human decides, the author fixes. Gates generic design docs, RFCs, standalone ADRs, specs, and plans — NOT the doc-library technical-design, architecture-doc + its linked ADRs, data-model, or api-spec artifacts (each routes to its dedicated reviewing-* twin). Keywords: design review, spec/plan/RFC/ADR review, gap analysis. - Skill: `bm629/design-review` (Agent Skill, multi-file: 2 files) - Install (CLI): `npx skillmds@latest add bm629/design-review` - Raw SKILL.md: https://api.skillmd.com/api/skills/bm629/design-review/raw - Safety review: pending - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: AI & ML - Author: bm629 (https://skillmd.com/u/bm629) - Updated: 2026-09-17 - Page: https://skillmd.com/skills/bm629/design-review --- ## Overview `design-review` performs an **adversarial pre-approval review** of a design document — a spec, design doc, RFC, ADR, or implementation plan. It hunts for the recurring gaps that otherwise surface late (or in production): missing bootstrap/ownership, misleading names, unconsidered scale, unstated assumptions, claims that don't match the code, missing failure handling, unexamined security surface, unjustified complexity, and incompleteness. Critically, it **verifies claims against the actual codebase** rather than trusting the prose, and it cites `file:line` evidence so every finding is grounded. It produces a findings report and a verdict; it **never edits the document and never approves** — that stays with the human reviewer and the author. Use it as the gate between writing a design and signing off on it. ## When to activate - ✅ A spec / design doc / RFC / ADR / implementation plan is about to be approved and you want gaps found first. - ✅ A document author wants a critical pass before presenting their design for sign-off. - ✅ An automated workflow needs a consistent pre-approval review step over design documents. **Do NOT activate when:** - The artifact is **code / a diff** — use a code-review skill (`requesting-code-review` / `code-review`); this skill reviews *design documents*, not implementations. - You are **authoring** the document — use a content/template skill to draft it; `design-review` only reviews an existing draft. - The document is the **doc-library `technical-design` artifact** — identified **authoritatively** by a `template: technical-design` frontmatter (a `# Technical Design:` heading is a fallback signal only when frontmatter is absent; a generic doc that merely titles itself "Technical Design" without the stamp stays here) — use **`reviewing-technical-design`**, its dedicated reviewer that asserts the TDD-specific implementability bar (requirement trace, reference-not-duplicate, observability, testing, rollout, delta-scoped amend). `design-review` still gates *generic* design docs, RFCs, ADRs, specs, and plans — only the doc-library TDD artifact is carved out. - The document is the **doc-library `architecture-doc` artifact** (a whole-system architecture document + its linked ADR files) — identified **authoritatively** by a `template: architecture-doc` frontmatter (a `# Architecture:` heading is a fallback signal only when frontmatter is absent; a generic doc that merely titles itself "Architecture" without the stamp stays here) — use **`reviewing-architecture-doc`**, its dedicated reviewer that asserts the architecture-quality bar (boundary+concerns, structure+altitude, diagram-sync, the ADR mechanism, NFR-realization, cross-cutting, ASR coverage, delta-scoped amend). **Boundary (load-bearing):** this carves out only the architecture-doc-library artifact and the ADR files it links; a **standalone ad-hoc ADR or RFC** (not part of an architecture-doc artifact) stays with `design-review`. - The document is the **doc-library `data-model` artifact** (a persistence/domain model — entities, keys, relationships, indexes, normalization, lifecycle) — identified **authoritatively** by a `template: data-model` frontmatter (a `# Data Model` heading is a fallback signal only when frontmatter is absent; a generic doc that merely titles itself "Data Model" without the stamp stays here) — use **`reviewing-data-model`**, its dedicated reviewer that asserts the integrity + queryability bar (typed+keyed entities, cardinality + referential rule, access-pattern-justified indexes, normalization + paradigm choice, lifecycle/migration, diagram⇄tables sync, one-directional-vs-the-api-spec, delta-scoped amend). The api-spec / feature-spec the model references stay with their own reviewers; only the data-model artifact is carved out. - The document is the **doc-library `api-spec` artifact** (an engineering API wire contract — operations, request/response schemas, auth, the error model, pagination, versioning, examples) — identified **authoritatively** by a `template: api-spec` frontmatter (a `# API Specification` heading is a fallback signal only when frontmatter is absent; a generic doc that merely titles itself "API Specification" without the stamp stays here) — use **`reviewing-api-spec`**, its dedicated reviewer that asserts the contract-completeness bar (every operation typed both sides + status codes, the complete error model, shared types referencing the data-model, per-operation authorization, pagination + tie-breaker, examples matching the schemas, one-directional-vs-the-api-reference, delta-scoped amend). Style-agnostic (REST/GraphQL/gRPC). The data-model / feature-spec it references + the downstream api-reference stay with their own reviewers; only the api-spec artifact is carved out. - The text is a one-line note or non-structured prose with no design to evaluate. ## Workflow ### Step 1: Receive inputs - **Required:** a path to the document to review. - **Optional:** explicit codebase pointers (dirs/modules the design touches). If omitted, **infer** the modules to verify from the document's own references (file paths, command/module names, links it cites). - **Optional:** an output preference — inline (default) or write a `review.md` beside the document. ### Step 2: Read the document fully Read the whole document before judging any part. Note its claims about existing behavior, its named commands/files/verbs, its inputs, its scale assumptions, and any code paths it references — these feed Steps 3–4. ### Step 3: Gap-hunt by category Walk the document against this rubric. The list is **extensible** — add categories as new gap types surface; never drop the core ones. | Category | The question to ask | |---|---| | **Bootstrap & ownership** | For every input the design consumes, *who/what creates it, and in what order?* Is the setup sequence stated? | | **Naming honesty** | Does each command / file / verb / field do what its name implies, in the order implied? Any misleading name? | | **Scale & limits** | What happens at 10× / 1000×? Per-item vs whole-collection operations; concurrency; unbounded growth. | | **Hidden assumptions** | Singular vs plural, one vs many, defaults, "obviously" — are they stated explicitly rather than assumed? | | **Consistency with shipped code** | Do claims about existing behavior, paths, or interfaces match how the code *actually* behaves? (verified in Step 4) | | **Idempotency / failure / re-run** | What on re-run, partial failure, crash, retry? Is the operation safe to repeat? | | **Security surface** | New authz/authn, secrets handling, external calls, destructive actions, untrusted input? | | **Necessity & simpler alternatives** | Does this need to be built at all? Is there a materially simpler approach the design skipped? | | **Completeness & clarity** | Are problem/goals/non-goals/alternatives/risks present? Would a first-time reader understand it? Is detail at the right altitude? | **Plan lens — when the document under review is an implementation/project plan, additionally check** (the 9 categories above still apply): | Plan check | The question to ask | |---|---| | **Task granularity** | Is each leaf task a single concern with **one testable exit check**, and atomic? Flag tasks that bundle concerns (an "and") or lack a stated exit check. | | **Dependency ordering / DAG** | Do `depends_on` / blocking edges form a valid DAG (no cycles), and does the execution order respect them? | | **Coverage vs the companion spec** | Does every spec scope item map to ≥1 task, with no task implementing out-of-scope work? | | **Exit-criteria testability** | Is each phase/task "done" a single testable statement (a command/observation), not subjective? | ### Step 4: Verify claims against the code (mandatory) For any finding that asserts something about **existing** behavior, paths, or interfaces, **read/grep the referenced module and confirm it** — then cite `file:line`. This is the difference between a grounded review and a guess. - A finding asserting an inconsistency **must quote/point at the real code** (`path:line`). - A gap you suspect but cannot confirm is marked **`unverified`** explicitly — never asserted as fact (this enforces "no fabrication"). - **Greenfield:** if the document describes brand-new behavior with no existing code to check, the *consistency* category is **N/A** — absence of code to verify is **never itself a blocker**. - **Bound the work:** verify only the modules the document references; do not scan the whole tree. ### Step 5: Assemble findings Each finding: ``` - [severity] — Gap: Fix: Evidence: | unverified ``` **Severity:** `blocker` (must resolve before approval — wrong/missing in a way that breaks the design or its premises) · `important` (should resolve — real gap, not fatal) · `minor` (nice to fix). ### Step 6: Verdict End with one line: **`ready-for-approval`** (no blockers) or **`has-blockers`** (list the blockers first, then important, then minor). The verdict is a recommendation to the human — not an approval. ### Step 7 (optional): Independent pass for large / high-risk docs For a large or high-stakes document, dispatch a **fresh reviewer with no prior context** (a subagent) given the same rubric, then merge its findings (de-duplicated). If subagent dispatch is unavailable, **fall back** to a second inline pass and say so. This mirrors the value of a fresh set of eyes; it is optional, not required. ### Step 8: Output - **Inline by default:** return the findings + verdict to the caller. - **Opt-in:** if requested, write a `review.md` beside the document. - **Never edit the document. Never approve.** Recommend; the author applies, the human decides. ## Rules **Hard rules (never violate):** - **Verify before asserting (no fabrication).** Any claim about existing behavior is confirmed against the code with a `file:line` citation, or it is labelled `unverified`. Never invent a gap. - **Review-only.** Never edit the document; never approve or reject it. Output is findings + a verdict recommendation; the human owns the decision and the author owns the fix. - **Greenfield is not a gap.** Absence of code to verify against is never, by itself, a blocker. - **Bound verification** to the modules the document references — do not scan the whole repository. - **Severity honesty.** Reserve `blocker` for things that genuinely must be fixed before approval; do not inflate. **Preferences (override-able):** - The category rubric is extensible — add categories as new recurring gap types emerge. - Surface blockers first; keep the report scannable; collapse minors if numerous. - Prefer one concrete suggested fix per finding over open-ended commentary. ## Gotchas - **Trusting the prose.** A spec can confidently describe behavior the code doesn't have. The whole value is Step 4 — verify against the code; a review that skips it is just opinion. - **Greenfield mis-flagged.** On a brand-new-system doc there is nothing to verify against; flagging "couldn't verify X" as a problem is wrong — mark consistency N/A. - **Whole-tree scan.** Trying to verify against the entire codebase is slow and noisy; bound it to referenced modules. - **Over-flagging.** Listing every nitpick as a blocker buries the real ones. Severity discipline keeps the verdict meaningful. - **Detail mistaken for completeness.** Lots of detail is not the same as a complete design; conversely, terse-but-complete is fine. Judge whether the *decisions* are present, not the word count. ## Anti-patterns - **"It probably doesn't match the code" without checking.** Fabricated or unverified gaps stated as fact — forbidden; verify or mark `unverified`. - **"I'll just fix the doc while I'm here."** Editing the document is out of scope; review-only. - **"Looks good to me."** Rubber-stamping — skimming and approving without running the rubric or verifying claims. - **"Let me approve it."** Deciding the outcome; the skill recommends, the human approves. - **"Let me grep the entire repo to be safe."** Unbounded verification; stay within referenced modules. ## Output A **findings report** (inline by default; optional `review.md`): zero or more findings in the Step-5 shape, ordered by severity, followed by a one-line `ready-for-approval` | `has-blockers` verdict. The artifact is a **recommendation consumed by the document's author and the human approver** at the approval gate — never an approval, never an edited document. ## Related - `requesting-code-review` / `code-review` — the analogous capability for **code/diffs**; `design-review` is the design-document counterpart that runs *before* code exists. - A content/template skill — authors the document that this skill reviews (authoring vs reviewing are distinct). - `reviewing-technical-design` — the dedicated reviewer for the doc-library **technical-design** artifact (a TDD), which is carved out of this skill's scope. Route a `template: technical-design` document there; `design-review` keeps generic design docs, RFCs, ADRs, specs, and plans. - `reviewing-architecture-doc` — the dedicated reviewer for the doc-library **architecture-doc** artifact (a whole-system architecture document + its linked ADR files), also carved out of this skill's scope. Route a `template: architecture-doc` document there; a standalone ad-hoc ADR/RFC (not part of an architecture-doc artifact) stays with `design-review`. - `reviewing-data-model` — the dedicated reviewer for the doc-library **data-model** artifact (a persistence/domain model), also carved out of this skill's scope. Route a `template: data-model` document there; `design-review` keeps generic design docs, RFCs, ADRs, specs, and plans. - `reviewing-api-spec` — the dedicated reviewer for the doc-library **api-spec** artifact (an engineering API wire contract), also carved out of this skill's scope. Route a `template: api-spec` document there; `design-review` keeps generic design docs, RFCs, ADRs, specs, and plans. - Fits the spec → plan → implement discipline as the pre-approval gate over the spec (and plan). ## Progressive disclosure - `references/sources.md` — research provenance for the rubric. Load only if auditing where the categories came from. This skill ships no `scripts/` or `assets/`; it runs via `Read` / `Grep` / `Glob` and (optionally) a subagent dispatch. ## Body budget - `description` ≤ 1,024 chars (agentskills.io cap); combined `description` + `when_to_use` truncated at 1,536 chars in the listing. - Body ≤ ~500 lines / 5,000 tokens. ## Changelog - **2.4.0** (2026-06-15) — scope carve-out: the doc-library `api-spec` artifact now routes to its dedicated reviewer `reviewing-api-spec`; the data-model/feature-spec it references + the downstream api-reference stay with their own reviewers. Additive only — the 9 gap categories and the plan lens are unchanged. (The fourth and last authoring-only carve-out — every doc-library type now has a dedicated reviewing twin.) - **2.3.0** (2026-06-15) — scope carve-out: the doc-library `data-model` artifact now routes to its dedicated reviewer `reviewing-data-model`; the api-spec/feature-spec it references stay with their own reviewers. Additive only — the 9 gap categories and the plan lens are unchanged. - **2.2.0** (2026-06-14) — scope carve-out: the doc-library `architecture-doc` artifact (+ its linked ADR files) now routes to its dedicated reviewer `reviewing-architecture-doc`; standalone ad-hoc ADRs/RFCs stay here. Additive only — the 9 gap categories and the plan lens are unchanged. - **2.1.0** (2026-06-14) — scope carve-out: the doc-library `technical-design` artifact now routes to its dedicated reviewer `reviewing-technical-design`; `design-review` keeps generic design docs, RFCs, ADRs, specs, and plans. Additive only — the 9 gap categories and the plan lens are unchanged. - **2.0.0** (2026-06-01) — initial reviewed release.