添加 claude code game studios 到项目
This commit is contained in:
panw
172
CCGS Skill Testing Framework/skills/analysis/code-review.md
Normal file
172
CCGS Skill Testing Framework/skills/analysis/code-review.md
Normal file
@@ -0,0 +1,172 @@
|
||||
# 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`.
|
||||
Reference in New Issue
Block a user