code-review-standards

これはレビュー時に保存された抜粋です。完全で最新の内容は外部ソースを確認してください。

---
name: code-review-standards
description: "Severity-tagged code review checklist (CRITICAL/HIGH/MEDIUM/LOW) used by code-critic agent"
user-invocable: false
disable-model-invocation: true
license: Apache-2.0
compatibility: claude-code
progressive_disclosure:
  entry_point:
    summary: "Severity-tagged checklist for code review. CRITICAL/HIGH block delivery; MEDIUM is flagged; LOW is noted. Critic outputs APPROVE (zero CRITICAL/HIGH), WARN (some HIGH, no CRITICAL), or BLOCK (any CRITICAL). 80% confidence filter — don't manufacture findings."
    when_to_use: "Loaded by code-critic agent to apply structured review criteria. Also loaded by engineers to self-check before requesting review."
    quick_start: "Apply checklist top-to-bottom: CRITICAL first, then HIGH, MEDIUM, LOW. For each finding: cite file+line, explain why it's a problem, provide the fix. Filter to >80% confidence. Output verdict with finding table."
  references:
    - criteria-critical.md
    - criteria-high.md
    - criteria-medium.md
    - criteria-low.md
    - criteria-efficiency.md
    - criteria-transferability.md
    - verdict-protocol.md
---

# Code Review Standards

## Purpose

This skill defines the structured checklist that the `code-critic` agent applies during
Stage 4 of the code production pipeline. The checklist is severity-tagged so that PM
and engineer both know exactly which findings block delivery and which are advisory.
Engineers may load this skill for self-review before requesting a critic pass.

The checklist exists because unstructured code review in multi-agent systems produces
inconsistent signal: one critic dispatch flags naming; another flags security; neither
flags the same things. Severity tagging makes the review deterministic across dispatches.

## The Severity-Tagged Checklist

### CRITICAL (must fix, blocks delivery)

- [ ] No secrets, API keys, or credentials hardcoded
- [ ] No SQL injection vectors (parameterized queries only)
- [ ] No arbitrary code execution paths (no `eval`, `exec`, unrestricted `pickle.loads`)
- [ ] Authentication/authorization not bypassable
- [ ] No infinite loops without escape conditions

### HIGH (must fix, blocks delivery)

- [ ] Type hints on all public functions and classes
- [ ] mypy --strict passes with zero errors
- [ ] pytest passes with zero failures
- [ ] Test coverage >= 90% on new code
- [ ] No bare except clauses
- [ ] No mutable default arguments
- [ ] No global mutable state
- [ ] No synchronous I/O inside async functions
- [ ] No N+1 query patterns
- [ ] Error cases handled explicitly (not silently swallowed)

### MEDIUM (flag, note in report, proceed)

- [ ] Functions <= 20 lines (prefer <= 10)
- [ ] No nested loops where hash map would reduce complexity
- [ ] list.pop(0) replaced with deque.popleft() where relevant
- [ ] asyncio.gather uses return_exceptions=True where appropriate
- [ ] Async operations have explicit timeouts
- [ ] Docstrings on public methods (Google or NumPy style)
- [ ] No Any types in production code paths

**Efficiency** (see [criteria-efficiency.md](references/criteria-efficiency.md)):

- [ ] No nested loops over two collections that should be a hash-map lookup (O(n*m) → O(n+m))
- [ ] No per-iteration I/O (queries/RPCs/fetches inside a loop) — batch outside the loop (HIGH on hot paths; see "No N+1 query patterns")
- [ ] Repeated deep property/selector resolution cached in a local (no greedy data access)
- [ ] String accumulation in loops uses list+join / StringBuilder, not `+=`
- [ ] No `SELECT *` / over-fetching in production query paths

**Transferability** (see [criteria-transferability.md](references/criteria-transferability.md)):

- [ ] No dead/unreachable code (statements after unconditional return/break/raise; uncalled private members)
- [ ] Long `if/else if` chains (3+ branches on one key) replaced by switch/match or dispatch map
- [ ] No nested `switch`/`match` — extract inner switch to a named function
- [ ] No actively misleading names (name contra