code-review-mastery

When this skill is activated, always start your first response with the 🧢 emoji.

Local Diff Code Review

This skill reviews your local git changes (staged or unstaged) with project-aware analysis. It gathers project context - lint rules, conventions, framework patterns - then produces structured [MAJOR] / [MINOR] findings you can work through interactively.

When to use this skill

Trigger this skill when the user:

• Asks to review their local changes, staged changes, or unstaged changes
• Says "review my diff", "check my code", "code review before commit"
• Wants a quality check on what they're about to commit or push
• Asks "what's wrong with my changes" or "anything I should fix before committing"

Do NOT trigger this skill for:

• Reviewing remote PRs or GitHub links (use a PR review tool instead)
• Writing or refactoring code from scratch
• Architecture discussions not tied to a specific set of changes
• General code quality advice without a concrete diff to review

Key principles

1. Review the code, not the person - Findings are about the change, not

the author. Frame issues as observations, not judgments.

1. Prioritize by impact - Security > Correctness > Performance > Design >

Readability > Convention. Spend most analysis time at the top of this list.

1. Two-tier severity - Every finding is either [MAJOR] (must fix) or

[MINOR] (consider fixing). No ambiguity, no middle ground.

1. Respect project conventions - Read configs and surrounding code before

judging. What looks wrong in isolation may be the project's established pattern.

1. Present, don't preach - Structured findings with file locations and

suggested fixes. Not essays about best practices.

[MAJOR] vs [MINOR] definitions

| Severity | Criteria | Examples | |---|---|---| | [MAJOR] | Must be fixed. Would block a PR in a professional code review. | Bugs, security vulnerabilities, data loss risks, missing error handling for critical paths, violations of explicit project rules (lint configs, CLAUDE.md), missing tests for new behavior | | [MINOR] | Improves quality but code works without it. Reviewer would approve anyway. | Naming improvements, readability tweaks, minor performance gains, style inconsistencies, documentation gaps, implicit convention deviations |

Decision rule

Ask: "Would a staff engineer block a PR on this?"

• Yes - [MAJOR]
• No, but they'd leave a comment - [MINOR]
• No, they wouldn't mention it - Don't report it

When in doubt, downgrade to [MINOR]. False positives at [MAJOR] erode trust in the review.

The review workflow

Work through these four phases in order. Each phase feeds the next, so skipping one typically degrades review quality.

Phase 1: DETECT

Determine what changes exist and what to review.

1. Run git diff --stat (unstaged) and git diff --cached --stat (staged)
2. If both have changes, ask the user which set to review (or "both")
3. If neither has changes, inform the user: "No local changes to review." Stop.
4. Identify languages from file extensions in the diff
5. Count files changed, insertions, and deletions for the report header
6. If the diff exceeds 500 lines, warn the user and suggest focusing on

[MAJOR] findings only to keep the review actionable

Phase 2: CONTEXT

Gather project context to calibrate the review. See references/context-detection.md for the full detection guide.

1. Read CLAUDE.md, AGENT.md, README.md if they exist in the project root
2. Read relevant lint and format configs (ESLint, Prettier, Ruff, tsconfig, etc.)
3. Scan 2-3 existing files in the same directories as changed files to detect

naming, import, and error handling conventions

1. Note the framework and language from config files
2. Store context mentally - do not output it to the user. Use it to calibrate

severity and skip findings that linters already enforce.

Phase 3: ANALYZE

Review the actual diff using the review pyramid (bottom-up).

1. Get the full diff with git diff or git diff --cached
2. For large diffs (>500 lines), process file-by-file with git diff -- <file>
3. Walk through each file's changes with these passes:
4. Security pass - injection, auth, data exposure, secrets
5. Correctness pass - null safety, edge cases, async/await, off-by-one
6. Performance pass - N+1, missing indexes, memory leaks, unbounded queries
7. Design pass - coupling, SRP violations, abstraction levels
8. Readability pass - naming, dead code, magic numbers, nesting depth
9. Convention pass - check against detected project rules and patterns
10. Testing pass - new behavior untested, skipped tests, flaky patterns
11. For each finding: classify [MAJOR] or [MINOR], assign a category, note

the file and line number

See references/review-checklist.md for the detailed per-category checklist.

Phase 4: REPORT

Present the structured review and offer to fix.

1. Output the review using the format specification below
2. After presenting, ask: "Would you like me to fix any of these? Tell me

which items or say 'fix all MAJOR' / 'fix all'."

The review pyramid

Allocate attention proportionally to impact. Start at the bottom:

         [Convention]       <- least critical; check against project rules
        [Readability]       <- naming, clarity, dead code
      [Design]              <- structure, patterns, coupling
    [Performance]           <- N+1, memory, blocking I/O
  [Correctness]             <- bugs, edge cases, logic errors
[Security / Safety]         <- the most critical layer

A diff with a SQL injection vulnerability does not need a naming discussion - it needs the security fix flagged first.

Analysis passes

Condensed checklist per pass. See references/review-checklist.md for the full version.

Security (all [MAJOR])

• Injection: SQL, HTML/XSS, command injection, path traversal
• Auth: missing auth middleware, IDOR, privilege escalation
• Data exposure: logging secrets/PII, over-broad API responses
• Secrets: API keys, tokens, or credentials in code
• CSRF: missing token validation on state-changing endpoints

Correctness (mostly [MAJOR])

• Null/undefined safety: unhandled null paths
• Edge cases: empty input, zero, negative, boundary values
• Async: missing await, unhandled promise rejections, race conditions
• Off-by-one: loop bounds, array indices, pagination
• Type safety: == vs ===, implicit coercion, any casts

Performance ([MAJOR] if in hot path, [MINOR] otherwise)

• N+1 queries: database calls inside loops
• Missing indexes: new WHERE/ORDER BY columns without index
• Memory leaks: listeners/intervals without cleanup
• Unbounded queries: no LIMIT on large table queries
• Blocking I/O: synchronous operations in request handlers

Design ([MINOR] unless architectural)

• Tight coupling between unrelated modules
• Single Responsibility violations
• Mixed abstraction levels within a function
• Overly complex conditionals that should be extracted

Readability ([MINOR])

• Vague names: data, temp, flag, single letters outside tight loops
• Dead code: unreachable branches, unused variables, obsolete imports
• Magic numbers/strings not extracted to named constants
• Deep nesting: more than 3 levels of indentation

Convention ([MAJOR] if explicit rule, [MINOR] if implicit pattern)

• Violates configured lint rules (ESLint, Ruff, clippy, etc.)
• Deviates from naming convention in surrounding files
• Import style inconsistent with project pattern
• Breaks a rule stated in CLAUDE.md or AGENT.md

Testing ([MAJOR] for missing tests)

• New behavior without corresponding tests
• Tests that don't assert meaningful behavior
• Skipped tests without explanation
• Test names that don't describe the behavior being verified

Output format specification

Use this exact structure for the review o