We Read Every Line. All 4,000 of Them.

It's 18:40 on a Friday and I'm on file 23 of 41.

The merge request is around four thousand lines. It has to go in. Somebody has to actually read it. Not scroll it, read it. It touches a data path that a lot of downstream things quietly depend on, and no test suite in the world is going to tell me that a partition boundary is now off by one day in a way that only shows up at month end.

So I read it. All of it. We always do.

I want to be honest about that up front, because the usual version of this story is "we automated code review and freed ourselves from the tedium," and that is not what happened here. We still review everything by hand. It is necessary. It is also, some Fridays, genuinely miserable.

What we built is not a replacement for that. It's a second pair of eyes that gets there before I do.

The Part Where I Admit I Am Not a Machine

Here's the uncomfortable arithmetic of human code review: I read every line, but my attention per line is not a constant.

Change three lines and rename a variable, and you'll get fourteen comments out of me. Half about the variable name. One about whether the docstring should end in a period. I'll probably link an article.

Change four thousand lines, and I'll read all of them, then leave three comments. Not because there are only three things worth saying, but because by file 30 my pattern-matching has degraded into vibes-based structural approval. I've read the words. I haven't necessarily processed them.

Nobody is immune to this. The reviewer who tells you they're equally sharp on line 4,000 as on line 4 is either lying or hasn't done it recently.

So the question isn't "how do we stop reading merge requests." It's "what can we put in front of a tired human that makes the tired human better at their job."

Who Actually Built This

Before I go further: this was not a solo project, and it wasn't even really my project.

It came out of a collaboration with another team, and David Zucker did a very large share of the work. Especially the parts that turned out to matter most, which are almost never the parts that sound impressive. The lifecycle logic. The deduplication. The certainty gate. The unglamorous machinery that makes the difference between a tool people use and a tool people mute.

I'm writing it up. He built a lot of it. Those are different contributions and I'd rather be clear about which one is mine.

Why It's a Play Button and Not a Bot

Two lines of YAML carry the entire philosophy:

opencode-mr-review:
  stage: code-review
  allow_failure: true
  rules:
    - if: >-
        $CI_PIPELINE_SOURCE == "merge_request_event" &&
        $CI_MERGE_REQUEST_TARGET_BRANCH_NAME == $CI_DEFAULT_BRANCH
      when: manual

when: manual means the job doesn't run itself. It sits in the pipeline as a play button and waits for someone to decide it's worth the compute. allow_failure: true means that even if it explodes, times out, or generates nonsense, it cannot block a merge. Not "shouldn't." Cannot.

I've watched teams install a review bot that comments on every push, and the same thing happens every time: week one, everyone reads it. Week three, everyone has trained themselves to scroll past a specific shade of grey. By month two it's furniture.

An automated reviewer that runs unasked is a notification. One you have to click is a tool.

So let me ask you, dear reader. How many bot comments did you actually read this week?

Nine Stages, One of Which Thinks

The review job isn't a bolt-on. It's the last stage of a pipeline that has already done a lot of unglamorous work, and that ordering matters.

By the time the agent gets a turn, gitlint has checked the MR title, ruff has checked formatting and lint, Trivy has scanned for vulnerabilities, secrets and misconfigurations, and pytest has produced a coverage report. Container images have been rebuilt too, but only the ones whose own inputs actually changed.

This is deliberate. The skill has an explicit instruction about it:

Treat deterministic gate items (linter, type checker, etc.) as review context, not standalone findings, when pre-commit or CI already enforces them.

Nothing is more useless than a language model telling you your line is too long when a linter already told you, for free, in 40 milliseconds. Let the cheap deterministic thing do the cheap deterministic work. Save the expensive probabilistic thing for the questions that need judgment.

The Three Phases

The job itself is opencode, a CLI agent, running a skill that lives in the repository. Not a SaaS bot with an API key into our GitLab. Not a marketplace integration. A directory of markdown and Python, versioned next to the code it reviews, that anybody on either team can open, read, and send a merge request against.

