Six Rounds of Bot Review on One PR
I’m a big fan of having multiple AI agents review code. I’ve written about why I use more than one model and what I found when I looked at the review data. Different reviewers catch different bugs, and I want those second opinions before I merge.
On this PR, they caught 32. The feature had passed its tests, static analysis, its own agent reviews, and a manual click-through in a browser. My triage process checked all 32 findings against the code and marked them legitimate. None were nits.
That took six rounds of review, five fix commits, and about 1,600 lines added to a feature I’d thought was finished. The bots were doing useful work. I wanted to understand why so much of it had to happen one push at a time.
The process, start to finish
The feature: physicians forward a continuing-education certificate to an email address, and the platform reads it and tracks the credit against their license renewal cycles. Think Ramp’s receipts inbox, but for CME. It’s a healthcare platform, so it touches inbound email, AI extraction, private documents, and a pile of compliance questions.
The whole pipeline ran inside one long Claude Code session, starting with the requirements work I’ve argued matters more now that agents write the code.
1. A grilling session. I gave the agent one sentence and ran /grill-me, a skill that interviews me one decision at a time and refuses to ask anything it can answer by reading the repo. Before the first question, it found that an earlier product decision had ruled this exact feature out, read why, and pointed out the reason didn’t apply to a tracker. Midway through, it dispatched a research sub-agent to read primary sources on a regulatory question and kept interviewing while it worked. Six decisions from me, eleven defaults it settled itself and listed so I could overturn them.
2. A spec. /to-spec turned the conversation into a PRD with a build contract, 27 user stories, and test seams. No second interview.
3. One-shot implementation. I skipped breaking it into tickets and told the agent to build the whole spec in a git worktree and open a PR. It generated the UI design in Claude Design: three directions first, then a build target, all on the app’s real design tokens. Then it wrote the backend test-first: migration, models, an inbound mail adapter behind the existing webhook inbox, a new governed AI purpose with its own eval dataset, reminders, translations in three languages, a Terraform module.
4. Its own review. Two sub-agents in fresh contexts: one checked the diff against the repo’s written conventions, one checked it against the spec. The spec reviewer caught a bug: starting a license’s next cycle early hid the cycle that was still running. The agent fixed it, with failing tests first.
5. A browser click-through. The agent seeded a database, linked the worktree in Herd, signed in as the demo physician, and drove every flow in a browser: upload, AI read, correction, delete, new license, next cycle, sender verification. That caught two more bugs the tests hadn’t caught, including one where pdftotext worked in every test and failed in the served app because PHP-FPM doesn’t have Homebrew on its PATH.
6. The PR. 121 files, about 8,100 lines, 70 feature tests, everything green.
What the bots found
Every PR in this repo gets GitHub Copilot and Claude Code Review. A separate agent runs my triage-github-pr skill, the same kind of workflow I described in the post about agents paying down my tech debt while I was on vacation. It reviews the diff itself first, before reading the bots’ conclusions, then classifies every bot finding against the code at the PR head, fixes what’s real, replies to each thread, resolves it, and loops. It has a hard cap of five fix rounds to keep the process from running indefinitely on fresh nits.
It used all five. A sixth review landed after the cap.
Final tally across the rounds: 32 legitimate, 4 dismissed as documented design choices, 4 minor, 1 stale, zero hallucinations. When I sorted the 32 by kind instead of by round, they fell into five families:
| Family | Count | Example |
|---|---|---|
| What a side effect does when it fails | ~10 | The storage disk returns false instead of throwing, so a failed PDF write still created the record |
| Success recorded before the work is durable | (in the above) | A “reply sent” marker committed before the notification was actually queued |
| Races and repeats | 4 | The overlap validation ran before the transaction, outside the lock that guarded rollover |
| A new entry path missing an old gate | 4 | Disabled accounts couldn’t log in, but could still forward certificates by email |
| Input bounds on one path but not another | 7 | Manual entry rejected future dates; the AI-read path didn’t |
| Two writers of the same fact disagreeing | 4 | The demo seeder hashed PDFs differently from real ingest, so a re-upload double-counted |
The agent had reviewed for convention violations and spec drift. The findings in that table concern failures, retries, concurrent work, and rules enforced on one path but missed on another.
Every check the agent ran before the PR answered a different question from the one the bots asked.
The convention review is explicitly “not a bug hunt.” The spec review checks fidelity. The browser click-through tests the happy path. The test suite covers what the author thought to test. None of those checks caught what happens when a queue push fails, what a disk returns on failure, or whether a webhook provider retries a 401. (SNS doesn’t. That was round six.)
I’d left those questions to the bots. They answered them one push at a time, with each review round taking about half an hour.
The treadmill
I expected the rounds to get smaller as we fixed things. But the fixes kept adding code that needed its own review.
- Round two found that processing ten attachments serially could overrun the webhook job’s 60-second timeout. The fix gave each PDF its own resumable job.
- Round three reviewed the new job and found that its “reply sent” marker committed before the reply was queued.
- Round three also flagged that deleting a certificate could remove a re-upload sharing its storage path. The fix gave every file a unique key. Round four reviewed that and found a failed delete still reported success, orphaning a private PDF with no record left to retry from.
- The reminder fix released its claim when a send failed, then rethrew. Round four found the rethrow now aborted every other physician’s reminders in the batch.
Each fix addressed the reported bug. It also introduced a job, a lock, a claim, or a retry with its own failure modes. Those became findings in the next round.
A fix that adds a mechanism is new code, and new code gets reviewed. If you don’t review it before you push, the bots will, one round at a time.
What I changed
The bots were right 32 times. I changed my skills to ask their questions earlier.
My implement skill used to end its review step with the convention and spec reviews. It now also runs a correctness pass in a fresh context before opening the PR, using questions drawn from these 32 findings:
- What does each side effect return on failure? (Check the driver config. A Laravel disk with
throw => falsefails silently.) - Is success recorded before the effect is durable? An after-commit dispatch hasn’t been queued when the commit lands.
- Does a catch turn a transient failure into a terminal one?
- Does one item’s failure abort the rest of the batch?
- Is serial work inside one job’s timeout? Is output sized by input bounded?
- What happens on a repeat, or two at once, on the same key? A validation before the transaction is not a guard.
- Does every new entry path enforce the gates the login does, and the bounds the form does?
- Do two writers of the same fact compute it the same way?
- What does the sibling module you copied do that your copy doesn’t?
That last one hurts. The Terraform module I imitated attached its S3 policy to two IAM principals. Mine attached it to one. I had read the sibling. Round two caught it.
The triage skill also used to fix, push, then re-review the new range. Now it reviews the unpushed fixes first, asks those questions about whatever mechanism the fix added, and fixes what it finds in the same round. That gives it a chance to catch the next bug before another half-hour bot review.
Some of the 32 findings depended on facts about this codebase: the disks don’t throw, the webhook job has a 60-second timeout, passedRecordMatch() doesn’t check disabled accounts. I put those in the agent’s persistent memory so it has them when it starts the next feature.
What this didn’t prove
The agent’s own review caught a bug before any bot saw the code: a contradiction between the implementation and the spec’s own rules. That’s what the spec reviewer was asked to look for. This PR doesn’t tell me the bots are better reviewers. It tells me my review instructions were missing questions they knew to ask.
I’d still try building a feature this way. One session got from interview to PR with a design, a research brief, a Terraform module, and three languages. Fixing the 32 findings took more review rounds and code, but none changed what the feature does for a physician.
I don’t know yet whether the checklist works. I wrote it after the fact, from one PR. On the next feature, I’ll be looking for whether it catches these failures before the bots do, and what they find that I still haven’t thought to ask about.