serhiy-bzhezytskyy Run the bot's review before it does
Purpose
An automated reviewer changes the order of events: it comments within minutes, before a human has looked, so its findings are the first thing on the PR and the frame through which the human reads the change. That makes two things worth doing. First, its rulebook is knowable — every finding it has ever posted is public on the repo's other PRs — so the checklist it will apply can be run before pushing, and it finds nothing. Second, when it does find something, the reply is not for the bot: it is for the reviewer who will scroll past an unanswered "Medium Severity" and read it as an open defect.
The bot is also genuinely useful in one specific way: it reads the implementation behind your call and reasons about what that implementation does asynchronously. That is a class of objection self-review tends to miss, because self-review checks what the code says and the bot checks what the code it calls actually does.
When to use
- Before the first push into a repo whose PRs carry bot comments — harvest first, then push.
- A bot finding has landed and needs an answer, or is about to be read by a human reviewer.
- Your change adds a wait, a guard, a helper, or a default that sits in front of something asynchronous — the bot's strongest category.
- The change is test-only. Counter-intuitively this is where a bot has a lot to say: whether the test exercises the path it claims to.
When NOT to use
- No automated reviewer on the repo. Check before assuming:
gh pr view N --json commentson a handful of recent PRs. - The rules you are worried about belong to a named human reviewer, recorded on other contributors' PRs — that is red-team-your-own-diff, and it finds a different class.
- The gap is in the input space, not the rulebook: fuzz-before-you-claim-done.
- The bot's finding is already fixed by the change you are about to push. Say so in one line; do not stage a refutation of something you agree with.
- The finding is about a file or behaviour you did not touch. Note it, do not adopt it.
- ⛔ Never treat the bot's verdict as a gate. It is often
NEUTRALin the check list for a reason. Measure the claim; the measurement decides, not the severity label.
The practice (checklist)
1 — Harvest its rulebook. Iterate recent PRs and pull the bot's inline comments:
for p in $(gh pr list --repo $O/$R --state all --limit 60 --json number --jq '.[].number'); do
gh api "repos/$O/$R/pulls/$p/comments" --jq '.[] | select(.user.login=="cursor[bot]") | .body'
done > /tmp/bot-raw.txt
tr -d '\0' < /tmp/bot-raw.txt > /tmp/bot.txt # ⚠️ see the NUL warning below
grep '^### ' /tmp/bot.txt | sed 's/^### //' | sort -u
grep -oE '(Low|Medium|High) Severity' /tmp/bot.txt | sort | uniq -c | sort -rn
⚠️⚠️ A single NUL byte in the harvested text makes grep treat the file as binary and print
nothing at all — not an error, just silence. That silence reads exactly like "the bot has
never posted anything". Pipe through tr -d '\0' or use grep -a before concluding the
corpus is empty.
2 — Cluster the titles into categories and keep them per house. They are stable, because the bot applies the same rulebook. Sort your own change against them.
3 — Run its checklist on your diff before pushing. For each category the bot actually uses in this repo, ask the question it would ask. The generic ones worth keeping:
- What does the thing I call do asynchronously, and do I wait for the right part of it? Read the implementation, not the name.
- Does my test exercise the path the fix changes — or would it pass with the fix reverted?
- Bounds and types: any arithmetic on a count, a length, a timeout; any narrowing cast.
- Lifetime: anything freed, unlinked, or unblocked while another pointer or list still refers to it.
- Blast radius: does this change behaviour for a caller I did not think about — a replica, a cron path, an older format?
- Platform and tags: guards for OS-specific syscalls, and the test tags this repo requires.
4 — When it does find something, answer with a measurement. Probe the exact mechanism it names, in the exact scenario it describes, several times. Then state which half of its claim you tested and what came out. ⚠️ And say what a measurement cannot settle: a race is not refuted by absence, so give the count of samples and the structural argument separately.
5 — Write the reply for the human. Name the mechanism, give the numbers, and keep it short. The audience is the reviewer who will read the bot's Medium-severity heading and want to know in two sentences whether it stands.
Rationalizations
| Shortcut | Why it fails |
|---|---|
| "It's a bot, I'll ignore it." | The human reviewer sees an unanswered Medium-severity finding above your change. Silence reads as an unaddressed defect, and it costs you the first impression. |
| "It's a bot, so it must be wrong." | On our PR it named a mechanism — an async PONG in the takeover path — that my own eleven-vector self-review had not tested. It was wrong about the outcome and right about the gap. |
| "It's a bot, so it must be right — I'll just add the wait it asks for." | Then you ship a change you cannot justify, and the next reviewer asks why. Measure first; the fix it proposes may be unnecessary or may hide the real question. |
| "I already self-reviewed, so the bot has nothing new." | Self-review checks what your code says. The bot reads what the code you call does. Those miss different things. |
| "There are no bot comments on this repo." | Check for the NUL-byte silence before believing it. A harvest that greps a binary-flagged file prints nothing and looks identical to an empty corpus. |
| "Answering a bot is beneath the PR." | The reply is not addressed to the bot. It is the paragraph a human reads instead of re-deriving your reasoning. |
RECEIPT
redis/redis, 2026-08-14. Cursor Bugbot reviewed PR #15636 minutes after a push and posted
one Medium Severity finding — before any human had looked at the revision.
Harvest: 28 findings across 15 of the 60 most recent PRs, distributed 18 Medium / 6 High / 4 Low, clustering into categories that are directly runnable as a pre-push checklist:
| category | its own titles |
|---|---|
| memory lifetime | Use-after-free when last blocked client is unblocked · Use-after-free when accessing freed clients list pointer · Multiple deferred CREATEs leak ae timer events |
| arithmetic & bounds | Throttle delay math can overflow · Off-by-one length check rejects valid abstract socket names · Unbounded memcpy in connect path · Truncating long long to int |
| ⭐ the test does not test the fix | Inverted regression test result · Regression test misses bug · Test doesn't exercise the actual bug fix path · Broken test failure message |
| incomplete waits / async | Incomplete ASM settle wait (ours) · Module iterator ranks go stale |
| races | IO thread races arming throttle · Throttle causes readable busy-loop |
| unintended blast radius | Replicas blocked by request throttle · Replica RDB expansion behavior changed · Unowned slots skip cron shrink |
| platform & tags | Unlink skip not guarded by Linux ifdef · Missing {needs:debug} tag |
| one error masking another | RDB error masks concurrent AOF error for master commands |
⭐ The category that matters most for test-only work is its own third row — four separate findings about tests that do not exercise what they claim. A test-only PR is not below a code-review bot's threshold; it is squarely inside its strongest lane.
Its finding on our PR, verbatim in part:
"
move_slots_to_nodereturns as soon as the destination task state matchescompleted, without waiting for the source to finalize or for cluster propagation. Destination takeover marks itself done and only schedules an async PONG, so the source can still believe it owns the slots or still hold an active migrate task."
⚠️ That was attack vector 12 of an eleven-vector self-review I had already run — and I had refuted it on weaker evidence. My probe measured "no active task or trim anywhere". The bot named a different mechanism: ownership propagation. Those are not the same property, and only the bot's version would produce the failure it describes.
Answered by measurement, not argument. Probed every node's ownership view immediately
after the helper returns, then ran the exact sequence the bot describes — move the range to
one node, then IMPORT on the former source — three times:
| former source still claiming the range | ⭐ 0 of 18 probes |
| active task or trim anywhere after return | ⭐ 0 |
[err]/[exception] in those runs |
⭐ 0 |
| the same sequence across the full 89-test sweep | ⭐ 0 failures, down from 9 |
⚠️ And the honest limit, stated rather than hidden: a race is not refuted by absence. What carries more weight than the samples is structural — the helper's check reads the same node's view that the subsequent command is validated against, so the decision and the validation share one source of truth.
⭐ Also caught by the harvest: a false negative of my own. The first two attempts to count
the bot's findings returned zero, and I nearly recorded "the bot has posted nothing" — the
file contained a single NUL byte, so grep treated 75 KB of text as binary and printed nothing.
tr -d '\0' turned the same file into 28 findings.
Lifecycle
- Harvest once per house, keep the category list in its playbook. The rulebook is stable; the second run costs nothing.
- Re-harvest when the bot changes (a new vendor, a new version string in its footer).
- ⚠️ The bot edits the PR body. It appends its own summary block; preserve that verbatim when you rewrite the description, or you will clobber or duplicate it.
- ⭐ Record which of its findings you refuted and how. The next one in the same category is answered in a minute, and a pattern of measured refutations is what stops a reviewer treating its labels as verdicts.