Code review
Status: 🟩 COMPLETE (🟦 LIVING — AI-assisted review is evolving fast) Last updated: 2026-06-19 Plain-English tagline: A second pair of eyes — human, AI, or both — on a code change before it merges. Catches what no tool can: design smells, missing edge cases, security concerns, and “this isn’t how this codebase does it.”
In plain English
Once your code passes linting (Linting), type checking (Type checking), and tests (Why test?), there’s still a class of issues those tools can’t catch:
- “This approach works, but it duplicates a utility we already have”
- “This name is misleading —
handleDatadoes three different things” - “You added a new database query but didn’t add an index — this will be slow at scale”
- “This is technically fine but reads weirdly. Could you split it into two functions?”
- “This is missing a check for the case where the user owns no posts”
- “Are you sure this should be public?”
These are JUDGMENT issues. They require human understanding of intent, codebase conventions, security context, scaling pressures, future needs. Tools can hint at some; a thoughtful reviewer catches them all.
Code review is the practice of having a SECOND READER examine a code change before it merges. In modern workflows, this happens on a pull request (PR) — the author proposes a change, the reviewer reads it, requests changes or approves, and only then does the code land.
In 2026, code review has expanded:
- Human review — the original. A teammate (or your future self, on a solo project) reads the diff and asks questions.
- AI review — Claude Code, Greptile, CodeRabbit, GitHub’s own code review tools — an LLM analyzes the diff for bugs, smells, and security issues. Fast, tireless, useful for catching common mistakes.
- Self-review — opening your own PR and reading the diff as a stranger would. Catches an embarrassing number of issues before anyone else sees them.
For solo developers, code review can feel pointless (“I’m reviewing my own code?”). It’s not. Self-review and AI-assisted review catch real things. Future-you, six months from now, IS the reviewer of present-you’s code.
Why it matters
Three concrete things code review catches:
-
Bugs the tools missed. A loop that’s off-by-one but compiles fine. An auth check that’s in the wrong order. A race condition. A test that doesn’t actually assert what it claims.
-
Architecture drift. New code that duplicates existing utilities, introduces a third way to do the same thing, or violates a pattern the rest of the codebase follows. Over time, ungoverned drift is what makes a codebase unmaintainable.
-
Knowledge sharing. Reading other people’s code is HOW developers learn a codebase. PRs become history — searchable explanations of why a thing was done a certain way. The discussion in a PR is often more valuable than the code itself.
For solo developers building on a long timeline, the second point matters most. Without review, every PR is a chance for the codebase to drift in a slightly different direction. Six months in, half the code feels foreign even to the person who wrote it.
What a good PR looks like
Before review even starts, the AUTHOR shapes how the review goes. The shape of a good PR:
| Element | What it looks like |
|---|---|
| Small diff | Ideally under 400 lines added/changed. Larger PRs get rubber-stamped. |
| Single concern | One feature, one bug fix, or one refactor. Not three at once. |
| Clear title | ”Fix: posts can be created without a title” beats “stuff”. |
| Description | Why the change, what changed, anything to watch for. Links to tickets / specs. |
| Tests | Especially for bug fixes — a test that fails BEFORE the fix, passes AFTER. |
| No commented-out code | Either keep it or delete it. |
| No unrelated changes | Reformatting one file you didn’t otherwise touch is noise; do it in a separate PR. |
| Passing CI | All checks green before requesting review. |
| Self-reviewed | The author has re-read the diff as if a stranger. |
A PR that follows these takes 5 minutes to review. A 1000-line PR with mixed concerns and “ran formatter on whole repo” takes an hour and gets a worse review.
What a reviewer actually looks for
In order of importance:
1. Correctness
- Does the logic actually do what the description says?
- Are there edge cases the code doesn’t handle? (Null inputs, empty arrays, zero, negatives, unicode, very large inputs)
- Is there an off-by-one in any loop?
- Is the database query right?
- Are race conditions possible?
2. Security
- Is user input validated at the boundary?
- Are auth checks present where needed (and in the right order)?
- Are secrets staying server-side?
- Could a malicious user exploit this code path?
3. Test coverage
- Is the new behavior tested?
- For bug fixes, is there a regression test?
- Are the tests meaningful, or do they just exercise code without asserting anything important?
4. Design
- Does this fit the existing patterns in the codebase?
- Could this be simpler?
- Is the new function/component named well?
- Could this duplicate something that already exists?
5. Readability
- Will future-me (or a teammate) understand this in 3 months?
- Are variable names clear?
- Are non-obvious decisions documented (in comments OR commit message)?
6. Performance
- Does this introduce a query in a loop?
- Are there obvious inefficiencies?
- Does it cache when it should, or cache when it shouldn’t?
7. Documentation
- Did the change touch something the README/AGENT_GUIDE/PROGRESS should reflect?
Most reviews catch issues in categories 1–4. Performance and docs are often noticed in passing.
A concrete example: a review conversation
The author posts a PR titled “Add password reset endpoint”:
// app/api/auth/reset-password/route.ts
export async function POST(req: NextRequest) {
const { email } = await req.json();
const user = await db.users.findFirst({ where: { email } });
if (user) {
const token = generateToken();
await db.passwordResetTokens.create({ data: { token, userId: user.id } });
await sendEmail(user.email, `Reset: ${process.env.SITE_URL}/reset?token=${token}`);
}
return NextResponse.json({ ok: true });
}A thorough review might leave these comments:
- “What if
emailis missing or not a string?” Use Zod to validate input. A POST with{ email: { $ne: null } }could shape-shift the query. - “
if (user)reveals which emails exist.” Return{ ok: true }always, regardless of whether the user was found. Otherwise this is a user enumeration vulnerability. - “No rate limiting.” Without it, an attacker can spam password resets. Add per-IP or per-email rate limit.
- “
generateToken— where does that come from?” Confirm it usescrypto.randomBytesnotMath.random. - “The token has no expiration.” Add
expiresAt: new Date(Date.now() + 30 * 60 * 1000)and check it on use. - “Email sending failures aren’t handled.” If
sendEmailthrows, the request returns 500 instead of the intentional{ ok: true }. Wrap in try/catch. - “Are we logging anything for audit?” A password reset is security-sensitive; log who, when, success/failure.
This is the kind of review NO automated tool catches. The compiler is fine with the code. The lint passes. Even tests might pass for the happy path. The issues are domain knowledge: how password resets ought to work.
This is the value: another mind, with security and edge-case experience, sees what the author didn’t.
AI-assisted code review in 2026
The landscape has shifted dramatically in the last 2-3 years. AI tools now do meaningful code review:
| Tool | What it does |
|---|---|
| Claude Code | Run /code-review (or ultrareview) on a branch — multi-agent cloud review with deep analysis. Reports correctness bugs and reuse/simplification findings. |
| GitHub Copilot Workspace / Code Review | Inline AI comments on PRs in GitHub. Catches typical bugs and suggests fixes. |
| Greptile | Cloud service that reviews PRs with awareness of the rest of the codebase. |
| CodeRabbit | Similar — PR-level AI review. |
| Cursor / Aider review modes | Editor-integrated AI review. |
What AI is good at:
- Catching typos and obvious mistakes (often before human review)
- Suggesting simplifications
- Pointing out missing edge cases (null, empty, boundary)
- Spotting some security smells (SQL injection patterns, missing auth checks)
- Recommending consistency with the rest of the codebase
What AI is NOT good at (yet):
- Deep domain knowledge (“this should use a CTE because we run this query during peak load”)
- Architectural fit (“we just discussed not adding a new utility module”)
- Subtle race conditions
- Performance issues that require profiling
- Genuinely creative design feedback
The 2026 best practice: AI review FIRST, then a human (even just future-you) reads the AI’s findings and the code. AI catches the obvious stuff; the human focuses on judgment calls.
For solo developers on the Bible Quest stack, /code-review or ultrareview before merging is now a normal part of the workflow. It catches genuine bugs regularly.
Self-review — the underrated skill
If you can’t get a human reviewer, BE one. The discipline:
- Open your own PR
- Wait 5-10 minutes (or until tomorrow)
- Read the diff as if someone else wrote it
- Comment on YOUR OWN PR every time you’d say “huh, that’s odd” or “wait, did I…?”
The time gap is the secret. Reading your own code immediately after writing it feels fine — your brain pattern-matches what you INTENDED. Reading it tomorrow forces you to read what’s actually there.
Solo developers who do this consistently catch 30-50% of issues they would have shipped otherwise. It’s free quality.
How to leave a good review comment
Good comments are SPECIFIC and ACTIONABLE:
❌ “This could be better.”
✅ “Consider extracting lines 12-25 into a validateOrder helper — they’re duplicated in the cancel-order route.”
❌ “Bug?”
✅ “If user.email is undefined, this throws on line 8. Should we early-return with a 400, or guarantee email exists upstream?”
❌ “Why?” ✅ “Why catch and swallow this error? If it fails, shouldn’t the user know?”
❌ “Nitpick: I’d name this differently.”
✅ “Nit: processUser is vague — could it be validateAndSaveUser? (Won’t block.)”
The pattern: start with the observation, give the reasoning, suggest a path forward. Tag tone-only suggestions clearly (“nit:”, “suggestion:”) so the author can decide whether to act.
Avoid:
- Personal attacks (“This is bad code”)
- Vague gripes (“Don’t love this”)
- Bikeshedding (arguing about variable names for an hour while a security issue sits unaddressed)
- Treating coding style preferences as if they were correctness issues
When to approve vs request changes vs comment
Most platforms (GitHub, GitLab, Bitbucket) offer three review states:
| State | Meaning |
|---|---|
| Comment | ”I read it. Here are notes. Use your judgment.” |
| Approve | ”I’m satisfied. You can merge.” |
| Request changes | ”Do not merge until you address these.” |
Use them with care:
- “Approve” doesn’t mean “perfect” — it means “good enough to ship, blocking issues addressed.”
- “Request changes” should be reserved for actual blockers: bugs, security issues, broken tests. Don’t request changes for nits.
- “Comment” is fine when you have suggestions but aren’t blocking.
For solo work, the GitHub UI lets you self-approve your own PR (which most teams configure to require ANOTHER reviewer for non-solo work). Even for solo projects, the formal “approve” click is a useful “I have reviewed this consciously” gesture.
The 24-hour rule
A pragmatic guideline: don’t merge your own PR within 24 hours unless it’s urgent. Reasons:
- You may notice issues yourself overnight (the “shower thought” effect)
- Tests may run on more environments / CI fully completes
- A human or AI reviewer has a chance to weigh in
- The deployment risk is lower if you can revert during the next workday
For hotfixes during outages: merge fast. For everything else: a day’s pause is cheap insurance.
How code review interacts with the rest of the workflow
The full quality pipeline for Bible Quest-style projects:
1. Author writes code locally
↓
2. Editor: TypeScript + ESLint catch typos and obvious bugs
↓
3. Pre-commit hook (Husky + lint-staged): auto-format + auto-fix
↓
4. Author opens PR
↓
5. CI runs: lint, type-check, tests, build
↓
6. AI review (Claude Code /code-review): catches bugs, suggests improvements
↓
7. Self-review: author re-reads after a pause
↓
8. Human review (if applicable): a second mind
↓
9. Merge → production
Each step catches a different class of issue. Skipping any one of them lets a category of bug through.
Common gotchas
-
Big PRs get rubber-stamped. A 2000-line diff is impossible to review meaningfully. People skim and approve. Either you trust the author, or the review is theater. Keep PRs small.
-
Auto-approval is a code review smell. Some teams have CI that auto-approves PRs if a label is set. Useful for trusted tooling; dangerous as a default.
-
“LGTM” without reading is a signature on a check you didn’t write. If you don’t have time, say so — don’t approve.
-
Don’t review when angry or tired. A tired reviewer misses obvious things. A reviewer in conflict with the author may be petty. Wait.
-
Tone in comments lives forever. A snarky review comment is a permanent record. Be kind, be specific, be helpful — assume the author had reasons even if they’re wrong now.
-
Don’t ship without re-reading after pushing. Force-pushes to a PR can silently overwrite something. Look at the final diff before merging.
-
Old PRs go stale. A PR that sits for two weeks may have merge conflicts, deprecated dependencies, or solve a problem that’s now solved differently. Rebase and re-review or close.
-
AI review can hallucinate problems. Claude may flag “this might be a SQL injection” on code that’s actually safe. Trust but verify; don’t blindly act on every suggestion.
-
AI review can miss real problems. It tends to focus on patterns it’s seen many times; novel architectures escape it. Pair AI with human review for high-stakes code.
-
Bikeshedding is the time-killer of code review. Arguing about variable names, tabs vs spaces, brace style — all topics that should be settled by automation (linter, formatter) at the project level, not relitigated per PR.
-
“This isn’t how WE do it” needs evidence. A reviewer claiming “we don’t do X” should point to the convention (a doc, another file, an ADR). Otherwise it’s a personal preference.
-
Don’t approve before CI passes. It tempts the author to merge before reviewing the failing CI. Wait.
-
Don’t review in the GitHub diff alone. For complex changes, pull the branch locally and run it. The diff hides interactions across files.
-
Reviewers can’t catch what authors hide. Code that’s “behind a feature flag” is exempt from review pressure in many teams; it shouldn’t be.
-
“Approve with comments” can mean nothing. Some reviewers approve with 20 comments. The author then ignores the comments and merges. Either request changes (blocking) or comment without approving until the author engages.
-
Reviews focused only on style miss the substance. A review that argues about variable names but misses the missing auth check is bad review. Triage priorities.
-
“This passes CI, so it must be fine” is wrong. CI catches mechanical issues. Reviewers catch judgment issues. They’re complementary.
-
Don’t review your own merged PRs and then push fix-ups. Open a new PR for the fix. Keeps the history clean and the second change reviewable.
-
git blamelies after large refactors. A review that adds value during the PR is more useful than archaeology a year later when blame points at someone who didn’t write the original logic. -
AI review tools differ in quality. Some are excellent; some are noise. Try a few and find what catches genuine issues for your codebase. Disable noisy ones rather than ignoring them.
-
For solo projects, structure beats discipline. Force yourself to open PRs (don’t push directly to main); force CI to run; force a 24-hour wait when possible. The structure does the work that a colleague would.
-
Code review is a teaching opportunity. A good review explains WHY, not just WHAT. “This is wrong” educates nobody; “This loses one second of data because Postgres TIMESTAMPS don’t have microseconds” teaches.
-
Comment threads can drift. A debate in the comments distracts from the actual review. Long discussions belong in chat, video calls, or a follow-up; the PR comments should be the conclusion.
-
The author is responsible for the merge. Reviewers don’t merge; reviewers approve. The author decides when to merge (after addressing feedback). This keeps responsibility clear.
-
AI review can violate the build budget. Heavy AI review on every PR can be slow and costly. Run it as a separate optional check, not a blocker, unless you’ve made the cost-benefit case.
-
A PR with “no changes” sometimes is a change. Look at whitespace, file moves, renames. Sometimes “this PR is empty” is actually a no-op refactor — but sometimes there’s a hidden change.
See also
- Why test? đźź©
- Unit tests đźź©
- Integration tests đźź©
- End-to-end (E2E) tests đźź©
- Linting đźź©
- Type checking đźź©
- Pull requests 🟩 — the mechanism review happens through
- Branches & merging đźź©
- CD 🟩 — automation that complements review
- Claude Code deep dive 🟩 — the canonical AI review tool in this stack
- Slash commands 🟩 —
/code-review,/security-review - OWASP Top 10 🟩 🟦 — what security-focused review checks for
- Glossary: Code review, Pull request, LGTM
Sources
- Google — Code review developer guide — the canonical large-org practice
- Conventional Comments — prefixing convention for review comments
- GitHub — About pull request reviews
- The Pragmatic Programmer (book) — chapters on code review and pair programming
- Anthropic — Claude Code Code Review skill — official AI review tooling
- Greptile, CodeRabbit — AI review tooling alternatives