pixelheros/CCGS Skill Testing Framework/skills/analysis/code-review.md

Skill Test Spec: /code-review

Skill Summary

/code-review performs an architectural code review of source files in src/, checking coding standards from CLAUDE.md (doc comments on public APIs, dependency injection over singletons, data-driven values, testability). Findings are advisory. No director gates are invoked. No code edits are made. Verdicts: APPROVED, CONCERNS, or NEEDS CHANGES.

---

Static Assertions (Structural)

Verified automatically by /skill-test static — no fixture needed.

• Has required frontmatter fields: name, description, argument-hint, user-invocable, allowed-tools
• Has ≥2 phase headings
• Contains verdict keywords: APPROVED, CONCERNS, NEEDS CHANGES
• Does NOT require "May I write" language (read-only; findings are advisory output)
• Has a next-step handoff (what to do with findings)

---

Director Gate Checks

None. Code review is a read-only advisory skill; no gates are invoked.

---

Test Cases

Case 1: Happy Path — Source file follows all coding standards

Fixture:

• src/gameplay/health_component.gd exists with:
  • All public methods have doc comments (## notation)
  • No singletons used; dependencies injected via constructor
  • No hardcoded values; all constants reference assets/data/
  • ADR reference in file header: # ADR: docs/architecture/adr-004-health.md
  • Referenced ADR has Status: Accepted

Input: /code-review src/gameplay/health_component.gd

Expected behavior:

1. Skill reads the source file
2. Skill checks all coding standards: doc comments, DI, data-driven, ADR status
3. All checks pass
4. Skill outputs findings summary with all checks PASS
5. Verdict is APPROVED

Assertions:

• Each coding standard check is listed in the output
• All checks show PASS when standards are met
• Skill reads referenced ADR to confirm its status
• Verdict is APPROVED
• No edits are made to any file

---

Case 2: Needs Changes — Missing doc comment and singleton usage

Fixture:

• src/ui/inventory_ui.gd has:
  • 2 public methods without doc comments
  • Uses GameManager.instance (singleton pattern)
  • All other standards met

Input: /code-review src/ui/inventory_ui.gd

Expected behavior:

1. Skill reads the source file
2. Skill detects: 2 missing doc comments on public methods
3. Skill detects: singleton usage at specific lines (e.g., line 42, line 87)
4. Findings list the exact method names and line numbers
5. Verdict is NEEDS CHANGES

Assertions:

• Missing doc comments are listed with method names
• Singleton usage is flagged with file and line number
• Verdict is NEEDS CHANGES when BLOCKING-level standard violations exist
• Skill does not edit the file — findings are for the developer to act on
• Output suggests replacing singleton with dependency injection

---

Case 3: Architecture Risk — ADR reference is Proposed, not Accepted

Fixture:

• src/core/save_system.gd has a header comment: # ADR: docs/architecture/adr-010-save.md
• adr-010-save.md exists but has Status: Proposed
• Code itself follows all other coding standards

Input: /code-review src/core/save_system.gd

Expected behavior:

1. Skill reads the source file
2. Skill reads referenced ADR — finds Status: Proposed
3. Skill flags this as ARCHITECTURE RISK (code is implementing an unaccepted ADR)
4. Other coding standard checks pass
5. Verdict is CONCERNS (risk flag is advisory, not a hard NEEDS CHANGES)

Assertions:

• Skill reads referenced ADR file to check its status
• ARCHITECTURE RISK is flagged when ADR status is Proposed
• Verdict is CONCERNS (not NEEDS CHANGES) for ADR risk — advisory severity
• Output recommends resolving the ADR before the code goes to production

---

Case 4: Edge Case — No source files found at specified path

Fixture:

• User calls /code-review src/networking/
• src/networking/ directory does not exist

Input: /code-review src/networking/

Expected behavior:

1. Skill attempts to read files in src/networking/
2. Directory or files not found
3. Skill outputs an error: "No source files found at src/networking/"
4. Skill suggests checking src/ for valid directories
5. No verdict is emitted (nothing was reviewed)

Assertions:

• Skill does not crash when path does not exist
• Output names the attempted path in the error message
• Output suggests checking src/ for valid file paths
• No verdict is emitted when there is nothing to review

---

Case 5: Gate Compliance — No gate; LP may be consulted separately

Fixture:

• Source file follows most standards but has 1 CONCERNS-level finding (a magic number)
• review-mode.txt contains full

Input: /code-review src/gameplay/loot_system.gd

Expected behavior:

1. Skill reads and reviews the source file
2. No director gate is invoked (code review findings are advisory)
3. Skill presents findings with the CONCERNS verdict
4. Output notes: "Consider requesting a Lead Programmer review for architecture concerns"
5. Skill does not invoke any agent automatically

Assertions:

• No director gate is invoked in any review mode
• LP consultation is suggested (not mandated) in the output
• No code edits are made
• Verdict is CONCERNS for advisory-level findings

---

Protocol Compliance

• Reads source file(s) and coding standards before reviewing
• Lists each coding standard check in findings output
• Does not edit any source files (read-only skill)
• No director gates are invoked
• Verdict is one of: APPROVED, CONCERNS, NEEDS CHANGES

---

Coverage Notes

• Batch review of all files in a directory is not explicitly tested; behavior is assumed to apply the same checks file by file and aggregate the verdict.
• Test coverage checks (verifying corresponding test files exist) are a stretch goal not tested here; that is primarily the domain of /test-evidence-review.