Code Review Skill
Review code changes following enterprise-grade engineering standards. The checklists, comment templates, and command patterns here apply to any software project regardless of stack (examples use TypeScript / Node / React; the discipline is stack-agnostic).
Review Process
- Understand the change - Read the PR description and commits
- Check for issues - Security, bugs, performance, maintainability
- Verify patterns - Ensure code follows project conventions
- Test coverage - Check if tests cover the changes
- Provide feedback - Clear, actionable comments
Review Checklist by Area
Security (CRITICAL)
- No hardcoded secrets, passwords, or API keys
- Input validation on all user inputs
- SQL injection prevention (parameterized queries)
- XSS prevention (output encoding)
- Authentication checks on protected endpoints
- Authorization checks (role-based access)
- No sensitive data in logs
- Secure error messages (no stack traces to client)
TypeScript/JavaScript
- No
anytypes - use proper type definitions - No unused variables or imports
- Async/await error handling (try/catch)
- No console.log in production code
- Proper null/undefined checks
- No hardcoded strings (use constants)
React/Frontend
- Components follow naming convention (PascalCase)
- Hooks follow rules (no conditional hooks)
- Loading states handled
- Error states handled
- Empty states handled
- Form validation implemented
- Accessibility (ARIA labels, keyboard nav)
- Responsive design verified
- Error handling uses the project's centralized error module
- API routes use the shared error handler utility, not inline ad-hoc error construction
- Components use the project's API call wrapper, never raw
new Error(message)directly - Error messages come from the project's error-code registry, no duplicated message strings
- Form errors use the shared form-error handler for consistent toast and field display
Backend/API
- DTOs defined for request/response
- Input validation with proper error messages
- Authentication middleware applied
- Structured logging at key points
- Error handling with proper status codes
- Multi-tenant queries are scoped to the correct tenant
- Transactions for multi-table operations
[CUSTOMIZE: additional language/runtime]
Add a section here for each additional language or runtime your project uses (Go, Python, Java, etc.). For each, list:
- Authorization check is FIRST operation
- All inputs validated
- Errors wrapped with context
- Events or side-effects emitted for state changes
- Idempotency handled for non-idempotent operations
Database
- Migrations are reversible
- Indexes on frequently queried columns
- Foreign keys properly defined
- No N+1 query patterns
- Transactions for related changes
Comment Templates
Request Change
**Issue:** [Brief description]
[Explanation of the problem]
**Suggestion:**
```suggestion
// Fixed code here
### Ask Question
Question: [Your question]
[Context or reason for asking]
### Approve with Note
Note: [Observation]
[Minor suggestion or future consideration - not blocking]
### Security Concern
Security: [Issue description]
[Explanation of risk]
Required fix: [How to fix it]
---
## Common Issues to Flag
### Security
```typescript
// BAD: SQL injection risk
const query = `SELECT * FROM users WHERE id = '${userId}'`;
// GOOD: Parameterized query (using your ORM)
const user = await db.user.findUnique({ where: { id: userId } });
Error Handling
// BAD: Swallowing errors
try {
await doSomething();
} catch (e) {
// Silent fail
}
// GOOD: Handle or propagate
try {
await doSomething();
} catch (error) {
logger.error({ error }, 'Failed to do something');
throw error;
}
Type Safety
// BAD: any type
const data: any = response.data;
// GOOD: Proper typing
interface UserResponse {
id: string;
name: string;
}
const data: UserResponse = response.data;
React Performance
// BAD: Creating function on every render
<Button => handleClick(item.id)}>Click</Button>
// GOOD: Memoized callback
const handleItemClick = useCallback((id: string) => {
handleClick(id);
}, [handleClick]);
Async State
// BAD: Not handling loading/error
const { data } = useQuery(...);
return <div>{data.name}</div>; // Crashes if data is undefined
// GOOD: Handle all states
const { data, isLoading, isError } = useQuery(...);
if (isLoading) return <Skeleton />;
if (isError) return <ErrorState />;
return <div>{data.name}</div>;
Review Commands
Get PR Diff
# View PR changes
gh pr diff <number>
# View specific file changes
gh pr diff <number> -- path/to/file.ts
# View PR details
gh pr view <number>
Checkout PR Locally
# Checkout PR branch
gh pr checkout <number>
# Run tests locally
npm test
# Build locally
npm run build
Add Review Comments
# Start review
gh pr review <number>
# Approve
gh pr review <number> --approve -b "LGTM!"
# Request changes
gh pr review <number> --request-changes -b "Please address the security concern."
# Comment only
gh pr review <number> --comment -b "A few suggestions..."
Review Response Format
## Review Summary
**Status:** [Approve / Request Changes / Comment]
### What I Reviewed
- [List of files/features reviewed]
### Findings
#### Critical (Must Fix)
- [ ] [Issue 1 with file:line reference]
- [ ] [Issue 2 with file:line reference]
#### Important (Should Fix)
- [ ] [Issue with explanation]
#### Minor (Consider)
- [ ] [Suggestion or optimization]
### What Looks Good
- [Positive feedback on well-done aspects]
### Questions
- [Any questions about the implementation]
Apply to: $ARGUMENTS
Source: mnzralee/claude-multi-agent-architecture — distributed by TomeVault.