smart-review

Smart Code Review

Perform thorough, actionable code reviews on git changes. Catches real bugs and security issues, not just style nits.

When to Activate

• User asks to review code changes
• User says /smart-review or "review my code" or "check this PR"
• User wants feedback before committing or merging

Instructions

Step 1: Determine Review Scope

Identify what to review based on user context:

# Option A: Review uncommitted changes
git diff --stat
git diff

# Option B: Review staged changes
git diff --cached --stat
git diff --cached

# Option C: Review branch vs base (PR review)
BASE_BRANCH="${1:-main}"
git log --oneline "$BASE_BRANCH"..HEAD
git diff --stat "$BASE_BRANCH"..HEAD
git diff "$BASE_BRANCH"..HEAD

# Option D: Review a specific commit
git show --stat <commit>
git show <commit>

If no changes are found, inform the user and stop.

For large diffs (>500 lines changed), read key files individually rather than relying solely on the diff — context from surrounding code matters.

Step 2: Analyze for Issues

Review the diff systematically, checking each category:

Critical (Must Fix)

• Bugs: Logic errors, off-by-one, null/undefined access, race conditions, infinite loops
• Security: SQL injection, XSS, command injection, hardcoded secrets, path traversal, insecure deserialization
• Data loss: Destructive operations without confirmation, missing error handling on writes, uncaught exceptions that could corrupt state

Important (Should Fix)

• Error handling: Missing try/catch on I/O, swallowed errors, generic catch blocks that hide failures
• Edge cases: Empty arrays, null inputs, boundary values, concurrent access, timeout handling
• API contracts: Breaking changes to public interfaces, missing validation on inputs, inconsistent return types
• Resource management: Unclosed connections, memory leaks, missing cleanup in finally blocks

Suggestions (Consider)

• Performance: N+1 queries, unnecessary re-renders, large allocations in hot paths, missing indexes
• Maintainability: Complex conditionals that could be simplified, duplicated logic, unclear variable names
• Testing: Untested code paths, assertions that don't verify the right thing, flaky test patterns
• Design: Tight coupling, mixed responsibilities, patterns that will cause pain as the codebase grows