Code Reviewer

Code Reviewer

Overview

The Code Reviewer skill provides a structured, systematic approach to evaluating code for correctness, security, performance, readability, and maintainability. It helps deliver actionable, prioritized feedback using defined severity levels so authors know exactly what must be fixed versus what is a suggestion. This skill draws on established best practices to ensure reviews are constructive, thorough, and consistent.

When to Use

• Reviewing a pull request before merging
• Auditing a module or file for quality issues
• Providing mentorship feedback on another developer's code
• Self-reviewing your own code before submitting
• Evaluating third-party or open-source code before adopting it

When NOT to Use

• Writing new features from scratch (use a code-generation skill instead)
• Debugging a runtime error with a stack trace (use the debugger skill)
• Profiling performance bottlenecks with tooling (use a profiler workflow)
• Reviewing non-code artifacts like diagrams or documentation prose

Quick Reference

Instructions

1. Understand the context — Read the PR description, linked issue, or stated goal. Know what the code is supposed to do before critiquing it.

2. Run a high-level pass — Scan the overall structure. Identify the files changed, the patterns used, and the approach taken. Flag architectural concerns at this stage.

3. Apply the correctness checklist:

   • Does the logic match the requirements?
   • Are edge cases handled (null, empty, zero, max values)?
   • Are error conditions caught and handled gracefully?
   • Are external calls (APIs, DBs) wrapped with error handling?

4. Apply the security checklist:

   • Is user input validated and sanitized before use?
   • Are secrets, tokens, or credentials hardcoded?
   • Is data properly escaped before rendering (XSS)?
   • Are SQL queries parameterized (no string concatenation)?
   • Are authorization checks in place on every sensitive operation?

5. Apply the performance checklist:

   • Are there nested loops with O(n²) or worse complexity?
   • Are database queries inside loops (N+1 problem)?
   • Are expensive operations cached where appropriate?
   • Are large payloads paginated or streamed?

6. Apply the readability checklist:

   • Are names descriptive and consistent with the codebase conventions?
   • Are functions short and single-purpose?
   • Is complex logic explained with comments?
   • Is dead code or commented-out code removed?

7. Apply the maintainability checklist:

   • Is there significant code duplication (DRY violations)?
   • Are tests included for new or changed logic?
   • Is the code properly modular and loosely coupled?
   • Does the change follow existing patterns in the project?

8. Assign severity to each finding:

   • Critical — Must fix before merge. Causes bugs, data loss, or security vulnerabilities.
   • Major — Should fix before merge. Significant quality or correctness problem.
   • Minor — Fix in a follow-up. Code smell or style inconsistency.
   • Nit — Optional polish. Purely aesthetic or preference-based.

9. Write feedback — For each finding, state: (a) what the issue is, (b) why it matters, (c) how to fix it. Use line references. Be respectful and specific.

10. Summarize — End with an overall assessment: Approve, Request Changes (with blockers listed), or Comment (non-blocking suggestions only).

Examples

Example 1: Review a Python function with bugs

Input:

def get_user_discount(user_id, db):
    user = db.query("SELECT * FROM users WHERE id = " + user_id)
    if user['role'] == 'premium':
        discount = 0.2
    elif user['role'] == 'vip':
        discount = 0.35
    return discount

Output:

[Critical] SQL Injection Vulnerability (line 2) The user_id is concatenated directly into the SQL string. An attacker can pass '; DROP TABLE users; -- as user_id. Fix: Use parameterized queries:

user = db.query("SELECT * FROM users WHERE id = %s", (user_id,))

[Critical] NameError on unmatched role (line 6–7) If user['role'] is neither 'premium' nor 'vip' (e.g., 'standard'), discount is never assigned and the return statement raises UnboundLocalError. Fix: Add a default:

discount = 0.0  # default at top of function

[Major] No error handling for missing user (line 2) If user_id doesn't exist, db.query() likely returns None, causing a TypeError on user['role']. Fix:

if not user:
    return 0.0

[Minor] Magic numbers (lines 4, 6) 0.2 and 0.35 have no explanation. Use named constants:

PREMIUM_DISCOUNT = 0.20
VIP_DISCOUNT = 0.35

Overall: Request Changes — Two critical issues block merge.

Example 2: Review a JavaScript React component

Input:

function UserList() {
  const [users, setUsers] = useState([]);

  useEffect(() => {
    fetch('/api/users')
      .then(res => res.json())
      .then(data => setUsers(data));
  }, []);

  return (
    <div>
      {users.map(u => (
        <div => deleteUser(u.id)}>
          {u.name} - {u.email}
        </div>
      ))}
    </div>
  );
}

Output:

[Critical] Missing key prop in list (line 12) React requires a unique key on list items. Without it, reconciliation produces incorrect UI updates. Fix: <div key={u.id}>

.catch(err => {
  console.error('Failed to load users:', err);
  setError('Could not load users. Please try again.');
});