Full-Stack Code Reviewer

Overview

A comprehensive skill that transforms Claude into a senior full-stack code reviewer. It systematically analyzes code changes across frontend, backend, database, API, and infrastructure layers — catching bugs, security vulnerabilities, performance bottlenecks, and maintainability issues before they reach production.

When to Use

• Reviewing pull requests before merging
• Conducting code audits on existing codebases
• Onboarding to an unfamiliar codebase
• Establishing code review standards for your team
• Pre-deployment safety checks
• Identifying technical debt

Quick Start

Copy
# Review staged changes
git diff --staged | claude "Review this code"

# Review a specific file
claude "Review src/services/auth.ts for security issues"

# Review an entire PR
gh pr diff 123 | claude "Full-stack review of this PR"

Review Process

Step 1: Understand Context

Before reviewing line-by-line, understand the big picture:

• What problem does this code solve? Read the PR description or ticket
• What files changed? Run git diff --stat for an overview
• What's the blast radius? Which systems are affected?
• Is there a test plan? How will changes be verified?

Step 2: Functionality Review

Check if the code actually works correctly:

Copy
## Functionality Checklist
- [ ] Code solves the stated problem
- [ ] All acceptance criteria met
- [ ] Edge cases handled (null, empty, boundary values)
- [ ] Error paths handled gracefully
- [ ] User input validated at system boundaries
- [ ] State transitions are correct
- [ ] Concurrent access considered (race conditions)

Common functionality issues:

Copy
// BAD: Off-by-one error
const lastPage = Math.floor(total / pageSize);  // Missing items when total % pageSize !== 0

// GOOD: Correct pagination
const lastPage = Math.ceil(total / pageSize);

Copy
// BAD: Missing null check
const userName = user.profile.name;  // Crashes if profile is null

// GOOD: Safe access
const userName = user?.profile?.name ?? 'Anonymous';

Step 3: Security Review

Check for OWASP Top 10 and common vulnerabilities:

Copy
## Security Checklist
- [ ] No SQL/NoSQL injection (parameterized queries used)
- [ ] No XSS vulnerabilities (output properly escaped)
- [ ] Authentication checked on all protected routes
- [ ] Authorization verified (user can access this resource?)
- [ ] Sensitive data not logged or exposed in errors
- [ ] CSRF protection in place for state-changing operations
- [ ] Rate limiting on authentication endpoints
- [ ] File uploads validated (type, size, content)
- [ ] Dependencies checked for known vulnerabilities
- [ ] Secrets not hardcoded (using env vars / secret managers)

Examples of security issues to catch:

Copy
// BAD: SQL injection
const query = `SELECT * FROM users WHERE email = '${email}'`;

// GOOD: Parameterized query
const query = 'SELECT * FROM users WHERE email = $1';
const result = await db.query(query, [email]);

Copy
// BAD: XSS vulnerability
element.innerHTML = userInput;

// GOOD: Safe rendering
element.textContent = userInput;
// Or with a framework: React auto-escapes by default

Copy
// BAD: Exposing sensitive info in error
catch (err) {
  res.status(500).json({ error: err.message, stack: err.stack });
}

// GOOD: Generic error for client, detailed log internally
catch (err) {
  logger.error('Payment failed', { error: err, userId });
  res.status(500).json({ error: 'Payment processing failed' });
}

Step 4: Performance Review

Identify bottlenecks before they hit production:

Copy
## Performance Checklist
- [ ] No N+1 query problems
- [ ] Database queries use proper indexes
- [ ] Large datasets paginated (not loaded all at once)
- [ ] Expensive computations cached where appropriate
- [ ] No unnecessary re-renders (React: useMemo, useCallback)
- [ ] Images and assets optimized
- [ ] API responses don't return excessive data
- [ ] Async operations parallelized where possible

Common performance issues:

Copy
// BAD: N+1 query problem
const orders = await Order.findAll();
for (const order of orders) {
  order.user = await User.findById(order.userId);  // Query per order!
}

// GOOD: Batch load with JOIN or include
const orders = await Order.findAll({
  include: [{ model: User }]
});

Copy
// BAD: Loading everything into memory
const allUsers = await db.users.find({});  // 1M+ records!
const active = allUsers.filter(u => u.active);

// GOOD: Filter at the database level
const active = await db.users.find({ active: true }).limit(100);

Copy
// BAD: Sequential async when parallel is possible
const user = await fetchUser(id);
const orders = await fetchOrders(id);
const notifications = await fetchNotifications(id);

// GOOD: Parallel execution
const [user, orders, notifications] = await Promise.all([
  fetchUser(id),
  fetchOrders(id),
  fetchNotifications(id)
]);

Step 5: Code Quality Review

Assess readability, maintainability, and adherence to standards:

Copy
## Code Quality Checklist
- [ ] Functions are focused (single responsibility)
- [ ] Names are descriptive and consistent
- [ ] No magic numbers or strings (use constants)
- [ ] DRY principle followed (no copy-paste code)
- [ ] Complex logic has explanatory comments
- [ ] Dead code removed
- [ ] Consistent formatting and style
- [ ] Error messages are helpful for debugging
- [ ] Types are used correctly (TypeScript)
- [ ] No `any` types without justification

