thoreinstein Code Review Mode
Perform a comprehensive code review with structured analysis across multiple dimensions.
When to Use This Skill
- When reviewing pull requests or merge requests
- Before merging feature branches into main/trunk
- When asked to review specific files or changes
- During pair programming review sessions
- For post-implementation quality checks
Workflow
Phase 1: Context Gathering
Identify the scope:
- Determine which files/commits to review (diff, specific files, or branch comparison)
- Get the diff:
git difffor the relevant range
Understand the change:
- Review commit messages and PR description (if available)
- Check git history for related recent changes
- Identify the purpose and expected behavior of the change
Map dependencies:
- Identify what other code depends on the changed files
- Check for related test files
- Note any configuration or schema changes
Phase 2: Multi-Dimensional Review
Evaluate the code against each dimension in the Review Dimensions section below. For each dimension:
- Walk through the relevant checklist
- Note any issues with severity (Critical/High/Medium/Low)
- Record the specific location (file:line)
- Document suggested fixes
Phase 3: Synthesis
Produce a structured review following the template in references/code-review-template.md.
Determine verdict:
- APPROVE - No critical/high issues, code is ready to merge
- NEEDS WORK - Has issues that must be addressed before merge
- BLOCK - Has critical issues, security vulnerabilities, or fundamental design problems
Organize findings by severity
Acknowledge strengths - what's done well
Provide actionable fixes - not just problems, but solutions
Review Dimensions
Correctness
- Logic is correct and handles all expected cases
- Edge cases are handled (null, empty, boundary values)
- Assumptions are documented or validated
- Error states are handled appropriately
- Types are correct and conversions are safe
Security
- No injection vulnerabilities (SQL, command, XSS)
- Input is validated and sanitized
- No secrets, credentials, or keys in code
- Authentication/authorization checks are correct
- Dependencies are secure and up-to-date
Performance
- No unnecessary loops or redundant operations
- Algorithms are appropriate for the data size
- Memory usage is reasonable
- No N+1 queries or unbounded fetches
- Caching is used appropriately
Reliability
- Errors are caught and handled gracefully
- Failures don't leave system in bad state
- Retry logic has backoff and limits
- Resources are properly cleaned up (connections, files, etc.)
- Timeouts are set for external calls
Maintainability
- Code is readable and self-documenting
- Functions/methods have single responsibility
- Naming is clear and consistent
- No unnecessary duplication
- Follows project conventions and patterns
Testing
- Tests exist for new/changed behavior
- Edge cases and error paths are tested
- Tests are deterministic (not flaky)
- Test names clearly describe what they verify
- Mocks/stubs are appropriate and not excessive
Escalation Criteria
Flag for additional human review when:
| Concern | Escalation Trigger |
|---|---|
| Security | Any auth changes, crypto usage, user data handling, or potential vulnerabilities |
| Reliability | Changes to error handling, retry logic, or failure recovery paths |
| Performance | Changes to hot paths, database queries, or algorithms with scale concerns |
| Architecture | New patterns, significant structural changes, or cross-cutting concerns |
| Testing | Reduced coverage, disabled tests, or changes to test infrastructure |
Constraints
- Review only what's in scope - don't expand to unrelated code unless it's directly affected
- Be specific - reference exact file:line locations for all findings
- Provide fixes, not just criticisms - every issue should have a suggested resolution
- Calibrate severity appropriately - not everything is critical
- Acknowledge good work - positive feedback matters too
Examples
Example: Reviewing a New API Endpoint
Scope: PR adds POST /api/users endpoint in internal/api/users.go
Phase 1 output:
Files changed:
- internal/api/users.go (new handler)
- internal/api/routes.go (route registration)
- internal/db/users.go (new repository method)
Related tests: internal/api/users_test.go (new)
Purpose: Add user creation endpoint for onboarding flow
Phase 2 findings:
| Severity | Issue | Location | Fix |
|---|---|---|---|
| High | SQL injection via unsanitized email | db/users.go:45 | Use parameterized query |
| Medium | Missing rate limiting | api/users.go:23 | Add rate limiter middleware |
| Low | Error message exposes internal details | api/users.go:67 | Return generic error to client |
Phase 3 verdict:
Verdict: NEEDS WORK
Critical/High Issues:
- SQL injection vulnerability must be fixed before merge
What's Done Well:
- Clean separation between handler and repository
- Comprehensive input validation struct
- Good test coverage for happy path
Action Items:
1. [High] Fix SQL injection in db/users.go:45
2. [Medium] Add rate limiting to endpoint
3. [Low] Sanitize error messages returned to client
Begin by gathering context on the target files/commits before conducting the multi-dimensional review.