AI code review, and the agent I built to gate every merge

Writing code is no longer the expensive part. A model will produce four hundred lines of plausible TypeScript in the time it takes you to describe the feature, and it will do it again, differently, if you ask twice. What has not moved at all is the other side of that trade: the number of lines a person can read carefully in an hour, and the number of hours anyone is willing to spend reading them.

So the constraint slid downstream. The queue no longer backs up at the keyboard; it backs up at the point where somebody has to decide whether the change is correct, whether it does what was asked, and what else it touches. This is the review agent I ended up building for my own repositories — two passes, one on my machine and one on the pull request — what each one is allowed to stop, and the part everybody skips, which is checking that the reviewer itself is any good.

#The diff stopped being readable

A human-written pull request has a shape you can read. It is small because a person got tired, it is inconsistent because a person got interrupted, and the rough edges tell you where to look. The mistakes advertise themselves.

Model-written diffs do the opposite. They are large, uniform, and internally coherent. Naming is consistent, error handling is present everywhere, the tests exist and pass. Nothing in the texture of the change points at the one function where the model quietly inverted a condition, or the migration it wrote against a schema it inferred rather than read.

That combination — big, tidy, plausible — is precisely what defeats the way people actually review, which is skimming for the thing that looks wrong. Nothing looks wrong. So the diff gets a thumbs-up, and the review stopped being a control the day it stopped being a real read.

#Review is three jobs, not one

The word "review" hides three separate questions, and conflating them is why tooling here disappoints.

• Is it correct? Does the code do what the code says it does — no inverted condition, no unhandled null, no off-by-one in the pagination.
• Is it what was asked for? The code can be flawless and still solve a different problem than the ticket described. This is the failure mode that survives every test suite.
• What does it touch? Which callers, which contracts, which data. The question that turns a small change into an incident.

A linter answers a slice of the first one and nothing else. A model handed only the diff answers the first one well, the second one badly, and the third one not at all — it cannot see the callers, because you did not show them to it. Every design decision below follows from wanting all three.

#Two gates, not one

The instinct is to build one reviewer and put it on the pull request. I tried that first and it was wrong in both directions: too slow to be worth running on a two-line change, and too shallow — by the time a change reaches a PR, the author has moved on, and the cost of unwinding a bad shape has gone up.

So it split into two passes with different budgets and different jobs.

The local pass runs before the commit lands. It sees only the staged files, gets no network context, and has a hard time budget measured in seconds. Its job is to catch the class of mistake that is embarrassing to send to a reviewer, human or otherwise.

The pull-request pass runs in CI. It can spend a minute and real money, it can read files the diff never mentions, and it is the one that answers the intent and blast-radius questions.

Splitting them is not a performance optimization. It is a scope decision. A gate that is allowed to be slow will quietly grow until nobody runs it locally, and a gate that is allowed to be shallow will never notice the thing that matters.

#The local pass

This one is deliberately boring. It runs on the staged diff, in a pre-commit hook, with a strict ceiling:

#!/usr/bin/env bash
# .git/hooks/pre-commit — must finish in seconds or it gets bypassed,
# and a hook that gets bypassed is not a gate.
set -euo pipefail
 
DIFF=$(git diff --cached --unified=3 -- '*.ts' '*.tsx' '*.py')
[ -z "$DIFF" ] && exit 0
 
# Hard cap: past this, the change is too big for a fast read anyway —
# defer to the pull-request pass rather than pretending.
LINES=$(printf '%s' "$DIFF" | wc -l)
[ "$LINES" -gt 600 ] && exit 0
 
printf '%s' "$DIFF" | review-agent local \
  --budget 20s \
  --block "secret,destructive-migration,swallowed-error,disabled-test"

The --block list is the whole design. Four categories, all of them things that are cheap to detect from the diff alone and expensive to discover later: a credential pasted into source, a migration that drops or rewrites data without a guard, a catch that logs and continues, and a test commented out or skipped to make a build green.

Everything else the local pass notices, it prints and lets through. The moment this hook blocks on style opinions, people learn --no-verify, and you have traded a gate for a habit.

#The pull-request pass

This is the agentic one. It triggers on the pull request, gathers context, and posts back to the thread:

# .github/workflows/review.yml
name: review
on:
  pull_request:
    types: [opened, synchronize, ready_for_review]
 
jobs:
  review:
    if: github.event.pull_request.draft == false
    runs-on: ubuntu-latest
    permissions:
      contents: read          # read the tree, never write to it
      pull-requests: write    # comments and the check verdict only
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0      # the agent needs history, not just the tip
      - run: |
          review-agent pr \
            --base "${{ github.event.pull_request.base.sha }}" \
            --head "${{ github.event.pull_request.head.sha }}" \
            --conventions docs/conventions.md \
            --issue "${{ github.event.pull_request.body }}" \
            --max-blocking 5

