code-review skill

!bash "${CLAUDESKILLDIR}/../../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; automationalwaysask 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.

test results, registries, source code).

1. List the inputs this skill reads (data files, prior reports, profiler output,

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.

1. For each, record FOUND or ABSENT — not "assumed present".
2. If any input required for a section is ABSENT, that section is

NOT ASSESSED — NO DATA as the whole verdict, naming what was missing and which skill produces it.

1. If every required input is ABSENT, stop and report

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 — -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 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, 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 (measured ~103k vs ~54k tokens on a 34k-token ADR). 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.

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)

Phase 7: Specialist Reviews (Parallel)

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

**Verify every specialist finding before reporting it. Do not pass findings

through unchecked.** For each finding, record in the report:

- File and line it refers to.

- Evidence — the quoted code, or the concrete input/state that triggers it.

- Confidence — VERIFIED (you checked it yourself) or UNVERIFIED —

specialist claim (you could not).

A finding you could not verify is reported as unverified or dropped, never

promoted to a defect on the strength of confident phrasing.

Why this is mandatory. Agents are reliable when deriving and unreliable when

diagnosing existing code. Measured in practice: three separate agents

produced three different wrong claims about the same six-line function,

every one fluent enough to pass a skim — including a spawned specialist here

alleging a float-precision bug that enumerating the inputs disproves. Without

this step the parent review is a laundering channel: a guess enters as a

specialist finding and leaves as a reviewed defect.

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