code-review

Files (2)

• SKILL.md
• code-review-template.md · references/

SKILL.md (first lines)

# 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

1. **Identify the scope:**
   - Determine which files/commits to review (diff, specific files, or branch comparison)
   - Get the diff: `git diff` for the relevant range

2. **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

3. **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:

1. Walk through the relevant checklist
2. Note any issues with severity (Critical/High/Medium/Low)
3. Record the specific location (file:line)
4. Document suggested fixes

### Phase 3: Synthesis

Produce a structured review following the template in `references/code-review-template.md`.

1. **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

2. **Organize findings** by severity
3. **Acknowledge strengths** - what's done well
4. **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

From thoreinstein/agents, MIT.

More from thoreinstein/agents