Architectural Code Review

!bash "${CLAUDE_SKILL_DIR}/../../hooks/yaml-helper.sh" resolve_config --keys automation

Every AskUserQuestion call follows .claude/docs/automation-modes.md (collaborative asks always · guided major-only · autonomous logs and proceeds; automation_always_ask categories always prompt).

Insufficient input — check this before producing any report

If the inputs this skill needs do not exist, the answer is "could not run" — not a filled-in report. Check first, and stop if the check fails.

1. List the inputs this skill reads (data files, prior reports, profiler output, test results, registries, source code).
2. For each, record FOUND or ABSENT — not "assumed present".
3. If any input required for a section is ABSENT, that section is NOT ASSESSED — NO DATA. Do not estimate it, do not infer it from an adjacent artifact, and do not leave a mandated cell to be filled by whoever reads the template next.
4. If every required input is ABSENT, stop and report NOT ASSESSED — NO DATA as the whole verdict, naming what was missing and which skill produces it.

A verdict of NOT ASSESSED is a success. It is the correct, useful answer to "what does the data say?" when there is no data. The failure mode this prevents is specific and has been observed in practice: report templates whose verdict enum had no "could not run" state produced false clean passes — an asset audit returning COMPLIANT on a project with no assets and no standards, and a performance profile reporting ">99% headroom against a 16.67ms budget" with zero profiler data and no budget ever set.

Absence of evidence is never evidence of absence. A scan that finds no matches because there are no files to scan has not verified anything. Say which of the two happened — a reader cannot tell from a green result.

---

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 the specialists block from project.yaml; if it is absent, fall back to the ## Engine Specialists section of .claude/docs/technical-preferences.md. Note:

• The Primary specialist — <engine>-specialist derived from engine.name (Godot→godot-specialist, Unity→unity-specialist, Unreal→unreal-specialist); used for architecture and broad engine concerns
• The Language/Code Specialist — specialists.code — used when reviewing the project's primary language files
• The Shader Specialist — specialists.shader — used when reviewing shader files
• The UI Specialist — specialists.ui — used when reviewing UI code

A value of null means UNSET — treat that key as absent and skip its specialist. Never spawn it as an agent name. The v1.0 migration writes null for any specialist the legacy file did not name, and it writes the whole block whenever one member is set — so a project that configured only its code specialist carries shader: null and ui: null. The config reader returns the four-character string "null" for these, which is not empty and therefore reads as configured. null, empty, and missing are the same state here.

If no engine is configured (no engine.name in project.yaml, and technical-preferences.md reads [TO BE CONFIGURED] or is missing), skip engine specialist steps. Record Engine validation: NOT ASSESSED — no engine configured (`engine.name` unset in `project.yaml`) in this run's output. A skipped check that says nothing is indistinguishable from a check that passed; the reader cannot tell engine guidance was never sought.

---

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 Assets/Scripts/Combat/Attack.cs 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-NNNN or docs/architecture/adr- (either case).

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, load only the sections this check needs — never an unbounded full read. A substantial ADR exceeds the 25k-token Read cap, and a capped read's only recovery is paging the remainder — the most expensive way to read a file. Use the same pattern as /dev-story and /create-stories:

1. Map the headings (cheap — line numbers only): Grep pattern="^## " path="[adr-file]" output_mode="content" -n
2. Bounded-read only ## Decision and ## Consequences, using the line numbers to set Read(offset, limit) spans that end where the next heading begins. If the heading map is empty (a nonstandard ADR predating the template), fall back to one full Read; if that truncates at the cap, grep for the decision/consequence content directly rather than paging the remainder.

A referenced ADR whose file is missing, or that cannot be read, makes ADR Compliance NOT ASSESSED — [ADR] could not be read: name it. No reference at all is NO ADRS FOUND, a different result.

From those two sections, 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)

---