I Stopped Asking the LLM What the Compiler Already Knew

Merge requests assigned to me as reviewer used to pile up, and the honest reason is that most of what I wrote in them was the same twenty comments. Field injection instead of constructor injection. A .block() in a reactive service. A ${VAR} in application.yaml that nobody added to values.yaml, which fails at deploy time rather than review time.

So I built a bot that reviews them. It polls GitLab every ten minutes, checks the MRs where I'm a reviewer, and either merges them or sends me a notification.

The interesting part isn't the model. It's deciding which stage catches what — and being willing to let the whole thing fail closed.

The version that doesn't work

Take the diff, send it to an LLM, ask "is this good?"

I tried that first. It fails in a specific and predictable way: the model spends its attention on things a compiler would have reported for free, and invents problems in code it can't see. It'll confidently flag a method as undefined because the definition wasn't in the diff. Meanwhile the actual syntax error three files down goes unmentioned, because by then it's 8,000 tokens deep and reconstructing the shape of a codebase from a unified diff.

An LLM reviewing a diff is doing two jobs at once: recovering context it doesn't have, and forming a judgement. It's bad at the first and that ruins the second.

Three stages, each doing what it's actually good at

MR assigned to me
      │
      ├─▶ 1. build check      does it compile?          deterministic, ~30s
      │
      ├─▶ 2. static analysis  ~80 known bad patterns    deterministic, instant
      │
      └─▶ 3. AI review        does this change make     the only stage
                              sense for its stated      that can be wrong
                              intent?

The rule I settled on: never ask the LLM something a compiler or a regex can answer. Not because the model can't — because every token it spends on a solved problem is attention taken from the one thing only it can do.

Stage 1: does it actually build

The bot clones the source branch, auto-detects the stack, and runs the matching compile command — ./gradlew compileJava compileKotlin, mvn compile, npx tsc --noEmit, python3 -m compileall. If it can't identify the stack, it skips cleanly and moves on.

The subtlety here isn't running the build. It's deciding what a failed build means.

A build fails for two very different reasons, and only one of them is the author's fault. If a private artifact registry times out, or a corporate proxy mangles a TLS handshake, that's my environment being broken — not the MR. Block on that and the bot becomes a nuisance that cries wolf every time the network hiccups.

So failures get classified before they get a verdict:

def _classify_jvm(output: str, rc: int) -> 'BuildResult':
    if rc == 0:
        return BuildResult(status='PASSED')
    lines = output.splitlines()
    dep_lines  = [l for l in lines if _JVM_DEP_RE.search(l)]
    code_lines = [l for l in lines if _JVM_CODE_RE.search(l)]
    compile_failed = bool(_COMPILE_TASK_RE.search(output) or _MAVEN_COMPILE_FAIL_RE.search(output))

    if code_lines and compile_failed:
        errors = _extract_jvm_errors(lines)
        return BuildResult(status='CODE_ERROR', errors=errors, raw_output=output)
    if dep_lines:
        return BuildResult(status='DEP_ERROR', raw_output=output)
    return BuildResult(status='SKIPPED', raw_output=output[:3000])

_JVM_DEP_RE matches resolution and network noise — Could not resolve, PKIX path building, connect timed out, plus whatever private artifact prefixes are configured. _JVM_CODE_RE matches real compiler output: Foo.java:42: error:, incompatible types, reached end of file.

Two details that matter more than they look. A line only counts as a real error if it matches the code pattern and not the dependency pattern — resolution failures often contain the word "error" and would otherwise poison the classification. And CODE_ERROR additionally requires evidence that a compile task actually ran, so a stray error: in unrelated log output can't fail an MR on its own.

Anything the bot can't confidently classify becomes SKIPPED, not FAILED. The build stage is allowed to have no opinion. It is not allowed to have a wrong one.

Stage 2: the eighty things I always say anyway

