One view out of three. Two templates out of three. Three prompt builders out of four. Six languages out of eleven. Two platforms out of three.
Each line is a fix that was correct where it landed, and absent from the places that needed the same fix.
Since May 7, an agent has read every commit pushed to our repositories over the previous days and written a report in French. I read it every morning; a colleague gets a copy on Slack. Each finding becomes a dated note with a state: open, fixed or withdrawn. Later runs check it again. The notes also keep what was verified and found correct, so the same code is not flagged twice.
After 90 reports, I read the notes as a set instead of one morning at a time. Very few are logic errors inside a diff. The diff is usually right. The defect sits somewhere the diff did not go.
Five shapes cover most of them. Each became a question I now ask of a change. Many of the reviewed commits are mine, and so are several of the examples below.
1. The twin nobody fixed
Where is the twin?
The ratios above belong here. A flag declared on one view out of three that share the same guard. A sentinel value handled by two templates and emitted raw by the third. A meta description corrected in six languages and left alone in five. A fix applied to one branch of a method, while the other branch calls super() into the parent version that still has the bug.
The clearest case sits in a test suite. A test was added to make sure a contract reached every prompt builder that generates code. It listed three builders by hand. Four builders generate code. The missing one handled refinement: the path taken by every "change this" message a user sends.
The same test module already told this story. An older test's docstring explains that two builders had once gone uncovered, and that they were the refine builders, the production path. That older test does not trust its own list. It introspects the functions that accept the parameter and asserts that both sets are equal. The new test repeated the gesture without that guard, and missed the same builder.
When a fix must reach every member of a family, compute the family. A list typed by hand is one more twin, waiting to drift.
2. The rule that lives in text
What enforces this, and does the text agree with the code next to it?
On August 11 at 15:31, I shipped a change in two halves. One half made our schema renderer supply defaults for non-nullable columns. The other added a sentence to the prompt of the model that writes those schemas: every non-nullable column MUST carry a default.
At 16:14, the renderer half was reworked and moved into the parser. The prompt file was not in that diff.
The next morning, a user's constraints forced an integer primary key. Told that every non-nullable column needs a default, the model wrote the identity clause into the default field. Postgres rejected the statement, every create came back as a 400, and our generic error path told the user to try again. Trying again could never work.
The deterministic half had been rewritten. The probabilistic half survived, and it steers the model that produces the renderer's input.
The shape does not need a model. A command-line module states that no command archives anything; the archive command sits in the same file, 360 lines further down, added by the same commit. A README says there is no build step, next to a build script. A tool's documentation forbids the cache refresh that its own raw SQL writes depend on, because the ORM hook that would clear that cache never fires.
A rule needs a home that can fail. When a prompt and a parser share a vocabulary, interpolate the tokens into the prompt from a single constant, and test that every token the prompt teaches survives the parser.
3. The proof that is not in the repository
Where is the proof, and can someone else run it again?
Thirty-six commit messages in one design repository cited a headless check: "headless 185/185", "detection proven ×3". The script lived in a gitignored scratch folder on one laptop. Across every remote ref, it did not exist. The handover document listed five mandatory verification commands, and three of them started with a cd into an absolute path under one person's home directory. Once flagged, its author moved most of it into the repository.
An audit journal, described in its own code as the only record of who did what because the API key is shared, lived in the same SQLite file that the repository declares a rebuildable cache. Deleting that file is documented as harmless.
Changes to a live ad account went through a desktop editor. The journal entry that records them cites a verification script five times. The script sits in a gitignored output folder, on one machine.
A commit message is a claim. The proof is whatever someone else can run again next quarter.
4. The kill switch that went away
How do we turn this off, and did this commit remove the way?
On September 23, I opened a feature from five pilot accounts to everyone, and deleted the flag along with the allowlist. The flag was also the only way to switch the feature off without a deploy. The downstream service keeps its own switch, but that one cuts the traffic while the product keeps generating code that depends on it.
A support assistant removes help articles about an extension the customer does not have. Its breaker turns that filter off. The line injected into the model's context, stating that the extension is not installed, stays. The breaker covers the part that was cheap to get wrong.
An exemption added "for a test" went into a hardcoded list that takes a deploy to change, next to a database table built for exactly that purpose and emptied with one query.
My own bulk-update command has guard rules that block unsafe writes, if you pass --guard. The write example in its docstring does not.
A guard you have to remember to request is a suggestion. When a flag graduates, keep it: default on, no allowlist, still a switch.
5. The failure nobody hears
When this fails, who finds out?
My favourite is mine. A function raises, under the comment "Fail loudly: a renamed tab silently disabled the archive import once already." Its only caller is a cron command wrapped in except Exception as e: print(e). The exception becomes one line in a log file and the process exits 0. The next rename will be exactly as silent as the first.
A production build script gained a timing echo as its last line. It has no set -e, so its exit status went from that of docker push to 0. If the image build fails, retagging latest fails too, and latest still points at the previous image. The next line tags that old image with the new commit SHA and pushes it. The deploy pulls the new SHA and ships old code, with the CI green. The same repository has a sibling script for another environment. It starts with set -e.
An alert script records items as alerted before posting them to Slack, and never checks whether the post succeeded. A timeout marks them alerted anyway. They never come back.
Every except should answer one question: who hears about this? In a cron job, print answers nobody.
When the review is the one that's wrong
On September 24, the review flagged a messaging channel still offered to users after the account behind it had been banned. No commit anywhere had turned the option off.
It had been turned off two days earlier, in a system the review does not clone. By the time the report ran, the ban had been lifted and the option restored. The finding was withdrawn in the next report.
The review's worst mistake of the month was shape three. It read the absence of proof in git as proof of absence. It now carries its own rule: when a switch can live outside the repositories, ask a question instead of stating a finding.
Five questions
These are the questions I now ask of a change, mine included:
- Where is the twin?
- What enforces this rule, and does the text agree with the code next to it?
- Where is the proof, and can someone else run it again?
- How do we turn it off, and did this commit remove the way?
- When this fails, who finds out?
None of them needs an agent. The agent was never given this list; the list came out of its notes. What it brings is stamina. It reads every commit, mine included, and writes the answer down where the next morning can check it.
Which of the five does your review already catch, and which one have you only met in production?
Top comments (3)
The idea that a diff can be correct but still belong in the wrong place is an underrated code review lesson. Moving logic into the right abstraction isn't just about cleaner files; it makes responsibilities clearer and future changes safer. I like the focus on recognizing patterns across reviews instead of treating every diff as an isolated problem.
The twin nobody fixed is the one I see most with coding agents too. They fix the exact spot the ticket points at and stop there. What helped me was asking the agent to list every caller and sibling of the changed code before it plans the fix, and then checking the diff against that list. Computing the family works for prompts as well as for tests.
Thanks Max. Listing callers and siblings before planning the fix is the right reflex, and checking the diff against that list afterwards is what makes it stick.
Two things I'd add. First, the list is only as good as the way it's produced. If the agent writes it from reading, it's one more hand-typed list, and it can miss a member exactly like the test missed the refine builder. When it comes out of a search (find references, grep on the guard or the string), the check gets much more reliable.
Second, the siblings are the hard half. Many of the twins in the article aren't linked by any call: the third template, the five languages, the other view with the same guard. They're copies, so they only show up when you search for the shape of the fix rather than for the function it touched.
Do you have the agent run that search itself, or do you hand it the list?