AI Code Review as a CI Merge Gate: What Should Block

The reviewer went in on a Thursday. By the middle of the next week it had commented on every open pull request in the repository, including a two-line copy change. The comments were not wrong, exactly: a variable name, a helper that could have been a map, a comment that no longer matched the line under it. And near the bottom of one of those reviews, past where anybody was still reading, a note that a migration in the diff dropped a column the deployed release still selected.

That is the failure worth writing about: not a reviewer that misses things, but one right often enough to install and noisy enough to mute. Wiring it into CI takes an afternoon. Deciding what it may block, and proving that decision with numbers before it blocks anything, is the work, and it is the part I got wrong first. What follows is the split I use now, the measurement that earns a class the right to block, and the rule that takes it away.

A muted gate is worse than no gate

Adoption and trust are moving in opposite directions, and a merge gate has to be designed for the gap. The 2025 Stack Overflow Developer Survey reports that 84% of respondents are using or planning to use AI tools in their development process, up from 76% the year before, and that 51% of professional developers use AI tools daily. The same survey reports that more developers actively distrust the accuracy of AI tools, 46%, than trust it, 33%, with only 3% highly trusting the output. Usage rose; trust in accuracy hit an all-time low.

That is why a gate needs evidence rather than enthusiasm. A team that doubts a tool's accuracy will not argue with it in public; it will learn where the collapse arrow is. In the first week people read the comments, by the second they scrolled past them to the diff, and by the third somebody asked whether the check was still running at all, which is the honest measure of a muted gate: nobody knows. A muted gate is worse than no gate, because the team now believes something is checking, and human reviewers skim harder when they think a machine looked first.

Three properties a class needs before it may block

A class of finding may block a merge only when all three of these hold at once. Two out of three is an advisory comment, and that is not a consolation prize.

• Objectively decidable. There is a fact of the matter a second engineer would agree with, without knowing the author or the deadline. A route either has an authorisation check on it or it does not.
• Cheap to verify. The author can confirm or refute it in about a minute from the comment alone, because the comment names the file, the line and the missing thing. A finding that needs a meeting to settle is not a gate, it is an agenda item.
• Rarely wrong, measured rather than assumed. A blocking class that is wrong one time in three teaches the team that red means maybe, and that lesson does not stay inside the review bot. It generalises to your test suite.

Three classes clear that bar, and all three are boring. A migration that drops or renames a column the deployed release still reads, decidable by grepping the deployed tag for the column name and expensive enough in production to deserve a red cross. A new HTTP route with no authorisation check, the risk OWASP Top 10:2021 ranks first, present in the 94% of applications tested for some form of Broken Access Control, and whose prevention rule is one sentence a machine can apply: except for public resources, deny by default. And a live credential in the added lines, which is barely an AI finding at all.

Everything requiring taste is advisory by construction

The other half of the rule is easier. Naming, file layout, whether a function could be simpler, whether an abstraction arrived too early: these are the findings an AI reviewer produces most fluently and has the least standing to enforce. Human reviewers solved this with vocabulary rather than tooling. Google's engineering practices tell reviewers to prefix a minor comment with Nit, meaning technically you should do it but it will not hugely impact things, to use Optional or Consider for an idea that is not strictly required, and FYI for something the author is not expected to act on in this change. The label stops the author reading every comment as a requirement. An AI reviewer shipped without that vocabulary is one whose every remark reads as mandatory.

The last row is the one that cost me. Simplification suggestions were the reviewer's highest-volume output, they were frequently reasonable, and letting them block for a single sprint is what taught everybody to ignore the bot for good. That is the shape of the mistake: a gate is muted by its most defensible-sounding class, not its most obviously wrong one.

Wire the block as its own required check

The split has to live in the pipeline, not the prompt. One job that comments on everything and fails on some of it puts the gate's behaviour inside a model's output, where you cannot change it without a redeploy and cannot audit it at all. Two jobs, one required and one not, moves the policy into YAML, where it is reviewable, diffable and revertible like any other change.

# .github/workflows/ai-review.yml
# Two jobs on purpose. Only the first is a required status check; the second
# can never turn the merge button red, whatever it thinks of your naming.
name: AI review
on: pull_request

jobs:
  blocking:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v7
        with:
          fetch-depth: 0          # need real history to find the merge base

      # A secret in a diff is a pattern match, not an opinion. Do not pay a
      # model to do a job a regex does deterministically and for free.
      - name: Secrets in the added lines
        run: |
          BASE=$(git merge-base "origin/$GITHUB_BASE_REF" HEAD)
          gitleaks git --log-opts="$BASE..HEAD" --redact --exit-code 1 .

      # Only the classes that earned the right to block. --fail-on=verified
      # means the model proposes and a checker confirms: the reviewer must
      # name the route, column or file, and the checker must find it there.
      - name: Reviewer, blocking classes only
        run: node scripts/ai-review.mjs
             --classes=secret,missing-authz,unsafe-migration
             --fail-on=verified

  advisory:
    runs-on: ubuntu-latest
    continue-on-error: true       # this job's conclusion is ignored on purpose
    steps:
      - uses: actions/checkout@v7
      - name: Reviewer, everything else
        run: node scripts/ai-review.mjs --classes=all --advisory --label=nit

The blocking job runs the deterministic checks first, then the reviewer restricted to the classes that earned their place, with every model finding held behind a verifier: the reviewer names the route, the column or the file, and something that is not the model has to find it there. Secrets are the clearest case for keeping the model out entirely, because gitleaks scans a git log range directly, exits 1 on a hit by default, and redacts the secret from its own output. Only the blocking job goes into the branch protection rule; the advisory job carries continue-on-error, so its verdict is recorded and never enforced.

