Code Review
Overview
Thorough code review methodology: correctness, security, error handling, concurrency, and design. Think like a senior engineer reading this for the first time.
Core principle: Read everything before writing anything. Understand what it's trying to do, trace what it actually does, find where those diverge.
The Process
1. Establish Context
- What language, framework, and stack?
- What kind of code is this? (API, CLI, library, frontend, data pipeline)
- What is this code supposed to do?
- What changed? (diff, PR description, commit messages)
2. Read Everything
Read every file completely. Do not skim.
For each function, ask:
- What is this supposed to do?
- What does it actually do?
- Under what conditions do 1 and 2 diverge?
3. Mentally Simulate Execution
Run it in your head under adverse conditions:
- Empty, null, zero, or malformed input
- Two requests at the same time
- Slow or failing dependencies (database, API, filesystem)
- The 1000th call. Large datasets. Deep recursion.
- Attacker-controlled input
- Error paths — does cleanup still run?
4. Trace Data Flow
For any external input (user input, request body, env var, file, network):
- Where does it go?
- Is it validated?
- Does it reach a query, command, file path, log, or response unmodified?
What to Look For
Correctness
- Does the code do what it claims?
- Variables mutated inside loops corrupting logic
- Conditions that can never be true
- Off-by-one errors
- Return values silently discarded
Error Handling
- Every operation that can fail must handle it, propagate it, or document why it's safe to ignore
- Silent failures and swallowed exceptions are bugs
- Partial failures leaving inconsistent state are serious bugs
Security
- Trace user input: does it reach a query, shell command, file path, or response without sanitization?
- Auth checks: does it verify identity AND ownership?
- Can auth be bypassed by hitting a different route or HTTP method?
- Are secrets, tokens, or PII being logged?
- Passwords hashed properly? (bcrypt/argon2, not MD5/SHA1)
- Hardcoded credentials anywhere?
- Input validation: type checked, length bounded, format validated
Concurrency
- Shared state with unsynchronized access?
- Races between response and background work?
- Leaked goroutines/threads?
Resource Management
- Anything opened must be closed
- Cleanup runs on error paths
- No resource leaks inside loops
Data Correctness
- Floats for money or precision values
- Timezone-naive datetimes
- Integer overflow
- ReDoS on user-controlled regex
Design
- Is this solving the right problem?
- Is the abstraction appropriate?
- Would this be hard to test, extend, or understand in 6 months?
Output Format
Critical — broken behavior, data loss, security vulnerability, crash risk
Important — incorrect logic, unhandled errors, performance problems
Suggestion — improvements, naming, style
For each finding:
- File and line number
- What the code does vs. what it should do
- Concrete fix
Only report findings grounded in actual code with a specific line. Do not speculate.
Open with a brief summary: stack, what the code does, overall quality. Close with highest-priority items first.
Do not make edits. Report only.
1---2name: code-review3description: Use when reviewing code for quality, bugs, security, and correctness. Load before any code review, whether reviewing your own work or someone else's.4---56# Code Review78## Overview910Thorough code review methodology: correctness, security, error handling, concurrency, and design. Think like a senior engineer reading this for the first time.1112**Core principle:** Read everything before writing anything. Understand what it's trying to do, trace what it actually does, find where those diverge.1314## The Process1516### 1. Establish Context1718- What language, framework, and stack?19- What kind of code is this? (API, CLI, library, frontend, data pipeline)20- What is this code supposed to do?21- What changed? (diff, PR description, commit messages)2223### 2. Read Everything2425Read every file completely. Do not skim.2627For each function, ask:281. What is this supposed to do?292. What does it actually do?303. Under what conditions do 1 and 2 diverge?3132### 3. Mentally Simulate Execution3334Run it in your head under adverse conditions:35- Empty, null, zero, or malformed input36- Two requests at the same time37- Slow or failing dependencies (database, API, filesystem)38- The 1000th call. Large datasets. Deep recursion.39- Attacker-controlled input40- Error paths — does cleanup still run?4142### 4. Trace Data Flow4344For any external input (user input, request body, env var, file, network):45- Where does it go?46- Is it validated?47- Does it reach a query, command, file path, log, or response unmodified?4849## What to Look For5051### Correctness52- Does the code do what it claims?53- Variables mutated inside loops corrupting logic54- Conditions that can never be true55- Off-by-one errors56- Return values silently discarded5758### Error Handling59- Every operation that can fail must handle it, propagate it, or document why it's safe to ignore60- Silent failures and swallowed exceptions are bugs61- Partial failures leaving inconsistent state are serious bugs6263### Security64- Trace user input: does it reach a query, shell command, file path, or response without sanitization?65- Auth checks: does it verify identity AND ownership?66- Can auth be bypassed by hitting a different route or HTTP method?67- Are secrets, tokens, or PII being logged?68- Passwords hashed properly? (bcrypt/argon2, not MD5/SHA1)69- Hardcoded credentials anywhere?70- Input validation: type checked, length bounded, format validated7172### Concurrency73- Shared state with unsynchronized access?74- Races between response and background work?75- Leaked goroutines/threads?7677### Resource Management78- Anything opened must be closed79- Cleanup runs on error paths80- No resource leaks inside loops8182### Data Correctness83- Floats for money or precision values84- Timezone-naive datetimes85- Integer overflow86- ReDoS on user-controlled regex8788### Design89- Is this solving the right problem?90- Is the abstraction appropriate?91- Would this be hard to test, extend, or understand in 6 months?9293## Output Format9495**Critical** — broken behavior, data loss, security vulnerability, crash risk96**Important** — incorrect logic, unhandled errors, performance problems97**Suggestion** — improvements, naming, style9899For each finding:100- File and line number101- What the code does vs. what it should do102- Concrete fix103104Only report findings grounded in actual code with a specific line. Do not speculate.105106Open with a brief summary: stack, what the code does, overall quality. Close with highest-priority items first.107108Do not make edits. Report only.
justincordova