code-review
Code Review Workflow
Systematic validation of code against all guardrails before committing. Supports both automated checking and interactive review modes.
When to Use
| Trigger | Mode | Description |
|---|---|---|
| Pre-Commit | Automated | Before any commit |
| PR Review | Interactive | During pull request review |
| Feature Complete | Both | After FEATURE/COMPLEX mode completion |
| Code Handoff | Interactive | Before transferring ownership |
| Quality Gate | Automated | CI/CD pipeline integration |
Review Modes
Automated Mode
Quick validation against all guardrails. Returns pass/fail/warning status.
Best for: Pre-commit checks, CI/CD integration, quick validation.
Interactive Mode
Guided review with questions and confirmations. Deeper analysis.
Best for: PR reviews, complex changes, code handoffs.
Prerequisites
Before starting review:
- Code is in a reviewable state (compiles, runs)
- All files to review are identified
- Access to test results (if available)
- Access to coverage reports (if available)
Review Process
Phase 1: Code Quality Guardrails
↓
Phase 2: Security Guardrails
↓
Phase 3: Testing Guardrails
↓
Phase 4: Git Hygiene
↓
Phase 5: Report Generation
Phase 1: Code Quality Guardrails
1.1 Function Length Check
Guardrail: No function exceeds 50 lines
Check: Count lines in each function/method
Pass: All functions ≤ 50 lines
Warn: Functions 40-50 lines (approaching limit)
Fail: Any function > 50 lines
If Failed:
- Identify functions exceeding limit
- Suggest extraction points for helper functions
- Recommend refactoring approach
1.2 File Length Check
Guardrail: No file exceeds 300 lines (components: 200, tests: 300, utils: 150)
Check: Count lines in each file
Pass: Files within limits
Warn: Files at 80%+ of limit
Fail: Files exceeding limits
File Type Limits:
| Type | Limit | 80% Warning |
|---|---|---|
| Components | 200 | 160 |
| Tests | 300 | 240 |
| Utilities | 150 | 120 |
| Other | 300 | 240 |
1.3 Cyclomatic Complexity
Guardrail: Complexity ≤ 10 per function
Check: Analyze control flow (if, for, while, switch, &&, ||)
Pass: All functions ≤ 10
Warn: Functions 8-10
Fail: Any function > 10
Complexity Calculation:
- Start with 1
- +1 for each: if, elif, for, while, case, catch, &&, ||, ?:
1.4 Type Signatures
Guardrail: All exported functions have type signatures
Check: Verify exported functions have types
Pass: All exports typed
Warn: Internal functions missing types
Fail: Exported functions missing types
1.5 Documentation
Guardrail: All exported functions have documentation
Check: Verify docstrings/JSDoc on exports
Pass: All exports documented
Warn: Documentation exists but incomplete
Fail: Exported functions undocumented
1.6 Code Hygiene
Guardrails:
- No magic numbers (use named constants)
- No commented-out code
- No TODO without issue reference
- No dead code (unused imports, variables, functions)
Check: Scan for violations
Pass: None found
Warn: Minor violations (1-2 magic numbers)
Fail: Multiple violations
Phase 2: Security Guardrails
2.1 Input Validation
Guardrail: All user inputs validated before processing
Check: Identify input sources (API params, form data, URL params)
Pass: All inputs validated with schema or type checks
Warn: Validation exists but not comprehensive
Fail: Raw user input used directly
Input Sources to Check:
- Request body/params
- Query strings
- Headers
- File uploads
- Environment variables from user
2.2 Database Queries
Guardrail: All queries use parameterized statements
Check: Scan for SQL/query construction
Pass: All queries parameterized
Fail: String concatenation in queries
Red Flags:
// FAIL: String concatenation
`SELECT * FROM users WHERE id = ${id}`
// PASS: Parameterized
db.query('SELECT * FROM users WHERE id = $1', [id])
2.3 Secrets Detection
Guardrail: No secrets in code
Check: Scan for secret patterns
Pass: No secrets detected
Fail: Hardcoded secrets found
Patterns to Detect:
- API keys (
sk_live_,pk_live_,api_key=) - Passwords (
password=,passwd=) - Tokens (
token=,bearer) - Connection strings with credentials
- Private keys (
-----BEGIN RSA PRIVATE KEY-----)
2.4 File Path Validation
Guardrail: All file operations validate paths
Check: Identify file operations
Pass: Path validation/sanitization present
Fail: Direct user input in file paths
Red Flags:
// FAIL: Directory traversal possible
fs.readFile(userInput)
// PASS: Validated
const safePath = path.join(baseDir, path.basename(userInput))
2.5 Async Operations
Guardrail: All async operations have timeout/cancellation
Check: Identify async calls (fetch, DB queries, external APIs)
Pass: Timeouts configured
Warn: Some operations without timeout
Fail: No timeout handling
Phase 3: Testing Guardrails
3.1 Coverage Thresholds
Guardrail: >80% business logic, >60% overall
Check: Review coverage report
Pass: Meets thresholds
Warn: 70-80% business / 50-60% overall
Fail: Below thresholds
3.2 Test Existence
Guardrail: All public APIs have unit tests
Check: Map public functions to test files
Pass: All public APIs tested
Warn: Most tested (>80%)
Fail: Significant gaps (<80%)
3.3 Regression Tests
Guardrail: Bug fixes include regression tests
Check: If fix, verify test added
Pass: Regression test present
Fail: No regression test for bug fix
3.4 Edge Cases
Guardrail: Edge cases explicitly tested
Check: Review test cases for boundaries
Pass: Null, empty, boundary values tested
Warn: Some edge cases missing
Fail: No edge case testing
Required Edge Cases:
- Null/undefined inputs
- Empty strings/arrays
- Boundary values (0, -1, MAX_INT)
- Invalid types
- Concurrent access (if applicable)
3.5 Test Independence
Guardrail: No test interdependencies
Check: Tests can run in any order
Pass: Tests isolated
Fail: Tests depend on execution order
Phase 4: Git Hygiene
4.1 Commit Message Format
Guardrail: type(scope): description (conventional commits)
Check: Verify commit message format
Pass: Follows convention
Fail: Non-conventional format
Valid Types: feat, fix, docs, refactor, test, chore, perf, ci
4.2 Atomic Commits
Guardrail: One logical change per commit
Check: Review commit scope
Pass: Single logical change
Warn: Related changes (acceptable)
Fail: Unrelated changes bundled
4.3 Sensitive Data
Guardrail: No sensitive data in commits
Check: Scan staged files
Pass: No sensitive data
Fail: Secrets, credentials, or PII found
Phase 5: Report Generation
5.1 Automated Report Format
## Code Review Report
**Date**: 2025-01-15
**Files Reviewed**: 12
**Status**: ⚠️ WARNINGS (3 issues)
### Summary
| Category | Status | Issues |
|----------|--------|--------|
| Code Quality | ✅ Pass | 0 |
| Security | ⚠️ Warn | 2 |
| Testing | ✅ Pass | 0 |
| Git Hygiene | ⚠️ Warn | 1 |
### Issues Found
#### Security Warnings
1. **Missing timeout** in `src/api/fetch.ts:42`
- External API call without timeout
- Recommendation: Add 30s timeout
2. **Input validation** in `src/routes/users.ts:15`
- Query parameter used without validation
- Recommendation: Add Zod schema
#### Git Hygiene Warnings
1. **Large commit scope**
- 8 files changed across 3 features
- Recommendation: Split into separate commits
### Passed Checks
- ✅ All functions ≤ 50 lines
- ✅ All files within size limits
- ✅ No secrets detected
- ✅ Coverage at 82%
- ✅ Commit message format correct
5.2 Interactive Report Format
Interactive mode includes prompts:
## Interactive Review: src/api/users.ts
### Function: createUser (lines 15-45)
**Observations**:
- 30 lines (within limit)
- Complexity: 6 (acceptable)
- Missing error handling for database failure
**Questions**:
1. Should we add explicit error handling for DB failures?
2. Should the validation schema be extracted to a separate file?
**Your Response**: [awaiting input]
Integration with CI/CD
GitHub Actions Example
# .github/workflows/code-review.yml
name: Code Review Checks
on: [pull_request]
jobs:
review:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Function Length Check
run: |
# Check no function > 50 lines
# Implementation depends on language
- name: Security Scan
run: |
# Run secret detection
# Run input validation check
- name: Coverage Check
run: |
npm test -- --coverage
# Verify thresholds
Checklist
Pre-Commit (Automated)
- All functions ≤ 50 lines
- All files within size limits
- Complexity ≤ 10
- No secrets in code
- Inputs validated
- Tests pass
- Coverage meets thresholds
- Commit message follows convention
PR Review (Interactive)
- All automated checks pass
- Code is understandable
- Edge cases handled
- Error handling appropriate
- No premature optimization
- No over-engineering
- Documentation updated
- Tests cover new functionality
Related Workflows
- security-audit.md - Deeper security analysis
- testing-strategy.md - Comprehensive test planning
- refactoring.md - When review identifies debt
- troubleshooting.md - When review finds issues