Shadow mode: measure it on merged pull requests first

Before the reviewer may block anything, run it where it can do no damage: over pull requests that already merged. The diffs are real, the outcomes are known, and nothing it says reaches an author. A couple of hundred merged pull requests gives a per-class count worth arguing about, and it costs a laptop and one evening rather than anybody's goodwill.

# Shadow mode: replay already-merged pull requests through the reviewer.
# Nothing is posted, nobody is notified. The output is a file you can count.
gh pr list --state merged --limit 200 --json number --jq '.[].number' \
  > /tmp/merged-prs.txt

while read -r pr; do
  gh pr diff "$pr" --patch > "/tmp/pr-$pr.patch"

  # Same binary, same prompt, same config as the CI job will use. Change one
  # of the three and the rate you measured stops describing what you shipped.
  node scripts/ai-review.mjs --patch "/tmp/pr-$pr.patch" --json \
    | jq -c --arg pr "$pr" '.findings[] | [$pr, .class, .file, .line, .message]' \
    >> /tmp/shadow-findings.jsonl
done < /tmp/merged-prs.txt

# What humans flagged on the same diffs, for the other half of the picture:
# a class the reviewer never raises is not precise, it is silent.
while read -r pr; do
  gh api "repos/OWNER/REPO/pulls/$pr/comments" --paginate \
    --jq '.[] | [.path, .line, .user.login, .body]' \
    >> /tmp/human-comments.jsonl
done < /tmp/merged-prs.txt

Then count. Precision per class is the number the gate decision turns on: of the findings the reviewer raised in a class, how many a human would have agreed with. Recall matters too, and the human review comments on those same diffs are the cheapest proxy available, because a class the reviewer never raises is not precise, it is silent. The adjudication step is where this goes wrong, so run it blind.

// scripts/shadow-report.mjs
// One precision figure per finding class, with the sample size behind it.
const MIN_SAMPLE = 20;   // policy, not a measurement: below this, no verdict
const PROMOTE_AT = 0.9;  // policy: one wrong finding in ten is the most I
const DEMOTE_AT = 0.8;   // will inflict on a gate, and one in five revokes it

const rows = readJsonl("/tmp/shadow-adjudicated.jsonl");
const byClass = new Map();

for (const r of rows) {
  // r.verdict was set by a human who saw the finding WITHOUT its class label
  // and WITHOUT being told the PR shipped fine. Adjudicate blind, or the
  // number is theatre: knowing it merged makes every finding look like noise.
  const c = byClass.get(r.class) ?? { agreed: 0, rejected: 0 };
  if (r.verdict === "agreed") c.agreed++;
  else c.rejected++;
  byClass.set(r.class, c);
}

for (const [cls, c] of byClass) {
  const n = c.agreed + c.rejected;
  const precision = c.agreed / n;
  const verdict =
    n < MIN_SAMPLE
      ? "insufficient sample"
      : precision >= PROMOTE_AT
        ? "eligible to block"
        : precision < DEMOTE_AT
          ? "advisory, and losing ground"
          : "advisory";
  console.log(cls.padEnd(20), String(n).padStart(4), precision.toFixed(2), verdict);
}

Blindness is not ceremony. An adjudicator who knows the pull request merged and shipped without incident marks almost everything as noise, because the absence of an incident feels like proof of harmlessness. Strip the class label and the outcome, shuffle the rows, and ask one question per row: would you have asked for a change here. The rate that comes out of that is the only one worth putting in a branch protection rule.

Promotion and demotion rules, written down before you need them

Commit these to the repository as a policy rather than deciding them under pressure while a release is blocked and three people wait. Mine are five lines.

1. A new class starts in shadow mode and posts nothing anywhere. It stays there until it has at least 20 blindly adjudicated findings, because a rate computed over four samples is a mood.
2. It graduates to advisory at any precision, labelled as a nit, and stays advisory for a minimum of two working weeks no matter how good the shadow numbers looked.
3. It may be promoted to blocking only at 90% precision or better on blind adjudication, and only when a deterministic verifier can confirm the finding without the model.
4. Every blocking class has a named owner and an override path that leaves a trace, a label on the pull request rather than a quiet word with whoever holds admin rights.
5. It is demoted the same day on one confirmed false block, or on precision under 80% in any month. Demotion needs no meeting and no consensus. Only promotion does.

The asymmetry is deliberate. Promotion has a cost that arrives later and spread thinly across everyone, so it should be slow and require evidence. Demotion prevents the exact failure this post is about, so it should be immediate and require nothing. In practice the demotion rule is what makes people willing to promote anything, because it turns a permanent-feeling decision into a reversible one.

Most of the value was in the boring checks anyway

After all that calibration, the reviewer's genuinely useful output turned out to be unglamorous. A summary of a large diff that lets a human choose which four files to read carefully. A test copied and left asserting nothing. A route added without the guard every one of its neighbours has. A column dropped in the same pull request that still reads it three files away. Fast, mechanical, and each one traceable to a fact sitting in the diff.

The design opinions were the part I wanted to be valuable and the part that had to be defanged. That is not a limitation of one tool or one model; it is what the trust figures predict. A team that distrusts a machine's accuracy will still accept its verdict on facts and reject its verdict on taste, and a gate built against that grain gets muted however good the model becomes. Sell the boring checks internally and the reviewer survives its first bad week.

One rule to carry: a class may block only when it is objectively decidable, cheap to verify and measured to be rarely wrong, and it stays advisory until all three are true at once. Build the shadow-mode harness before the gate rather than after the argument, write the demotion rule down while nothing is on fire, and accept that a good AI reviewer's best work looks more like a checklist than a critique.