Code Review Workflow¶

Purpose: 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]

Quick Commands¶

Run Automated Review¶

# Conceptual - AI performs these checks
@.claude/skills/code-review/SKILL.md --mode=automated

Run Interactive Review¶

# Conceptual - AI guides through review
@.claude/skills/code-review/SKILL.md --mode=interactive

Review Specific Files¶

# Conceptual - Focus on specific files
@.claude/skills/code-review/SKILL.md --files=src/api/*.ts

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)¶

PR Review (Interactive)¶

security-audit.md - Deeper security analysis
testing-strategy.md - Comprehensive test planning
refactoring.md - When review identifies debt
troubleshooting.md - When review finds issues