review-design-low-level

name	review-design-low-level
description	Review a low-level design document for gaps in code structure, class design, error handling, and testing strategy. Use after producing a low-level design and before starting implementation.
argument-hint	[prefix or path to low-level design file, or leave blank to auto-discover]

Review a low-level design document for implementation-readiness gaps, unsound class design, incomplete testing strategy, and unclear specifications. This review is tailored to concerns at the code-structure level — not architecture decisions (those belong in the high-level review).

Input

The user may provide: $ARGUMENTS

Expected Folder Structure

project/
├── design/
│   ├── {prefix}-requirements.md              # Read for cross-reference
│   ├── {prefix}-high-level-design.md         # Read for consistency checks
│   └── {prefix}-low-level-design.md          # Input: document to review
└── reviews/
    └── {prefix}-low-level-design-review.md   # Output

Process

Phase 1: Locate Documents

1. Low-level design: Determine the file to review:

   • If $ARGUMENTS is a file path (contains / or ends in .md), use it directly and infer the prefix from the filename (e.g., design/foo-low-level-design.md → prefix is foo)
   • If $ARGUMENTS is a prefix word (no /, does not end in .md), look for design/{prefix}-low-level-design.md
   • If no arguments provided, scan design/ for files matching *-low-level-design.md (also check for unprefixed low-level-design.md)
     • One match: Use it and infer the prefix from the filename
     • Multiple matches: Use AskUserQuestion listing the discovered files and ask the user which design to review
     • No matches: Use AskUserQuestion to ask the user where to find the low-level design
   • If no prefix can be determined, ask the user

2. Requirements document: Once the prefix is established, look for design/{prefix}-requirements.md

3. High-level design: Once the prefix is established, look for design/{prefix}-high-level-design.md

4. If any are missing: Use AskUserQuestion to ask the user where to find them. The requirements and high-level design are strongly recommended for cross-referencing but not blocking.

Phase 2: Read and Understand

Read all three documents. Before reviewing, understand:

• What requirements must be satisfied
• What architectural decisions were already made (and are not up for debate in this review)
• What the low-level design proposes for code structure, class design, and testing

Phase 3: Review Analysis

Analyze the low-level design across these areas. Not every area will be relevant to every design — focus on what matters for this specific system.

3.1 Package/Module Structure

• Do package boundaries align with the component boundaries from the high-level design?
• Is the dependency direction correct? (Dependencies should flow toward stable abstractions, not toward volatile details.)
• Are there circular dependencies between packages?
• Is there an appropriate separation between public interfaces and internal implementation?
• Could any packages be merged without loss of clarity? (Over-decomposition.)
• Are there packages doing too much? (Under-decomposition.)
• For workspace-based languages (Rust Cargo, Go modules): do the package boundaries support independent compilation and testing?

3.2 Class/Type Design

• Does each class/struct have a clear single responsibility?
• Are abstractions (traits, interfaces, protocols) used where they provide value — not just for the sake of abstraction?
• Are there God objects — classes that know too much or do too much?
• Are there data classes that should have behavior, or behavior classes that should be split?
• Is the type system used effectively? (Strong types vs. primitive obsession, newtype wrappers, sum types for states.)
• Are mutability and ownership semantics clear? (Especially important for Rust, but relevant everywhere.)
• Do the class diagrams match the class interactions? (A class used in a sequence diagram should appear in the class diagram.)

3.3 Class Interactions & Workflows

• For each sequence diagram: is the chain of responsibility clear? Does each class do its part and only its part?
• Are there workflows in the requirements that don't have corresponding interaction diagrams?
• Do the interactions match the package dependency graph? (A class in package A calling a class in package B implies A depends on B.)
• Are there deep call chains that could be simplified?
• Is error propagation shown? Do sequence diagrams cover failure paths or only happy paths?
• For async workflows: is the concurrency model clear? Are race conditions addressed?

3.4 Data Access Layer

