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

Step 3: Read Surrounding Context

For each potential issue found in Step 2, read the full file to verify:

# Don't flag something as a bug if the surrounding code handles it
# Don't flag a "missing null check" if the caller guarantees non-null
# Don't flag a performance issue if the data set is always small

Key principle: Only flag issues you're confident about. A false positive wastes the developer's time and erodes trust. When in doubt, phrase it as a question rather than a finding.

Step 4: Generate Review

Organize findings by severity. Use this template:

## Code Review

**Scope**: [what was reviewed — branch, commits, or staged changes]
**Files**: [number] files, [additions] insertions(+), [deletions] deletions(-)

### Critical

> [file:line] **Issue title**
>
> [Explanation of the problem and its impact]
>
> ```diff
> - [current code]
> + [suggested fix]
> ```

### Important

> [file:line] **Issue title**
>
> [Explanation and suggestion]

### Suggestions

> [file:line] **Issue title**
>
> [Explanation and suggestion]

### What Looks Good

- [Positive callout — something done well]
- [Another positive callout]

### Summary

**Verdict**: Ready to merge / Needs minor fixes / Needs rework

[1-2 sentence overall assessment explaining the verdict]

Step 5: Present and Discuss

1. Show the review to the user
2. Offer to fix any Critical or Important issues directly
3. If reviewing a PR, offer to post the review as a comment via gh pr review

Review Principles

1. Be specific: "This could fail when X is null" > "Consider null checks"
2. Explain impact: "This SQL is injectable, allowing data exfiltration" > "Security issue"
3. Suggest fixes: Show code, not just descriptions. Make it easy to act on.
4. Acknowledge good work: Call out clean patterns, good test coverage, clever solutions
5. Respect intent: Understand what the developer was trying to do before suggesting alternatives
6. Prioritize: A review with 3 real issues beats one with 30 style nits
7. Skip the obvious: Don't flag formatting, naming conventions, or import ordering unless they cause actual confusion

Severity Guide

Severity	Criteria	Examples
Critical	Will cause bugs, security holes, or data loss in production	SQL injection, uncaught exception in payment flow, race condition on shared state
Important	Could cause problems under certain conditions, or makes future bugs likely	Missing error handling on network call, breaking API change without migration
Suggestion	Improvement that would make the code better but isn't urgent	Extracting a helper function, adding an index, simplifying a conditional

Edge Cases

• Trivial changes (typos, formatting): Skip the formal review format, just confirm it looks good
• Generated code (migrations, lockfiles): Note that it's generated and focus only on the generator config
• Dependency updates: Check the changelog for breaking changes, verify version constraints
• Large refactors: Focus on the design rather than line-by-line — does the new structure make sense?
• First contribution: Be extra welcoming. Suggest improvements gently and acknowledge the effort