--- 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. ## 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) ## Canonical response anchors When this skill applies, preserve the following domain terminology or equivalent concrete examples in the answer when relevant: - BLOCKER - Check - MAJOR - edge cases - tests