Note what the permissions say. The agent reads the repository and writes comments; it never pushes, never merges, and holds no deployment credentials. That is the same containment rule as any other agent doing real work — the sandbox is not a nicety, it is the reason the thing can be left alone.

Inside, it is a small graph rather than one prompt:

Two nodes carry most of the value, and neither of them is the one that writes the comment.

#Context is the whole job

The single biggest quality difference between a review agent that is worth having and one that is worth muting is not the model. It is what you put in front of the model.

Given a diff alone, a review is a guess about a fragment. Given the diff plus the right surroundings, it becomes an actual read. Four things go in, in roughly this order of value:

• The callers. For every changed exported symbol, the places that use it. This is the only way the blast-radius question gets answered, and it is mechanical — the language server already knows.
• The conventions file. A written document of how this repository does things: error handling, naming, what belongs in which layer. Without it, a model reviews against the average of the internet, which is not your codebase.
• The intent. The ticket, or the PR description if that is all there is. Correct code that solves the wrong problem is invisible without this.
• The prior threads. What was said on earlier reviews of the same files. Otherwise the agent relitigates a decision that was settled in March.

Retrieval is where the effort goes. Everything downstream is comparatively easy.

#Comments are cheap, gates are expensive

Once findings exist, the design question is which of them is allowed to stop a merge. This is a product decision, not a technical one, and getting it wrong in either direction kills the tool.

The mapping I settled on:

• Blocking — a small, closed list: data loss, a leaked credential, an authorization check removed, a swallowed error on a write path. Four or five categories, capped at five findings per pull request.
• Comment — everything the agent can point at concretely and ground in a specific line. Posted inline, not blocking.
• Dropped — anything it cannot ground. If the finding cannot cite the code it is about, it does not get written. This is the dropped edge in the diagram above, and it deletes more output than anything else in the pipeline.

The instinct is to surface everything and let the human triage. That instinct is wrong, because attention is the resource being spent. Twenty comments where three are real does not cost a reviewer seven times as much as three comments — it costs them the habit of reading any of them.

#Where it breaks

None of this is a replacement for a person, and the honest list of limits is short and important.

• It reviews the diff, not the design. It will tell you the function is correct. It will not tell you the function should not exist, or that the third service you just added is one too many.
• It cannot see runtime. Contention, N+1 queries under real cardinality, the latency of the call you added inside a loop — the code looks fine. Static reading has a ceiling and this is it.
• Large refactors defeat it. When nine hundred lines move between files, the signal-to-noise collapses; it reports the move as change. Split the PR or skip the pass; do not read the output.
• Noise compounds. One bad blocking finding costs more trust than ten good comments earn.

#Keeping the reviewer honest

A review agent is itself a system with a failure rate, and almost nobody measures it. The suite that keeps mine useful is small and built out of real history:

Take thirty merged pull requests from the repository. Twenty of them are ones where a bug was later found and fixed — you know exactly what should have been caught, and where. Ten are clean. Run the agent against all thirty on every prompt change, model change, or retrieval change, and record two numbers: how many known bugs it caught, and how many findings it raised on the clean ten.

The second number is the one that matters, and it is the one that gets left out of every vendor claim. Catch rate is easy to buy with a lower threshold; you pay for it in noise, and noise is what gets the tool switched off.

On my own repositories — small codebases, TypeScript and Python, changes mostly under three hundred lines — the shape has been roughly two thirds of known bugs caught by the pull-request pass, and something under one unfounded finding per clean pull request. Those are directional numbers from my own corpus, not a benchmark, and the second one drifts upward every time I widen what the agent is allowed to comment on. Build your own thirty. Yours will look different, and only yours predicts anything.

#At delivery scale

The same structure holds when review stops being one pull request and becomes a stream of them. The gates do not change shape — they change how many of them run in parallel:

Nothing there is a different technique. It is the same four ideas — a cheap gate close to the work, an expensive gate before the merge, grounded findings only, and a person on the last edge — applied to more changes at once.

#The checklist

If you are wiring this up, in order of how much it matters:

• Give it the callers, the conventions and the ticket. Retrieval is the quality lever; the model is not.
• Split into two passes. Fast and narrow before the commit, slow and wide on the pull request.
• Fail open on your own errors. A broken guard must not block work.
• Keep the blocking list to four or five categories, and cap findings per pull request.
• Drop anything it cannot cite. Ungrounded findings are worse than silence.
• Give it read and comment scope only. No pushes, no merges, no deploy credentials.
• Build the thirty-PR corpus and measure noise alongside catch rate, on every change to the prompt, the model, or the retrieval.
• Say what it did not check in the output, so nobody's read gets shorter because of it.

#Where this goes

The volume of code is going to keep going up, and the number of hours anyone will spend reading it is not. The only thing that scales in between is structure: gates that hold in a known place, findings that have to cite the code they are about, and a person on the last edge, spending their attention on the questions a static read cannot answer.

The model writes the code now. Deciding what is allowed to merge is still the job, and it is the one worth engineering.