Make the Build the Reviewer

I run a lot of AI code reviews, and for a while I kept hitting the same wall. I'd load the review instructions and our coding conventions into the agent, point the agent at the change, and get a decent review. Then I'd run it again and get a different one. It would catch a rule one time and miss it the next. Sometimes it skipped files entirely.

None of that is surprising once you think about what a model is. But it bothered me most on the rules that weren't matters of opinion. Whether a request handler returns the agreed result type is not a judgment call. Whether a catch block logs the exception or rethrows it isn't either. I was relying on a probabilistic reviewer to check things a compiler-adjacent tool could check every time.

So I changed the split. If a review comment can be checked mechanically, it's a candidate for a test that runs in the build. AI review and human review get what's left, which is the part that actually needs judgment.

Every repeated review comment is a candidate for a check.

Not every comment becomes one. But the question is now always asked.

Everything here comes from one .NET 8 service

I applied this approach to a real .NET 8 service. The examples and measurements below come from that project.

Some context on its shape, because the rules depend on it. It follows Clean Architecture: a Domain layer at the center, an Application layer that holds the use cases, and an Infrastructure layer that talks to databases and outside systems. Use cases are written as requests (commands that change something, queries that read) and dispatched to handlers with MediatR. Handlers return ErrorOr<T>, a result type that carries either a value or a list of errors instead of throwing. Persistence goes through EF Core. The code is also split into modules, one per business area.

Architecture tests that read code, not text

The easy version is regex over text: scan the files, flag the pattern. That breaks as soon as formatting, comments or generics get involved, and it can't see across files.

So each rule uses whichever engine actually understands the thing it checks. Roslyn parses source into syntax trees, so a rule about catch blocks or conditions looks at real statements. Reflection inspects compiled types: method names, return types, constructor parameters. Mono.Cecil, directly and through NetArchTest, reads compiled IL, so a rule about which layer references what looks at real references, not using lines. The real dependency-injection container is built in a test to validate its registrations. Regex hasn't disappeared, it still matches words inside identifiers and shows up in some tooling, but it's not standing in for a parser.

Each rule carries a comment in the same four parts: Rule, Why, Example, Fix. When a test fails, the developer (or the agent) reading the failure gets the reason and the remedy.

Here's a representative slice of the rule catalogue:

That's twelve rows from a catalogue of 92 entries, and an entry is not the same thing as a test: some group several analyzer rules. Five entries are AI warnings: they don't fail the build, but they go into the findings report that AI review works through, described below. The other 87 are failure-level, but that count includes opt-in and manual gates, so it is not 87 rules failing every CI run.

Outside the table, the same approach covers naming (method names start with an allowed verb, checked mostly by reflection, with a spell checker as a separate gate, more on that below), loops that must log why they skip an item, XML documentation summaries whose type references the compiler can check (see cref), where types live inside modules, and one shared retry mechanism built on the Polly resilience library.

The spell checker is really a naming rule

The check I find cleverest is also the least obvious one: a spell checker.

cspell is an open-source spell checker built for code. It doesn't only read prose. It splits identifiers on their casing and underscores, so fetchCustomerFeedAsync is checked as "fetch", "customer", "feed" and "async", and it reads comments and documentation summaries too. Every word has to be found in one of the dictionaries the project turns on.

In my setup those are American English, C# and .NET terms, general software vocabulary and well-known company names, plus one controlled project glossary of about 130 domain and vendor terms. The glossary has one rule: no abbreviations.

That turns a typo checker into a readability check. I set the minimum word length to three characters so short tokens get checked as well, which is meant to catch msg, mgr and svc. A name like GetMsgSvc fails, and the failure points at the file and line. The fix is to rename it to GetMessageService with Roslyn's rename refactoring, so every reference moves with it. A typo in a comment just gets corrected. A genuinely new domain term goes into the glossary, where the addition is visible in review. "It's only an abbreviation" is not a way in. Build output, generated code and database migrations are excluded.

What I like most is what it does to reviews. "Should this be cust or customer?" stops being a comment someone has to write and someone else has to argue with. The check gives the same answer on every run.

It has limits. A dictionary can't tell a good name from a bad one made of real words, and some abbreviations are dictionary words in their own right, so it won't catch all of them. Like the other checks, it runs on changed C# files locally and across the whole solution in CI.

