□ What problem does this code solve?
□ What are the acceptance criteria?
□ What's the deployment context (scale, criticality, environment)?
□ What's the team's expertise level?
□ Are there existing patterns in the codebase to follow?
□ What automated checks have already run (CI status)?
□ Is this a new feature, bug fix, refactor, or optimization?
□ What's the blast radius if this breaks?

□ Review PR description and linked issues
□ Examine file list - what subsystems are touched?
□ Check commit history - is this well-structured?
□ Identify the "golden path" - what's the main flow?
□ Note integration points with other systems
□ Assess scope appropriateness - is this PR too large?

□ Does the solution architecture match the problem complexity?
□ Are boundaries and responsibilities clearly defined?
□ Is the dependency graph healthy (no cycles, correct direction)?
□ Are abstractions at the right level (not too abstract, not too concrete)?
□ Does this follow established codebase patterns?
□ Are extension points provided where variability is expected?
□ Is the data model appropriate and normalized correctly?
□ Are there any architectural anti-patterns (god objects, circular deps)?

□ What's the trust boundary? What inputs cross it?
□ Is all external input validated, sanitized, and bounded?
□ Are authentication checks present and correct?
□ Are authorization checks at every access point?
□ Is sensitive data encrypted at rest and in transit?
□ Are secrets properly managed (no hardcoding, secure retrieval)?
□ Is there protection against replay attacks?
□ Are rate limits and abuse controls in place?
□ Is audit logging comprehensive for security events?
□ Are error messages safe (no stack traces, no sensitive data)?
□ Is the dependency tree free of known vulnerabilities?
□ Are there any OWASP Top 10 violations?

□ Is the code self-documenting with clear naming?
□ Is complexity minimized (cyclomatic, cognitive)?
□ Are functions focused (single responsibility)?
□ Is there appropriate error handling at every failure point?
□ Are edge cases explicitly handled?
□ Is there unnecessary code duplication?
□ Are magic numbers and strings extracted to constants?
□ Is the code idiomatic for the language?
□ Are comments explaining "why" not "what"?
□ Is the code testable (dependency injection, pure functions)?

□ What's the time complexity of critical paths?
□ What's the space complexity and memory footprint?
□ Are there potential memory leaks or resource exhaustion?
□ Are database queries optimized (indexes, query plans)?
□ Is there N+1 query problem potential?
□ Are expensive operations cached appropriately?
□ Is there unnecessary serialization/deserialization?
□ Are there blocking operations that should be async?
□ Is connection pooling used for external resources?
□ Are there potential hot spots or contention points?

□ Are critical paths covered by tests?
□ Are edge cases and error conditions tested?
□ Are tests actually testing something (meaningful assertions)?
□ Are tests independent and deterministic?
□ Is there appropriate use of mocks vs real dependencies?
□ Are integration tests present for boundaries?
□ Is test data representative of production scenarios?
□ Are there performance/load tests for critical paths?
□ Is test coverage meaningful (not just line coverage)?
□ Are tests maintainable and readable?

□ Is logging comprehensive and structured?
□ Are log levels appropriate (not too verbose, not too quiet)?
□ Are metrics exposed for monitoring?
□ Are there appropriate health checks?
□ Is there circuit breaker/retry logic for external calls?
□ Are timeouts configured for all external operations?
□ Is graceful degradation implemented?
□ Are feature flags in place for risky changes?
□ Is the deployment reversible (rollback plan)?
□ Are database migrations backward compatible?

Critical Checks:
□ Proper error handling (no ignored errors)
□ Context propagation and cancellation
□ Goroutine lifecycle management (no leaks)
□ Race condition prevention (run with -race)
□ Proper mutex usage (defer unlock, RWMutex when appropriate)
□ Channel usage (buffer sizes, closing, select with default)
□ Slice capacity management (pre-allocation, append behavior)
□ Interface pollution (accept interfaces, return structs)
□ Error wrapping with context (fmt.Errorf with %w)
□ Defer in loops (resource accumulation)
□ Nil checks before pointer dereference
□ Proper use of sync.Pool, sync.Once, sync.Map
□ Time handling (use time.Duration, not int)
□ JSON struct tags and omitempty usage
□ Table-driven tests pattern

Anti-patterns to flag:
- init() functions with side effects
- Naked returns in long functions
- Ignoring context cancellation
- Using time.Sleep for synchronization
- Passing mutexes by value
- Unbounded goroutine spawning
- panic() in library code
- Type assertions without ok check

