--- name: code-review description: "Performs an architectural and quality code review on a specified file or set of files. Checks for coding standard compliance, architectural pattern adherence, SOLID principles, testability, and performance concerns." argument-hint: "[path-to-file-or-directory]" user-invocable: true allowed-tools: Read, Glob, Grep, Bash, Task context: fork agent: lead-programmer --- ## Phase 1: Load Target Files Read the target file(s) in full. Read CLAUDE.md for project coding standards. --- ## Phase 2: Identify Engine Specialists Read `.claude/docs/technical-preferences.md`, section `## Engine Specialists`. Note: - The **Primary** specialist (used for architecture and broad engine concerns) - The **Language/Code Specialist** (used when reviewing the project's primary language files) - The **Shader Specialist** (used when reviewing shader files) - The **UI Specialist** (used when reviewing UI code) If the section reads `[TO BE CONFIGURED]`, no engine is pinned — skip engine specialist steps. --- ## Phase 3: ADR Compliance Check Search for ADR references in the story file, commit messages, and header comments. Look for patterns like `ADR-NNN` or `docs/architecture/ADR-`. If no ADR references found, note: "No ADR references found — skipping ADR compliance check." For each referenced ADR: read the file, extract the **Decision** and **Consequences** sections, then classify any deviation: - **ARCHITECTURAL VIOLATION** (BLOCKING): Uses a pattern explicitly rejected in the ADR - **ADR DRIFT** (WARNING): Meaningfully diverges from the chosen approach without using a forbidden pattern - **MINOR DEVIATION** (INFO): Small difference from ADR guidance that doesn't affect overall architecture --- ## Phase 4: Standards Compliance Identify the system category (engine, gameplay, AI, networking, UI, tools) and evaluate: - [ ] Public methods and classes have doc comments - [ ] Cyclomatic complexity under 10 per method - [ ] No method exceeds 40 lines (excluding data declarations) - [ ] Dependencies are injected (no static singletons for game state) - [ ] Configuration values loaded from data files - [ ] Systems expose interfaces (not concrete class dependencies) --- ## Phase 5: Architecture and SOLID **Architecture:** - [ ] Correct dependency direction (engine <- gameplay, not reverse) - [ ] No circular dependencies between modules - [ ] Proper layer separation (UI does not own game state) - [ ] Events/signals used for cross-system communication - [ ] Consistent with established patterns in the codebase **SOLID:** - [ ] Single Responsibility: Each class has one reason to change - [ ] Open/Closed: Extendable without modification - [ ] Liskov Substitution: Subtypes substitutable for base types - [ ] Interface Segregation: No fat interfaces - [ ] Dependency Inversion: Depends on abstractions, not concretions --- ## Phase 6: Game-Specific Concerns - [ ] Frame-rate independence (delta time usage) - [ ] No allocations in hot paths (update loops) - [ ] Proper null/empty state handling - [ ] Thread safety where required - [ ] Resource cleanup (no leaks) --- ## Phase 7: Engine Specialist Review If an engine is configured, spawn engine specialists via Task in parallel with the review above. Determine which specialist applies to each file: - Primary language files (`.gd`, `.cs`, `.cpp`) → Language/Code Specialist - Shader files (`.gdshader`, `.hlsl`, shader graph) → Shader Specialist - UI screen/widget code → UI Specialist - Cross-cutting or unclear → Primary Specialist Also spawn the **Primary Specialist** for any file touching engine architecture (scene structure, node hierarchy, lifecycle hooks). Collect findings and include them under `### Engine Specialist Findings`. --- ## Phase 8: Output Review ``` ## Code Review: [File/System Name] ### Engine Specialist Findings: [N/A — no engine configured / CLEAN / ISSUES FOUND] [Findings from engine specialist(s), or "No engine configured." if skipped] ### ADR Compliance: [NO ADRS FOUND / COMPLIANT / DRIFT / VIOLATION] [List each ADR checked, result, and any deviations with severity] ### Standards Compliance: [X/6 passing] [List failures with line references] ### Architecture: [CLEAN / MINOR ISSUES / VIOLATIONS FOUND] [List specific architectural concerns] ### SOLID: [COMPLIANT / ISSUES FOUND] [List specific violations] ### Game-Specific Concerns [List game development specific issues] ### Positive Observations [What is done well -- always include this section] ### Required Changes [Must-fix items before approval — ARCHITECTURAL VIOLATIONs always appear here] ### Suggestions [Nice-to-have improvements] ### Verdict: [APPROVED / APPROVED WITH SUGGESTIONS / CHANGES REQUIRED] ``` This skill is read-only — no files are written. --- ## Phase 9: Next Steps - If verdict is APPROVED: run `/story-done [story-path]` to close the story. - If verdict is CHANGES REQUIRED: fix the issues and re-run `/code-review`. - If an ARCHITECTURAL VIOLATION is found: run `/architecture-decision` to record the correct approach.