DEV Community: Codzee.io

Your CODEOWNERS File Might Be Correct And Still Send Your PR to the Wrong Person

Codzee.io — Mon, 31 Aug 2026 11:15:03 +0000

You open a PR. GitHub looks at the changed paths, checks them against CODEOWNERS, and auto-assigns a reviewer. This happens so often you probably don't think about it anymore. It's infrastructure — it just works.

Except sometimes it doesn't, and it fails in a way that never throws an error.

Here's a realistic example:

# CODEOWNERS
/payments/  @engineer-a

Engineer A hasn't touched payments/ in eight months — they moved to a different team in Q1. Engineer B has made the last 18 commits in that directory and has reviewed most of the recent PRs that touched it. Engineer B isn't in the file.

Your PR touches payments/refunds.go. It gets routed to Engineer A.

Nothing is misconfigured. The glob pattern matches. The username resolves to a real, active employee. GitHub does exactly what it's supposed to do. The routing logic is just missing context it was never designed to have.

This is the gap this article is about: CODEOWNERS tells you who owns a file. It doesn't tell you who should review a specific change. Those turn out to be different questions, and the difference gets bigger as your team and codebase grow.

1. What CODEOWNERS Gets Right

Before picking it apart, it's worth being clear that CODEOWNERS solves a real problem well, and nothing below is an argument for ripping it out.

Explicit ownership. Anyone can look at one file and see who's accountable for a given path. No tribal knowledge required.
Required reviews. You can enforce that certain paths always get eyes from the right team before merge — critical for things like auth, billing, or infra-as-code.
Clear accountability. When something in /payments/ breaks, there's no ambiguity about who's supposed to be paying attention to that code.
Simple, predictable behavior. It's a text file with glob patterns. No ML model, no black box, no surprises. You can git blame it.

For a small team, or for stable, slow-moving parts of a codebase, this is close to sufficient. The problem shows up specifically as team size and code churn increase — which is exactly when the cost of getting routing wrong also goes up.

2. Where Static Routing Breaks

CODEOWNERS answers one question — who owns this path — and organizations often treat that as a proxy for a different question: who should review this change. Here's where those diverge:

Signal	Static CODEOWNERS	Dynamic routing
File ownership	Yes	Yes
Recent contribution	No	Yes
Current expertise	Limited	Yes
Review history	No	Yes
Reviewer load	No	Yes
Expertise decay	No	Yes

Walking through the "No" column:

Recent contribution — CODEOWNERS has no time dimension. An entry from two years ago carries the same weight as one from yesterday.
Current expertise — "Limited" because ownership is often a reasonable proxy for expertise at the moment the file was written, but it's frozen after that. It doesn't update when someone's knowledge of an area grows or fades.
Review history — the file doesn't know who's actually been reviewing changes in an area, only who's nominally responsible.
Reviewer load — https://www.codzee.io/will happily assign a fifth simultaneous PR to someone already buried in review requests. It has no concept of "currently busy."
Expertise decay — there's no mechanism for an owner's relevance to degrade over time as they stop working in an area, and no mechanism for someone else's relevance to rise as they start.

None of this is a criticism of CODEOWNERS as a tool — it's just outside its scope. It's a static ownership file, not a reviewer-relevance model. The mistake is using it as if it were the second thing.

3. Model Expertise as Something That Changes

The core idea: expertise isn't binary, and it isn't permanent. "Alice owns payments" is a snapshot. What you actually want to know is closer to "how relevant is Alice's expertise, right now, to this specific diff."

Useful inputs for that:

Last contribution date — when did this person last touch this area?
Number of recent relevant changes — not lifetime commits, recent ones. Someone with 3 commits last month is a stronger current signal than someone with 200 commits two years ago.
Similarity to the current PR — has this person touched the same files, the same functions, the same data model recently?
Review activity — have they been reviewing PRs in this area, even if they haven't authored changes there?
Historical ownership — were they ever formally responsible for this, even if the file has since changed?

You can express this as a rough conceptual model — not a formula anyone should treat as an industry standard, just a way to make the tradeoff explicit:

reviewer_relevance = ownership_weight
                    + expertise_weight
                    + recency_weight
                    - load_penalty

The actual weights are going to be organization-specific, and honestly team-specific within an org. A security-critical path might want ownership weighted heavily no matter what. A fast-moving product surface might want recency and review history to dominate. The point of writing it this way isn't to hand you a formula — it's to make explicit that "who reviews this" is a function of multiple decaying/growing signals, not a single static lookup.

4. Work Through a Sample PR

Say you've got a repo with three top-level domains:

/payments/
/identity/
/notifications/

A PR comes in touching payments/refunds.go. CODEOWNERS says payments/ belongs to Engineer A. Here's how three candidate reviewers actually stack up:

Reviewer	Ownership	Recent work	Similar reviews	Current load
A	High	Low	Medium	High
B	None	High	High	Medium
C	Medium	Medium	Low	Low

Engineer A is the listed owner, but hasn't done recent work in this directory, and is already carrying a heavy review load.
Engineer B isn't in CODEOWNERS at all, but has been actively working in this area recently and has reviewed similar changes — they've probably seen the exact class of bug this PR could introduce.
Engineer C has moderate ownership and moderate recent activity, low similarity to this specific change, but has bandwidth right now.

A purely static system routes to A every time, because A is the only name it knows how to check against. But on the actual signals that predict a useful review — recent, hands-on context and demonstrated familiarity with similar changes — B is the stronger candidate, and C is a reasonable fallback if B is unavailable.

This doesn't mean A should be removed from the review entirely — for something in a sensitive path, keeping the formal owner in the loop for accountability might still make sense. It means A shouldn't be the only signal the system checks.

5. A Practical Routing Framework

Distilling this into something you could actually reason about (or build) for your own repo:

Ownership — who's formally accountable, per CODEOWNERS or equivalent?
Recency — who's touched this code lately?
Expertise — who has a track record of relevant, substantive contributions here?
Review history — who's reviewed similar changes before, and reviewed them well (not just approved fast)?
Load — who has the bandwidth to actually do this review in a reasonable timeframe?

None of these should unilaterally win. Ownership keeps governance intact — you don't want a recency model quietly routing security-sensitive changes away from the team accountable for them. Load is a modifier, not a primary filter — someone who's slightly busy but the clear best fit is often still the right call over someone who's fully free but has no context. The five signals are meant to be combined, with the weighting depending on what kind of change you're looking at (a one-line config tweak needs way less of this than a cross-cutting refactor).

6. The Routing-Decay Loop

Here's the pattern that causes CODEOWNERS files to quietly go stale, laid out as a loop:

People change teams / roles
        ↓
Code changes (new services, refactors, deprecations)
        ↓
Expertise shifts (some people gain context, others lose it)
        ↓
