# Code Review

> Review code changes for correctness, security, performance, and code quality. Use when the user asks to review a diff, review code changes, review commits, or perform a code review. Input can be: (1) a text diff pasted directly, (2) one or more git commit hashes to extract the diff from, or (3) a git range like abc123..def456. The user may also provide task description or requirements that motivated the change.

- Skill: `aravinds-wick/code-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add aravinds-wick/code-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/aravinds-wick/code-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Security
- Author: AravindS-Wick (https://skillmd.com/u/aravinds-wick)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/aravinds-wick/code-review

---

# Code Review

Expert code reviewer combining rigorous analysis with deep expertise in clarity, consistency, and maintainability. Prioritize readable, explicit code over overly compact solutions while ensuring correctness and security.

## Inputs

Accept any combination of:
1. **Text diff** — pasted directly by the user
2. **Git commit hashes** — one or more SHAs; extract the diff with git
3. **Task description / requirements** — context for what the change is supposed to accomplish

## Review Workflow

### Step 1: Obtain the diff

- If the user provided a text diff, use it directly.
- If the user provided commit hashes, extract the diff with git:
  ```bash
  # Single commit — show its diff:
  git diff "<commit>^..<commit>"
  # Two commits — diff between them:
  git diff "<commit1>..<commit2>"
  # Range syntax (abc123..def456) — pass directly:
  git diff "<range>"
  ```
- If the user provided a range (e.g. `abc..def`), pass it as a single argument.
- If neither diff nor commits are provided, ask the user for input.

### Step 2: Gather context

- Read the changed files fully (not just the diff hunks) to understand surrounding code.
- Search the codebase for code that depends on or is affected by the changed code — callers, importers, subclasses, consumers of modified interfaces/APIs/types. The actual version of the code after the diff is applied is already checked out, so use file search tools to find dependent code and read it.
- If the user provided task requirements, keep them in mind — flag deviations where the implementation doesn't match stated intent.

### Step 3: Analyze changes

Review against two tiers using the checklist below.

#### Priority Levels

| Level | Meaning | Action |
|-------|---------|--------|
| P0 | Critical — security vulnerability, data loss risk, crash | Must fix |
| P1 | Major — significant bug, performance regression, broken feature | Must fix |
| P2 | Minor — code smell, clarity issue, inconsistency | Nice to fix |
| P3 | Suggestion — improvement idea, optional refactor | Optional |

#### Critical Issues (P0–P1)

**Correctness:**
- Logic errors and off-by-one mistakes
- Unhandled edge cases (null, empty, boundary values)
- Broken control flow (early returns, missing breaks)
- Incorrect type conversions or comparisons
- State mutation side effects

**Security:**
- Injection vulnerabilities (SQL, command, XSS)
- Exposed secrets, tokens, or credentials
- Unsafe deserialization
- Missing input validation at system boundaries
- Improper access control or authorization checks

**Performance:**
- Inefficient algorithms (quadratic where linear is possible)
- N+1 queries or unbounded database calls
- Memory leaks or unbounded growth
- Missing pagination on large datasets
- Blocking operations in async contexts

**Data Integrity:**
- Race conditions in concurrent code
- Missing transactions for multi-step writes
- Data loss on error paths
- Inconsistent state after partial failures

#### Code Quality (P2–P3)

**Clarity:**
- Unnecessary complexity or deep nesting
- Poor naming (vague, misleading, or inconsistent)
- Confusing logic flow or convoluted conditionals
- Nested ternary operators (prefer switch/if-else)
- Magic numbers or unexplained constants

**Consistency:**
- Violations of project conventions
- Inconsistent naming conventions
- Mixed patterns for the same concern
- Import style inconsistencies

**Maintainability:**
- Missing abstractions for duplicated logic
- Tight coupling between unrelated modules
- Over-engineering simple problems
- Dead code or unreachable branches

**Simplification:**
- Redundant null checks or type guards
- Overly verbose constructs with simpler alternatives
- Unnecessary intermediate variables
- Code that reimplements standard library functions

**Principles:**
- Only flag issues **introduced by the change**, not pre-existing problems.
- Preserve functionality — suggest changes to HOW, never WHAT.
- Prefer explicit code over clever one-liners.
- Consider the user's stated requirements when judging correctness.

### Step 4: Produce the review

Output this format:

```
## Code Review

**Verdict**: [APPROVE | REQUEST CHANGES | NEEDS DISCUSSION]
**Confidence**: [HIGH | MEDIUM | LOW]

### Summary
[1-2 sentences: what the change does and overall assessment]

### Findings

| Priority | Issue | Location |
|----------|-------|----------|
| P0 | Description | file:line |
| P1 | Description | file:line |
| P2 | Description | file:line |

### Details

#### [P0/P1] Issue title
**File:** `path/to/file.ext:line`

Description of the issue and why it matters.

**Suggested fix:**
\```
code suggestion
\```

(Repeat for each P0/P1 finding. P2/P3 items only need the table entry unless a code suggestion adds clarity.)

### Recommendation
[Concise actionable recommendation for the author]
```

**Rules:**
- Use `APPROVE` only when there are no P0 or P1 findings.
- Use `REQUEST CHANGES` when P0 or P1 findings exist.
- Use `NEEDS DISCUSSION` when findings are ambiguous or require author's context.
- Include detailed write-ups with suggested fixes for every P0 and P1 finding.
- P2/P3 findings go in the table; add detail sections only when a code suggestion helps.
- Keep it concise — don't pad with praise or filler.