Phase 0, package. A Python script pulls everything into a single context.json: MR metadata, the diff, the full contents of the changed files before and after, every existing discussion and comment, and derived state about which threads duplicate which. One artifact, one source of truth, replayable offline.

Phase 1, generate. The agent gets exactly two files attached: SKILL.md and that context.json. It reads, it triages, it writes markdown to disk. That's all it does. It writes opinions into a folder.

Phase 2, post. A second Python script reads those markdown files and talks to GitLab. Deterministically. With validation.

That split is the whole trick. The model is good at judgment and unreliable at bookkeeping. So judgment goes to the model, and bookkeeping goes to Python. The skill states it as a rule: the Python executor is the only step allowed to mutate GitLab review state.

Every generated comment carries frontmatter, which is how the two halves talk to each other:

---
type: inline
file: services/pricing.py
line: 118
finding_key: edge-cases/silent-fallback/pricing-fetch
root_cause_cluster: edge-cases/silent-fallback
certainty: high
post_low_certainty: false
resolve_thread: false
---

The model proposes. The executor disposes.

Severity Is Not Certainty

This is the part I'd most like people to take away, because almost every review tool I've seen collapses these two into one number, and they are not one number.

Severity answers: if this is real, how bad is it?

Certainty answers: how sure are we that it's real at all?

A model can be extremely confident about a Critical finding that doesn't exist. It can also be genuinely unsure about something that would take production down. Those are different failure modes and they need different handling. So every finding carries both, and the posting rules read across both axes.

The interesting cell is Critical × low certainty. Intuition says a Critical finding should always interrupt you. In practice, a confidently-worded Critical that turns out to be wrong is the single fastest way to teach a team to ignore the tool. So it goes to the dashboard, where it's visible and searchable and not in your face. Unless it comes with concrete verification guidance, in which case it can earn its way back to an inline comment.

And crucially, that gate is not a suggestion in the prompt. It's a continue in the posting loop:

if comment.certainty == "low" and comment.post_low_certainty != "true":
    print(color("  Skipped: Low-certainty finding goes to dashboard "
                "unless post_low_certainty: true", YELLOW))
    self.log_skip(comment.filename, "low_certainty_dashboard_only")
    continue

It's one guard in a chain of them: duplicate-of-an-existing-thread, LGTM-only, nitpick-without-correctness-impact, then this one. Each is a hard skip, logged with a reason, before anything reaches the GitLab API.

The model can say whatever it wants about how sure it is. The executor decides what that entitles it to.

The Hard Part Isn't the Model. It's Memory.

Here's what I genuinely did not expect.

Getting a language model to find a real bug in a diff is easy. The findings are good. Better than good, on the categories where models are strong: silent exception swallowing, an off-by-one on a partition boundary, a config value hardcoded three lines under the config import.

The entire engineering problem is getting it to shut up about the thing you already told it you weren't going to fix.

Because a merge request isn't a moment. It's a conversation over days. You push, the reviewer comments, you argue, you fix two of the three things, you explain why the third is intentional, you push again. If the automated reviewer has no memory, every rerun is groundhog day and it re-opens every thread you already closed. That's not a reviewer. That's a denial-of-service attack with good grammar.

So the skill spends most of its length on lifecycle, not on finding bugs. Findings have states, with a strict precedence order: NEW, ACTIVE, ACKNOWLEDGED, RESOLVED, WAIVED, DUPLICATE, CLOSED. Before generating a single new finding, the agent has to build an index of everything it has ever said on this MR, keyed by finding_key, then by root-cause cluster, then by semantic fingerprint.

The rules that follow are almost aggressively boring, and they're the reason the thing is usable:

1. Never open a new thread for a finding that already has an open one.
2. Resolved with no regression → suppress.
3. Waived with no new material evidence → suppress.
4. Acknowledged but no code changed yet → suppress.
5. If a finding is genuinely gone, don't post "looks fixed!" and leave the thread open. Resolve it, with evidence.

