The PR Was Too Clean to Merge: When AI Code Ignores the Repo's Hard-Won History

Last month I reviewed a PR, and thinking about it now still gives me chills.

The change was small, a minor tweak to a checkout page, with a pricing.ts added along the way. Inside was a formatMoneyCents(), code written very nicely: complete types, clean function decomposition, unit tests with three cases, all green. CI passed, build passed, the page ran fine.

My mouse was already hovering over "Approve" when I suddenly felt something was off.

It wasn't that the code was wrong. It was that it was too clean. Too clean to have grown out of this repo.

I scrolled down two screens and found a formatCurrency() in packages/currency/src/format.ts. It did the same thing, but carried one extra responsibility — it executed the rounding rule that Finance signed off on after a real cent-inaccuracy incident years ago.

Not round-half-up. Banker's rounding.

Nothing in that PR hinted that "a function already exists here". The agent that generated it found a "missing utility function" in the files it could see, and filled it in. From its point of view, this was a perfect, self-contained, shortest-path fix.

But it didn't know why that formatCurrency existed. It didn't know that those few lines of rounding logic sat on top of a real incident, a Finance sign-off, a rule that could never be broken again. It just saw "a function is needed here", and built one.

1. All tests pass because the tests and the implementation came from the same mind

I later looked into why this PR sailed through with green lights.

The unit tests for formatMoneyCents() used 10.00, 99.99, 500.00, 1280.00. All passed.

But the real fork point isn't there. Banker's rounding and round-half-up only diverge at exactly half values: 0.005, 2.675, 5 cents. None of these values appeared in the tests.

Not because the test writer was lazy. It's because the test writer and the implementation writer were the same model. It won't proactively construct a boundary case it hasn't thought of. It verifies "this function is consistent with itself", not "this function is consistent with the only implementation in the system".

What's worse, this kind of defect doesn't error out. A cent off, the API returns 200, the page renders normally, monitoring stays quiet. It only surfaces months later during a reconciliation, by which time the merge is several weeks in the past.

Tests all pass. But it will fail silently, precisely when you least want to be disturbed.

2. AI loves to reinvent the wheel, not because it's dumb, but because it has no sense of ownership

That technical commentary gave four explanations, and I can match every one of them to the engineering scene.

Local context preference. The related utility was three packages away, and nothing in the prompt pointed to it. The agent solved the problem within the few files it could see.

Naming mismatch. The existing function was called formatCurrency; the agent was thinking in terms of "money cents". Fuzzy search would miss it too.

Completion reward. A self-contained little function turns the current change green without touching shared code. It's the shortest path to green.

Barrel exports hide the source. Everything is re-exported from a single index, so the agent doesn't realize the function it needs is already on the public interface.

Not one of the four is "insufficient model capability". They're all retrieval and ownership problems.

It doesn't know which code was "paid for in blood", which abstractions are off-limits, which seemingly redundant lines of logic sit on top of a real incident. It only knows "a function is missing here", and fills it in.

This is not a problem that a "please don't reinvent the wheel" prompt can solve.

3. Now when I look at AI-written utility functions, I ask one more question

"Is there already a function like this in the repo?"

Not "is there something similar", but "is there something that exists for a real reason?".

I've developed a plain habit of my own: every time I see AI create a new utility function, I don't read its implementation first. I search for the same verb first. format, validate, parse, convert — these are the words most likely to grow "functions that look new but already exist".

Once I find the old one, I ask: why does it look like this?

If any line in the old function looks "redundant" — a strange rounding, an awkward default, a roundabout error handling — it's probably not that the original author was unskilled, but that someone paid a price for that line.

AI doesn't know these costs. It only sees "this isn't elegant enough", and optimizes it away.

4. That commentary finished with advice on the testing side, and I think it hit the mark

Boundary values for money should not wait for the model to "remember" them on its own — they should be written as a fixed little table, hardcoding the fork points like 0.005, 2.675, 5 cents. Let the test hold the line, rather than letting the implementation "understand" the line.

But I think there's a deeper layer beyond testing.

What AI-generated code lacks is not correctness, but provenance.

It doesn't know why formatCurrency uses banker's rounding. It doesn't know that the seemingly redundant useEffect exists to work around a bug in a certain Safari version. It doesn't know that the "extra" null check is a patch from a production incident two years ago.

It sees only the code, not the incidents, sign-offs, and conventions behind it.

And those are precisely the parts of a system that must never be touched, and are the easiest to "optimize" away.

In closing

In the end, I didn't approve that PR directly.

I deleted formatMoneyCents and had the change wire into formatCurrency instead. The resulting diff was uglier than the original, with one extra import and one less "clean little function".

But it aligned with the only implementation in the system. It knew who signed off on that rounding logic.

AI can write functions very cleanly. But it doesn't know which things shouldn't be made clean.