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

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