Every review round found a bug in the previous round's fixes

This is obvious in hindsight and it is very easy not to act on. A fix is written under time pressure, with the reviewer's framing rather than the author's, by someone who has stopped thinking about the whole and started thinking about one line. It gets less scrutiny than the original code precisely because it *is* a fix — it has a rationale attached, and the rationale reads like an argument for correctness.

Three of ours from that change set, all introduced while fixing something real:

- Normalising a trailing slash in one predicate but not its twin. Two functions have to agree about which pages are safe to cache publicly; one learned to treat `/pricing/` as `/pricing` and the other did not. The result was an authenticated render of a marketing page held in a shared cache for ten minutes. The existing test could not see it, because it iterated the *sets* those predicates are built from rather than calling the predicates. - A redirect added to fix that, which then broke app launching, because for that one route a trailing slash is load-bearing — it is what makes relative URLs in the app document resolve. - A widening of which member ids count as "implicated" in an event, which looked like a straightforward gap-closing change. It was reverted. The visibility rule requires *every* implicated id to be in the viewer's set, so adding an id that is not a household member hid the event from everyone, and a side effect of the same edit flipped a notification from "alert the stewards" to "alert everyone."

We now budget a full review round on the fixes. Not as diligence theatre — as the round most likely to find something.

The deeper problem the loop kept surfacing is that a passing test is weak evidence. Not because tests are written carelessly, but because there are so many ways to accidentally assert something other than what you meant.

The cheapest example we found asserted this:

expect(laneCostLines().length).toBeGreaterThanOrEqual(0);

That is true of every array that has ever existed. It was green from the day it was written.

The more interesting ones look nothing like that. A test named for checking that a metering hook is installed in *every* worker entry point opened its own metrics scope before calling the code — and the hook is a no-op outside such a scope. So the test created the exact condition it was supposed to detect the absence of. Three of the four lanes in production were counting into nothing, the dashboard read healthy, and the test was green.

Another: a guard that throws `RowPolicyError` when a channel cannot be found, tested with `expect(...).toThrow(RowPolicyError)`. Delete all three of the "channel not found" guards and an unresolved channel falls through to a later membership check, which throws the same error class with a different message. The test passes with the guards and without them. **Assert the message, not the class.**

And the one we think about most, because it involved a real database and therefore felt trustworthy. A query needed an `ORDER BY` to pick deterministically among candidate rows. The test asserted which row came back, against real SQLite. We deleted the `ORDER BY` to check the test would catch it. It did not — SQLite walks the unique index on that column and returns the same row either way. The test was correct about the outcome and blind to the property. A green outcome test over a database that would produce that outcome anyway is not coverage of anything.

Because we kept finding tests like that, we ran mutation testing over the piece of the system where being wrong is worst — the compiler that rewrites app SQL to enforce row-level access rules.

That module had 97.3 per cent line coverage and 92.1 per cent branch coverage. Its mutation score was **78.4 per cent**: of fifteen hundred deliberate defects injected into it, two hundred and sixty-seven were not detected by any test. Three of the survivors were authorization guards — lines whose entire job is to stop one member reading another's rows — and deleting them broke nothing anybody had written.

We wrote tests for those and re-ran it; the two worst files went from 76 per cent to 96, and the engine as a whole now sits in the mid-eighties, and the runner is configured to exit non-zero below eighty. But the number is not the point of telling you this. The point is that the gap between 97 and 78 was invisible from the coverage report, and every one of those uncovered defects was in code that a coverage badge would have called done.

The single most useful technique to come out of that run is narrow and very transferable. The compiler is a switch over policy kinds, and each arm reads a different field. Every test fed it a fixture containing only the field its own arm needed. So deleting an arm made the code fall through to a neighbouring arm that reads a *different* field, which was absent, and the fallback produced the same answer — every test still passed. The fix is to feed one policy kind a fixture carrying *every* arm's field at once. Then a deleted arm falls into a neighbour that finds real data and confidently returns the wrong thing, which is exactly what you want a mutant to do.

Mutation testing has a failure mode of its own, which is chasing a number. Some mutants are equivalent — the mutated program is genuinely indistinguishable from the original, so no test can kill them and any test that appears to is testing something else.

We have several proven ones, written down so nobody re-litigates them: an early return whose tail reduces to `!x && x` when the flag is falsy; a string constant whose only load-bearing property is not being equal to a *different* constant; a fallback array that can never collide with the values it is compared against. Each of those cost somebody half an hour to prove and is now a comment. The score is a diagnostic, not a target.

The loop only works if you are willing to distrust it in both directions, and the case that taught us this was two successive review passes over the same six lines reaching opposite conclusions. One cleared a query's `OR` chain as safe because the rewriting is done on a parsed syntax tree. The next called it a data leak.

Feeding the actual SQL to the actual rewriter and printing the result settled it in about a minute:

-- what the app wrote, plus the guard the hub appends
WHERE (A) OR (B) AND owner_id = ?
-- how SQL actually groups it
WHERE (A) OR ((B) AND owner_id = ?)

Every row matching `A` came back to any member. The second reviewer was right, and the first reviewer's reasoning was *also* right — AST rewriting does avoid string-splicing precedence bugs — it was just answering a question about a different failure mode.

The rule we took from it: a review finding is a hypothesis, and the "all clear" is the more dangerous of the two verdicts to accept on authority. Reproduce it before you fix it *and* before you dismiss it. Then mutate your own fix and confirm a test goes red — four times on one recent change a test passed both with and without the fix, every time because a fixture was subtly wrong, usually a column name that had been aliased somewhere in between.

One more, because it cost a day and the reasoning felt airtight at the time.

A change produced twenty-seven failures across the full suite. Every one passed when run on its own. The repository has a documented pattern of scattered timeouts under parallel load, and these matched it exactly. The conclusion — pre-existing flakiness, unrelated to the change — was reported with some confidence.

The clean-tree baseline came back **8,653 passed, 0 failed**. All twenty-seven were caused by the change. The culprit was a log call sitting one branch too high, so it fired for every synthetic tenant the tests provision, which flooded the console spies other suites assert on and slowed the run enough to time unrelated files out.

Passing in isolation is exactly as consistent with "a defect that only manifests under load" as it is with "an unrelated flake." It distinguishes nothing. Take the baseline — in a separate checkout rather than by stashing, if there is uncommitted work you would not enjoy losing.

Very little of this is about tooling. The practice that came out of eight rounds is four habits:

1. Review the fixes as a first-class round. They are the least-reviewed code in the change.
2. Before fixing or dismissing a finding, make the real code show you its answer — the smallest script that prints the rewritten query, the actual thrown message, the real plan.
3. After fixing it, break the fix on purpose and confirm something goes red. If nothing does, the test pins nothing, and that is usually a wrong fixture rather than a wrong assertion.
4. Never diagnose a suite failure by how it feels. Baseline it.

None of that is novel. What was novel to us was the *rate*: eight rounds, and the last one still found real defects in the seventh round's work. We do not think that is a statement about the people doing the reviewing. We think it is a statement about how much of a codebase's apparent correctness rests on tests that are green for reasons nobody has checked.

• Apps run inside your family hub and still can't read your family's data
• Rotating encryption keys under a database per household, with no downtime