Code Review Antipatterns That Slow Teams Down

Code review slows teams down when it stops being focused risk reduction and turns into unstructured judgment.

The pull request may still get comments. The checklist may still pass. The required approvals may still be collected. But feedback arrives late, low-risk preferences crowd out real defects, authors rewrite the same code several times, and reviewers leave the review feeling busy rather than confident.

That is the uncomfortable part: code review can look active while doing a poor job of protecting the system.

A useful review process helps engineers answer a smaller set of questions quickly: what changed, what can break, what risk needs human judgment, what belongs in this pull request, and which comments are blocking versus optional.

For the wider set of engineering habits around decisions, maintainable code, debugging, testing, and AI-assisted review, see the Software Engineering Fundamentals hub.

Why Code Review Starts Feeling Expensive

Most teams do not have slow reviews because engineers dislike quality.

They have slow reviews because review is carrying too many hidden jobs at once:

• finding defects
• teaching local conventions
• enforcing style
• approving architecture
• catching missing tests
• transferring knowledge
• checking product behavior
• protecting production reliability
• negotiating scope

Those are all legitimate concerns. The problem is that many pull requests do not tell reviewers which concern matters most.

So reviewers guess.

One reviewer comments on naming. Another asks for a different abstraction. A third notices the missing authorization check. The author treats all comments as equally blocking because the review tool displays them in the same shape. The important issue eventually gets fixed, but only after several cycles of mixed-priority feedback.

That is review friction, not review quality.

GitHub's pull request review docs describe the mechanics clearly: reviewers can comment, approve, or request changes, and repository rules may require approving reviews before merge. See GitHub's docs on pull request reviews. Those mechanics are useful, but they do not decide what feedback should matter most.

The team still has to design that part.

The Reasonable Assumption That Fails

The common assumption is:

More careful review should produce better code.

That is only partly true.

Careful review helps when the carefulness is aimed at the real risk. It slows the team down when every concern gets the same weight.

A team can have strict review and still miss production defects if the process optimizes for comment volume, approval count, or stylistic consistency instead of system risk.

The better question is not "was this reviewed?"

The better question is:

Did review reduce the uncertainty that mattered for this change?

That question changes how the team treats pull request size, descriptions, comment severity, tests, and reviewer assignment.

Antipattern 1: The Pull Request Has No Review Target

Consider this pull request description:

Title: Update billing flow

Description:
- refactor invoice service
- add retry handling
- clean up old helper
- update tests

The author may know the real risk. Reviewers do not.

Is the important part the retry behavior? The invoice state transition? The refactor? The tests? A migration? A dependency change?

Without a review target, reviewers scan everything with the same attention and comment on whatever is easiest to notice.

A better description gives reviewers a path:

Title: Add retry handling for invoice creation timeouts

Intent:
When the invoice provider times out after accepting a request, retry the operation
without creating duplicate invoices.

Risk:
The risky path is provider timeout after remote success but before local state is saved.

Review focus:
1. Does the idempotency key prevent duplicate provider invoices?
2. Does the state transition preserve enough information for support?
3. Do the tests cover timeout-after-success and retry-after-failure separately?

Out of scope:
The helper rename is mechanical; no behavior change intended there.

That description does not make the change safe by itself. It makes the review more likely to inspect the right thing first.

Google's review guidance recommends taking a broad view of a change first, then examining the main parts before moving through the rest. See Google's guide on navigating a change in review.

The pull request description should make that broad view cheap.

Antipattern 2: Oversized Pull Requests Hide The Risk

Large pull requests create two different problems:

• reviewers need more time to build a mental model
• the real risk gets hidden among unrelated changes

That second problem is the dangerous one.

If the same pull request changes the payment state machine, renames files, updates formatting, adds tests, moves helpers, and changes retry policy, the review becomes a search problem. The risky lines are mixed with noise.