• Is the abstraction level appropriate? (Repository pattern where it helps, direct access where it doesn't.)
• Are transaction boundaries clear? Which operations are atomic?
• Is connection management addressed (pooling, lifecycle, cleanup)?
• Are query patterns appropriate for the expected data volumes?
• Is the migration strategy defined and practical?
• Does the data access layer match the data model from the high-level design?

3.5 Error Handling

• Is there a coherent error type hierarchy, or are errors ad-hoc?
• Is it clear how errors propagate from the data layer through business logic to the API boundary?
• Are all error types mapped to user-facing responses?
• Are retryable vs. non-retryable errors distinguished?
• Is there a consistent pattern for logging vs. returning vs. swallowing errors?
• Are there error scenarios from the requirements that aren't covered?

3.6 Configuration & Wiring

• Is the startup sequence realistic? Are dependencies initialized in the right order?
• Is dependency injection explicit and traceable, or is it magic?
• Are all configuration parameters typed and validated?
• Is the configuration loading order clear (env vars, config files, defaults, overrides)?
• Could the system start with a misconfiguration and fail at runtime rather than at startup?

3.7 Testing Completeness

This is a critical section — the testing strategy is one of the most important parts of a low-level design.

Unit test coverage:

• Does every class with significant logic have unit tests defined?
• Are the unit tests testing behavior (what the class does) rather than implementation (how it does it)?
• Are edge cases covered — not just the happy path?
• Do the tests map to specific requirements (FR-x.x.x)?

Integration test coverage:

• Is there at least one integration test for every functional requirement?
• Are the test descriptions specific enough to implement? (Setup, exercise, assert all defined.)
• Are the integration tests practical given the test infrastructure described?

Requirements traceability:

• Check the traceability matrix: is every FR-x.x.x from the requirements doc present?
• Are there requirements covered only by unit tests that really need integration test coverage?
• Are there requirements with no test coverage at all?

Test infrastructure:

• Are test doubles (mocks, fakes, stubs) defined for the right interfaces?
• Is the integration test infrastructure realistic? (Testcontainers, docker-compose, in-memory fakes — is it specified?)
• Are there shared test utilities that would reduce test boilerplate?

3.8 Consistency with High-Level Design

• Do the packages/modules correspond to the components described in the high-level design?
• Does the data access layer implement the data model from the high-level design?
• Are all the data flows from the high-level design represented as class interactions?
• Are the technology choices from the high-level design reflected correctly? (Right libraries, right patterns for the chosen language.)
• Are there low-level decisions that contradict or undermine high-level architectural choices?

3.9 Specification Clarity

Flag anything that:

• Uses ambiguous language ("may", "could", "might", "TBD", "TODO")
• Describes a class or method without specifying its contract (inputs, outputs, invariants)
• Has pseudocode that could be interpreted multiple ways
• References types or interfaces that aren't defined elsewhere in the document
• Leaves implementation decisions to the implementer without guidance ("choose an appropriate strategy")

Phase 4: Generate Review

Create the review document at reviews/{prefix}-low-level-design-review.md using the template in template.md.

Conventions:

• Finding IDs with category prefixes: PKG-*, CLASS-*, INTERACT-*, DAL-*, ERR-*, CONF-*, TEST-*, CONSIST-*, UNCLEAR-*
• Priority ratings: Must Address (blocking — do not proceed to implementation), Should Address (high priority), Consider (medium)
• RFC 2119 language (SHALL, SHOULD, MAY) for recommendations
• Mermaid diagrams where they clarify a gap or proposed improvement
• Cross-reference requirement IDs (FR-x.x.x) when flagging testing gaps

Mermaid Diagram Rules (required for renderer compatibility — GitLab, GitHub, VS Code use older Mermaid.js):

• Always quote flowchart node labels: A["Start here"] not A[Start here]
• Always quote decision nodes: B{"Is it ready?"} not B{Is it ready?}
• Always quote subgraph titles: subgraph sg["My Group"] not subgraph sg[My Group]
• Prefer A -- "label" --> B link syntax over A -->|"label"| B
• Keep labels short — move detail into surrounding prose
• NO <br/> in any label or node text
• NO par / and blocks in sequence diagrams — use Note over A,B: description instead
• NO <--> bidirectional arrows — use two separate directed arrows
• NO }o--|| at start of erDiagram lines — reverse to ||--o{
• NO special characters in state/flowchart labels: () . / → __ — use plain words

Phase 5: Present Summary

After creating the review, summarize for the user:

• Overall assessment of the design's readiness for implementation
• Verdict for each review area
• Top 5 most critical items
• Count of findings by priority
• Specific requirements (FR-x.x.x) lacking test coverage, if any
• Whether the design is ready for implementation, needs targeted fixes, or needs significant rework

Output

Create reviews/{prefix}-low-level-design-review.md (create reviews/ directory if needed).