Code quality examples:

Copy
// BAD: Magic numbers, unclear intent
if (user.role === 3 && Date.now() - user.ts > 86400000) {
  // ...
}

// GOOD: Named constants, clear intent
const ADMIN_ROLE = 3;
const SESSION_TIMEOUT_MS = 24 * 60 * 60 * 1000; // 24 hours

if (user.role === ADMIN_ROLE && Date.now() - user.lastActive > SESSION_TIMEOUT_MS) {
  // ...
}

Copy
// BAD: Function doing too many things
function processUser(data: any) {
  // validates, transforms, saves, sends email, logs... 200 lines
}

// GOOD: Single responsibility
function validateUserInput(data: UserInput): ValidatedUser { ... }
function saveUser(user: ValidatedUser): Promise<User> { ... }
function sendWelcomeEmail(user: User): Promise<void> { ... }

Step 6: Test Review

Verify that tests are meaningful and comprehensive:

Copy
## Test Checklist
- [ ] New code has corresponding tests
- [ ] Tests cover happy path AND error paths
- [ ] Edge cases tested (empty, null, max values)
- [ ] Tests are independent (no shared mutable state)
- [ ] Test names describe the behavior being tested
- [ ] No flaky tests (time-dependent, network-dependent)
- [ ] Integration tests for critical paths
- [ ] Mocks are appropriate (not over-mocking)

Test quality examples:

Copy
// BAD: Test name doesn't describe behavior
test('user test', () => { ... });

// GOOD: Describes expected behavior
test('should reject login when password is incorrect', () => { ... });
test('should return 404 when user does not exist', () => { ... });

Copy
// BAD: Only testing happy path
test('creates user', async () => {
  const user = await createUser({ email: '[email protected]', password: 'pass123' });
  expect(user).toBeDefined();
});

// GOOD: Testing edge cases too
test('rejects user creation with invalid email', async () => {
  await expect(createUser({ email: 'not-an-email', password: 'pass123' }))
    .rejects.toThrow('Invalid email');
});

test('rejects user creation with duplicate email', async () => {
  await createUser({ email: '[email protected]', password: 'pass123' });
  await expect(createUser({ email: '[email protected]', password: 'pass456' }))
    .rejects.toThrow('Email already exists');
});

Review Output Format

The reviewer generates structured feedback:

Copy
## Code Review Summary

**Overall Assessment:** ✅ Approve / ⚠️ Request Changes / ❌ Block

### Critical Issues (Must Fix)
1. **[Security]** SQL injection in `userService.ts:45` — use parameterized queries
2. **[Bug]** Off-by-one in pagination logic `listController.ts:23`

### Suggestions (Should Fix)
1. **[Performance]** N+1 query in order loading — use eager loading
2. **[Quality]** Extract validation logic into separate function

### Nitpicks (Optional)
1. **[Style]** Inconsistent naming: `getUserData` vs `fetchUserInfo`
2. **[Docs]** Add JSDoc for exported function `calculateDiscount`

### What's Good
- Clean separation of concerns in the service layer
- Comprehensive error handling in the API routes
- Good test coverage for edge cases

Layer-Specific Review Guides

Frontend (React/Next.js)

• Component renders correctly with different props
• Loading and error states handled
• Accessibility attributes present (aria-labels, roles)
• No direct DOM manipulation (use refs sparingly)
• Proper key props on list items
• useEffect cleanup functions prevent memory leaks

Backend (Node.js/Express)

• Input validation at controller level
• Business logic in service layer (not controllers)
• Database transactions for multi-step operations
• Proper HTTP status codes returned
• Request/response logging for debugging

Database

• Migrations are reversible
• Indexes exist for frequently queried columns
• Foreign keys and constraints enforced
• No raw SQL without parameterization
• Large text fields use appropriate column types

API Design

• RESTful conventions followed
• Consistent response envelope structure
• Proper pagination for list endpoints
• Versioning strategy in place
• Rate limiting configured

Best Practices

1. Be constructive — Suggest improvements, don't just point out problems
2. Explain why — Don't just say "this is wrong," explain the consequence
3. Prioritize — Distinguish critical issues from style preferences
4. Praise good work — Acknowledge well-written code
5. Be specific — Reference exact lines and files
6. Suggest alternatives — Show what better code looks like
7. Consider context — A quick fix has different standards than a core library
8. Ask questions — "Why was this approach chosen?" is better than "This is wrong"

Anti-Patterns to Flag

Anti-Pattern	Problem	Solution
God function	200+ line function	Split into focused functions
Shotgun surgery	One change requires editing 10 files	Better abstraction
Primitive obsession	Passing 5+ params	Use an options object or class
Feature envy	Class A mostly uses Class B's data	Move logic to Class B
Dead code	Commented-out or unreachable code	Delete it (git remembers)
Copy-paste code	Same logic in 3+ places	Extract shared function
Hardcoded config	URLs, keys in source code	Environment variables
Catch-all errors	catch(e) {} silently swallows	Log and handle specifically