The rule that came from moving files

One of the rules about what may appear in the Application layer came from cleanup work. Names of external integrations appeared in the Application layer. Rather than only moving the individual files, I wrote a generalized architecture test: the business Application layer may not reference external-system names. If one appears, the build flags it before review.

That's the pattern I look for now. When a fix is really about a rule rather than one file, it should probably be a test.

Strict where it's certain, warnings where it isn't

Not everything checkable deserves to fail the build.

Command success logging is a good example. When a command succeeds, its log line must include an identifier taken from the request, and that's enforced. The outcome value is only a warning: it's useful, but some commands legitimately only have the ID to log.

The same goes for methods over 30 lines, and for summaries full of jargon, semicolons or type names. Those are signals worth surfacing, but they are judgment calls, so they show up as warnings.

The Sonar static-analysis rules follow the same split: 121 defect and security rules are configured as errors, and 16 readability rules go to AI review as warnings.

Adopting it without stopping the world

You can't turn on a hundred rules against an existing codebase and expect a green build. So every rule can have a baseline allowlist: a list of the violations that already existed when the rule was turned on.

The allowlist works in both directions. A new violation fails the build. An entry for a violation that's already been fixed also fails the build, until the stale entry is deleted. Debt stays visible, and without a deliberate edit the list can only shrink.

That last part matters. A person can still add an entry on purpose. The test doesn't make that impossible, and it doesn't catch every way around a rule. What it does is turn a silent drift into a change someone has to make, and that someone else can see in review.

In the current snapshot, 66 of the 70 allowlists are empty. The other four hold 12 entries between them, and some of those are permanent exceptions an owner decided to keep, not debt waiting to be fixed.

One findings report, three kinds of reviewer

Deterministic checks don't replace review. They change what review is for.

The checks run through one pipeline that produces a single findings report. It merges the architecture tests' findings with Sonar's readability warnings. An agent workflow then groups the affected files and hands them to parallel fixer agents, the same pattern I described in my last article on parallel code review. A collector step then rebuilds and rechecks until nothing fails. After that, an independent read-only agent audits the result.

Documentation gets its own reviewer. A separate AI agent compares every changed XML documentation summary with the code it describes. "Does this summary still tell the truth?" is a question no syntax tree can answer.

And humans still review. Design, product behavior and taste stay with people, because those are the decisions I actually want people making.

Checks have to be fast enough to run every time

All of this runs through a single verification command that builds the code, runs the tests and runs the quality checks. It has to be cheap, or people stop running it.

It doesn't run everything everywhere, and I'd rather be precise about that. Locally, by default, it builds the whole solution, runs every architecture test, runs the unit test classes affected by changes since the parent branch, and checks spelling and formatting on changed C# files. In CI, a full mode widens that to the whole solution. Integration tests run only in a separate opt-in mode and aren't part of the CI run, and one deployment-configuration check is run by hand. Every step prints its timing, so slow steps stand out.

One did. A lookup that matches words against type names took about 45 seconds. I rewrote it with a trie and it now takes 0.46 seconds. An equivalence test runs the new implementation against the old one, so the speedup didn't quietly change the answers.

Today the repository has about 177 architecture tests and 1,091 unit tests. A complete verification run, covering build, tests, formatting, spelling, the documentation-summary check and Sonar, takes about four to seven minutes.

Those test counts are a different measure from the 92 catalogue entries, so I don't add them together. Either way, they describe how much is checked. They aren't proof of quality on their own.

Rules are shared, config is per project

The rules, templates and the agent instructions that run the fix workflow are shared. Project-specific settings live in a small configuration: which folders play which architectural role, which outside systems exist, and which topics are allowed in the shared folders for generic building blocks.

The team doesn't read Roslyn code to learn the rules. They get plain-language visual rule cards with the same Rule, Why, Example and Fix.

What the build can't review

This doesn't encode every convention, and I wouldn't want it to. A test can make sure a method name starts with an allowed verb. It can't tell you whether the name is the right one. It can make sure a summary exists and references real types. It can't tell you whether the summary is true, which is why a separate reviewer checks that.

What changed is where effort goes. The things that should never vary are checked the same way whenever the checks run, whoever or whatever is reviewing. Review time goes to the questions that actually need judgment.

Make the build the reviewer for what can be checked. Keep people, and models, for the rest.