Catalin Bozianu

How review changes when the author is a machine

On the twelfth of May a commit landed in one of our repositories with an author that is not a person. It fixed a token refresh that failed when a call outlived the credential it started with, and added a fallback for when the audio device disappears mid call.

I read it, asked for one change, merged it. By the following week there were four.

What I expected to feel was something about the milestone. What actually happened is that three concrete things about reviewing had to change, and all three turn out to be improvements regardless of who is writing.

One: ask who approved it, not who wrote it

When a commit carries a person's name you read it differently. You know who to ask. You calibrate care against who they are, and that calibration is usually fair.

None of that is accountability. It is a shortcut to accountability that works well enough that we stopped examining it. The moment the author field names something that cannot answer a question in six months, the shortcut is gone and you are left with the real mechanism: did somebody competent read this, and do they understand it well enough to be responsible for it.

I now ask who approved it about human commits too. It is a better question.

Two: one concern per pull request stops being a style preference

A pull request with three unrelated changes from a colleague is annoying. The same request from an agent is unreviewable, because you cannot fall back on trusting the author's judgement about what belongs together.

So the rule became mechanical: one concern, or it goes back. Not for tidiness. Because review is only possible at all when the unit under review has a single claim attached to it.

Three: read for agreements, not for logic

This is the one that took longest to work out.

The old habit is to read a diff for logic errors. Machine written code is usually locally correct, so the yield there is low. The yield is in contracts: a field that is a string here and a list there, an event that is listened for and never dispatched, an error code one side treats as fatal and the other as empty.

Those are defects of agreement between two places, and each place looks fine on its own. When more code arrives, the proportion of defects with this shape goes up, so review has to go looking for it specifically.

What did not change

It does not merge. It does not release. It opens a pull request and waits, same as anyone.

The question I get is always some version of "do you trust it", and trust is not the mechanism. I do not merge my own work unreviewed either, and I have fifteen years of history to point at. It gets the same treatment for the same reason: not because of what it is, but because that is how code gets into production here.

The uncomfortable part, offered as a diagnostic

If your review process only worked because you implicitly trusted the people writing the code, you do not have a review process. You have a culture, and it is about to be tested by a contributor who writes four times as much as anyone else and has no reputation to protect.

Mine held, mostly. The places where it did not were the places where I had been skimming for years, and I would not have found them any other way.

Back to the diary