← All posts
ENGINEERING

The reviewer that never merges its own code

Why we split the coding agent from the review agent — and the four defects that caught on this blog's own code.

For the first six months, one agent wrote the code and checked it. It was fast, and it was wrong in a specific, predictable way: an author is a poor judge of whether the thing they just built does what was asked. A model that has spent forty thousand tokens convincing itself of an approach will not reverse that in the last two hundred.

So we separated them. The coding seat opens the pull request. A second seat, with no memory of the drafting session, reads the diff against the spec and the tests, and holds the merge.

What the reviewer can see

The reviewer gets the diff, the linked spec section, the test run, and the deploy history for the environment the change is headed to. It does not get the author's reasoning trace. That omission is the whole point — a fresh reader has to reconstruct intent from the artifact, which is exactly the test we want the artifact to pass.

Findings have to be falsifiable

An early version produced findings like "consider extracting this into a helper." Nobody acted on them, and their presence taught the humans on the team to skim the review pane. We now require every blocker to cite something checkable: a spec line, a failing assertion, a prior incident, or a named invariant.

A review comment that cannot be proven wrong is not a review comment. It is an opinion with a badge on.

blocker:
  rule: spec/auth/sessions.md#L42
  claim: "refresh token reuse is not revoked"
  evidence: tests/auth/replay_spec.ts:118 (failing)
  proposed: revoke_family(token.family_id) before issue()

The page you are reading is the argument

This blog was built by the platform it describes, and the review gate is the reason it is not broken.

Two independent review passes over the blog's own code found defects the test suite had signed off on. The search index's backfill treated "absent from the page I just listed" as "orphaned" — with a list capped at 200 posts, that would have silently deleted the search entries of every post past the cap. An edit to a published post never re-indexed it, so search would have kept answering with text the post no longer contained. And the public container was being handed the platform's session signing key, turning a compromise of an unauthenticated surface into session forgery.

The sharpest one was a fake. The egress scanner returns the name of a secret it matched, or an empty string when the text is clean. The code used it as if it returned redacted text, which would have erased every clean draft the writing assistant produced. The unit test passed — because the test double had been written to match the mistaken assumption rather than the real interface.

CAVEAT

A test double that encodes your misunderstanding will agree with your bug forever. The review caught this by reading the real implementation; no amount of coverage would have.

What we would do differently

We should have made findings expire from the start. The first month accumulated a standing backlog of warnings nobody would ever clear, and a backlog you never clear is a signal you eventually stop reading.

NO
Nadia Okonjo
Works on the engineering seats and the merge gate at Sapient Works.

KEEP READING

COMPANY

What shipping this blog taught us

ENGINEERING

Soak, then ship: our production release policy