| name | code-review-and-quality |
| description | Conduct staff-engineer-level code reviews across correctness, security, performance, readability, and architecture |
| difficulty | staff |
| domains | ["general"] |
Overview
Code review is the most important quality gate in software development. A good code review catches bugs, prevents security vulnerabilities, improves design, and transfers knowledge — before code reaches production. This skill runs a structured review across five dimensions.
When to Use
- Before any PR is merged
- When reviewing AI-generated code
- As part of the
/review workflow
Process
Dimension 1: Correctness
- Does the code do what the spec says?
- Are all edge cases handled? (empty inputs, null, zero, max values, concurrent access)
- Are error conditions handled? (network failure, invalid input, resource exhaustion)
- Are there off-by-one errors?
- Are there race conditions in concurrent code?
- Does the code handle partial failures?
Dimension 2: Security
- Is all user input validated before use?
- Are all database queries parameterized?
- Is no sensitive data logged?
- Are all auth/authz checks correct?
- Are there injection vulnerabilities? (SQL, shell, LDAP, XML)
- Are there new attack surfaces (new endpoints, new data types)?
- See
security-and-hardening for full checklist.
Dimension 3: Performance
- Are there N+1 query patterns?
- Are there missing database indexes?
- Is expensive computation done in a hot path?
- Are large data sets loaded into memory when streaming would work?
- Are there blocking synchronous calls that should be async?
- Does the change affect the critical path? Profile it.
Dimension 4: Readability and Maintainability
- Does the code express its intent clearly?
- Are names accurate? (Function names that lie are worse than no names)
- Is complexity necessary, or is there a simpler solution?
- Are there long functions that should be extracted?
- Is there duplicated logic that should be shared?
- Are there magic numbers or strings that should be constants?
- Will a new engineer understand this in 6 months?
Dimension 5: Architecture
- Does this change fit the existing design?
- Does this create new coupling that will be hard to remove?
- Does this introduce a new pattern when an existing one could be used?
- Does this create a new dependency that adds risk?
- Does this make the system harder or easier to test?
Review process
- Read the PR description. If it's missing, ask for it.
- Understand the spec/ticket the PR is implementing.
- Review the tests first — they tell you what the author thinks the code should do.
- Review the implementation against each dimension above.
- Run the code locally for non-trivial PRs.
- Leave specific, actionable comments. Not "this is wrong" — "this will fail when X because Y; consider Z."
Anti-Rationalizations
"The tests pass — it's good to go"
Tests passing proves the code passes the tests. It does not prove the tests are correct, complete, or that the code is secure.
"It's a small change — it doesn't need a full review"
Small changes have caused large incidents. The review checklist is short enough to run on any change.
Verification Requirements
