| name | code-review-and-quality |
| description | Performs multi-axis code review before merge. Use when a Kanban card enters the review column, when evaluating code another agent produced, or before merging any change. Reviews across correctness, readability, architecture, security, and performance. |
Code Review and Quality
Overview
Every change gets reviewed before merge — no exceptions. Review covers five
axes: correctness, readability, architecture, security, performance.
Approval standard: Approve when the change definitely improves overall
code health, even if it isn't perfect. Don't block because it isn't exactly
how you would have written it. Don't rubber-stamp either.
When to Use
- A Kanban card has moved to the
review column
- Before merging any PR
- When evaluating code another agent produced
- After completing a feature implementation
Process
Step 1: Understand the Context
- What is this change trying to accomplish?
- What spec or task does it implement?
- What is the expected behavior change?
Step 2: Review the Tests First
- Do tests exist for the change?
- Do they test behavior (not implementation details)?
- Are edge cases covered?
- Would the tests catch a regression if the code changed?
Step 3: Review the Implementation
For each file changed, walk the five axes:
- Correctness — Does this code do what the test says it should? Edge cases, error paths, off-by-one, race conditions.
- Readability — Can another engineer understand this without the author explaining it? No clever tricks. Could this be done in fewer lines?
- Architecture — Does this fit the system's design? Existing patterns? Clean module boundaries? No circular deps?
- Security — Input validated at boundaries? No secrets in code? Auth checks in place? External data treated as untrusted?
- Performance — N+1 queries? Unbounded loops? Sync ops that should be async? Missing pagination?
Step 4: Categorize Findings
| Prefix | Meaning |
|---|
| (no prefix) | Required change — must address before merge |
| Critical: | Blocks merge — security, data loss, broken functionality |
| Nit: | Minor — author may ignore |
| Optional: / Consider: | Suggestion worth considering |
| FYI | Informational only |
Step 5: Verify the Verification
- What tests were run? Cite the command and result.
- Did the build pass?
- Was the change tested manually?
- Screenshots for UI changes?
Common Rationalizations
| Rationalization | Reality |
|---|
| "It works, that's good enough" | Working code that's unreadable, insecure, or architecturally wrong creates debt that compounds. |
| "I wrote it, so I know it's correct" | Authors are blind to their own assumptions. Every change benefits from another set of eyes. |
| "We'll clean it up later" | Later never comes. The review is the quality gate — use it. |
| "AI-generated code is probably fine" | AI code needs more scrutiny, not less. It's confident and plausible, even when wrong. |
| "The tests pass, so it's good" | Tests are necessary but not sufficient. They don't catch architecture problems, security issues, or readability concerns. |
Red Flags
- PRs merged without any review
- Review that only checks if tests pass (ignoring other axes)
- "LGTM" without evidence of actual review
- Security-sensitive changes without security-focused review
- Large PRs that are "too big to review properly" — split them
- No regression tests with bug-fix PRs
- Comments without severity labels
Verification
After review is complete:
