Code Review

Phase 1: Load Target Files

Read the target file(s) in full. Read AGENTS.md for project coding standards.

Phase 2: Identify Engine Specialists

Read ../ccgs-references/references/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:

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

Look for patterns like ADR-NNN or 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 using the role-reference workflow — 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 load the qa-tester role reference and perform that role pass 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 request_user_input:

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

Source: arkilstsko/Codex-Game-Studio — distributed by TomeVault.