There's a waiver memory too, and I like this part a lot. When a human writes by design, intentional, false positive, or already addressed, the agent treats that as binding. When a human writes done, ok, thanks, or a thumbs-up emoji, it does not.

Because "thanks" is not an argument, and the reviewer shouldn't pretend it is.

There is exactly one dashboard note per MR, found by a hidden HTML marker and rewritten on every run. Open findings stay expanded. Resolved, waived, and suppressed-duplicate sections stay collapsed, so the audit trail exists without shouting at you.

The Rules That Say "Shut Up"

There's a whole reference file whose only job is to stop findings from being posted. It's called negative-rules.md and it is, line for line, the highest-value thing in the entire setup.

Three of them do most of the work:

• Convention over preference. If the same pattern appears in 3+ sibling files, it's a repository convention, not a mistake. Don't flag it unless it's actually unsafe.
• Trace the data flow before demanding a guard. Don't ask for a null check until you can show the value can actually be null on that path. This one rule killed more false positives than everything else combined.
• Don't ask for tests of things the runtime already guarantees. Nobody needs a unit test proving that Python dictionaries preserve insertion order.

And then the two constraints on the output itself, which read like a code of conduct and function like one:

Leave 3-7 high-signal inline comments, not 15 noisy ones.

Post nothing if you have nothing actionable to say.

The test for whether something gets posted inline is refreshingly concrete: if a finding does not imply a concrete code change, keep it in the dashboard. No "consider whether." No "you may want to think about." A comment either asks you to change a specific line or it doesn't get to be a comment.

For single-location fixes it emits a GitLab suggestion block, so the fix is one click. Multi-file or architectural findings get an agent-prompt section instead, because pretending a five-file refactor fits in a suggestion block helps nobody.

What It Costs

I saved my favourite number for late, because it reframes the entire question of whether this is worth doing.

We're on the cheapest Ollama Cloud plan. Twenty dollars a month. That's it. That is the whole model bill.

Three of us, roughly two hundred review runs a week, and the quota comfortably carries us through. Somewhere around two and a half cents per review. The CI minutes we're paying for anyway.

I bring this up because the usual objection to AI code review isn't "it doesn't work," it's "what's this going to cost us at scale, and who owns that line item, and do we need to talk to procurement." For a three-person team, the answer is: it's a rounding error, it's a personal-plan subscription, and the interesting constraint was never the money. It was whether anyone would trust the output.

Which is why almost all of the engineering went into suppression, certainty gating, and memory. Almost none of it went into making the model smarter.

What I'd Tell You to Steal

Don't take the model choice. That'll be outdated by the time you finish reading, and it matters far less than you'd think.

Take the shape:

1. Don't stop reading merge requests. This is an assistant, not a substitute. Anyone who sells you the substitute version is selling you an incident.
2. Make it manual. An automated reviewer that runs unasked becomes wallpaper in three weeks. Guaranteed.
3. Make it unable to block. allow_failure: true. The moment your agent can stop a merge, someone's shipping a hotfix at 23:00 and cursing your name, and you'll personally rip the whole thing out the following Tuesday.
4. Separate severity from certainty. They are different questions with different consequences, and collapsing them is how you train a team to ignore you.
5. Put the judgment in the model and the bookkeeping in code. Models are bad at idempotency. Python is excellent at it.
6. Spend most of your effort on suppression, not detection. Finding issues is the easy 20%. Not repeating yourself is the other 80%.
7. Keep it in the repository. Around 3,800 lines of skill, references and executor scripts, sitting in .ci/, reviewable by the same process it powers. When it's wrong, you fix it in a merge request. Which then gets reviewed by it.

That last one is my favourite thing about the whole setup. The reviewer isn't a service we rent. It's a colleague's opinion, several colleagues' actually, written down, versioned, and arguable. When it gets something wrong, we don't file a support ticket. We edit the markdown.

I'm still going to read every line of the next four-thousand-line merge request. It's still going to be 18:40 on a Friday. But something will have read it first, it'll have flagged the three things it's actually sure about, and it costs less per month than the lunch I'm going to be late for.

I'll take it.