Back to blog
Engineering

Turn Every Repeated Review Comment Into a Failing Test

A style guide is read once; a check fails at the moment of the mistake. How a 49-line test and a list that could only shrink took our API from 82 test files mocking our own code to zero in 22 days.

A chrome ball landing in water, throwing up a crown of droplets and reflected in the dark surface below.

On October 6, a little before 7pm Pacific, five commits landed on our main branch and a file in our API's test folder went empty. It was a list of test files still allowed to mock our own modules. On September 14 it had 82 entries. That evening it had none (our git log, October 6, 2026).

The list belonged to a ratchet: a test that scans the source, fails on any new file that mocks one of our modules, and fails again when a listed file stops mocking and nobody takes it off the list. The list could only get shorter, and 22 days later it was empty.

Here is the practice I took from it. A style guide is read once; a check fails at the moment of the mistake. When the same review finding shows up twice, encode it as a test that scans the source and fails with the file, the rule and the fix. Then hold the drift you already have to a baseline that may only fall.

The rule we had already written down

On September 13 we counted: 97 of the API's 314 test files mocked our own modules, with 413 mock.module calls between them. Modules named auth were mocked 106 times and db 68 times (our repository's agent instructions, counted September 13, 2026). A test that replaces your own auth module proves only that the mock was written the way the code expected. Those tests touched no database, no GitHub and no running engine.

We wrote the rule into the instructions every agent in the repository reads at the start of a session: new tests do not mock our own modules. Prose like that has one moment of influence, the start of the session, long before the agent is three files deep in a test that would be quicker with a mock. Agents write most of our commits, about 10,800 since February by the same September 13 count, so a review comment we make twice we will make two hundred times. Review is the scarce input once agents produce faster than people read, the variable no demo shows.

