Your Coding Agent Passes the Tests and Still Fails Review

A team I was helping had a coding agent wired into their issue tracker. Pick up a ticket, read the repository, write the patch, open a pull request. On paper it worked: the agent closed issues, the test suite went green, and CI waved the pull request through to a human. Then the senior engineer who owned review would read the diff and reject roughly half of them. Not because the tests were wrong. The tests passed. He rejected them because the change was the wrong change: it widened a function signature the whole codebase leaned on, or it fixed the reported bug by editing a shared helper and quietly broke three callers the ticket never mentioned, or it was simply more code than the problem deserved.

The frustrating part was that every rejected pull request had a green bar on it. The agent had done exactly what it was measured on. It had made the failing case pass. Nobody had told it, and nothing in the pipeline could tell it, that passing the case and being mergeable are not the same thing.

The gap between a green grader and a merge

This is not a local quirk of one team. In March 2026, METR ran the experiment properly: they took 296 AI-generated pull requests and had four active maintainers from three SWE-bench Verified repositories, scikit-learn, Sphinx, and pytest, decide whether they would actually merge each one. The result is the number worth memorising. Maintainer merge rates ran about 24 percentage points below the automated grader after normalising against human-written patches, and about 35 points below it unnormalised. (Many SWE-bench-Passing PRs Would Not Be Merged into Main, METR, March 2026.)

Read that as an engineer, not as a benchmark watcher. The grader is your CI. It runs the hidden tests and reports pass or fail. The maintainer is your reviewer. The grader said yes and the reviewer said no about a quarter of the time more often than a human baseline would explain. The reasons the maintainers gave are the ones you already know from your own queue: code quality, breaking code the change was not supposed to touch, and failing the core functionality the issue actually asked for even though some narrower test went green.

The honest caveat, which METR states plainly, is that this is not a capability ceiling. The agents got one shot with no feedback, and the newest model in the study was Claude Sonnet 4.5. A stronger model with a revise loop will close some of this gap. But the structural point survives any model: the thing you are gating on, a green test run, is not the thing you care about, a change you would keep. The same way a living eval set fed by production traces catches regressions your fixed suite never imagined, a merge gate has to measure things the test suite was never built to see.

Why passing tests is the wrong gate

A test suite answers one question: does the machinery run as specified. The targeted case passes, the schema validates, the service answers. That is necessary and it is cheap, and it is also silent about everything that makes a change mergeable. It will not tell you that the agent solved a narrow bug by broadening a public interface, that the diff deletes an assertion that was load-bearing, or that the fix touches a file whose reverse dependencies the agent never looked at.

There is a deeper reason the grader misleads, and a recent position paper names it well. Benchmarks graded against a single reference solution penalise equally valid alternatives and fold the model, the harness, and the environment into one end-to-end number, so a green result tells you almost nothing about which part earned it. (Position: Coding Benchmarks Are Misaligned with Agentic Software Engineering, June 2026.) Flip that around and the production lesson is sharp: a pass is a coarse, entangled signal, and treating it as a merge decision imports all of that entanglement into your main branch.

This is the same mistake as grading an agent only on its final answer while ignoring how it got there, which is why grading the path and not just the destination matters for any agent you run in anger. A coding agent has a path too, and the diff is its trajectory. The question a merge gate asks is not did the final case pass. It is: is this diff the kind of change we would accept, given everything it touched on the way to green.

What a merge gate actually checks

A merge gate is a stage you insert between CI and the human, and it checks three things the suite does not.

The first is collateral regression. The agent runs the tests it was pointed at. The gate runs the tests that the diff could have broken, which is a different and usually larger set: the reverse dependencies of every file the change touches. A fix that makes the reported bug pass while breaking an unrelated module is the single most common way an agent earns a green bar it does not deserve, and it is exactly what the maintainers in the METR study rejected as breaking other code.

The second is diff scope. Most unmergeable agent diffs are not subtly wrong, they are too big or in the wrong place. A change that edits a public signature, deletes an assertion, or sprawls across a dozen files to close a one-line issue is a scope smell, and scope smells correlate with the review rejections that cost the most time. You do not need a model to catch these. You need a diff parser and a short list of things that require a human to look twice.

The third is a review-grade pass: an explicit rubric, scored by an LLM-as-judge or a reviewer agent, that asks the taste questions a suite cannot. Did the change stay in scope. Does it handle the edge cases the issue implied. Is the error handling real or swallowed. This is where you spend model tokens, and it only works if you have done the unglamorous work of calibrating the judge so its scores mean something before you let it block a merge.

A regression check that runs before review

Start with the cheapest and most deterministic layer, because it costs nothing in tokens and never hallucinates. Compute the blast radius of the diff from the import graph, run the tests that cover it, and fail the gate on any breakage outside the files the issue was scoped to.