Ownership file stays the same (nobody's job to update it proactively)
        ↓
Routing quality declines

Nothing in that loop is anomalous — it's just normal organizational and codebase churn. The failure isn't that people move around or that code changes. The failure is that CODEOWNERS has no way to hear about any of it. It's a file, not a feed.

The fix isn't "update the file more often" — that's a losing battle against how fast real orgs change. It's closer to: continuously derive dynamic signals from data you already have (commit history, PR review history, recent activity) and layer them on top of the static file, rather than trying to keep the static file itself perfectly current. Contribution and review data refresh automatically as people work — no one has to remember to edit a YAML-adjacent file when Bob becomes the de facto payments expert. The signal is already sitting in your git history and PR review logs; it's just not being used for routing.

7. What Teams Can Measure Today

You don't need new tooling to start getting visibility into how well your current routing is actually working. Most of this is derivable from data already in GitHub:

Reviewer reassignment rate — how often does the auto-assigned reviewer get swapped out before the PR merges? High rate = your default routing isn't matching reality.
Time to first meaningful review — not first comment, first substantive review. If the assigned reviewer routinely takes days to engage, they may be overloaded or not the right fit.
Review load concentration — what percentage of review volume in a given area lands on one person? If it's consistently one name, you likely have a bottleneck, whether or not that person is technically the best expert.
PRs routed to inactive contributors — a rough version of this is: for each CODEOWNERS entry, check git log --since="6 months ago" -- <path> and see if the listed owner shows up. If not, that entry is a candidate for drift.
Review handoffs — PRs where the review gets passed from one person to another before merge are a decent proxy for "the first routing decision missed."
Recent contribution coverage — what share of merged PRs in an area were reviewed by someone who'd actually committed to that area in the last N weeks?

None of these require a new system to start tracking. A few are a git log or a GitHub API query away. The value isn't in any single metric — it's in noticing, over a few months, whether your routing is drifting in a direction nobody's paying attention to.

8. Practical Takeaway

None of this is an argument for dropping CODEOWNERS. It's still the right mechanism for encoding accountability — who's on the hook for a given part of the codebase, and which teams need guaranteed visibility into changes that touch their area.

What it's not sufficient for, on its own, is figuring out who should actually review a given PR today.

Keep CODEOWNERS for accountability. Add dynamic signals for relevance. Ownership answers "who's responsible." Recency, expertise, review history, and load together answer "who's actually the right person to look at this, right now." A routing setup that only asks the first question will keep sending PRs to people who used to be the right answer. A routing setup that ignores the first question entirely gives up the governance guarantees CODEOWNERS exists to provide. You want both.

How are you handling reviewer routing today — static CODEOWNERS, team rotation, tribal knowledge ("just tag whoever's around"), or something more dynamic? Curious what's actually working at different team sizes — drop your setup in the comments.

Our AI code reviewer problem wasn't that it was wrong. It was that it wouldn't shut up

Codzee.io — Thu, 27 Aug 2026 13:16:37 +0000

I've been thinking about a problem with AI code review that I don't see discussed enough: a reviewer can be technically correct and still make the whole review experience worse.

Here's the PR that made this click for me. Small change, maybe 200 lines, nothing exotic. New validation path, a couple helper functions. The kind of thing that used to get a comment or two and an approve.

The AI reviewer came back with something like a dozen comments. A naming suggestion. A "this null check might be redundant" note. A theoretical race condition that would only show up under conditions the code was never going to hit in prod. A "you could extract this into a function" suggestion on a function that was already about ten lines long.

And here's the annoying part most of these weren't wrong. The variable name genuinely could've been clearer. The null check probably was redundant. Individually, I could defend almost every comment on that list.

But there was one comment in there about an actual edge case in the validation logic that genuinely mattered. And it was sitting in the middle of eleven other comments that didn't. The dev opening the PR skimmed most of it, dismissed a few without really reading them, and nearly skipped the important one too — because by comment seven or eight, he'd already stopped expecting any of it to be worth his time.

Nobody wanted "more thorough." They wanted to know what actually mattered.

That's the distinction I think gets missed constantly: "is this technically an issue" and "is this worth interrupting someone for" are completely different questions, and most tooling in this space only ever answers the first one.

The assumption baked into a lot of AI review tools is basically: if finding more issues is good, finding even more must be better. But review was never just an issue-detection problem, it's a signal-to-noise problem, and honestly kind of a trust problem too. Once a reviewer starts treating a PR like a checklist to maximize, it stops mattering how accurate any individual comment is — the aggregate experience is what people remember, and the aggregate experience was "annoying."

That frustration eventually became one of the reasons I started working on Codzee. Not going to turn this into a pitch — the short version is the goal isn't "find everything," it's figuring out what actually deserves a developer's attention. Still early, still figuring a lot of it out.

Genuinely curious how other people think about this though:

How many AI review comments do you actually want to see on a normal PR? Is there a number where it tips from "helpful" to "annoying" for you?
Do you find yourself tuning out AI-generated comments after a tool has burned you with noise a few times?
Where's your personal line between a useful warning and something that's just noise dressed up as a finding?
Would you rather a reviewer miss a low-confidence issue entirely, or flag it anyway and let you decide?
What actually makes you trust an automated reviewer, versus just tolerate it?

Not trying to manufacture a debate here, I'm actually unsure about some of these myself — like the miss-vs-flag question, I go back and forth on what the right default even is. Curious what this sub thinks.

AI code review: Are more comments actually

Codzee.io — Tue, 25 Aug 2026 12:23:34 +0000

we’ve been thinking about something that seems easy to overlook with AI-assisted code review.

Imagine a PR gets 24 AI-generated comments.

That sounds thorough—but how many of those comments would you actually want a developer to spend time on?

There’s a big difference between:

“This could cause a production bug.”
“This might create a security issue.”
“This code could be more readable.”
“You could rename this variable.”
“Consider refactoring this small section.”

All of these may technically be valid observations, but they don't have the same value.

For developers, the real challenge isn't just finding issues anymore.

It's prioritizing them.

Too many low-value comments can create review fatigue, hide important findings, and slow down the person who actually needs to make the decision.

I'm curious how other developers handle this:

When using AI for code review, what do you prefer?

AI should report everything and let developers filter it.
AI should only report high-confidence, high-impact issues.
AI should report everything but rank comments by severity.
Something else?

We've been exploring this problem at Codzee and believe that AI code review should optimize for signal, not comment count.

Would love to hear how this works in your team and what you've found useful—or annoying—about AI code review.
www.codzee.io

How I'd Benchmark an AI Code Reviewer Before Putting It on 100 Engineers' PRs

Codzee.io — Mon, 24 Aug 2026 12:55:11 +0000

AI code-review tools are easy to demo.

Give one a pull request, wait a few seconds, and it produces a handful of comments that look surprisingly useful. Maybe it catches a missing authorization check. Maybe it spots a race condition. Maybe it complains about a test that doesn't cover an edge case.

That's a good demo.

It is not a benchmark.

If you're considering putting an AI reviewer on the pull requests of 100 engineers, the question isn't "How good does this look?" It's:

How often does this tool find problems that matter in our code, how often does it waste developers' time, and does it improve the review process enough to justify the cost?

I'd answer that question with an evaluation built around your own repositories, languages, review practices, and historical bugs.

Here's how.

1. Build a representative PR dataset

Start with a dataset of pull requests that looks like your actual engineering workload.

Don't build it entirely from toy examples. A benchmark containing 50 artificially constructed security bugs will tell you something about the model, but not necessarily much about what happens when it encounters your production code.

I'd aim for a few categories:

Historical production bug fixes
PRs that introduced bugs but were later fixed
Normal feature PRs with no known bugs
Refactoring PRs
Dependency updates
Performance-related changes
Security-sensitive changes
Database/schema changes
API changes
Test-only changes

The important part is preserving the context available during a real review: the PR diff, repository state, relevant files, tests, configuration, and whatever other context your normal review process exposes.

You should also keep a hidden ground-truth record for each PR.

For example:

PR-1842
Repository: payments-api
Language: Go
Known issue: authorization bypass
Severity: Critical
Introduced in: PR
Fixed by: PR-1911
Expected finding: Yes

For clean PRs, record that too:

PR-2017
Repository: customer-service
Language: Kotlin
Known issue: None
Expected finding: No

That gives you both positive and negative examples.

The negative examples are essential. An AI reviewer that finds something suspicious in every PR can look impressive until developers have to deal with the resulting noise.

2. Include known real-world bugs

Historical bugs are some of the most valuable test cases you have.

Look through incident reports, postmortems, security fixes, bug tickets, and reverted changes. Find bugs that were actually introduced into your codebase.

Then construct benchmark cases that reproduce the review situation.

For example, suppose six months ago a PR introduced:

if user.is_admin:
    return export_all_accounts()

when the intended condition was:

if user.is_admin and user.can_export_accounts:
    return export_all_accounts()

The benchmark should test whether the reviewer catches the authorization problem from the PR context—not whether it can identify a deliberately planted TODO: SECURITY BUG comment.

This distinction matters.

A useful benchmark measures performance on problems engineers genuinely encounter.

3. Include different bug severities

Not every finding has the same value.

A reviewer that catches one critical security issue but misses 20 minor problems may still be extremely valuable. Conversely, a tool that produces hundreds of "potential issue" comments while missing serious defects can become a productivity problem.

Classify your known issues.

One simple taxonomy is:

Severity	Example
Critical	Data loss, privilege escalation, severe security vulnerability
High	Production outage, major correctness issue
Medium	Significant edge case or reliability problem
Low	Minor correctness issue, maintainability problem

You can adapt these categories to your existing incident or vulnerability classification.

Then measure recall separately by severity.

For example:

Critical: 4/5 found
High:     11/18 found
Medium:   17/31 found
Low:      9/22 found

That tells you much more than a single "82% accuracy" number.

4. Test multiple repositories and languages

If your organization has six repositories and four languages, don't benchmark the tool on one favorite service written in the language it handles best.

Split the dataset across representative environments.

For example:

payments-api       Go
web-frontend       TypeScript
mobile-backend     Kotlin
data-platform      Python
infrastructure     Terraform

You don't necessarily need equal numbers of PRs in every repository. You do need enough samples to identify obvious differences.

The question isn't only "Does the tool work?"

It's:

"Does it work consistently enough across the environments where we're planning to deploy it?"

A reviewer that performs brilliantly on TypeScript but poorly on Terraform may still be useful. You just need to know that before making it mandatory across the organization.

5. Give every reviewer the same context

Benchmark fairness is surprisingly easy to get wrong.

If you're comparing two AI reviewers, make the input as equivalent as possible.

Give each one the same:

PR diff
Base revision
Repository contents
Relevant configuration
Tests
Documentation
Existing review context
Tool permissions, where applicable

Don't manually explain a tricky bug to one system because "it needed a little help."

Likewise, don't give one tool access to information that the other cannot see unless that difference is part of the product you're evaluating.

Context also needs to be documented.

For each benchmark run, record exactly what the reviewer was allowed to inspect.

Otherwise, six months later, you'll have two benchmark results that look comparable but weren't generated under comparable conditions.

6. Measure bugs correctly identified

The first metric I'd track is bug recall.

At its simplest:

Bug recall = known bugs correctly identified / known bugs

Suppose your dataset contains 100 known bugs and the tool identifies 72 of them.

That's 72% recall.

But there's a catch: what counts as "identified"?

A comment saying:

"This code might have an issue."

isn't necessarily equivalent to correctly identifying the actual defect.

Define acceptance criteria before running the benchmark.

For example, a finding might count as correct only if it:

Points to the relevant code.
Describes the actual failure mode.
Explains why the change causes the problem.
Provides enough information for an engineer to validate it.

This prevents optimistic scoring.

7. Measure false positives

Now run the other half of the experiment.

Give the reviewer PRs with no known bugs and see what it reports.

A useful metric is false-positive rate, but I also like tracking the simpler number:

False findings per PR

If a tool produces:

Known bugs found:      72
False findings:       180
PRs reviewed:          100

then its 72% recall doesn't look nearly as exciting.

You can also calculate precision:

Precision = correct findings / all findings

If the reviewer produces 100 findings and only 40 are genuinely actionable, precision is 40%.

For developers, this metric can matter enormously.

A reviewer that catches everything but complains about everything isn't necessarily useful.

8. Measure duplicate and noisy findings

False positives aren't the only source of noise.

AI reviewers can report the same underlying problem multiple times.

For example, one missing validation might produce:

"Input isn't validated."
"Potential invalid state."
"Unexpected value may reach database."
"Consider adding defensive checks."

Four comments, one issue.

I'd explicitly measure unique actionable findings.

You can group findings that describe the same underlying defect and count them once.

Also track findings that are technically true but don't deserve review attention.

For example:

"This function could be refactored to reduce complexity."

That may be reasonable advice, but if your benchmark is evaluating bug detection, it shouldn't count as a valuable bug finding.

A useful classification is:

Correct + actionable
Correct + low value
Duplicate
False positive
Not a bug

This makes the output much easier to analyze.

9. Measure severity accuracy

Finding the bug isn't enough.

The reviewer should also understand its impact.

Suppose your ground truth says:

Authorization bypass → Critical

and the AI says:

Potential code quality issue → Low

Technically, it noticed something. Operationally, it failed an important part of the review.

Compare predicted severity with your ground truth:

                  Actual
Predicted       Critical  High  Medium  Low
Critical            7      2      0      0
High                3     14      4      1
Medium              0      5     19      7
Low                 0      1      8     11

You don't need sophisticated statistics to start. Even a confusion matrix like this will show whether the tool systematically underestimates serious defects.

10. Measure whether developers actually accept findings

Eventually, the benchmark has to leave the spreadsheet.

Ask engineers to review the AI findings without knowing which tool produced them.

For every finding, capture something like:

Valid issue?       Yes / No
Actionable?        Yes / No
Would fix it?      Yes / No
Would mention it?  Yes / No

If you're running a live pilot, you can also measure what happens to findings in actual PRs:

Accepted
Fixed
Dismissed
Ignored
Marked duplicate

This is arguably one of the strongest signals you can get.

A finding with 95% technical accuracy but a 10% acceptance rate may not be particularly useful.

A finding that engineers consistently validate and act on is much more valuable.

11. Measure review latency

Don't forget the economics of the workflow.

Measure how long it takes from:

PR opened
    ↓
AI review starts
    ↓
AI findings available

Then compare that with the existing review process.

Latency matters because an AI reviewer that takes 45 minutes to analyze every PR may be technically impressive but operationally awkward.

You should also test different PR sizes.

For example:

Small:   <200 changed lines
Medium:  200–1,000
Large:   >1,000

A tool might be excellent on small PRs and degrade badly on large ones.

That's something you want to discover in a benchmark, not after deployment.

12. Measure developer time saved

This is where the benchmark becomes a business case.

Estimate the human effort associated with the findings.

For example, during a pilot you might measure:

PRs reviewed:                  200
Valid AI findings:              86
Findings accepted by authors:   61
Estimated review effort saved:  24 hours
Additional triage time:          7 hours
Net time saved:                 17 hours

Be conservative.

If an AI comment catches something a human reviewer would definitely have found anyway, don't automatically count the entire human review as "saved."

Instead, ask:

"How much work did this finding actually remove from the engineering process?"

You can estimate time through short developer surveys, sampled review sessions, or controlled experiments.

The goal isn't to manufacture a perfect number. It's to get a defensible estimate.

13. Repeat the evaluation over time

One benchmark run isn't enough.

AI systems change. Your repositories change. Prompting changes. Models change. Vendors ship new detection logic.

Run the benchmark periodically.

I'd keep a fixed golden dataset containing a representative sample of your most important historical cases, then add new cases over time.

For example:

Baseline
├── 100 historical bugs
├── 100 clean PRs
└── 50 security-sensitive PRs

Quarter 2
├── +25 new production bugs
└── +25 new clean PRs

Quarter 3
├── +20 new production bugs
└── +20 new clean PRs

Now you can track whether performance is actually improving.

You can also detect regressions.

If version 4.2 of your AI reviewer goes from 78% to 84% recall but doubles false findings, that's not simply "better."

It's a trade-off.

A simple scoring framework

You don't need a complicated machine-learning benchmark.

Here's a framework I'd start with:

Metric	Weight
Bug recall	30%
Precision / false positives	20%
Severity accuracy	10%
Developer acceptance	15%
Duplicate/noise rate	10%
Review latency	5%
Developer time saved	10%

Score each category from 0–100, then calculate the weighted total.

For example:

Recall                  82 × 0.30 = 24.6
Precision               76 × 0.20 = 15.2
Severity accuracy       80 × 0.10 =  8.0
Developer acceptance    71 × 0.15 = 10.7
Noise                   85 × 0.10 =  8.5
Latency                 90 × 0.05 =  4.5
Time saved              68 × 0.10 =  6.8

Total                               78.3 / 100

The exact weights are less important than agreeing on them before you see the results.

And I'd add one more rule: define minimum thresholds for critical metrics.

For example:

Overall score       ≥ 75
Critical bug recall ≥ 90%
Precision            ≥ 60%
P95 review latency   ≤ 10 minutes

That prevents a tool from compensating for catastrophic performance in one area with excellent performance in another.

What about vendor benchmarks?

Vendor benchmarks are useful.

They can tell you how a tool performs against standardized datasets, and they're worth looking at when you're building your shortlist.

But they answer a different question.

A vendor benchmark asks:

"How well does our system perform on this benchmark?"

Your benchmark asks:

"How well does this system work for our engineers, in our codebases, under our review process?"

That's the number I'd use when deciding whether to put the tool on 100 engineers' PRs.

For example, if you're evaluating a tool such as Codzee, treat its published benchmark results as one input into your research—not as the final deployment decision.

The same methodology should apply to every vendor.

The benchmark I'd actually run

If I were setting this up for an engineering organization tomorrow, I'd make the process roughly:

1. Select 300–500 representative PRs
              ↓
2. Label known bugs and expected severity
              ↓
3. Include clean PRs as negative examples
              ↓
4. Run each AI reviewer with equivalent context
              ↓
5. Normalize and deduplicate findings
              ↓
6. Blind-score correctness and severity
              ↓
7. Run a developer acceptance study
              ↓
8. Measure latency and engineering effort
              ↓
9. Calculate weighted score
              ↓
10. Repeat periodically

Most importantly, keep the benchmark independent from the vendor.

You choose the PRs.

You define what constitutes a correct finding.

You decide how severity is classified.

You determine which findings are useful.

And you keep the dataset private enough that it can't simply become another benchmark for vendors to optimize against.

That's how you turn an AI code-review demo into an engineering evaluation.

The goal isn't to find the AI reviewer with the highest benchmark score.

It's to find the one that makes your code-review process better without creating a new category of developer noise.

What would you add to this benchmark?

I Put an AI Reviewer on Every PR. Here's What Happened After the 100th Review.

Codzee.io — Thu, 20 Aug 2026 12:03:16 +0000

At first, adding an AI code reviewer to every pull request feels like cheating.

Every PR gets another pair of eyes.

Every diff gets analyzed.

Potential bugs are pointed out before a human reviewer even opens the PR.

The first few reviews are impressive.

Then something strange happens.

You stop reading every comment.

Not because the reviewer is always wrong.

Because you learn that most comments aren't worth stopping for.

That's the part of AI code review I think we don't talk about enough.

The reviewer's first job isn't finding bugs

It's earning the developer's attention.

Imagine this PR:

AI review

✓ Possible null handling issue
✓ Consider extracting this function
✓ This variable could be renamed
✓ Potential race condition
✓ Missing test case
✓ This comment could be clearer
✓ Consider using...
✓ Consider using...
✓ Consider using...

The problem isn't that every comment is bad.

Some might be useful.

The problem is that the developer now has to perform another review:

“Which of these comments should I care about?”

That's review fatigue.

And it can happen surprisingly quickly.

There are four kinds of AI review noise

I've found it useful to think about review noise in four categories.

1. Wrong

The reviewer identifies something that isn't actually a problem.

Example:

This condition can never be true.

Except it can.

The reviewer simply misunderstood how the value reaches the function.

This is the easiest type of noise to recognize.

2. Technically correct

This one is more dangerous.

Example:

This function is 45 lines long.
Consider splitting it into smaller functions.

Fine.

Maybe that's a good idea.

But if the PR is fixing a production incident and the function is intentionally kept together for transactional behavior, this isn't the problem the reviewer should be talking about.

The comment isn't wrong.

It just isn't useful right now.

3. Missing context

This happens constantly.

The reviewer sees:

if (!user) {
    return;
}

and says:

“Potentially unsafe null handling.”

But the API intentionally permits anonymous users.

The reviewer doesn't understand the business rule.

The developer does.

Now the developer has to explain the business rule to the AI reviewer.

That's backwards.

4. Repetition

This is the fastest way to turn useful tooling into background noise.

The same type of comment appears over and over.

Eventually developers recognize the pattern.

They stop reading.

And once they stop reading, the good comments get ignored too.

The worst metric for AI code review

I don't care how many comments an AI reviewer generates.

Seriously.

Imagine two systems.

System A

100 comments.

8 accepted.

System B

15 comments.

9 accepted.

Which one would you rather have?

Obviously B.

But comment volume is seductive because it's easy to put on a dashboard.

PRs reviewed: 2,381

Issues detected: 14,921

That sounds impressive.

It doesn't tell you whether developers found those issues useful.

I'd rather see:

Findings: 1,200

Accepted: 780
Dismissed: 310
Duplicates: 70

Now we're measuring something interesting.

Try a review-noise audit

If your team already uses AI code review, here's a simple experiment.

Take the last 100 AI-generated review comments.

Put each one into one of these buckets:

A — Fixed
B — Discussed / investigated
C — Dismissed
D — Duplicate
E — Not actionable

Then calculate the percentages.

For example:

Fixed:              31%
Investigated:       18%
Dismissed:           27%
Duplicate:           12%
Not actionable:      12%

Now you have a much more useful picture of your reviewer.

The interesting number isn't “31% fixed.”

It's everything else.

Because every dismissed or low-value comment still consumed some amount of developer attention.

Then look at severity

This is another place where AI reviewers can lose trust.

Suppose a normal PR gets:

5 critical issues

You investigate them.

Three are style suggestions.

One is a minor maintainability concern.

One is actually important.

After a few PRs like that, what does “critical” mean to you?

Probably not much.

Severity needs a contract.

For example:

Critical

Could realistically cause:

security compromise
data loss
major outage
broken core functionality

High

Meaningful production risk.

Medium

Worth fixing but not urgent.

Low

Minor concern.

And then there's another category:

Don't comment

If the reviewer isn't confident enough, it should stay quiet.

The uncomfortable lesson

Developers don't need an AI reviewer that notices everything.

They need one that knows what not to mention.

That's a very different optimization target.

A human reviewer naturally filters.

They might see 20 questionable things and comment on 2.

Why?

Because they understand that every comment costs the author something.

Time.

Attention.

Discussion.

Potential context switching.

AI systems don't naturally feel that cost.

They need to be designed around it.

AI writing code makes this more important

There's another reason this problem is getting bigger.

AI coding agents can produce much larger changes much faster.

A developer might ask:

“Add OAuth login.”

The agent might modify:

auth/
middleware/
routes/
database/
frontend/
tests/
configuration/
documentation/

Now someone has to understand that entire change.

The bottleneck has moved.

We used to ask:

“How can we write code faster?”

Now we increasingly need to ask:

“How can we verify more code without increasing cognitive load at the same rate?”

That is a much harder engineering problem.

So what should an AI reviewer actually do?

My ideal reviewer has five rules.

Rule 1: Don't repeat deterministic tools

If ESLint already catches it, don't produce another paragraph explaining it.

If the compiler catches it, don't create an AI warning.

AI should focus on things that require reasoning.

Rule 2: Understand the surrounding code

A diff without context is often not enough.

The reviewer should understand relevant:

functions
tests
interfaces
configuration
repository conventions
PR intent

Rule 3: Prioritize risk

A potential authentication vulnerability should not appear underneath 12 variable-naming suggestions.

Rank findings.

Rule 4: Explain why the finding matters

Don't say:

“This could cause a problem.”

Say what could happen.

For example:

“This query now executes once per customer because it sits inside the loop. At 10,000 customers, this turns the operation into an N+1 query pattern.”

That's actionable.

Rule 5: Be willing to say nothing

This may be the most important rule.

A review that returns:

“No high-confidence issues found.”

can be a successful review.

Silence is better than noise.

Where Codzee fits

This is the problem space where Codzee is interesting.

The useful question isn't:

“Can we put AI on every pull request?”

That's relatively easy now.

The harder question is:

“Can we put AI on every pull request without making developers hate the review process?”

That requires better prioritization, useful context, and a focus on actionable findings rather than maximizing comment volume.

That's the direction AI-assisted code review needs to move.

A simple mental model

Think about AI review like a security guard.

A bad security guard stops everyone.

“Bag check.”

“Bag check.”

“Bag check.”

“Bag check.”

Eventually everyone stops paying attention.

A good security guard doesn't stop everyone.

They identify the situations that actually deserve attention.

AI code review should work the same way.

Don't maximize interruptions.

Maximize useful interruptions.

The real success metric

After a few months of using an AI reviewer, ask your team one question:

“When this reviewer leaves a comment, do you usually read it?”

If everyone says yes, you've probably built trust.

If people say:

“I usually just dismiss them.”

You have a problem.

And it's not necessarily model accuracy.

It's relevance.

The reviewer has become background noise.

That's what I mean by earned irrelevance.

The AI didn't necessarily fail because it couldn't find bugs.

It failed because developers stopped believing that its comments were worth their attention.

And in code review, attention is the resource that matters most.

What about your team?

If you're already using an AI code reviewer, try the 100-comment audit.

Take your last 100 comments.

Classify them.

Then see how many were actually useful.

I'd be genuinely interested in what teams are finding.

Are AI reviewers reducing your review workload, or have they just created another thing you have to review?

Disclosure: This article discusses Codzee as an example of AI-assisted code-review tooling. The broader argument is about the design of AI review workflows, not a claim that any single tool solves the problem.

The AI Code Review Checklist I Actually Use Before Merging a PR

Codzee.io — Wed, 19 Aug 2026 12:48:48 +0000

I've been using AI coding tools heavily for a while now, and the thing that changed my workflow the most wasn't the code generation itself, it was building a review habit that actually catches the failure modes specific to generated code. AI output tends to look more finished than it is. The variable names are good, the structure is clean, the comments are confident. That polish is exactly what makes it easy to under-review.

This is the checklist I actually run through, not the one I aspire to run through. Eight categories, each with a quick explanation of why it matters and a small example of the kind of thing that slips past a fast skim.

1. Correctness: does it do what was asked, not just something plausible

AI models are very good at producing code that satisfies the letter of a prompt while missing the actual intent. If you ask for "a function that returns the most recent order for a user," you need to check what "most recent" means in the data. Sorted by creation date? Updated date? What if there are two orders with the same timestamp?

def get_most_recent_order(orders):
    return sorted(orders, key=lambda o: o.created_at)[-1]

This works until two orders share a timestamp, and then the result is whichever one Python's sort happens to place last, which is not "most recent" in any meaningful sense. It's arbitrary. Nobody asked for arbitrary.

Why it matters: correctness bugs from AI output are rarely syntax errors, they're semantic gaps between what was asked and what got built. You have to read the code against the requirement, not just against itself.

2. Edge cases: the inputs nobody put in the prompt

If your prompt didn't mention empty lists, null values, negative numbers, or duplicate entries, the model probably didn't think about them either. It optimizes for the happy path described in the request.

function calculateAverage(scores) {
  const total = scores.reduce((sum, s) => sum + s, 0);
  return total / scores.length;
}

Pass an empty array and you get NaN silently propagating into whatever calls this next. No error, no crash, just a quietly wrong number working its way downstream.

Why it matters: edge cases are where AI-generated code fails most often, because the model is pattern-matching against the common case in its training data, not reasoning about your specific data guarantees.

3. Security: don't assume the model thought about it unless you told it to

If you didn't explicitly ask for input sanitization, parameterized queries, or auth checks, don't assume they're there. They often aren't, because the prompt didn't ask for them.

query = f"SELECT * FROM users WHERE email = '{user_email}'"
cursor.execute(query)

This is straightforward SQL injection, and it's also one of the most common patterns I see generated when someone asks for "a function that looks up a user by email" without mentioning security at all.

Why it matters: generated code defaults to the simplest version of a solution, and the simplest version is very often the insecure version. Treat every generated data access or input-handling function as unreviewed for security until you've explicitly checked it.

4. Performance: check the complexity, not just the output

Generated code frequently reaches for the most obvious algorithm rather than the most efficient one, especially for anything involving lookups or nested iteration.

def find_duplicates(items):
    duplicates = []
    for i in range(len(items)):
        for j in range(i + 1, len(items)):
            if items[i] == items[j] and items[i] not in duplicates:
                duplicates.append(items[i])
    return duplicates

This is O(n²) for a problem that's a few lines away from O(n) with a set or a counter. It'll pass every test on a small list and quietly become a bottleneck the moment someone runs it against production-sized data.

Why it matters: small inputs during testing hide complexity problems that only show up at scale, and AI output doesn't come with a complexity analysis attached. You have to do that part yourself.

5. Maintainability: will the next person understand this without you

Generated code is often correct in isolation but disconnected from the conventions of the rest of your codebase. Different error handling style, different naming pattern, a slightly different way of structuring similar logic that already exists three files over.

# existing pattern in the codebase
class UserNotFoundError(Exception):
    pass

# what got generated for a new function
def get_user(user_id):
    user = db.query(user_id)
    if not user:
        return None  # inconsistent with the rest of the codebase's error pattern
    return user

Why it matters: consistency is a maintainability feature. Code that solves the problem correctly but ignores established patterns adds friction for every future reader, even if it never causes a bug.

6. Tests: check what's actually being asserted, not just that tests exist

Generated tests can create a false sense of safety. It's common to get tests that run without error but don't assert anything meaningful.

def test_process_payment():
    result = process_payment(order)
    assert result is not None

This test passes whether the payment succeeded, failed silently, or returned an empty error object. It confirms the function returned something, not that it did the right thing.

Why it matters: a test suite with weak assertions is worse than no test suite in some ways, because it creates confidence that isn't backed by actual verification. Read the assertions, not just the test count.

7. Dependencies: verify the package actually exists and does what's claimed

Models occasionally reference packages, methods, or APIs that don't exist, or that existed in an older version and have since changed. This is less common than it used to be but still worth a direct check, especially for less mainstream libraries.

import pandas as pd
df.iteritems()  # removed in pandas 2.0, replaced by df.items()

Code like this can look completely reasonable to someone who hasn't kept up with a specific library's recent changes, and it'll fail immediately in CI or, worse, only in a slightly older pinned environment.

Why it matters: a broken import or a deprecated method is an easy fix once caught, but it's an annoying one to debug blind if it ships and fails somewhere downstream instead of at review time.

8. Production behavior: what happens when a dependency it relies on goes down

This is the category that's easiest to skip because it doesn't show up in normal testing at all. Does the code handle timeouts? Partial failures? What does logging look like when something goes wrong at 3am and someone's trying to debug it without you in the room?

def send_notification(user_id, message):
    response = requests.post(NOTIFICATION_API_URL, json={"user_id": user_id, "message": message})
    return response.json()

No timeout, no retry, no handling of a non-200 response, and no logging if it fails. When the notification service has a bad five minutes, this function will hang or throw an unhandled exception with zero context about what actually happened.

Why it matters: this is the category of bug that doesn't show up until it's already in production, because local development rarely simulates a flaky dependency. It's worth explicitly asking "what does this look like when the thing it depends on is unavailable" for anything that talks to an external service.

The pattern across all eight

Every one of these examples looks fine at a glance. That's not a coincidence, it's the actual challenge with reviewing generated code. The syntax is clean, the naming is sensible, the code reads as intentional. The gaps aren't in how the code looks, they're in what it doesn't account for, and you only catch that by asking specific questions of it rather than scanning for things that look wrong.

Save this, print it, stick it next to your monitor, whatever works. The categories don't change much project to project, only the specifics do.

What would you add to this list? Curious what's bitten people that isn't covered here.

Before You Merge AI-Generated Code, Ask These 12 Questions

Codzee.io — Fri, 14 Aug 2026 12:21:07 +0000

I've merged plenty of AI-generated code that was genuinely fine. I've also caught myself almost merging code that looked fine and wasn't, because it read like something a competent person wrote and my brain filled in the rest.

Over the last year I've settled into a rough set of questions I run through before approving anything I didn't write line by line myself, generated or not. Here they are, in the order I actually ask them.

1. What problem is this code actually solving?

It's easy to review whether code works and skip whether it solves the right thing. AI tends to answer the literal prompt, not the intent behind it.

def get_active_users():
    return db.query("SELECT * FROM users WHERE active = true")

If "active" was supposed to mean "logged in within 30 days" and not a boolean flag that's rarely updated, this passes every test and still solves the wrong problem.

Reviewer tip: Read the original ticket or request before reading the diff. Check the code against the intent, not just the literal ask.

2. Do I actually understand the implementation?

Not "does it look reasonable," actually understand it, line by line, well enough to explain it to someone else.

Reviewer tip: Try to explain the function out loud in one sentence per major step. If you get stuck anywhere, that's the part you haven't actually reviewed yet, just skimmed.

3. What assumptions is it making?

Every implementation bakes in assumptions about the shape of the data, the order things happen in, or what "normal" looks like.

function getLatestOrder(orders) {
  return orders[orders.length - 1];
}

This assumes orders is sorted chronologically and never empty. Neither assumption is stated anywhere.

Reviewer tip: Ask "what does this assume about its inputs that isn't checked anywhere?" Write the answer down, literally, in the PR comment if it matters.

4. What happens with bad input?

Bad input isn't an edge case, it's a certainty over a long enough timeline.

def parse_age(value):
    return int(value)

Pass it "25" and it works. Pass it "twenty-five", None, or -5 and you get a crash or a nonsensical value with no complaint.

Reviewer tip: Pick three inputs that would never appear in a demo but could plausibly appear in production: empty, wrong type, absurdly large. Trace through what actually happens.

5. What happens when an external service fails?

Generated code frequently assumes the network, database, or third-party API always responds successfully.

async function getExchangeRate(currency) {
  const response = await fetch(`https://api.example.com/rates/${currency}`);
  const data = await response.json();
  return data.rate;
}

No timeout, no handling for a non-200 response, no fallback. If that API is slow or down, this fails in whatever way fetch and .json() happen to fail, which may not be a clear error at all.

Reviewer tip: For every external call, ask "what does the caller see if this times out or returns an error status?" If the answer is "an unhandled exception," that's worth a comment.

6. Are permissions actually enforced, not just checked?

There's a difference between "there's an auth check" and "it's the right auth check."

if (!req.user) return res.status(401).send('Unauthorized');
const doc = await db.documents.findById(req.params.id);
res.json(doc);

This confirms someone is logged in. It never confirms they own or have access to this document.

Reviewer tip: For any endpoint touching a specific resource, ask "does this check the resource belongs to the requester, or just that the requester is logged in?"

7. Is sensitive data exposed anywhere?

Look at what actually goes into logs, error responses, and API payloads, not just what the happy path returns.

catch (err) {
  res.status(500).json({ error: err.message, stack: err.stack });
}

Fine in local development. In production this can leak file paths, query fragments, or internal structure to whoever triggers the error.

Reviewer tip: Grep the diff for console.log, print, and catch blocks. Check what they actually expose.

8. Is this more complex than the problem needs?

Generated code sometimes over-engineers a simple problem with extra configuration, unnecessary abstraction layers, or generic solutions to specific problems.

Reviewer tip: Ask "could this be half the length and still be correct?" If yes, that's worth pushing back on, complexity has an ongoing cost even when it's not technically wrong.

9. Are the tests actually meaningful?

Generated tests often confirm the code does what it does, not that it does what it should.

def test_apply_discount():
    assert apply_discount(100, 10) == 90

This confirms the arithmetic. It says nothing about a discount over 100%, a negative price, or invalid input.

Reviewer tip: For each test, ask "what wrong implementation would still pass this?" If you can think of one easily, the test isn't pinning down enough.

10. Does it fit existing conventions?

Locally correct code can still be a long-term problem if it introduces a new pattern the codebase doesn't already use, a different error-handling style, a new HTTP client, a different logging approach.

Reviewer tip: Before approving, check one comparable file elsewhere in the codebase. If the patterns don't match, ask whether that's intentional.

11. What happens under unusual load or concurrency?

Code that's correct for one request at a time can break under concurrent access, especially anything involving shared state or caching.

counter = {}

def increment(key):
    counter[key] = counter.get(key, 0) + 1

Fine single-threaded. Under concurrent requests this can lose increments, since read-then-write isn't atomic.

Reviewer tip: For anything touching shared state, ask "what happens if this runs twice at the exact same moment?"

12. Will the next developer understand this without me?

The final check. If someone opens this file in eight months with zero memory of this PR, can they figure out what it does and why from the code and comments alone?

Reviewer tip: Read the diff as a stranger would, not as someone who already knows what it's supposed to do. If it doesn't hold up, add a comment now while the reasoning is still fresh.

Copy-paste checklist for your PR template

## AI-Generated Code Review Checklist

- [ ] Solves the actual requirement, not just a literal reading of it
- [ ] I understand every part of this well enough to explain it
- [ ] Assumptions are identified and actually hold true
- [ ] Handles empty, invalid, and unexpected input
- [ ] Handles external service failures and timeouts
- [ ] Permissions checked against the specific resource, not just login state
- [ ] No sensitive data leaked via logs, errors, or responses
- [ ] No unnecessary complexity for the problem size
- [ ] Tests check behavior, not just current output
- [ ] Matches existing codebase conventions
- [ ] Considered behavior under concurrent access or load
- [ ] Understandable to a developer with no context on this PR

None of this is exotic review practice. It's the same discipline good engineers apply to any code they didn't personally trace through, generated or not. The only thing that's changed is how often that situation comes up, and how easy it is to skip these questions when the first draft already looks like someone competent wrote it.

What should an AI code reviewer never be allowed to decide?

Codzee.io — Thu, 13 Aug 2026 11:31:55 +0000

What should an AI code reviewer never be allowed to decide?

Automated code analysis has gotten genuinely good at finding things. Unhandled exceptions, missing null checks, patterns that historically correlate with bugs, inconsistent formatting, code that duplicates logic sitting somewhere else in the repo. That part of the job, pattern detection across a large surface area, is exactly the kind of task a machine does faster and more consistently than a tired human scrolling through a 400-line diff at the end of the day.

But there's a difference between finding a potential problem and deciding what the team should do about it, and I think that line is where a lot of the current conversation about automated review gets confused.

The data backs up the caution here. In StackOverflow's 2025 Developer Survey, adoption of AI tools kept climbing, with the majority of professional developers now using them regularly, yet trust in the accuracy of their output actually dropped compared to the year before. When asked what would still make them reach out to a human instead of relying on AI, the single most common answer developers gave was simply not trusting the AI's answer. That's not a story about a tool getting worse. It's a story about developers getting a clearer sense of where the tool's judgment should stop and theirs should start.

Finding a problem vs deciding what to do about it

Here's the distinction in practice. "This function doesn't handle a null input" is a finding. "This function should be rewritten to reject null inputs at the boundary instead of checking for them internally" is a decision, because it depends on how the rest of the system is structured, what the team's conventions are, and whether that boundary even exists yet. A tool can flag the first. Only a human with context can respond to the second responsibly.

That distinction matters more once you list out the kinds of decisions that come up constantly in real review, and notice how few of them are actually technical questions in disguise.

Product requirements. Whether a feature does what the business actually needs isn't something code analysis can evaluate. It can check if the code matches a spec. It has no way of knowing if the spec itself is right.

Business priorities. Whether this PR should ship this week or wait for a more complete solution next sprint depends on revenue timing, customer commitments, and competitive pressure, none of which live in the diff.

Architectural tradeoffs. Microservice or monolith, synchronous or event-driven, these choices ripple across a system for years and depend on team size, deployment maturity, and organizational appetite for operational complexity. A tool can tell you a pattern is unusual. It can't tell you it's wrong for your context.

Acceptable technical debt. A shortcut taken deliberately, with a clear owner and a plan to revisit it, is a completely different thing from the same shortcut taken carelessly. The code looks identical either way. The judgment about whether it's acceptable lives entirely outside the code.

User experience. Whether an extra confirmation step protects users or just adds friction is a design and empathy question, not a static analysis question.

Security risk tolerance. A tool can flag that input isn't validated. Whether that gap is acceptable for an internal admin tool used by three trusted employees, versus unacceptable for a public signup form, requires knowing who's exposed and what the actual threat model is.

Performance tradeoffs. Whether shaving fifty milliseconds off a response is worth the added code complexity depends entirely on whether that fifty milliseconds affects anyone's experience at all.

Organizational conventions. Naming patterns, folder structure, preferred libraries. These exist because a team agreed on them, not because one option is objectively correct.

Whether a feature should exist at all. No amount of analysis on the code tells you if building it was the right call in the first place.

Whether a shortcut is strategically acceptable. Sometimes shipping something slightly rough now, on purpose, is the correct business decision. That's a judgment about timing and risk appetite, not about the code.

A concrete example

Picture a PR that adds a new caching layer in front of a slow database query. Automated analysis can correctly flag that the cache has no invalidation strategy, and that's a genuinely useful finding, the kind of thing that's easy to miss under deadline pressure. What it cannot tell you is whether stale data for up to five minutes is a real problem for this particular feature, or a complete non-issue because the underlying data barely changes. That call depends on what the data represents and who's looking at it, and only someone with product and business context can make it responsibly.

Why this balance actually works

None of this is an argument against using automated analysis in review. It's genuinely useful for exactly what it's built for, catching the categories of mistakes that are easy to miss under time pressure, at a scale no human reviewer can match across a large codebase. The 2025 Sonar developer survey found that a large share of committed code now involves some form of AI assistance, and developers report reviewing and validating it more carefully as a result rather than less, which suggests the healthy pattern here is already forming naturally on a lot of teams.

The honest position is that these tools are excellent at expanding what gets checked, and completely unqualified to decide what should happen once something is found. Engineering judgment, the part that weighs tradeoffs, understands business context, and takes responsibility for the outcome, has to stay human.

I write about this kind of review culture regularly over at www.codzee.io, mostly because it's a distinction I think gets lost in a lot of the current hype cycle.

Eight principles for responsible AI-assisted code review

Automated findings are a starting point for discussion, not a verdict.
A flagged issue always needs a human owner to decide what happens next.
Never let a tool's confidence score substitute for a reviewer's understanding of context.
Business, product, and architectural decisions stay with people who carry the consequences.
Use automation to widen coverage, not to replace judgment on what matters most.
Document the reasoning behind decisions, not just the fact that a tool flagged something.
Treat every automated suggestion as a hypothesis worth checking, not a fact worth trusting blindly.
If no human could explain why a decision was made, the decision wasn't actually made yet.

I Asked 10 Developers What Makes Them Reject a Pull Request

Codzee.io — Wed, 12 Aug 2026 12:10:36 +0000

A quick note before this starts: I didn't actually run ten interviews for this post. What follows is a synthesis — patterns I've seen show up over and over in my own review comments, in other people's, and in the general "why did you block this" conversations that happen on pretty much every team that takes code review seriously. If you've been reviewing code for more than a year, I'd bet most of this is going to feel familiar rather than new. That's kind of the point. These aren't ten hot takes. They're ten things that keep independently showing up across very different teams, which probably means they're just true.

I've grouped them into ten reasons, each with a small example and, more importantly, a note on how to raise the issue without it turning into a fight or a demoralized author. Because the "what" of a rejection is usually easy. The "how you say it" is where reviews go well or badly.

1. Broken Behavior

This is the obvious one, but it's worth stating plainly: if the code doesn't do what it claims to do, nothing else matters yet. Style, naming, architecture — none of it is relevant if the function returns the wrong value.

function isEligibleForDiscount(user) {
  return user.age > 65 || user.age < 18;
}

If the actual business rule is "65 and older," this excludes someone who just turned 65. Small, easy to miss, and exactly the kind of thing that survives a glance because the code reads fine — it's fluent, grammatical, wrong.

How to raise it constructively: State the discrepancy, not the failure. "I think this excludes someone exactly at 65 — should the comparison be >=?" is a question, not an accusation, and it gives the author room to say "actually that's intentional" if there's context you're missing.

2. Security Problems

Security issues get treated differently from ordinary bugs because the cost of being wrong isn't "annoying," it's "someone's data leaked." That difference in stakes is why these tend to be non-negotiable even when they're inconvenient to fix right before a release.

query = f"SELECT * FROM users WHERE email = '{email}'"

String-interpolated SQL is the classic example, but the pattern shows up everywhere — unescaped output going into HTML, secrets logged in plaintext, auth checks that verify identity but not permission.

How to raise it constructively: Don't soften this one into vague language. "Nit: might want to look at this" undersells a real risk. Be direct about why it matters without being alarmist: "This is vulnerable to SQL injection since email comes straight from the request — can we use a parameterized query here?" Directness isn't the same as harshness.

3. No Tests for Risky Changes

Not every change needs a wall of new tests. But changes that touch money, permissions, data deletion, or anything hard to undo are a different category, and "it worked when I tried it" isn't the same as "it's covered."

def refund_order(order_id, amount):
    order = db.get_order(order_id)
    order.refunded_amount += amount
    db.save(order)

No check that amount doesn't exceed the order total, no test verifying the accumulation logic, no test for calling this twice. This is exactly the kind of function where a bug doesn't show up as a crash — it shows up as a quiet accounting discrepancy three weeks later that someone in finance has to chase down.

How to raise it constructively: Explain the category of risk rather than just asking for "more tests," which can feel like busywork. "Since this touches refund amounts directly, could we add a test for double-refunding and for a refund larger than the order total? Mostly want to make sure double-submission doesn't double-refund." That gives the author a concrete, reasoned target instead of an open-ended homework assignment.

4. Unclear Naming

This one is easy to dismiss as bikeshedding, and sometimes it is. But naming is usually a proxy for something else — either the author isn't totally sure what the variable represents, or the code is doing something slightly different from what its name implies.

data = get_data(id)
if data:
    process(data)

What is data? An order? A user? A raw API response? A parsed object? The name tells you nothing, and six months from now, "what does data actually contain" becomes an archaeology project.

How to raise it constructively: Ask instead of dictating. "What does data represent here — maybe pending_order or similar would make the next line clearer?" Sometimes the act of answering that question makes the author realize the variable is doing two different jobs, which is a more valuable outcome than just picking a better name.

5. Excessive Complexity

Code that's technically correct but takes five reads to understand is still a liability — it's just a slower one. Complexity tends to accumulate gradually, one small conditional at a time, until a function that started simple is six levels of nested logic deep and nobody remembers why.

function getStatus(order) {
  if (order.cancelled) {
    if (order.refunded) {
      return "cancelled_refunded";
    } else {
      if (order.refundPending) {
        return "cancelled_refund_pending";
      }
      return "cancelled";
    }
  } else {
    if (order.shipped) {
      return order.delivered ? "delivered" : "shipped";
    }
    return order.paid ? "processing" : "pending";
  }
}

Every branch here is individually reasonable. Together, they're hard to hold in your head at once, and that's the actual cost — not that any single line is wrong, but that verifying correctness requires simulating every path manually.

How to raise it constructively: Point at the shape, not the person's competence. "This has a lot of nested branches — would a lookup table or early returns make the cases easier to scan?" Offering a concrete alternative (rather than just "this is too complex") turns the comment into something actionable instead of a vague judgment.

6. Duplicated Logic

A little duplication is often fine, even healthy — premature abstraction can be worse than a few repeated lines. But when the same rule is implemented in more than one place, you've created a situation where fixing a bug requires remembering every location it was copied to, and someone always forgets one.

# in billing.py
if not re.match(r"^[^@]+@[^@]+\.[^@]+$", email):
    raise ValueError("invalid email")

# in signup.py, written by someone else, months later
if "@" not in email or "." not in email:
    raise ValueError("bad email")

Two different validation rules for the same concept, drifting further apart every time either one gets touched.

How to raise it constructively: Name the pattern rather than just the instance. "I think this is the third place we validate emails, and they're not all doing the same check — might be worth pulling into a shared function at some point, doesn't need to block this PR." Flagging it without demanding an immediate refactor respects the scope of what the author was actually trying to ship.

7. Poor Error Handling

The failure path is where a lot of real bugs live, precisely because it's the path least exercised during normal development. Code that handles success gracefully and failure carelessly tends to look complete right up until it isn't.

try {
  await processPayment(order);
} catch (e) {
  console.log("payment failed");
}

The payment failed, the user probably still thinks their order went through, and the only record of what happened is a console log nobody's watching.

How to raise it constructively: Ask what the user or caller experiences on failure, not just what the code does. "If this catch fires, what does the customer see? Right now I think they'd get a success response — should we surface the failure back to them?" Grounding the comment in the actual downstream experience makes the stakes concrete instead of abstract.

8. Unexpected Side Effects

A function whose name promises one thing but does another is a specific, sneaky kind of bug, because it's invisible at every call site — the code reads correct everywhere it's used, and the mismatch only becomes apparent when something downstream breaks for reasons nobody can trace back to this function.

def get_user_display_name(user):
    user.last_seen = datetime.now()
    db.save(user)
    return f"{user.first_name} {user.last_name}"

A function that looks like a pure getter is quietly writing to the database on every call. Call it in a loop to render a list of a hundred names, and you've just issued a hundred writes nobody intended.

How to raise it constructively: Point out the mismatch between name and behavior specifically. "This reads like a pure getter but it's also updating last_seen — is that intentional? If so, might be worth splitting the write out or renaming so it's obvious from the call site." The goal is making the side effect visible, not implying the author was being sneaky — usually it just accreted there over time.

9. Architectural Problems

Sometimes a change is locally correct and still a problem, because of where it lives rather than what it does. A new responsibility bolted onto an existing module, a new dependency pointing the wrong direction, a boundary quietly crossed — these are the hardest comments to make well, because they're rarely about the diff itself.

# inside the OrderService
def send_slack_notification(order):
    requests.post(SLACK_WEBHOOK_URL, json={"text": f"New order {order.id}"})

Nothing here is "wrong" in isolation. But now OrderService — which should own order logic — also owns notification formatting and an external HTTP dependency, and every future change to how notifications work has to route through a class that was never meant to care about them.

How to raise it constructively: These comments land best when they're explicitly framed as bigger-than-this-PR, so the author doesn't feel blocked on solving something out of scope. "This works, but I think notification logic living inside OrderService is going to get tangled as we add more notification types — worth a separate NotificationService down the line? Not blocking this PR on it." Naming the concern without demanding it be resolved right now respects both the code and the person's afternoon.

10. Code That Future Developers Cannot Understand

This is the hardest one to pin down because it's not really a rule — it's a judgment call about whether someone with less context than the author, arriving in a year, can safely modify this code without breaking something they can't see.

def calc(x, y, z):
    return x * 0.85 + y * (1 if z > 3 else 0.5) - 12

Every one of these numbers means something. None of that meaning is written down anywhere. Whoever touches this function next either has to reverse-engineer the business rule from the arithmetic, or — more likely — just leaves it alone forever because nobody wants to be the one who breaks a formula they don't understand.

How to raise it constructively: Ask the author to explain it in the comment thread, and then ask if that explanation could live in the code instead of in the PR discussion. "Could you walk me through what 0.85 and the z > 3 threshold represent? If it's a business rule, I think it'd help future-us to have that as a comment or named constant." Often the author does know exactly what it means — the review is really just prompting them to write it down somewhere more permanent than their own memory.

Rejecting Code Without Discouraging the Developer

All ten of the reasons above can be delivered in a way that either helps someone grow or makes them dread opening their next PR. The difference usually comes down to a few habits.

Separate the code from the person. "This has a bug" and "you wrote a bug" describe the same fact but land completely differently. The first is about the artifact. The second, even unintentionally, is about the author's competence.

Distinguish blocking issues from everything else. Not every comment needs to hold up a merge, and treating a naming nit with the same tone as a security hole trains people to either panic at every comment or ignore all of them. Say plainly when something isn't blocking.

Ask before asserting, when there's room to be wrong. A reviewer often doesn't have the full context the author does. "Why did this end up this way?" invites an explanation you might be missing. "This should be done differently" doesn't leave room for one, even when you're right.

Explain the why, not just the what. "Move this to a separate function" is an instruction. "This function is doing three things, which makes it hard to test them independently — could we split it up?" is a reason, and reasons generalize to the next PR in a way that instructions don't.

Say something positive when it's actually true. Not as a cushioning tactic, but because reviews that are 100% critique start to feel like a performance review nobody asked for, and people stop putting effort into approaches worth praising because the effort never gets acknowledged.

None of this means lowering the bar. A PR with a real security hole should still get blocked, clearly and without hedging. The point isn't to be gentler about what gets rejected — it's to be more deliberate about how, so that the person on the other end of the review comes away having learned something, instead of just having been told no.

The reasons a PR gets rejected are, in the end, pretty consistent across teams and codebases. What varies enormously is whether the developer who wrote it feels like the review made their code better, or just made their afternoon worse. That difference is almost entirely in the delivery, not the content.

7 Things I Look For Before I Approve a Pull Request

Codzee.io — Tue, 11 Aug 2026 10:54:13 +0000

7 Things I Look For Before I Approve a Pull Request

Code review is one of those skills nobody really teaches you. You get thrown into it, you start by nitpicking variable names, and eventually — usually after shipping a bug that a five-second look would have caught — you develop an actual system.

This is the system I use. It's not exhaustive and it's not fancy. It's just seven questions I ask myself on every PR, roughly in this order, before I click approve. I've included real code examples for each one — a bad version, a better version, and what I'm actually looking for as a reviewer.

Let's get into it.

1. Does the code actually solve the problem?

This sounds obvious, but it's the check people skip most often, because it's tempting to jump straight into reading syntax instead of first asking "does this match the ticket?"

I read the PR description and the linked issue before I read a single line of code. Then I ask: if I ran this exact scenario, would it produce the right outcome? Not "does this look like reasonable code" — does it solve the actual problem.

Bad version — closes the ticket "filter out inactive users" but silently changes behavior elsewhere:

function getUsers(users) {
  return users.filter(u => u.active && u.role !== 'guest');
}

The ticket asked for inactive users to be filtered out. This PR also drops guests — a requirement nobody asked for, that might break another screen relying on guest visibility.

Improved version:

function getActiveUsers(users) {
  return users.filter(u => u.active);
}

Scoped exactly to what was requested. If guest filtering is genuinely needed, it belongs in its own PR with its own ticket and its own tests.

What to notice as a reviewer: scope creep disguised as a fix. Extra changes "while I was in there" are one of the most common sources of surprise regressions. If the PR does more than the ticket asked for, ask why, out loud, in the comments.

2. Is the code understandable?

Working code and readable code are not the same thing. I try to read each function the way I'd read it cold, six months from now, with zero memory of writing it.

Bad version:

def calc(d, t):
    r = []
    for x in d:
        if x[2] > t:
            r.append((x[0], x[1] * 1.1))
    return r

I have no idea what this does without tracing through it. What's d? What's index 2? Why 1.1?

Improved version:

def apply_discount_to_high_value_orders(orders, threshold):
    discounted_orders = []
    for order in orders:
        if order.total > threshold:
            discounted_orders.append((order.id, order.total * 1.1))
    return discounted_orders

Same logic, but now I can understand the function's purpose from its name and its variables, without running it in my head.

What to notice as a reviewer: if you have to open a second tab, trace three function calls, or ask the author "wait, what does this do?" in a comment, that's a readability problem — not a you problem. Flag it. Naming and structure are load-bearing, not cosmetic.

3. What happens when something goes wrong?

Most demos work. The question is what the code does when the network drops, the API returns a 500, or a file doesn't exist. I specifically look for what happens on the unhappy path, because that's the path nobody tests manually before opening the PR.

Bad version:

async function fetchUserProfile(userId) {
  const response = await fetch(`/api/users/${userId}`);
  const data = await response.json();
  return data;
}

No handling for a failed request, a non-200 response, or malformed JSON. One flaky network call and this throws an unhandled exception somewhere upstream, with a stack trace that tells the user nothing useful.

Improved version:

async function fetchUserProfile(userId) {
  let response;
  try {
    response = await fetch(`/api/users/${userId}`);
  } catch (err) {
    throw new Error(`Network error fetching user ${userId}: ${err.message}`);
  }

  if (!response.ok) {
    throw new Error(`Failed to fetch user ${userId}: ${response.status}`);
  }

  return response.json();
}

Now failures are explicit, informative, and distinguishable — a caller can tell the difference between "the network died" and "the server said no."

What to notice as a reviewer: any await, file read, external call, or parse step with no corresponding failure path. Ask: "what does the user see if this line throws?" If the honest answer is "a blank screen" or "an unhandled promise rejection in the console," that's not ready to merge.

4. Are edge cases handled?

This is different from error handling. Edge cases are inputs that are technically valid but easy to forget: empty lists, zero, negative numbers, duplicate entries, the very first or very last item, extremely long strings.

Bad version:

function getAverageScore(scores) {
  const total = scores.reduce((sum, s) => sum + s, 0);
  return total / scores.length;
}

Pass in an empty array and you get NaN, silently, with no error and no signal that anything went wrong.

Improved version:

function getAverageScore(scores) {
  if (scores.length === 0) {
    return null; // no scores yet — caller decides how to display this
  }
  const total = scores.reduce((sum, s) => sum + s, 0);
  return total / scores.length;
}

Now the empty case is a deliberate decision instead of an accident, and it's documented right there in the code.

What to notice as a reviewer: try to mentally run the function on an empty input, a single-item input, and a huge input. If the author clearly only ever tested with "3-5 typical items," that's usually where the bugs are hiding. Ask specifically: "what happens with zero items?" — it's a cheap question that catches a lot.

5. Is the code secure?

You don't need to be a security engineer to catch the common stuff. Most security bugs in day-to-day PRs aren't exotic — they're unsanitized input, secrets in code, or overly trusting data from outside the system.

Bad version:

app.get('/search', (req, res) => {
  const query = `SELECT * FROM products WHERE name = '${req.query.name}'`;
  db.execute(query, (err, results) => {
    res.json(results);
  });
});

Classic SQL injection — anything the user types goes straight into the query string.

Improved version:

app.get('/search', (req, res) => {
  const query = 'SELECT * FROM products WHERE name = ?';
  db.execute(query, [req.query.name], (err, results) => {
    if (err) return res.status(500).json({ error: 'Search failed' });
    res.json(results);
  });
});

Parameterized queries mean user input is data, never executable SQL, no matter what someone types into the search box.

What to notice as a reviewer: any spot where user input touches a query, a shell command, a file path, or gets rendered directly into HTML. Also scan for hardcoded API keys, tokens, or passwords — even in a comment, even "temporarily." And check whether data from external APIs is validated before being trusted, the same way you'd validate user input.

6. Are the tests meaningful?

A PR with 100% coverage can still have useless tests. I look at whether the tests actually verify behavior, or whether they just exercise the code without asserting anything meaningful.

Bad version:

test('calculates discount', () => {
  const result = applyDiscount(100, 0.1);
  expect(result).toBeDefined();
});

This test passes no matter what applyDiscount returns, as long as it returns something. It would still pass if the function returned -999.

Improved version:

test('applies a 10% discount correctly', () => {
  expect(applyDiscount(100, 0.1)).toBe(90);
});

test('returns original price when discount is zero', () => {
  expect(applyDiscount(100, 0)).toBe(100);
});

test('throws on a negative discount rate', () => {
  expect(() => applyDiscount(100, -0.1)).toThrow();
});

Each test verifies a specific, checkable outcome — including an edge case and an invalid input.

What to notice as a reviewer: look for assertions like toBeDefined(), toBeTruthy(), or no assertion at all — these are common signs of a test written to satisfy a coverage number rather than to catch a regression. Also check whether the tests cover the failure paths and edge cases from sections 3 and 4 above, not just the happy path. A test suite that only tests success is only half a test suite.

7. Will another developer understand this six months from now?

This is the catch-all question, and I ask it last because by this point I've usually already spotted the answer. It's about intent, not syntax: will someone (possibly you) be able to figure out why this code exists, not just what it does.

Bad version:

// fix
if (user.type === 3) {
  discount = 0.15;
}

What is type 3? Why 0.15? Six months from now, this is an archaeology project.

Improved version:

const USER_TYPE_ENTERPRISE = 3;
const ENTERPRISE_DISCOUNT_RATE = 0.15;

// Enterprise customers get a 15% discount per the Q3 pricing agreement (JIRA-4821)
if (user.type === USER_TYPE_ENTERPRISE) {
  discount = ENTERPRISE_DISCOUNT_RATE;
}

Named constants explain what, the comment explains why, and the ticket reference gives anyone a way to dig deeper if they need to.

What to notice as a reviewer: magic numbers, cryptic comments like // fix or // hack, and any logic that encodes a business rule without saying where that rule came from. If you, the reviewer, have to ask "why does this exist?" in the PR comments, future-you (or future-someone) will have to ask the same question with nobody around to answer it.

The Pull Request Checklist

Here's the condensed version I actually use, PR by PR:

[ ] Solves the problem — matches the ticket, no unrelated scope creep
[ ] Readable — I can follow the logic without re-reading it three times
[ ] Handles failure — network errors, bad responses, and exceptions are caught and explicit
[ ] Handles edge cases — empty input, zero, single item, duplicates, extremes
[ ] Secure by default — no raw string interpolation into queries/commands, no hardcoded secrets, external input is validated
[ ] Tests actually assert something — happy path, at least one edge case, at least one failure case
[ ] Understandable in six months — no unexplained magic numbers, comments explain why not what

None of these checks take more than a minute or two individually. Together, they take maybe ten to fifteen minutes on a normal-sized PR — and they catch the vast majority of the bugs and headaches that would otherwise surface in production, at 2am, with much less context than you have right now.

Review is cheap. Production incidents aren't. That trade-off is really the whole point of this checklist.