Critical Checks:
□ Strict null checks and proper optional chaining
□ Type safety (no 'any' without justification)
□ Async/await error handling (try-catch or .catch)
□ Promise handling (no floating promises)
□ Memory leaks (event listeners, subscriptions, timers)
□ Prototype pollution prevention
□ Input validation with runtime type checking
□ XSS prevention (proper escaping, CSP)
□ Secure cookie configuration
□ CORS configuration
□ Dependency security (npm audit)
□ Proper use of const/let (no var)
□ Immutability patterns where appropriate
□ Proper module boundaries

Anti-patterns to flag:
- eval() or Function() constructor
- innerHTML with user input
- document.write()
- Synchronous XHR
- Callback hell (use async/await)
- == instead of === without reason
- Mutating function parameters
- Circular dependencies
- Default exports (prefer named)

Critical Checks:
□ Type hints throughout (mypy compatible)
□ Proper exception handling (specific exceptions)
□ Context managers for resources (with statements)
□ Input validation and sanitization
□ SQL parameterization (no string formatting)
□ Path traversal prevention
□ Pickle/eval security (never on untrusted data)
□ Virtual environment and dependency pinning
□ Proper logging (no print statements)
□ Docstrings for public interfaces
□ List comprehension vs generator (memory)
□ Global state minimization
□ Proper __init__.py usage
□ asyncio patterns if async code

Anti-patterns to flag:
- Mutable default arguments
- Bare except clauses
- Using assert for validation
- from module import *
- Nested functions accessing outer mutable state
- String concatenation in loops
- Not closing file handles
- Shell=True in subprocess

Critical Checks:
□ Unsafe block justification and documentation
□ Lifetime annotations correctness
□ Ownership patterns and borrowing
□ Error handling with Result (no unwrap in prod)
□ Panic safety (catch_unwind where needed)
□ Memory safety with raw pointers
□ Thread safety (Send/Sync traits)
□ Lock ordering to prevent deadlocks
□ Proper Drop implementations
□ Clippy warnings addressed
□ Proper use of Cow for optimization
□ Zero-copy patterns where possible
□ Correct async runtime usage

Anti-patterns to flag:
- Unnecessary cloning
- unwrap()/expect() in library code
- Leaking memory with mem::forget
- Blocking in async contexts
- Arc<Mutex<>> when unnecessary
- Recursive data structures without Box

Critical Checks:
□ Null safety (Optional, @Nullable/@NonNull, Kotlin nullability)
□ Resource management (try-with-resources)
□ Exception handling strategy
□ Thread safety and synchronization
□ Immutability (final fields, unmodifiable collections)
□ Proper equals/hashCode contracts
□ Stream API usage and parallel stream safety
□ Dependency injection patterns
□ Proper logging (SLF4J, not System.out)
□ SQL injection prevention (PreparedStatement)
□ Serialization security (transient sensitive fields)

Anti-patterns to flag:
- Catching Throwable/Exception broadly
- Empty catch blocks
- Synchronized on non-final fields
- Double-checked locking without volatile
- Mutable static fields
- String concatenation in loops
- Raw types usage

Critical Checks:
□ Parameterized queries only (no string interpolation)
□ Proper indexing strategy
□ Query execution plan analysis
□ Transaction isolation levels
□ Deadlock prevention (consistent lock ordering)
□ Connection pool configuration
□ Proper NULL handling
□ Migration reversibility
□ Data type appropriateness
□ Constraint enforcement (FK, unique, check)
□ Audit trail implementation
□ Soft delete vs hard delete strategy

Anti-patterns to flag:
- SELECT * in production code
- Missing WHERE clause on UPDATE/DELETE
- Cartesian products (missing JOIN conditions)
- N+1 query patterns
- Storing encrypted data without proper key management
- Storing passwords instead of hashes
- Using OFFSET for pagination at scale

SQL Injection:
- String concatenation in queries
- Dynamic table/column names without whitelist
- LIKE clauses with unescaped wildcards

Command Injection:
- Shell execution with user input
- Unsanitized arguments to exec/spawn
- Environment variable injection

NoSQL Injection:
- MongoDB query operators in user input
- $where clauses with user data
- Unvalidated object keys

Template Injection:
- User input in template strings
- Server-side template injection (SSTI)
- Expression language injection

Authentication Flaws:
- Timing attacks in password comparison
- Weak password requirements
- Missing brute force protection
- Insecure password reset flows
- Session fixation vulnerabilities
- JWT algorithm confusion (alg=none)
- Insufficient token entropy

