DEV Community

Dimitris Kyrkos
Dimitris Kyrkos

Posted on

Your AI code reviewer isn't a second opinion. It may be the first opinion again.

A model writes the change. A model reviews the change and says it looks good. The pull request now has two positive signals on it, and it feels like two checks happened.

Often it was closer to one.

AI review is useful, and I'm not arguing against using it. I'm arguing against what teams quietly start treating it as: an independent security check.

The demo vs. the merge queue

In the demo, the reviewer catches an unsanitized input, explains it clearly, and suggests a fix. That's real value, and it does happen.

In the merge queue, the hard cases look different. The code is clean, the summary is confident, and the flaw is something neither the author nor the reviewer had any reason to consider, because neither was told about it.

The scenarios below are illustrative, not real incidents.

Failure mode 1: shared assumptions

The ticket says: "Add an endpoint so users can download their invoices." A coding assistant produces this:

@login_required
def download_invoice(request, invoice_id):
    invoice = Invoice.objects.get(id=invoice_id)
    return FileResponse(open(invoice.pdf_path, "rb"))
Enter fullscreen mode Exit fullscreen mode

An AI reviewer reads the diff and reports: authentication is enforced, the ORM prevents SQL injection, no obvious issues. Each of those statements is true.

Any logged-in user can still download any invoice by changing the ID.

The generator treated "logged in" as "allowed." If the reviewer starts from the same ticket, the same diff, and the same mental shortcut, it can make the same mistake. Using a different model may help, but it doesn't make the review independent. A gap in the context is a gap for every model that receives that context.

The useful check starts somewhere else, at the requirement: who is allowed to read this invoice?

Failure mode 2: the diff isn't the control flow

A reviewer that sees only the diff sees only what changed. Security behavior often lives in what didn't.

Say a PR adds a new router in routes/export.js. The file looks fine. But the auth middleware is mounted per path in app.js, a file the PR never touches, and the new path isn't covered. No line in the diff is wrong. The bug exists in the relationship between that diff and the rest of the system.

Before trusting "looks safe" on a sensitive change, trace the path a request actually takes. If the tool can't show you that path, it hasn't checked it.

Failure mode 3: the confident explanation

"This change validates input and enforces appropriate permissions" is a sentence about the code. It isn't evidence about the code.

Fluent summaries are easy to accept, especially at the end of a long day with ten PRs left. A simple habit helps: for any important claim, ask what would demonstrate it. Which line enforces the permission? Which test fails if it's removed? If nobody can point to anything, the claim is still a hypothesis.

Failure mode 4: tests that inherit the blind spot

Generated tests can repeat the same assumption as generated code. A test that logs in as the owner and gets a 200 passes on the vulnerable endpoint above. It proves the happy path works.

A test written from the requirement looks different:

def test_user_cannot_download_someone_elses_invoice(client, alice, bobs_invoice):
    client.force_login(alice)
    response = client.get(f"/invoices/{bobs_invoice.id}/download")
    assert response.status_code == 404
Enter fullscreen mode Exit fullscreen mode

That test fails until the query is scoped to the owner, for example with get_object_or_404(Invoice, id=invoice_id, owner=request.user). This is also a good place for AI to help: ask it to suggest abuse cases, then keep the ones that map to real requirements.

Failure mode 5: the summary gets merge rights

The riskiest setup is the one where a model's output moves the process forward without a person: auto-approving a PR, or closing a high-severity finding because the explanation sounded reasonable.

There's a second problem here. The code under review is also input to the reviewer, so text in a diff can try to steer the result. That's one more reason not to let a model's verdict be the only gate.

Different checks fail in different ways

Independence comes from combining methods that don't share the same blind spots:

  • Human review: is this the right behavior, and does it match the business rules?
  • Tests: does it do what the requirement says, including what it must refuse?
  • Static analysis: do known risky patterns and data flows appear?
  • Dependency checks: did this pull in a version with a known vulnerability?
  • Secret scanning: did a credential get committed?
  • AI review: what looks suspicious, what is this unfamiliar code doing, and what should we test?

None of these is enough alone, and that's the point. They disagree usefully.

A policy you could adopt this week

  • AI review comments are advisory. They never count as an approval.
  • A model can't close a high-severity finding. A person closes it and records why.
  • Changes touching auth, payments, secrets, or personal data need a human reviewer plus the relevant automated checks.
  • Important AI findings get checked against the requirement, the control flow, and other evidence.
  • Accountability for the merge stays with a named person.

AI review is a way of directing attention. Use it to find where to look, then go look. The value is better attention, not outsourced accountability.

Where does AI review sit in your pipeline right now? Has it ever been the only thing standing between a security-sensitive change and main, even by accident?

Top comments (3)

Collapse
 
jacksonxly profile image
Jackson Ly •

AI review was nearly the only check once, on an event app I build. An agent reviewed the new helper role's grant table and tests and reported no permission bugs. I asked for a pass on staging as a seeded helper, clicking through everything. That found a ticket-pricing page showing sales totals to helpers and crew, plus an unrelated double booking. The grants matched the written spec the whole time, and the problems only showed up once a helper opened the screens.

Collapse
 
cyclopt_dimitrisk profile image
Dimitris Kyrkos •

That's a great example of something that's easy to miss when reviewing code in isolation. The grants can be perfectly correct against the spec and you can still have problems in how the app actually behaves. The seeded helper walkthrough is exactly the kind of independent check I had in mind. It tests the experience and the boundaries, not just whether the implementation matches what was asked for.

Collapse
 
alexshev profile image
Alex Shev •

Independence is the missing variable. A review model that shares the author model’s prompt, context, and assumptions can repeat the same blind spot with more confidence. Use a contrast set with hidden defects and measure disagreement by defect class, not just whether the reviewer produces plausible prose.