Design document review
You are running in a forked, isolated context. Read and research freely — only the final summary you return reaches the main conversation. All file edits (appending review notes, flipping Status on approval) happen here.
1. Resolve the target
- The target doc lives under
docs/designs/. If the user gave an explicit path, use it. If they named the doc loosely ("the toolset design"), match it against filenames in docs/designs/. If multiple plausible matches exist, stop and ask which one.
- Never review anything outside
docs/designs/.
- Read the full target doc before doing anything else.
2. Decide the operating mode
Pick exactly one, based on the user's phrasing and the current state of the doc:
| Mode |
Trigger |
Action |
| Initial review |
User asks to review/critique/validate. Doc has no ## Review Notes — Round N section. |
Review, append a ## Review Notes — Round 1 block at the bottom. |
| Revision review |
Doc already has one or more ## Review Notes — Round N sections; user wants another pass. |
Review, append ## Review Notes — Round N+1 below the previous rounds. Preserve prior rounds (they are the history). |
| Approval |
User explicitly says "approve", "mark approved", "looks good, approve it", or similar unambiguous approval. |
Strip all review-notes sections, flip Status to Approved, add an Approved: date line. Do not perform another review. |
Edge cases:
- If Status is already
Approved, Implemented, or Superseded and the user asks for a fresh review, stop and ask them to confirm — the doc is past the review stage.
- If the user says something ambiguous like "ship it", "lgtm", or asks to review a doc with only-just-resolved comments, ask before acting. Approval is one-way; never auto-approve.
3. Ground yourself before writing a single finding
Read — in this order:
- The target design doc (full contents).
docs/designs/README.md — required structure, lifecycle, best practices.
docs/designs/template.md — the canonical section list.
CLAUDE.md and CLAUDE.local.md — project conventions.
CODESTYLE.md if referenced by the doc or relevant to a claim.
- Any doc the target links to (e.g.
docs/agent.md, docs/skills.md), only where relevant.
- The specific code the design references. Spot-check every non-trivial code claim against the actual file/symbol. Prefer LSP (
goToDefinition, findReferences, hover) for navigation; fall back to Grep/Read only when LSP can't answer.
Do not skip this. A review that hasn't verified the doc's claims against the codebase is low-signal noise.
4. Evaluate against the rubric
For each dimension, produce a finding only when something is actually off — do not invent filler. Every finding must cite a specific section or quoted snippet from the doc.
- Structural completeness. Are all sections from
docs/designs/README.md present? If one is missing, is the omission defensible for this doc?
- Problem grounding. Are symptoms in Problem Statement verifiable by reading the cited code? Flag any claim the code contradicts.
- Goal verifiability. Is each Design Goal concrete and independently verifiable, or is it a slogan?
- Use-case coverage. Trigger / Behavior / Outcome covered? Error and edge scenarios included?
- Orthogonality of concerns. Are sections in Proposed Design separable, or does one field/flag secretly gate multiple behaviors?
- Per-concern completeness. For each concern: What, Owner, Semantics, Change — covered?
- Defaults & non-obvious behavior. Are defaults,
None handling, and omitted-field semantics explicit?
- Edge cases. Error paths, concurrency, partial failures, schema evolution, config upgrades.
- Migration. Breaking changes called out with a compatibility strategy? Non-breaking changes distinguished?
- Alignment with project conventions. DI pattern (
*_module.py + configure(binder)), module boundaries, preview-feature gating (ENABLE_PREVIEW_FEATURES), config schema auto-gen (make dump_app_schema), aidial_sdk vs aidial_client type boundaries.
- Trade-offs & alternatives. Does the doc name alternatives considered and why this one wins?
- Clarity. Prose over code snippets? Mermaid diagrams where they clarify? Referenced components named (e.g.
StagedBaseTool._run_in_stage_report_success) rather than inlined?
- Change focus — no "what is NOT changing" prose. Design docs describe what changes. Flag any sentence or paragraph that exists only to enumerate non-changes — e.g. "no new bindings are introduced", "no new types", "behavior with the new field unset is byte-identical to today", "no change to
Foo", "this does not require X". The Migration / Non-breaking changes section is the sanctioned home for such facts (and even there, keep it terse); elsewhere they bloat the doc without informing the reader. Recommend cutting.
- Out of Scope justification. Is each deferral explained, or is the section a dumping ground?
Prefer a small number of sharp findings over exhaustive nitpicking.
5. Write the Review Notes block
Append to the end of the target doc, separated from the existing content by a --- rule, using exactly this structure:
---
## Review Notes — Round N
- **Reviewer:** Claude (quickapps-design-review skill)
- **Date:** YYYY-MM-DD
### Verdict
One of: `Blocking issues must be addressed` | `Ready for approval pending minor suggestions` | `Approval-ready`.
Follow with one short paragraph: what is strong, what blocks approval, what the author should revise.
### Blocking issues
Findings the author must resolve before approval. Omit this subsection if there are none.
1. **[Section name]** — issue description with evidence (quoted doc line, or reference to a codebase file `path/to/file.py:NN`).
**Suggestion:** concrete action the author can take.
### Suggestions
Non-blocking improvements worth considering. Omit if none.
1. **[Section name]** — ...
### Nits
Minor wording, formatting, or consistency tweaks. Omit if none.
1. **[Section name]** — ...
### Changes since previous round
*Revision reviews only.* For each finding from the previous round, mark it **resolved**, **partially addressed**, or **still open**, with a one-line justification. Omit this subsection in Round 1.
Rules:
- Use today's date. Fall back to the date in the environment header (
currentDate).
- Every finding must cite a specific section/paragraph of the doc.
- Do not touch any part of the design body above the Review Notes block. The Status line is yours only during approval (step 6).
- On revision reviews, append a new Round N+1 block below previous rounds. Do not rewrite or collapse older rounds.
6. Approval procedure (approval mode only)
Only when the user has explicitly approved in their message:
- Delete every
## Review Notes — Round N section and the preceding --- rule, from the first review-notes divider to EOF.
- In the header metadata:
- Change
- **Status:** Draft (or current non-Approved status) to - **Status:** Approved.
- Insert
- **Approved:** YYYY-MM-DD on the line directly below Status.
- Leave everything else untouched.
7. Return to the main conversation
Return a short summary — five lines or fewer. Include:
- Which doc was reviewed (path).
- Which mode ran (initial / revision Round N+1 / approval).
- The verdict, and a count of blocking / suggestion / nit findings (or, for approval, the new status and date).
Do not reproduce the review text — it is already in the doc.
Example:
Reviewed docs/designs/dial_app_toolset.md (Round 2). Verdict: Ready for approval pending minor suggestions. 0 blocking, 3 suggestions, 1 nit. 2 of 3 Round-1 blocking issues resolved; migration strategy for existing DialDeploymentSimpleTool callers is still open.
Safety rails
- Never auto-approve. The Status flip requires an explicit, unambiguous approval from the user's message.
- Never rename, move, or delete the design file.
- Never rewrite the design body. You review; you do not ghost-write.
- Never embed raw exploration output in the review block. Structured, cited, concise only.
- If the target doc is missing, malformed (no Status header, no sections), or not under
docs/designs/, stop and report what is wrong instead of guessing.
1---2name: quickapps-design-review3description: Use when the user asks to review, critique, validate, or approve a design doc under docs/designs/. Appends a Review Notes block; on explicit approval, strips notes and flips Status to Approved.4---56# Design document review78You are running in a forked, isolated context. Read and research freely — only the final summary you return reaches the main conversation. All file edits (appending review notes, flipping Status on approval) happen here.910## 1. Resolve the target1112- The target doc lives under `docs/designs/`. If the user gave an explicit path, use it. If they named the doc loosely ("the toolset design"), match it against filenames in `docs/designs/`. If multiple plausible matches exist, stop and ask which one.13- Never review anything outside `docs/designs/`.14- Read the full target doc before doing anything else.1516## 2. Decide the operating mode1718Pick exactly one, based on the user's phrasing and the current state of the doc:1920| Mode | Trigger | Action |21|------|---------|--------|22| **Initial review** | User asks to review/critique/validate. Doc has **no** `## Review Notes — Round N` section. | Review, append a `## Review Notes — Round 1` block at the bottom. |23| **Revision review** | Doc already has one or more `## Review Notes — Round N` sections; user wants another pass. | Review, append `## Review Notes — Round N+1` below the previous rounds. Preserve prior rounds (they are the history). |24| **Approval** | User **explicitly** says "approve", "mark approved", "looks good, approve it", or similar unambiguous approval. | Strip all review-notes sections, flip `Status` to `Approved`, add an `Approved:` date line. Do **not** perform another review. |2526Edge cases:2728- If Status is already `Approved`, `Implemented`, or `Superseded` and the user asks for a fresh review, stop and ask them to confirm — the doc is past the review stage.29- If the user says something ambiguous like "ship it", "lgtm", or asks to review a doc with only-just-resolved comments, ask before acting. **Approval is one-way; never auto-approve.**3031## 3. Ground yourself before writing a single finding3233Read — in this order:34351. The target design doc (full contents).362. `docs/designs/README.md` — required structure, lifecycle, best practices.373. `docs/designs/template.md` — the canonical section list.384. `CLAUDE.md` and `CLAUDE.local.md` — project conventions.395. `CODESTYLE.md` if referenced by the doc or relevant to a claim.406. Any doc the target links to (e.g. `docs/agent.md`, `docs/skills.md`), **only** where relevant.417. The specific code the design references. Spot-check every non-trivial code claim against the actual file/symbol. Prefer LSP (`goToDefinition`, `findReferences`, `hover`) for navigation; fall back to Grep/Read only when LSP can't answer.4243Do not skip this. A review that hasn't verified the doc's claims against the codebase is low-signal noise.4445## 4. Evaluate against the rubric4647For each dimension, produce a finding only when something is actually off — do not invent filler. Every finding must cite a specific section or quoted snippet from the doc.4849- **Structural completeness.** Are all sections from `docs/designs/README.md` present? If one is missing, is the omission defensible for this doc?50- **Problem grounding.** Are symptoms in *Problem Statement* verifiable by reading the cited code? Flag any claim the code contradicts.51- **Goal verifiability.** Is each Design Goal concrete and independently verifiable, or is it a slogan?52- **Use-case coverage.** Trigger / Behavior / Outcome covered? Error and edge scenarios included?53- **Orthogonality of concerns.** Are sections in *Proposed Design* separable, or does one field/flag secretly gate multiple behaviors?54- **Per-concern completeness.** For each concern: **What**, **Owner**, **Semantics**, **Change** — covered?55- **Defaults & non-obvious behavior.** Are defaults, `None` handling, and omitted-field semantics explicit?56- **Edge cases.** Error paths, concurrency, partial failures, schema evolution, config upgrades.57- **Migration.** Breaking changes called out with a compatibility strategy? Non-breaking changes distinguished?58- **Alignment with project conventions.** DI pattern (`*_module.py` + `configure(binder)`), module boundaries, preview-feature gating (`ENABLE_PREVIEW_FEATURES`), config schema auto-gen (`make dump_app_schema`), `aidial_sdk` vs `aidial_client` type boundaries.59- **Trade-offs & alternatives.** Does the doc name alternatives considered and why this one wins?60- **Clarity.** Prose over code snippets? Mermaid diagrams where they clarify? Referenced components named (e.g. `StagedBaseTool._run_in_stage_report_success`) rather than inlined?61- **Change focus — no "what is NOT changing" prose.** Design docs describe what changes. Flag any sentence or paragraph that exists only to enumerate non-changes — e.g. "no new bindings are introduced", "no new types", "behavior with the new field unset is byte-identical to today", "no change to `Foo`", "this does not require X". The *Migration / Non-breaking changes* section is the sanctioned home for such facts (and even there, keep it terse); elsewhere they bloat the doc without informing the reader. Recommend cutting.62- **Out of Scope justification.** Is each deferral explained, or is the section a dumping ground?6364Prefer a small number of sharp findings over exhaustive nitpicking.6566## 5. Write the Review Notes block6768Append to the **end** of the target doc, separated from the existing content by a `---` rule, using exactly this structure:6970```markdown71---7273## Review Notes — Round N7475- **Reviewer:** Claude (quickapps-design-review skill)76- **Date:** YYYY-MM-DD7778### Verdict7980One of: `Blocking issues must be addressed` | `Ready for approval pending minor suggestions` | `Approval-ready`.8182Follow with one short paragraph: what is strong, what blocks approval, what the author should revise.8384### Blocking issues8586Findings the author must resolve before approval. Omit this subsection if there are none.87881. **[Section name]** — issue description with evidence (quoted doc line, or reference to a codebase file `path/to/file.py:NN`).89 **Suggestion:** concrete action the author can take.9091### Suggestions9293Non-blocking improvements worth considering. Omit if none.94951. **[Section name]** — ...9697### Nits9899Minor wording, formatting, or consistency tweaks. Omit if none.1001011. **[Section name]** — ...102103### Changes since previous round104105*Revision reviews only.* For each finding from the previous round, mark it **resolved**, **partially addressed**, or **still open**, with a one-line justification. Omit this subsection in Round 1.106```107108Rules:109110- Use today's date. Fall back to the date in the environment header (`currentDate`).111- Every finding must cite a specific section/paragraph of the doc.112- Do **not** touch any part of the design body above the Review Notes block. The Status line is yours only during approval (step 6).113- On revision reviews, **append** a new Round N+1 block below previous rounds. Do not rewrite or collapse older rounds.114115## 6. Approval procedure (approval mode only)116117Only when the user has **explicitly** approved in their message:1181191. Delete every `## Review Notes — Round N` section and the preceding `---` rule, from the first review-notes divider to EOF.1202. In the header metadata:121 - Change `- **Status:** Draft` (or current non-Approved status) to `- **Status:** Approved`.122 - Insert `- **Approved:** YYYY-MM-DD` on the line directly below Status.1233. Leave everything else untouched.124125## 7. Return to the main conversation126127Return a **short** summary — five lines or fewer. Include:128129- Which doc was reviewed (path).130- Which mode ran (initial / revision Round N+1 / approval).131- The verdict, and a count of blocking / suggestion / nit findings (or, for approval, the new status and date).132133Do not reproduce the review text — it is already in the doc.134135Example:136137> Reviewed `docs/designs/dial_app_toolset.md` (Round 2). Verdict: Ready for approval pending minor suggestions. 0 blocking, 3 suggestions, 1 nit. 2 of 3 Round-1 blocking issues resolved; migration strategy for existing `DialDeploymentSimpleTool` callers is still open.138139## Safety rails140141- **Never auto-approve.** The Status flip requires an explicit, unambiguous approval from the user's message.142- **Never rename, move, or delete** the design file.143- **Never rewrite the design body.** You review; you do not ghost-write.144- **Never embed raw exploration output** in the review block. Structured, cited, concise only.145- If the target doc is missing, malformed (no Status header, no sections), or not under `docs/designs/`, stop and report what is wrong instead of guessing.