by Serguey Shinder
A colleague pushed a branch on the Friday and the automated review blocked it. He was annoyed, went home, came back on the Monday and pushed an empty commit to make the pipeline run again, because that is what people do. It passed. Nothing in the branch had changed over the weekend.
He mentioned it as a joke in stand up. I took forty recent pull requests and ran the same check twice against the identical commit. Thirteen came back materially different. In four of those, one run blocked and the other approved.
We had put that check in a gate position about five months earlier, as a required status, which meant it could stop work and, more importantly, could permit it. I had thought of it as a reviewer who never got tired. What we had actually installed was a reviewer who did not remember Friday, and we had given it the power to sign off.
The cost arrived in June on a refund path. The first run flagged a missing check on a negative amount, in clear language, and named the line. The author had moved on to something else, the branch sat for two days, and when it was rerun to pick up a change on the main branch, the finding was not there. It was approved with a green tick and merged by a person who reasonably believed the tick meant something. We refunded about two thousand three hundred pounds to accounts that should not have been able to request it, and found out three weeks later from our own reconciliation rather than from anything in the pipeline.
Neither of the two changes we made is a complaint about the model. It is genuinely good at this work, it has found things in our code that three experienced people missed, and we are keeping it.
What it cannot be is a gate, because a gate is a promise about the same input producing the same answer, and ours could not make that promise. The check now writes a comment, with its output stored on the pull request exactly as produced so two runs can be compared, and a named human decides. Blocking is a thing only people do here.
The habit I have taken into everything since is embarrassingly simple and I had never once thought to do it. Before you let anything hold a position of authority in your process, run it twice on the same input and see whether it agrees with itself. We do that with our tests and we have a word for the ones that fail it. We had somehow not thought to do it with the reviewer, and a check you can reroll teaches everybody who works with it, quickly and silently, that green is a thing you wait for rather than a thing you earn.
– Serguey Asael Shinder
Leave a Reply