|
| 1 | +--- |
| 2 | +name: review-spec |
| 3 | +description: >- |
| 4 | + Review a design specification in Specs/ against the actual codebase to find |
| 5 | + inaccuracies, potential issues, or things that need correction before |
| 6 | + implementation. Use when asked to: "review a spec", "check a spec for issues", |
| 7 | + "validate a spec", "find problems in a spec", "review the spec for [feature]", |
| 8 | + "audit a spec against the codebase", or any request to verify that a |
| 9 | + specification accurately reflects the current code and conventions. |
| 10 | +disable-model-invocation: true |
| 11 | +argument-hint: "spec filename or feature name" |
| 12 | +--- |
| 13 | + |
| 14 | +# Review a Design Specification |
| 15 | + |
| 16 | +Review a design specification against the actual codebase to surface inaccuracies, missing details, convention violations, and potential issues — before implementation begins. The goal is to catch problems that would cause confusion or rework during implementation. |
| 17 | + |
| 18 | +**Spec to review:** $ARGUMENTS |
| 19 | + |
| 20 | +If no specific spec is given, ask the user which spec in `Specs/` they'd like reviewed. |
| 21 | + |
| 22 | +## Process |
| 23 | + |
| 24 | +### 1. Read the Spec |
| 25 | + |
| 26 | +Read the full spec file. As you read, collect: |
| 27 | + |
| 28 | +- **Referenced files**: Every source file mentioned (implementation touchpoints, code examples, "see also" references) |
| 29 | +- **Factual claims**: Specific assertions about existing code (option names/defaults, function signatures, symbol locations, data structures, file paths) |
| 30 | +- **New additions**: Symbols, options, files, or behaviors the spec proposes to add |
| 31 | +- **Behavioral descriptions**: How existing functions work, what they accept, what they return |
| 32 | + |
| 33 | +### 2. Cross-Reference with the Codebase |
| 34 | + |
| 35 | +Read all referenced files in parallel using the Explore agent or direct reads. For each file, verify: |
| 36 | + |
| 37 | +- **Options and signatures**: Do the current options match what the spec claims? Are default values correct? Does the spec list all required changes? |
| 38 | +- **Symbol locations**: Are symbols declared where the spec says they should be (e.g., CommonSymbols.wl vs private context)? Follow existing conventions — if similar symbols are private in other files, new ones should be too. |
| 39 | +- **Function behavior**: Does the code actually work the way the spec describes? Look at the actual implementation, not just the signature. |
| 40 | +- **Naming and conventions**: Do proposed names follow codebase conventions (lowerCamelCase for internal functions, UpperCamelCase for exported, etc.)? |
| 41 | +- **Implementation touchpoints completeness**: Are there files that would need changes but aren't listed? For example, if a new path variable is added, does it need a declaration in CommonSymbols.wl AND a definition in Files.wl? |
| 42 | + |
| 43 | +### 3. Check for Common Issues |
| 44 | + |
| 45 | +Look specifically for these categories of problems: |
| 46 | + |
| 47 | +**Factual inaccuracies:** |
| 48 | +- Option names or defaults that don't match the current code |
| 49 | +- Function signatures that accept different arguments than described |
| 50 | +- Symbols described as being in one location when they're in another |
| 51 | +- Properties or features attributed to objects that don't exist yet |
| 52 | + |
| 53 | +**Missing touchpoints:** |
| 54 | +- Files that need changes but aren't listed in the implementation table |
| 55 | +- Intermediate helpers or utilities the spec assumes exist but don't |
| 56 | +- Test files, documentation, or configuration that should be updated |
| 57 | + |
| 58 | +**Vague or ambiguous language:** |
| 59 | +- "such as" or "e.g." where a concrete decision is needed |
| 60 | +- Placeholder names that should be pinned down |
| 61 | +- Underspecified behavior for edge cases (e.g., what happens with `File[...]` targets?) |
| 62 | + |
| 63 | +**Logical and ordering issues:** |
| 64 | +- Operations that could fail partway through, leaving inconsistent state |
| 65 | +- Missing error handling |
| 66 | +- Race conditions or atomicity concerns in multi-step operations |
| 67 | + |
| 68 | +**Convention violations:** |
| 69 | +- Symbols proposed for shared contexts that are only used in one file (should be private) |
| 70 | +- Patterns that diverge from how similar features are already implemented |
| 71 | +- Error handling that doesn't follow the project's Enclose/Confirm/throwFailure patterns |
| 72 | + |
| 73 | +### 4. Triage and Act |
| 74 | + |
| 75 | +For each issue found, decide: |
| 76 | + |
| 77 | +- **Straightforward fix** (wrong default value, vague language, missing file in touchpoints table): Edit the spec directly to correct it. |
| 78 | +- **Needs discussion** (design trade-offs, ordering concerns, architectural decisions): Present the issue clearly with the trade-offs so the user can decide. |
| 79 | + |
| 80 | +Focus on the **top three issues by importance**. If there are more, mention them briefly but don't overwhelm. Importance is determined by how much implementation pain the issue would cause if left uncorrected. |
| 81 | + |
| 82 | +### 5. Report |
| 83 | + |
| 84 | +For each issue addressed: |
| 85 | +- State what the issue is |
| 86 | +- Explain why it matters (what would go wrong during implementation) |
| 87 | +- Show what you changed (for fixes) or present the options (for discussions) |
| 88 | + |
| 89 | +If no significant issues are found, say so — then consider whether the spec could be improved in other ways (clarity, readability, completeness of examples). |
0 commit comments