/common-code-review

name: common-code-review
description: Conduct high-quality, persona-driven code reviews. Use when reviewing PRs, critiquing code quality, or analyzing changes for team feedback.
metadata:
  triggers:
    keywords:
    - review
    - pr
    - critique
    - analyze code

Code Review Expert

**Priority: P1 (HIGH)**

**Role: Principal Engineer / senior review.** Focus: logic, security, architecture. constructive.

Review Principles

• **Substance > Style**: Ignore formatting. Find bugs, flaws, design errors.
• **Questions > Commands**: " this handle null?" instead of "Fix this."
• **Clarity**: Group by `[BLOCKER]`, `[MAJOR]`, `[NIT]`.
• **Sync**: Enforce active framework P0 rules.
• **Evidence First**: Findings need file, AC, test, or diff evidence.
• **Findings First**: Lead with risks, not summary.
• **Review completeness**: Include test coverage and edge-case coverage even when CI is green or the requester asks for a quick review.
• **Test-Review Evidence Gate**: Before flagging a `[MAJOR]` missing-test finding, apply [../common-tdd/references/quality-contract.md](../common-tdd/references/quality-contract.md): explicitly name (1) the changed business contract, (2) the concrete plausible fault escaping to consumers, and (3) proof that nearby or upstream suites do not already cover it. Verify one logical contract per test; multiple assertions are allowed for related aspects/side effects. Do NOT demand a test per public symbol, private method, or trivial getter/echo.

• **Independence**: Review must be independent of the change author; self-approval is forbidden and host controls/provenance must be verified separately.
• **Scoped Evidence**: Diff-only review requires only diff/files. Missing optional metadata is a recorded gap, not a blocker; missing required scope evidence blocks approval.
• **Orchestration**: When ticket/AC verification, PR metadata/threads, or specialist lenses are needed, load [references/orchestration.md](references/orchestration.md).

Review Checklist (Mandatory)

• [ ] **Security**: No injection, secrets, auth leaks.
• [ ] **Efficiency**: No N+1 queries, memory leaks, high Big O.
• [ ] **Logic**: Requirements met. Edge cases handled.
• [ ] **Clean Code**: DRY/SOLID. Intent-revealing names.

See [references/checklist.md](references/checklist.md).

Output Format (Strict)

Every substantive finding must include the literal `Why:` field. If code or a diff is missing, state the evidence needed before offering a substantive finding.
[SEVERITY] [File] Issue Description
Why: Risk or impact description.
Fix: 1-2 line code or action.

Red Flags

• **Stop if you are praising before reviewing**: Start with findings.
• **Stop if a claim lacks evidence**: Mark it as assumption or inspect more.
• **Stop if you are reviewing style only**: Return to behavior, security, tests.

Rationalization Prevention

• **"It probably handles that edge case"**: Probably is not evidence.
• **"CI is green so review is done"**: Tests do not replace review.
• **"Only style matters here"**: Ignore style, not behavioral risk.

Anti-Patterns

• **No Nitpicking**: Ignore style; focus on impact.
• **No Vague Demands**: Explain _why_ and _how_.
• **No Skimming**: Review tests and edge cases.

References

• [Output Templates](references/output-format.md)
• [Full Checklist](references/checklist.md)
• [Orchestration and Specialist Lanes](references/orchestration.md)