Omegaswift
AI

Using AI in Code Review Without Lowering the Bar

Machine review is tireless and literal. It catches what a tired reviewer skims past and misses what matters most. A person should still hold the merge button.

The Omegaswift engineering teamAI and product engineering10 min read

What Machine Review Is Genuinely Good At

It is consistent and it never gets bored, and that covers a large class of real defects. Error paths nobody exercised. A value that can be empty on one branch and is checked on the other. Resources opened and never closed. A function signature changed with one caller missed. These survive human review not because they are hard but because they are dull, and dull is where attention goes first.

It is good at the mechanical security checks too. Keys committed by accident. Queries built by pasting user input into a string. Output written into a page without escaping. Dependencies with known vulnerabilities. Configuration that opens something wider than anybody intended. None of that requires judgement, and all of it requires somebody to look every single time, which people do not.

And it is good at absence. Tests missing for the code that changed. A new failure mode with no logging near it. A configuration value added in one environment and forgotten in the others. Absence is the hardest thing for a human reviewer to notice, because a diff shows what was added rather than what should have been.

It also reviews everything to the same standard, which people do not. The change that lands at five on a Friday. The one from the developer everybody trusts. The one that is mostly reformatting with a single real line buried in the middle. All of those get the same attention from a machine, and all of them get less than they deserve from a tired reviewer at the end of a long week.

What It Misses

It cannot tell you whether the change should exist. A clean, well tested implementation of the wrong feature passes everything. Only somebody who understands what the ticket was really for will catch it, and that is one of the more expensive things to catch late.

It struggles with anything depending on knowledge outside the diff. That this service is called by a batch job on Sunday night. That the field is populated from a supplier file and arrives blank every January. That the team agreed last quarter to stop adding to this module because it is being replaced. Those facts live in people's heads and in decisions nobody wrote down.

It is weak on architecture in the way that actually matters. It will comment on the structure of the file in front of it. It will not tell you that this is the fourth place the same rule has now been implemented, or that the abstraction being extended has been subtly wrong for a year and ought to be deleted rather than grown.

And it is poor at consequence. A change that is correct and will put far more load on the database, or one that is correct and quietly breaks a commitment you made to a customer, reads as fine at the level of the code. Consequence is a product question wearing a technical costume.

Why a Person Still Holds the Merge

Accountability does not distribute. When something breaks in production, somebody has to understand what shipped and why, and that person needs to have made a decision rather than watched one being made. Keeping the merge with a human is what preserves that, and it costs almost nothing.

There is a practical reason as well. Review comments are advice, and advice with no cost attached gets skimmed. Once a named person is signing, the awkward questions get asked, because the signature belongs to them.

In EnsureFix the pipeline runs in stages and each team chooses which stages proceed on their own and which stop and wait. Automatic merging exists, and the confidence bar for it is set by the customer rather than by us. Almost everybody starts with that bar high enough that nothing merges unseen, then moves it for narrow categories where they have watched the results themselves. That order is the right one.

Teams get into trouble by treating this as one policy rather than as a set of categories. Merging a dependency bump automatically is a completely different risk from merging a change to how invoices are calculated. A single rule covering both is either too loose or pointlessly strict, and usually both at once.

Noise Is the Real Failure Mode

A reviewer who receives a wall of comments per change, most of them technically true and not worth acting on, stops reading them. From that moment the tool has made your review worse than it was, because it has trained people to scroll past the exact place where the important comment appears.

So volume needs managing from day one. Rank by severity and put the top band where it cannot be missed. Switch off whole categories your team does not care about rather than asking people to ignore them every day. And watch the acceptance: when most comments of a given type get dismissed, that type is wrong for your codebase and should go.

EnsureFix tracks which of its fixes are accepted and which are rejected, and adjusts to match. That loop matters more than raw detection ability. A tool that finds everything and gets ignored finds nothing.

Attach the patch wherever you can. A comment saying something is wrong costs the reviewer time and thought. A comment arriving with the correction already written costs them a glance and a click, and it gets acted on far more often.

What Changes for Human Reviewers

With the mechanical layer handled, human review changes shape. The questions worth a person's attention move up. Is this the right change. Does it fit what we already have. Will the next person understand it in a year. What happens when it fails, and who finds out.

That is a harder review to perform and a considerably more valuable one. It is also less comfortable, because those questions have no checklist, and a reviewer can no longer demonstrate diligence by spotting a missing check. Say plainly that this is now the job. Otherwise people keep doing the old job alongside the machine and quietly resent it.

Time per review usually falls while the value of each comment rises. Watch for the opposite pattern though. Teams that keep every mechanical habit and add the tool on top end up slower than they were before, with two passes doing the same work and neither one trusted.

There is a quieter benefit worth naming. Machine review does not care who wrote the change, and it takes over a category of comment that reviewers used to spend social capital on. Nobody has to be the colleague who always mentions the missing test, which makes the human comments that remain considerably easier to hear.

Reviewing Code a Machine Wrote

This is a different job from reviewing a colleague's work, and treating it the same way is where standards actually slip. Generated code is plausible by construction. It follows the conventions of the file it sits in, the names look right, and the shape is familiar. Plausibility is precisely what makes skimming dangerous.

Read the tests first, and read them suspiciously. Tests written alongside the code by the same process can encode the same misunderstanding. A passing test proves the code does what the author intended, which is a different claim from doing what the ticket asked. Check that the test would genuinely fail if the behaviour were wrong.

Then read the edges rather than the happy path. No rows. A duplicate. A number that arrives as text. The third party call that times out. Generated changes tend to be strongest in the middle of the case and thinnest at the boundaries. Reviewers usually look in the opposite order.

Ask for the reasoning alongside the diff. EnsureFix shows why it made each decision, and that turns reviewing into reading rather than guessing. Where a tool cannot tell you why, you are reverse engineering intent out of code, and that takes longer than writing the change would have.

Rolling It In Without a Revolt

Start advisory and non blocking, on one repository. Let the team see the comments for a few weeks with nothing gated on them, and collect the complaints. Then agree together which categories are allowed to block a merge, and keep that list short to begin with.

Let the team switch things off. A reviewer who can silence a noisy rule in one click stays engaged with the tool. One who has to raise a request with a platform team and wait learns to ignore the whole thing instead, and you will never be told.

Keep the standard visible. Publish what the machine checks and what it does not, so nobody assumes a green run means the change was reviewed. Bars rarely drop by decision. They drop because people gradually come to believe something else is doing the checking, and nobody ever says that out loud.

Watch how long changes take to merge while you turn things on. If reviews are getting slower rather than quicker, something is either too noisy or gating the wrong category. The team will feel that weeks before any report shows it, so ask them directly rather than waiting for the numbers.

Written by

The Omegaswift engineering team

AI and product engineering at Omegaswift. Filed under AI.

Ask us about this

Ready to talk about your IT?

We are happy to answer any questions you have and help you work out which of our services fit your needs.