def regression_gate(diff, repo, issue_scope) -> list[str]:
    alarms = []
    touched = diff.changed_files()

    # Reverse dependencies: every module that imports a touched file.
    blast = repo.reverse_deps(touched) | touched
    result = repo.run_tests(cover=blast)   # not just the agent's chosen tests

    for failed in result.failures:
        owner = repo.module_of(failed)
        if owner not in issue_scope:
            alarms.append(f"collateral break in {owner}: {failed}")

    # Scope smells: public API edits and deleted assertions need a human.
    if diff.touches_public_signatures():
        alarms.append("diff changes a public signature")
    if diff.deletes_assertions() > 0:
        alarms.append(f"diff removes {diff.deletes_assertions()} assertions")
    return alarms

The important line is cover=blast, not the agent's chosen tests. The whole failure mode is that the agent proved the thing it was looking at and was blind to what it broke elsewhere, which is the coding-agent version of the silent failures that keep every test green while the run is quietly wrong. A reverse-dependency run turns that blind spot into a hard signal, and it does it deterministically, so it never argues with you.

A reviewer agent in the loop

Deterministic checks catch collateral damage and scope, but they cannot judge whether a change is any good. For that you run a review-grade pass against a rubric. The recent work here is encouraging: a reviewer agent that inspects the repository, given the issue and the generated pull request, and returns an accept or revise decision with structured feedback beats single-turn, fixed-context review on both decision accuracy and the resolve rate after revision. (SWE-Review: Closing the Loop on Issue Resolution with Agentic Code Review, July 2026.)

The shape that matters is the loop, not the score. You are not asking the judge for a number to display. You are asking it for feedback the authoring agent can act on, so a weak first draft gets revised before a human ever opens it.

REVIEW_RUBRIC = {
    "in_scope":      "Changes only what the issue requires. No unrelated edits.",
    "correct":       "Fixes the reported behaviour, including implied edge cases.",
    "no_regression": "Does not weaken or delete existing guarantees.",
    "clarity":       "A maintainer could read the diff and understand why.",
}

def review_pass(issue, diff, judge) -> dict:
    scores = judge.grade(issue, diff, rubric=REVIEW_RUBRIC)  # 0 to 1 each
    decision = "accept" if min(scores.values()) >= 0.7 else "revise"
    return {
        "decision": decision,
        "scores": scores,
        # Feedback the authoring agent revises against, not prose for a human.
        "feedback": judge.feedback(issue, diff, scores),
    }

Gate on the weakest dimension, not the average, so a diff that is clean and clear but out of scope does not sail through on a high mean. And run this as a loop with a hard cap on attempts: accept, or revise and retry up to a small bound, then escalate to a human. An agent that can retry forever will talk itself into a confident wrong answer, so the bound is not a convenience, it is a safety rail. The reviewer agent's job is to raise the floor and shrink the human queue, not to own the decision.

The parts that bite

The reviewer agent is a model, so it drifts and it has taste of its own, which may not be your team's. If you never calibrate it against diffs your maintainers actually accepted and rejected, its 0.7 threshold is a number you made up, and it will cheerfully block good changes and pass bad ones with equal confidence. Treat the judge as code under test: hold out a labelled set of real review decisions and check that the gate agrees with your humans before you let it block anyone.

The gate also costs real tokens and real latency on every pull request, and a loop multiplies both. If your agents open a lot of pull requests, run the deterministic regression and scope checks first and only pay for the review pass on diffs that clear them, so you are not asking a model to review code that a free check already rejected.

The deepest trap is the one that feels like success: a reviewer agent that approves everything and a human who, seeing the green review stamp, stops reading. That is worse than no gate, because it launders agent output through a second layer of false confidence. The gate exists to shrink the human's queue, not to retire the human. Keep a person as the real decision on anything that touches authentication, money, data migrations, or a public contract, which is the same reason you keep a human approval step on the actions an agent cannot take back. And keep the gate itself inside your CI so a regression in the gate gets caught like any other, because a gate that silently degrades is just a slower way to ship the diffs you were trying to stop.

The takeaway

The number from METR is the one to carry: maintainers merged about 24 points less often than the grader passed, and the reasons were code quality, collateral breakage, and missing the real ask. None of that is visible in a green test run, because a test run was never built to see it. If your coding agent closes issues and your reviewers keep bouncing the results, you do not have a weak model, you have a missing gate. The green bar is answering a question you were not asking.

Build the gate in the order the checks earn their cost. A deterministic regression and scope check first, because it is free and never lies. A review-grade pass against a calibrated rubric second, run as a revise loop so the human sees a cleaner diff. And a human kept firmly on anything that can hurt you. If you want a second pair of eyes on where that gate should sit in your pipeline, and what it should refuse to let through, I am happy to talk it through. Thirty minutes, no pitch.