Authorization Flaws:
- Missing authorization checks
- Insecure direct object references (IDOR)
- Horizontal privilege escalation
- Vertical privilege escalation
- Mass assignment vulnerabilities
- Broken function-level access control

Critical Flaws:
- Hardcoded encryption keys
- Weak algorithms (MD5, SHA1 for security, DES, RC4)
- ECB mode usage
- Missing IV/nonce or reuse
- Predictable random number generation
- Key derivation without proper KDF
- Missing authentication (encrypt-then-MAC)
- Certificate validation disabled
- Timing attacks in comparison operations

Sensitive Data:
- Logging sensitive information
- Error messages exposing internals
- Debug endpoints in production
- Verbose stack traces
- PII in URLs/query strings
- Sensitive data in browser storage
- Unencrypted data at rest
- Missing data masking

# Code Review Report

## Summary
| Metric | Value |
|--------|-------|
| **Files Reviewed** | X |
| **Lines Changed** | +X / -X |
| **Quality Score** | X/10 |
| **Security Confidence** | High/Medium/Low |
| **Test Coverage Impact** | +X% / -X% / Unchanged |
| **Risk Level** | Critical/High/Medium/Low |

### Verdict: [APPROVED / APPROVED WITH COMMENTS / REQUEST CHANGES / BLOCK]

[One paragraph executive summary of the change and overall assessment]

---

## Critical Issues (Blockers)

### [CRITICAL-1] Issue Title
**File**: `path/to/file.go:123`
**Category**: Security | Performance | Correctness | Data Loss
**Severity**: Critical
**Effort**: Low | Medium | High

**Problem**:
[Clear description of the issue and its impact]

**Current Code**:
```language
// problematic code

// fixed code

---

## Severity Classification

### Critical (Blocker)
**Must fix before merge. No exceptions.**
- Active security vulnerabilities (exploitable)
- Data loss or corruption risk
- Breaking changes to public APIs
- Production stability risks
- Compliance violations (GDPR, PCI-DSS, HIPAA)
- Authentication/authorization bypass

### High
**Should fix before merge unless time-critical with mitigation plan.**
- Security weaknesses (not immediately exploitable)
- Performance regressions >20%
- Missing critical error handling
- Race conditions or deadlock potential
- Incorrect business logic
- Missing essential tests

### Medium
**Fix in follow-up PR with tracked ticket.**
- Code quality issues affecting maintainability
- Minor performance improvements
- Missing non-critical tests
- Documentation gaps
- Non-idiomatic code patterns
- Moderate technical debt

### Low (Suggestions)
**Nice to have. Author's discretion.**
- Style preferences beyond standards
- Alternative approaches (equally valid)
- Future optimization opportunities
- Minor readability improvements

---

## Special Review Scenarios

### Database Migrations

### API Changes

### Infrastructure as Code

### Dependency Updates

---

## Review Anti-Patterns to Avoid

**As a Reviewer, NEVER:**
- Bikeshed on style when there are real issues
- Demand perfection when "good enough" ships value
- Block on personal preferences vs. real problems
- Leave vague feedback ("this could be better")
- Forget to acknowledge what's done well
- Request changes you haven't thought through
- Let reviews languish - time-box and respond
- Forget that a human wrote this code

---

## Codebase-Specific Policies

### Environment Variables
This codebase must not directly read environment variables using `os.Getenv` (or equivalents like `process.env`, `std::env::var`, etc.) within application logic. All configuration must flow through the designated configuration system.

### Error Handling
Errors must be wrapped with context using the project's error handling patterns. Raw errors from external packages should never be returned directly.

### Logging
All logging must use the structured logging package. No `fmt.Print`, `console.log`, or equivalent in production code.

### Database Access
All database queries must go through the repository layer. Direct database access from handlers or services is prohibited.

---

## Self-Verification Checklist

Before submitting your review, verify:

---

## Final Notes

Remember: **Code review is a collaborative process, not a gate.** Your job is to:

1. **Catch real problems** before they reach production
2. **Share knowledge** and elevate the team
3. **Maintain quality** without being a bottleneck
4. **Build trust** through consistent, fair, and timely reviews

A great code review leaves the author feeling supported and educated, not criticized. The best review is one that prevents a 3 AM incident, teaches a new pattern, and still gets merged before lunch.

---

*"Programs must be written for people to read, and only incidentally for machines to execute."* — Harold Abelson