code-review

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

Argument:

/code-review [file(s)]

may optionally include a story file path as the last argument (e.g.,

/code-review src/combat/attack.gd production/epics/combat/story-001.md

). If a story path is provided, read it to extract the governing ADR reference.

Search for ADR references in, in priority order:

The story file (if provided as argument)
Header comments at the top of the implementation files
Commit messages referencing these files (
```
git log --oneline -- [file]
```
)

Look for patterns like

ADR-NNN

docs/architecture/ADR-

If no ADR references found, note: "No ADR references found — ADR compliance check skipped. For full ADR compliance review, provide the story path:

/code-review [files] [story-path]

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: Specialist Reviews (Parallel)

Spawn all applicable specialists simultaneously via Task — do not wait for one before starting the next.

Engine Specialists

If an engine is configured, determine which specialist applies to each file and spawn in parallel:

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).

QA Testability Review

For Logic and Integration stories, also spawn

qa-tester

via Task in parallel with the engine specialists. Pass:

The implementation files being reviewed
The story's
```
## QA Test Cases
```
section (the pre-written test specs from qa-lead)
The story's
```
## Acceptance Criteria
```

Ask the qa-tester to evaluate:

Are all test hooks and interfaces exposed (not hidden behind private/internal access)?
Do the QA test cases from the story's
```
## QA Test Cases
```
section map to testable code paths?
Are any acceptance criteria untestable as implemented (e.g., hardcoded values, no seam for injection)?
Does the implementation introduce any new edge cases not covered by the existing QA test cases?
Are there any observable side effects that should have a test but don't?

For Visual/Feel and UI stories: qa-tester reviews whether the manual verification steps in

## QA Test Cases

are achievable with the implementation as written — e.g., "is the state the manual checker needs to reach actually reachable?"

Collect all specialist findings before producing output.

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]

### Testability: [N/A — Visual/Feel or Config story / TESTABLE / GAPS / BLOCKING]
[qa-tester findings: test hooks, coverage gaps, untestable paths, new edge cases]
[If BLOCKING: implementation must expose [X] before tests in ## QA Test Cases can run]

### 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

Use

AskUserQuestion

Prompt: "Code review complete — verdict: [APPROVED / CHANGES REQUIRED / MAJOR REVISION]. How would you like to proceed?"

Options (adjust based on verdict):

If APPROVED:

[A] Run /story-done to mark the story complete

```
[B] Stop here
```

If CHANGES REQUIRED or MAJOR REVISION:

[A] Fix the issues and re-run /code-review

[B] Run /story-done anyway with noted exceptions

```
[C] Stop here
```

If an ARCHITECTURAL VIOLATION is found:

If the violation contradicts an existing ADR: fix the implementation to comply with
```
docs/architecture/[adr-file].md
```
. If the design has legitimately changed, run
```
/architecture-decision
```
to formally revise the existing ADR — do not create a competing one.
If no ADR exists for the pattern that was violated: run
```
/architecture-decision
```
to document the correct approach before fixing the code.