The Check I Hadn't Written

In July I wrote about an interface an agent got wrong. The compiler was happy. The linter was happy. Every test passed. The design was still wrong. I finished that post by saying the only thing that catches this is a person reading the code.

I have since written a check that catches it.

So the sentence I ended on was premature. The argument holds up, some things still need a person reading them. That example was not one of them.

Two jobs called review

The queue in front of a reviewer got longer this year. The people telling me are describing their own teams, not a survey.

But the word “review” hides two jobs.

Some of what you catch was decided long before the diff showed up. Formatting, import order, file structure, the naming convention the team agreed on in March. You are not exercising judgment there. You are performing a lookup, slowly, using a person. That is a linter's job, not yours.

The rest was only decided in the diff itself. Whoever wrote the code chose whether this is the right design. They chose whether the code belongs in this module. They chose whether the abstraction is worth what it costs.

No check answers those, and nothing in this post is an attempt to build one that does. It needs a person who has seen enough systems to know what this one is about to become. That work stays, and all you can change is how much unrelated work you have to get through first.

Moving a convention into the linter and applying it in a hook dealt with the first kind completely. So when review is the bottleneck, the first question is how much of that queue is lookup work, still being done by hand. It is rarely most of a diff. But it is reliably more of one than it needs to be.

That is the easy part, and it is not why I am writing this.

The check

Here is the interface from the July post, before I fixed it.

interface CompilerAdapter {
  sourceFilesFromTsconfig(projectRoot: string, tsconfig: string): SourceFile[];
  sourceFilesFromMemory(files: Record<string, string>): SourceFile[];
}

Production never calls the second method. Only the tests do. I presented that as an example of design damage no automated check could see, which is why you have to read the code.

I read it again later and it looked oddly mechanical. It is a member on a production interface with zero call sites outside the test directory. That is not taste. That is something a script can check.

So I wrote it as a lint script in the repository the July post was about. It runs next to the checks that were already there. It parses the code with the TypeScript compiler the project already depends on, collects the members declared on interfaces in src/, then asks which of them only the tests ever reach.

You may not have to write anything. Depending on your language and toolchain, Semgrep, Knip or an architecture testing library may already cover it. Mine did not, so I wrote it myself, and it came to about 150 lines.

I did not want to argue this from a toy example, so I put the bad interface back into the real source tree. The extra method on CompilerAdapter, implemented in all four adapters, called from the test file that wanted it. Then I ran everything.

The first three rows are the July post. The fourth row is the part I said only a person could catch.

This check is not protecting me from the mistake I made, because I caught that one by reading. It is protecting me from the next one, on a day when I am reading the diff at half past five with something else on my mind.

The line moves

I believed that problem needed my eyes, while writing a post arguing that design is what automated checks cannot measure. It was not that the problem needed judgment. It was that I had not written a check for it yet.

A problem that needs judgment and a problem you have not written the check for look the same in review. Until the check exists, both of them end with you reading the code and having to catch it. The difference only shows up when you sit down and try to write the check. Many of us never try, because “that needs judgment” is a flattering explanation and easy to believe.

Anything you have caught twice in review is a check you have not written yet. That is not true of everything you catch, and it is not true the first time. The second time is the signal, because it tells you this is a pattern rather than an accident, and a pattern is something a query can describe. Some of them will turn out to need you after all, and you only find that out by trying.

You still have to review the code. What changes is what you are reviewing. Every check you write takes out something that was never judgment, so what is left is the part that needs you. You have fewer things to look at overall, and every one of them is a call no automated check can make for you.

Two is enough here

Normally I wait for three examples. I want the third case before I extract an abstraction. Two cases will fit whatever abstraction I already have in mind, and the third one tells me whether I got it right.

I still do that with abstractions. Checks are different, and the difference is what happens when you get one wrong.

Get an abstraction wrong and it can get expensive to undo. It shapes the code around it, and the next thing you write grows to fit it. Unpicking it a year later is a project with a planning meeting. So you wait, and by waiting you find out what varies between the cases.

Get a check wrong and you delete the check. Nothing grew around it, no design bent to accommodate it, and all you lose is the morning you spent writing it.

An abstraction is a prediction. You are guessing at the shape of cases you have not seen yet, which is exactly why you want a third one before you commit to it. A check is more of a description. It describes something that already happened, and it happened twice, so there is nothing left to guess about.

It will happen a third time either way. The only question is whether the check catches it or you do.

Change what arrives

Checks remove the mechanical work, not the judgment calls. What makes those cheaper is reviewing a plan instead of a finished diff, and reviewing small changes instead of big ones.

The last post ended on making a claim before the code lands, and I wrote that as a way to keep learning from work an agent did. It does something else too. A plan you agree to before any code exists is cheap to review. The same decision, arriving as a finished diff you have to reverse engineer, is expensive. The judgment is the same but the price is different. The only thing that changed is when you were asked.

The rest is unglamorous. Smaller tasks produce smaller diffs, and a small diff gets reviewed properly rather than skimmed. A change that touches one module asks you one question. A change that touches nine asks you nine, and you will answer about four of them honestly.

None of this is new advice either. It is how we kept pull requests reviewable long before any of this. Now we are pointing it at a contributor who opens them faster than we ever did.

Still read the code

This argument is easy to misread, and I would rather repeat myself than be misread.

The check I added does not catch every design problem. Test-only members are one narrow kind of test-induced design damage, and test-induced design damage is one narrow kind of bad design. I have not automated design review. I have taken one thing off the list.

But that is how the list gets shorter. You write one check at a time, each one on the day you notice you have caught something twice.

Everything else is still my job. I decide whether the abstraction is worth what it costs. I decide whether the name matches how the team talks about the thing. I decide whether this belongs here at all. I cannot think of a query that describes any of those.

Every check I write leaves me more attention for the rest of the diff, and it does not do the thinking for me. The formatting, the structure, the conventions, and now one specific kind of design damage are all handled before I open the diff.

I still read every line. I just have less to read before I get to the parts that need me.