Skip to content

code-review.skill.md

Latest commit

 

History

History
94 lines (70 loc) · 3.75 KB

code-review.skill.md

File metadata and controls

94 lines (70 loc) · 3.75 KB
name code-review
version 0.3.0
description Systematic code review protocol for AI agents — how to review code (your own or others') with rigor and specificity.
extends Project Workflow Protocol

Code Review Skill

This skill defines how to perform a structured code review. Use it when reviewing your own output before presenting to the user, or when asked to review existing code.

Review Mindset

  • You are a reviewer, not an author. Your job is to find problems, not defend choices.
  • Every comment must be actionable. "This could be better" is not a review comment. "Extract this into a utility function to reduce duplication with line 45" is.
  • Prioritize correctness over style. A bug matters more than a naming preference.

Review Checklist

For every code change, check the following in order:

1. Correctness

  • Does the code do what it claims to do?
  • Are all edge cases handled? (null, empty, boundary values, error states)
  • Are async operations properly awaited? Error-handled?
  • Are types correct and strict? (no any escapes, no type assertions without justification)

2. Security

  • Is user input sanitized before rendering or database writes?
  • Are secrets kept out of client code?
  • Are permissions checked server-side, not just hidden in UI?
  • Are dependencies from trusted sources?

3. Readability

  • Can you understand the code without the author explaining it?
  • Are variable and function names descriptive and consistent?
  • Is the code structured logically? (related things together, clear flow)
  • Are comments present where the why isn't obvious? Absent where it is?

4. Performance

  • Are there unnecessary re-renders, re-fetches, or re-computations?
  • Are large operations debounced or throttled?
  • Are heavy modules lazy-loaded?
  • Is data fetching efficient? (no N+1 queries, no over-fetching)

5. Maintainability

  • Is the code DRY without being over-abstracted?
  • Are there magic numbers or hardcoded values that should be constants?
  • Is the change scoped correctly? (no mixed concerns in one commit)
  • Will the next developer understand this in 6 months?

6. Testing

  • Is the change tested? (new behavior has tests, bug fixes have regression tests)
  • Are tests testing behavior, not implementation?
  • Are test names descriptive?

Severity Levels

Level Meaning Action Required
Blocker Bug, security flaw, or data loss risk Must fix before merge
Warning Logic concern, missing edge case, performance issue Should fix; merge with caution if deferred
Suggestion Readability improvement, naming preference, minor optimization Optional; author decides
Nit Formatting, whitespace, trivial style Fix if easy; ignore if not

Comment Format

[SEVERITY] file:line — description

Example:
[Blocker] src/auth/login.ts:42 — Password comparison uses == instead of timing-safe comparison. Vulnerable to timing attacks.

[Warning] src/hooks/useCart.ts:18 — Missing cleanup in useEffect return. Will cause memory leak if component unmounts during fetch.

[Suggestion] src/utils/format.ts:7 — Consider renaming `fmt` to `formatCurrency` for clarity.

Self-Review Protocol

Before presenting any code to the user:

  1. Re-read every line you changed
  2. Run through the checklist above mentally
  3. Flag anything you're less than 90% confident about
  4. Verify the change compiles and builds
  5. Confirm you haven't modified files outside the declared scope

Anti-Patterns

  • Rubber-stamp reviews: Approving without actually reading. Never.
  • Style-only reviews: Catching semicolons but missing null pointer exceptions.
  • Ego reviews: Rewriting working code to match personal preference.
  • Drive-by comments: Pointing out issues without suggesting fixes.