The next day we turned the sentence into a check. It was 49 lines long. It read every test file, matched mock.module("./ or mock.module("../, and compared the matches with a plain text allowlist of the 82 files that did it that day. A newcomer failed. A listed file that no longer mocked also failed, with the message "no longer mock an own module, drop from allowlist", so the list tightened in the same PR that earned it.

Test files allowed to mock our own modules, September 14 to October 6, 2026A stepped line falls from 82 files on September 14 to 58 on October 5, with one rise from 68 to 69 on September 20 that was undone the same afternoon. On October 6 five commits take it from 58 to 0, marked in gold.82 files to zero in 22 days020406080+1, Sep 2058 on Oct 50five commits, Oct 6Sep 14Sep 21Sep 28Oct 6Allowlist entries after each commit. Source: our git log.
Each step is a commit that removed files from the list. The gold point is the evening the list emptied.

Why a ratchet, and why per file

The alternative was to fix all 82 files first and switch the check on afterwards. That plan rarely ships: the fix takes weeks, and every day of those weeks agents add more mocks. A ratchet switches the check on today, against today's drift, and asks one thing of each PR: leave it no worse. Each PR that touches a listed file then has a reason to finish the job, because the check says so by name.

The list fell in small steps for three weeks as we deleted and rewrote code around it: 72 by the end of the first night, 60 by September 29, 58 by October 5. On October 6 five commits rewrote the remaining test files to run against a real database and the real app. Those five commits changed 77 files, added 7,045 lines and deleted 11,680 (our git log). The list went 58, 46, 34, 17, 6, 0.

There is one bump in that line, and it is the most useful point on the chart. On September 20 a PR added a file to the list, and the check passed because the file was on the list. Two hours later the next commit took it off again. A ratchet is enforced by a file anyone can edit, so the rule that keeps it honest is a human one: never raise the baseline to land a change. Fix the code. When the list truly has to grow, that is a decision for review, and the diff of the list is where review sees it.

Five counts, one morning

Early on October 6, the same day the list emptied, we generalised the idea. One file, ratchets.test.ts, now scans the API's source for five shapes we kept flagging in review: an inline call to the auth function instead of the shared middleware, a handler that takes a raw Request, a module spread in the database file, an error body with no machine-readable code, and a failed status write whose error is swallowed by an empty .catch. Each count has a per-file number in a baseline file. A total above the baseline fails. A total below it passes and prints the drop, so the PR that lowered it lowers the baseline too.

Thirty-two minutes after the check landed, the swallowed-failure count went from 25 to zero. By 6:40 that morning three inline auth calls had gone as well.

The five structure counts and their baselines at the end of October 6, 2026Error bodies with no code: 474. Handlers taking a raw Request: 160. Inline auth calls: 148, down from 151 that day. Module spreads in db.ts: 22. Failed statuses written and then swallowed: 0, down from 25 the same morning, in gold.Five counts that may only fallError body with no code474Handler taking a raw Request160Inline auth call148151 → 148 on Oct 6Module spread in db.ts22Failed status written, then swallowed025 → 0 on Oct 6Totals in the per-file baseline file, end of October 6, 2026. Source: our git log.
Each bar is a count of one shape across the API's source. The gold one went from 25 to zero 32 minutes after the check landed.

The matchers are regular expressions run over a copy of the source with comments and string contents masked out, so a pattern inside a comment does not count. The file carries its own self-test: a fixture in testdata/ holds multi-line examples of two of the shapes, and the check must find exactly those two lines. A check that has never caught its own example is one you are trusting on faith.

What the agent reads when it fails

To see the message an agent gets, I took the October 6 version of the check, removed the code key from one rate-limit response, and ran it.

Anatomy of a ratchet failure messageA dark terminal panel shows the real failure: error response without code rose above 474 in src/middleware/auth.ts at lines 30 and 39 and eight more, expected at most 474, received 475. Callouts name the rule, the baseline, the file and line, and the count. A dashed row marks the fix line the message does not carry yet.What the agent reads when it fails $ bun test tests/ratchets.test.ts error: error response without code rose above 474 in: src/middleware/auth.ts:30, src/middleware/auth.ts:39, and 8 more Expected: <= 474 Received: 475 1The rule, by name 2The baseline it crossed 3Where: file and line 4How far over 5 The fix, written for the agent: add a typed code, e.g. code: "RATE_LIMITED" Rows 1 to 4: real output. Row 5: what I would add.
The October 6 check names the rule, the baseline, every line and the count. It does not yet say what to do instead; that is row five.

It names the rule, the baseline it crossed and the count. It names lines too, more of them than I changed: the baseline is per file, so the check knows auth.ts has one too many and cannot tell which one is new. It lists all ten. An agent finds the new one by reading its own diff, which works, and a check that pointed at the changed line would work better.

The message is also missing its most useful line, the fix. "Error response without code" tells an agent what is wrong. "Add a typed code such as RATE_LIMITED" tells it what to type. The newest tools in this space are built around that second sentence. @shadcn/lint, first published on September 14, calls itself an agent-first linter: "When an agent breaks a rule, the error explains what's wrong and suggests a fix based on your components, variants, and theme" (shadcn-ui/lint README). Vercel shipped its Web Interface Guidelines as an agent command on January 12, and its review skill reports findings "in the terse file:line format" (vercel-labs/agent-skills, January 14, 2026). Both assume the reader of the error is an agent, and both write the error for that reader.

The recipe

What I would tell anyone whose code is mostly written by agents:

  1. The second time you write the same review comment, write the check. The first time is a judgment call. The second is a pattern.
  2. Scan the source in the test suite you already run. No new job, no new service. Ours are ordinary test files. Our engine's comment policy (no ticket ids in comments, no two comment-only lines in a row, no block comment across lines) has run that way since September 14.
  3. Fail with the file and line, the rule, and the fix. Write the fix for the agent that will trip it, in words it can act on.
  4. Hold existing drift to a per-file baseline that may only fall. Print the drop when it falls, so the baseline follows the code down.
  5. Give the check a self-test. A fixture with the bad shape in it, and an assertion that the check finds it.
  6. Never raise the baseline to land a change. When it has to rise, make that a review decision.

This is the small, daily version of an idea from earlier this year: that agents make an organisation's judgment loops executable. A review comment is judgment. A failing test is the same judgment, running on every PR.

Honest limits. A ratchet counts shapes and knows nothing of intent. An agent can satisfy "error body with no code" with code: "ERROR" and the check will pass. The regular expressions find only what they were written for. These checks keep a known mistake from spreading; finding the unknown ones is still a reviewer's job. What a check takes off the reviewer is the fifth identical comment.

Where this runs in Rush

This is the habit I want every agent we run to meet. When an agent works on a repository in Rush, it does the work in its own sandbox and returns a PR, and the repository's own checks run on that PR, ratchets included. When one fails, the file and line sit on the PR, a follow-up sends them back into the same run, and the trajectory shows what the agent did on the way there. A run that comes back without its proof ends as needs_review. The model is rented; what a team remembers about its own mistakes is what it builds, and a check is that memory speaking at the moment it matters.

Every review comment you make twice is a test you have not written yet. Ours took 49 lines and a list of 82 names, and on October 6 the list was empty.

Read Next