Google's CL author guide argues for small, self-contained changes because they are reviewed more quickly and thoroughly, are easier to reason about, and are simpler to roll back. See Google's guide on small CLs.

For a normal product team, the practical rule is:

Splitting the work is not bureaucracy. It protects review attention.

A useful split might be:

1. rename helper without behavior change
2. add missing tests around current behavior
3. change retry state transition
4. remove old code after rollout

Each pull request now has a clearer review target.

Antipattern 3: Review Comments Have No Severity

This is a typical slow review:

Can we rename this?
This needs an authorization check.
Maybe extract this helper?
Should this have a test?
Nit: blank line.
This retry can duplicate a charge.

The second and last comments are dangerous. The others may be useful. The review tool often displays all of them as comments with equal visual weight.

That creates two bad outcomes:

• authors treat every comment as mandatory
• reviewers understate the few comments that really block merge

Google's guidance on review comments recommends explaining reasoning and labeling comment severity so authors can distinguish required changes from suggestions or informational notes. See Google's guide on writing code review comments.

A higher-signal review labels intent:

Blocking: this endpoint updates user roles without checking whether the caller can
administer this account.

Blocking: this retry path can send the same provider request twice with no idempotency key.

Important: the test covers provider failure before remote commit, but not timeout after
remote success. That is the ambiguous case this change is meant to handle.

Suggestion: consider renaming `state` to `invoiceState` because there are three state
machines in this file.

Nit: extra blank line.

Severity labels do not make reviewers more polite by magic. They make review intent explicit.

That matters because review is not only a correctness tool. It is also a communication protocol.

Antipattern 4: Review Optimizes For Style Instead Of Failure Modes

Style matters. Consistent formatting, naming, and structure make code easier to read.

But if style comments dominate every review, the team can end up with tidy diffs and weak production reasoning.

Imagine this change:

export async function updateUserRole(actorId: string, userId: string, role: string) {
  const user = await db.user.findUnique({ where: { id: userId } })

  if (!user) {
    throw new Error('User not found')
  }

  await db.user.update({
    where: { id: userId },
    data: { role },
  })
}

Low-impact comments are easy to make:

Suggestion: rename `user` to `targetUser`.
Nit: can we use a custom error type?
Suggestion: extract the update call to a helper.

Those may be reasonable.

The review-blocking question is different:

Blocking: where do we verify that `actorId` is allowed to change this user's role?
This function can currently assign admin privileges across account boundaries.

The antipattern is not "style comments are bad." The antipattern is letting the easy comments consume the review before the risk is named.

Google's "what to look for" review guide puts design, functionality, complexity, tests, naming, comments, style, documentation, and context into one review model. It specifically calls out thinking about edge cases, concurrency problems, and whether a change fits the system. See Google's guide on what to look for in a code review.

That is the useful ordering: system risk first, style after.

Antipattern 5: Tests Are Reviewed As Presence, Not Evidence

"Tests added" is not enough.

Tests are review evidence. They should tell reviewers which production assumption the change protects.

For the role update example, a weak test proves only the happy path:

it('updates a user role', async () => {
  await updateUserRole(admin.id, member.id, 'admin')

  await expectRole(member.id, 'admin')
})

That test may pass while the authorization bug remains.

A review should ask:

This connects directly to the broader testing problem in Why Tests Pass but Production Still Breaks: a test suite can be green while modeling the wrong reality.

During review, the useful question is not "are there tests?"

It is:

Would these tests fail for the production bug we are worried about?

Antipattern 6: Review Starts Too Late For Design Feedback

Some feedback belongs before a pull request exists.

If a reviewer first sees a change after the author has implemented a new abstraction, migrated three call sites, and updated tests, design feedback becomes expensive. Even if the reviewer is right, the author has already paid most of the implementation cost.

That is how review comments start sounding like obstruction when they are really late design discovery.

Use a short pre-review note for changes with broad design impact:

Planned change:
Move invoice creation behind an idempotent command object.

Reason:
Provider timeouts can currently leave us unsure whether the invoice was created.

Alternatives considered:
1. retry the existing service method
2. add provider-specific dedupe only
3. store command state locally and retry from that state

Open question:
Should the command table be owned by billing-api or billing-worker?

That can be a design note, issue comment, short document, or early draft pull request. The format matters less than the timing.

For the decision-making side of this habit, see How Software Engineers Make Decisions.

Review should catch local risks. It should not be the team's first design conversation for every cross-cutting change.

Antipattern 7: AI Review Adds Noise Without Risk Ownership

AI review can help with mechanical coverage: suspicious null handling, missing tests, confusing names, repeated code, and likely mistakes.

It can also multiply low-impact comments.

If human reviewers see a long list of AI suggestions and shift into "verify the tool output" mode, the review can become faster while still missing system-level risk.

That is why AI-assisted review needs a clear split:

The companion article Why AI Code Review Misses Real Risks covers that failure mode in more detail.

The short version: AI can expand review coverage. It should not replace reviewer judgment.

A Better Review Workflow

A higher-signal review process does not need to be heavy.

It needs a clearer sequence.

1. Read The Description Before The Diff

Before opening the files, ask:

• What is this pull request trying to change?
• What behavior should be different after merge?
• What risk does the author want reviewed?
• What is explicitly out of scope?

If the description cannot answer those questions, ask for that context first.

2. Find The Riskiest Path

Do not review files only in alphabetical order.

Start with the main behavior path:

• request handler
• state transition
• permission boundary
• database write
• retry loop
• worker handler
• migration step
• public API contract

Then use the surrounding changes to validate that path.

3. Check Tests Against The Risk

Read tests as claims:

This is the reality the author thinks matters.

If the tests miss the risky path, the review should say that directly.

4. Label Comments By Impact

Use consistent labels:

Blocking:
Important:
Suggestion:
Nit:
Question:

The label is not ceremony. It prevents accidental priority inversion.

5. Move Big Disagreements Out Of Comment Loops

If the discussion is about architecture, ownership, rollout, or product behavior, a comment thread may be the wrong medium.

Use the thread to name the disagreement, then move to a short design discussion:

Blocking until we decide ownership: this state transition affects billing-api and
billing-worker. I do not think the PR thread is the right place to settle that.
Can we decide the owner and update the PR description with the result?

Review comments should preserve decisions, not force the team to hold every design meeting inside a diff.

Pull Request Review Checklist

Before requesting review, authors should check:

• Is the primary behavior change stated in the description?
• Is unrelated cleanup split out or clearly labeled?
• Does the description tell reviewers where the risk is?
• Are tests tied to the risky behavior, not only the happy path?
• Is there a rollout or rollback note when production state changes?

During review, reviewers should check:

• Does the change belong in this part of the system?
• What production behavior can fail if this merges today?
• Are permission, data, retry, concurrency, and API-contract boundaries still safe?
• Do the tests fail for the failure mode being reviewed?
• Are comments labeled by impact?
• Are style comments separated from blocking risk?

Before merge, the team should check:

• Are blocking issues resolved in code, tests, or explicit follow-up work?
• Are deferred risks documented somewhere visible?
• Would another engineer understand the change from the pull request after the fact?
• Is the change small enough to revert safely?

This checklist is deliberately practical. It keeps review focused on evidence, not performance theater.

Final Takeaway

Code review slows teams down when every pull request becomes a vague negotiation over quality.

It gets faster when the team treats review as focused risk reduction: small changes, clear descriptions, explicit review targets, tests that model the dangerous path, severity-labeled comments, and early design conversations for changes too broad to settle in a diff.

The goal is not fewer comments. The goal is better signal per review cycle.

When reviewers know what risk they are inspecting and authors know which feedback blocks merge, review becomes less defensive, less random, and much more useful.