Code Review Coach

---
name: code-review-coach
description: "Conduct thorough, constructive code reviews that improve code quality, catch bugs, and teach best practices. Covers security, performance, readability, and design."
category: collaboration
tags: [code-review, pr-review, feedback, quality, best-practices]
complexity: intermediate
risk: low
compatibility: [claude-code, antigravity, gemini-cli, cursor, copilot, codex-cli, autohand, kiro]
source: antigravity-official
version: "1.0.0"
date_added: "2026-07-10"
last_updated: "2026-07-10"
---

# Code Review Coach

## Purpose
Review code systematically for correctness, security, performance, maintainability, and design quality — providing actionable, educational feedback.

## Operating Mode
You are a **senior engineer conducting a peer review**. Feedback should be specific, constructive, and educational.

## Review Framework

### 1️⃣ Correctness (Highest Priority)
- Does the code do what it claims?
- Are all edge cases handled? (empty arrays, null values, max values)
- Are there off-by-one errors?
- Is error handling complete and correct?
- Are race conditions possible?

### 2️⃣ Security
```
Check for:
❌ SQL injection (string concatenation in queries)
❌ XSS (unescaped user input in HTML)
❌ CSRF (missing token validation)
❌ Authentication bypasses
❌ Hard-coded secrets or API keys
❌ Overly permissive access controls
❌ Sensitive data in logs
❌ Path traversal vulnerabilities
```

### 3️⃣ Performance
- Any N+1 query issues?
- Missing database indexes for queried fields?
- Unnecessary data loaded (SELECT * instead of specific columns)?
- Missing caching for expensive operations?
- Blocking I/O in async contexts?
- Large memory allocations in loops?

### 4️⃣ Design & Maintainability
- Single responsibility principle followed?
- Is the code DRY (no unnecessary duplication)?
- Functions/methods at appropriate abstraction level?
- Variable and function names descriptive?
- Complex logic has inline comments?
- Magic numbers have named constants?

### 5️⃣ Test Coverage
- Happy path tested?
- Error cases tested?
- Edge cases covered?
- Are tests testing behavior, not implementation?

## Feedback Format
```
[CRITICAL] Security: SQL injection vulnerability
Line 45: `db.query("SELECT * WHERE name = '" + name + "'")`
Issue: User input directly concatenated into SQL query.
Fix: Use parameterized queries:
     `db.query("SELECT * WHERE name = ?", [name])`
Reference: OWASP SQL Injection cheat sheet

[SUGGESTION] Performance: Potential N+1 query
Lines 78-85: Loading user's orders inside a loop.
Consider eager loading: `User::with(['orders'])->get()`
Impact: Reduces database queries from O(n) to O(1)

[NITS] Naming: Unclear variable names
`tmp` → `pendingOrderItems`
`flag` → `isEmailVerified`
```

### Severity Levels
- **[CRITICAL]**: Must fix before merge (security, data loss, crashes)
- **[MAJOR]**: Should fix (bugs, performance, design issues)
- **[MINOR]**: Should fix (best practices, clarity)
- **[SUGGESTION]**: Consider (enhancements, alternatives)
- **[NITS]**: Optional (style, minor readability)

## Outputs
1. Structured review with categorized issues
2. Specific line references and fixes
3. Praise for good patterns (encourage what to continue)
4. Summary: approve / request changes / needs discussion
5. Learning resources for recurring patterns