Regex-based static analysis, run on every MR regardless of stack. Roughly eighty rules mapped to their Sonar equivalents: .block() in a reactive path, hardcoded credentials, new Random() for anything security-adjacent, string concatenation in SQL, == on strings, empty catch blocks, printStackTrace, field @Autowired, any in TypeScript, bare except: in Python.

None of this needs a model. It's pattern matching, it's instant, and it never hallucinates. Test files, generated code, node_modules and vendored directories are excluded, because a linter that flags test fixtures is a linter everyone learns to ignore.

The two rules I'd actually recommend stealing aren't code rules at all:

Duplicate keys in config files. YAML silently takes the last one. A duplicate spring.datasource.url forty lines apart, the wrong one wins, and it surfaces in staging.

Helm completeness. Every ${VAR} referenced in application.yaml must exist in values.yaml. This is a five-line check that catches a class of failure that otherwise surfaces as a pod crash-looping at deploy time, long after the review is closed and everyone has moved on.

Neither is clever. Both have caught real bugs that three humans and a CI pipeline had already signed off on.

Stage 3: the model, kept on a short leash

Only now, with a diff that compiles and has no known bad patterns, is there a question worth spending a model on: does this change do what the MR says it does?

It runs on qwen2.5-coder:14b through Ollama, entirely locally. That's not a preference, it's a hard constraint — this is proprietary code on a corporate network, and shipping diffs to a third-party API is not a decision a side project gets to make. A 14B model on a laptop is meaningfully worse at this than a frontier model. Stages 1 and 2 exist partly so it doesn't have to be good at everything.

Large diffs are split per file and grouped into batches of ~40,000 characters, capped at 10 batches. Each batch is reviewed independently, and the aggregation is deliberately paranoid:

final_verdict = (
    'APPROVE_MERGE'
    if all(v == 'APPROVE_MERGE' for v in batch_verdicts)
    else 'NOTIFY_HUMAN'
)

One uncertain batch out of ten and the whole MR escalates to me. An empty diff also defaults to NOTIFY_HUMAN. Every ambiguous path leads to a human, never to a merge.

The single highest-leverage thing in this stage is that the model is allowed to say I don't know. If it's unsure about a specific file, it puts that filename in a needs_context list instead of guessing. The bot then fetches those complete files from GitLab and runs a second pass with full context.

That one affordance removed most of the hallucinated review comments. The failure mode I described at the top — confidently flagging a method as undefined because its definition wasn't in the diff — is a model being forced to answer without the information. Give it a way to ask, and it asks.

Why I trust it to merge

A bot that merges code needs a much better answer than "the model said it was fine."

It never resolves its own threads. If it posts a comment, a human resolves it — the bot has no path to marking its own concern addressed. Once all its threads are resolved, it re-reviews from scratch.

APPROVE_MERGE with open threads doesn't merge. Approval and unaddressed comments are not a contradiction the bot gets to resolve on its own; it waits.

It defers to other reviewers. If anyone else has unresolved threads, the bot doesn't review at all. Human review in progress beats machine review finished.

Merge conflicts stop it before anything else runs, and it re-reviews automatically once they're resolved.

It never deletes the source branch after merging. If it gets something wrong, the branch is still there.

And when it escalates, it actually escalates — a macOS notification that re-fires every two minutes until the MR is merged or closed. An alert that can be ignored is an alert that teaches me to ignore alerts.

What it isn't

It's not a senior engineer. It cannot say that an approach is right but the abstraction is wrong, and it will approve a well-formed change that solves the wrong problem.

The honest accounting is that stages 1 and 2 — the compiler and the regexes — catch most of what actually gets caught. The model's real contribution is a second pass on intent, and a willingness to escalate when something feels off. That's a genuinely useful reviewer. It's just not the reviewer the demo videos promise.

Which is the lesson I keep coming back to: the value wasn't in adding an LLM. It was in working out how little the LLM needed to do.

Code is on GitHub as mr-review-bot. The review guidelines live in prompt.md as a template — copy it to prompt.local.md, put the real platform conventions in there, and that file stays out of version control.