Any idea why is “@codex review” on PRs so better at findings mistakes/bugs than bugbot?
I use both heavily - on PRs I run in cursor as much bugbot review rounds as it takes to get to clean result. And then I run codex review on the PR and it almost always ( anecdotal ) finds something, and often important bugs/misses.
Would love it if Cursor stepped up the game in AI PR review
Hey @David_Stredansky, thanks for the detailed feedback!
Could you share a few specific examples we can pass along to the team? The most useful things would be:
.cursor/BUGBOT.md rules set upEven 1-2 examples would give us something concrete to act on. Really appreciate you taking the time!
Hey @Colin, here is a summary my agent generated accross my repos ( private, so I cannot share the exact commits ) + I will also include a detailed incident from today where I ran 3 rounds of Bugbot → then there was a clean result from Bugbot and after that Codex returned 3 more findings ( at least one was relevant and important )
TypeScript / Next.js / Postgres. Private repos (a platform monorepo plus a product app). Typical PRs are medium-large. No .cursor/BUGBOT.md. Flow is local or cloud Bugbot rounds until clean, then @codex review loops on the PR.
Classes Bugbot repeatedly misses (after it has already gone clean): concurrent writers and lock/fence gaps; quota and send-state edges after the happy path is correct; cross-job ordering (migrate vs deploy); admission checks applied on one surface but not another; merge/precedence bugs (false overwriting true).
Example 1 — idle connected-account parking. Cloud Bugbot: 3 passes, all findings fixed (park while reserved outbound exists, crash-after-DELETE, owner mismatch). Then @codex review. Codex still filed P1s: bypass the queue guard for already-missing remotes; make the queue recheck atomic with remote deletion; serialize reconnect vs delete with a nonce. Eight Codex rounds after Bugbot was clean.
Example 2 — daily send-slot packing. Three local Bugbot rounds until clean. Bugbot did catch a real high (skipped drafts still counted as quota siblings). After that, Codex found P1s Bugbot never raised: in-flight sending rows dropping out of daily quota; expired sending not reclaimable after a crashed worker; provider POST using the wrong idempotency key.
Same pattern on an invite/connection PR (GitHub Cursor review + Bugbot-style fixes, then Codex P1: already-connected target dropping the message; classifying arbitrary callback payloads as a free-tier account) and on a staging-migrate workflow (Codex P1: schema can advance on main before the matching staging app is deployed).
Counterexamples exist: after Bugbot, a couple of PRs came back Codex-clean. The issue is the non-trivial concurrent / cross-file / state-machine misses, not that Bugbot finds nothing.
Type: internal engineering note
Scope: one pull request that rebuilt outbound webhook contact-field handling
Reviewers compared: Bugbot (iterative, on-branch) vs Codex (single review of the “clean” head)
Anonymization: product name, repository, customer, workspace, and individual names are omitted. Examples use fictional values.
We iterated on a webhook contact-field fix until Bugbot reported no remaining bugs. That took six Bugbot runs: five consecutive finding rounds, then one clean pass.
A Codex review of that same “clean” commit then filed three findings. All three were real. Two were P2 (correctness / compatibility). One was P1 and sat in the same enrichment path Bugbot had already reviewed twice.
The useful conclusion is not “Bugbot is useless.” Bugbot found five genuine defects we would have shipped. The useful conclusion is that a clean Bugbot pass is not a closed correctness argument, especially after several tight fix-the-finding loops. Each loop trained both the code and the reviewer on the last bug class. Codex, arriving once on the finished shape, still saw holes that the loop had normalized as “already handled.”
A published multi-page funnel collected contact fields (email, first name, last name, phone) on page 1 and submitted on a later page. The thank-you path ran. The third-party webhook received HTTP traffic. The body was missing the contact fields the destination needed.
Two local causes:
selectedOptions, but not in the webhook body.{{email}} / {{email_address_0523}} were left as literals, or bound to the wrong key.The intended contract after the fix:
email, phone, first_name, last_name, and name when we have them.{{placeholders}} by exact alias or extracted contact fields — never by prefix.Compatibility stance: additive keys, do not overwrite a value the destination already has, unless that value is clearly missing or junk.
That last sentence is where later reviews kept finding gaps. “Do not overwrite” is easy to implement as if (payload[key] !== undefined). Several Bugbot rounds, and then Codex, were all variations of what counts as a real existing value.
| Step | Reviewer | Result | What we changed |
|---|---|---|---|
| Initial implementation | — | Multi-page contact fields dropped | Merge earlier pages; restore standard contact keys; lead fallback |
| Bugbot 1 | Bugbot | Prefix template match | Exact-alias / contact-field template resolution |
| Bugbot 2 | Bugbot | Marketing labels steal email/phone | Reject opt-in / type / marketing labels |
| Bugbot 3 | Bugbot | Unvalidated lead.data.email / phone |
Only trust values that look like email/phone/name |
| Bugbot 4 | Bugbot | First-name-only name beats “First Last” |
Prefer assembled first+last |
| Bugbot 5 | Bugbot | Empty string treated as present | Treat blank / whitespace as missing |
| Bugbot 6 | Bugbot | No bugs | — |
| Codex (same clean HEAD) | Codex | 3 findings, all valid | Validate existing values; broader name labels; keep numeric suffixes distinct |
Six Bugbot runs. Five defects. One clean bill. Then three more defects from a different reviewer on the same commit.
{{placeholders}}Severity: high (first finding on the new helper).
The first template resolver treated “close enough” keys as a match. {{email}} could bind to email_marketing_opt_in. {{name}} could bind to name_suffix. {{phone}} could bind to phone_type.
That is exactly the production poisoning shape: a boolean or enum sitting next to a real contact field, winning because it shares a prefix.
// Buggy idea (simplified): first key that contains the placeholder wins
for (const [dataKey, value] of Object.entries(leadData)) {
if (dataKey.startsWith(wanted) || wanted.startsWith(dataKey)) {
return String(value);
}
}
Fix: resolve only by exact key / exact slug / registered alias, then fall back to extracted contact fields. No prefix walk.
This was a good catch. It also framed the rest of the Bugbot loop: don’t let a lookalike key pretend to be email/phone/name.
Severity: medium / high, same family as round 1.
After prefix matching was removed, extraction still used loose label checks. A field labeled Email Marketing Opt In with value "true" was treated as an email field. Phone Type = "mobile" was treated as a phone.
// Too loose
if (label.includes("email")) result.email = value;
if (label.includes("phone")) result.phone = value;
Those values then occupied the standard aliases (email, phone) on the outbound payload. Destinations that map the first email key would get "true".
Fix: exact / suffix-safe contact labels, plus a deny-list for opt, marketing, consent, subscribe, preference, status, type, verified. Values must also look like an email or a phone.
const NON_CONTACT_LABEL_TERMS =
/\b(opt|marketing|consent|subscribe|preference|status|type|verified)\b/;
// "Email Marketing Opt In" → rejected
// "Email Address" → accepted, if the value looks like an email
email / phoneSeverity: medium.
The server-side fallback read lead.data.email and lead.data.phone and trusted them whenever they were non-empty. Historical submit logic had used the same loose includes("email") / includes("phone") walk, so saved leads could already contain "true" and "mobile".
// After Bugbot 2, form extraction was careful.
// Saved lead.data was not.
return {
email: dataEmail || fromForm.email,
phone: dataPhone || fromForm.phone,
};
Fix: only use lead.data.email / phone / name when they pass the same shape checks (looksLikeEmailValue, looksLikePhoneValue, looksLikePersonName). Junk loses to the form.
This is the first time Bugbot said “you validated one source and forgot the other.” Codex later said the same thing about a third source: the inbound payload sitting in leadData at apply time.
name beats assembled full nameSeverity: medium.
Submit stored data.name from the first label containing "name". On a form with First Name then Last Name, that is the first name only.
if (label.includes("name") && !name) name = value;
// "First Name" = "Jane" wins. "Last Name" never updates data.name.
The webhook helper then preferred that saved data.name over the form-assembled "Jane Smith".
// After Bugbot 3
name: (looksLikePersonName(dataName) ? dataName : "") || fromForm.name
// dataName = "Jane" looks like a name → payload.name stays "Jane"
Fix (and we widened it, because the same bug existed in three places):
extractFunnelContactFields prefers assembled first+last over a shorter name.contactFieldsFromLeadRecord prefers the form-assembled name, using a “richer name” helper ("Jane" vs "Jane Smith").name without overwriting an unrelated existing name ("Acme Inc" stays).label.includes("name") and started using the same extractor.This round is important later. The submit-route switch is what Codex’s second finding is about.
Severity: high.
After the name work, Bugbot found the next hole in “do not overwrite existing values”:
if (existing === undefined || isUnresolvedTemplate(existing)) {
payload[key] = contactValue;
}
email: "" and phone: " " are not undefined. They are also not templates. Apply left them blank. Template resolution did the same: a present empty string was “usable,” so {{email}} never fell through to the saved lead.
This is the production-shaped failure mode again: the lead row has the email; the outbound body does not.
Fix:
function isMissingWebhookValue(value: unknown): boolean {
if (value === undefined || value === null) return true;
if (typeof value === "string" && value.trim() === "") return true;
return isUnresolvedWebhookTemplate(value);
}
Used both when applying contact fields and when deciding whether a template key is already bound.
Prompt: remaining real bugs only; do not re-litigate prefix matching, marketing labels, or unvalidated lead.data email/phone unless a new hole remains.
Result: no bugs.
At this point the branch had five review-driven commits on top of the original fix, a focused QA file (~14 tests, later 17), and a reviewer that had just spent five rounds on this exact module. The clean result was honest about that reviewer’s remaining search. It was not a proof that the enrichment contract was closed.
Codex reviewed the commit that Bugbot had just cleared. It filed three inline comments. None were style. None were duplicates of an unfixed Bugbot item. All three were merged and covered by tests.
When incoming
leadDataalready contains a nonempty but invalid contact value — such asemail: "true",phone: "mobile", orname: "true"—isMissingWebhookValuereturns false, so the valid contact loaded from the saved lead is not applied. Template resolution then also prefers that bad direct value.
This is the same family as Bugbot 3 and Bugbot 5, one step further along the pipe.
| Source | Bugbot 3 | Bugbot 5 | Codex P1 |
|---|---|---|---|
Saved lead.data.email = "true" |
fixed | — | — |
Payload email = "" |
— | fixed | — |
Payload email = "true" |
not checked | not checked | found |
{{email}} with leadData.email = "true" |
— | empty only | found |
After Bugbot 5 the guard was:
if (isMissingWebhookValue(existing)) {
payload[key] = value; // only undefined / null / "" / {{...}}
}
"true" is none of those. The saved lead could hold [email protected]. The body still left with email: "true".
Why Bugbot missed it: the previous two rounds had just taught the helper that (a) saved lead.data must be validated and (b) blanks are missing. The apply path still treated any nonempty non-template string as sacred, because the compatibility rule was “don’t overwrite.” Codex asked the more precise question: don’t overwrite a value that is valid for that key.
Fix:
function isInvalidExistingContactValue(key: string, existing: unknown): boolean {
const current = stringifyContactValue(existing);
if (!current) return true;
if (key === "email" || key === "email_address") return !looksLikeEmailValue(current);
if (key === "phone" || key === "phone_number") return !looksLikePhoneValue(current);
if (key === "name" || key === "first_name" || key === "last_name") {
return !looksLikePersonName(current);
}
return false;
}
if (isMissingWebhookValue(existing) || isInvalidExistingContactValue(key, existing)) {
payload[key] = value;
}
Template lookup uses the same idea: if the placeholder is a contact synonym and the bound value fails the shape check, skip it and use extracted contact fields.
This was the highest-value Codex finding. It is the original customer failure mode (junk in a contact key, real value sitting on the lead) after five Bugbot rounds had already circled that mode.
For funnels whose person-name field has a customized label such as
Legal Name,Your Name, orApplicant Name, this exact comparison no longer recognizes it. The submit route now uses this helper in place of the previouslabel.includes("name")extraction, so those submissions savedata.nameas undefined and also fail to populate the standard webhooknamefield.
Bugbot 4 correctly killed includes("name") because it treated Company Name and First Name as a full name. The replacement was too tight:
if (label === "name" || label === "full name") {
result.name = value;
}
That is safe. It is also a product regression for any funnel that renamed the field, which is common in this editor.
Why Bugbot missed it: Bugbot 4 was solving “first name leaked into name.” The tests we added were First Name + Last Name + Company Name. Nobody asked “what legitimate labels did includes("name") used to catch?” Codex reviewed the substitution, not only the bug being closed.
Fix: accept name, full name, and * name / * full name, while denying company / business / first / last / middle / user / account / display and the existing marketing deny-list.
// accepted: Legal Name, Your Name, Applicant Name, Contact Name
// rejected: Company Name, First Name, Last Name, Display Name
Submit and webhooks share the helper, so the stored lead and the outbound body stay aligned.
When a funnel contains multiple fields with the same normalized label, a custom placeholder such as
{{company_name_1234}}can now bind tocompany_name_5678: both keys have their numeric suffix stripped, and the first matchingleadDataentry wins.
The editor disambiguates repeated labels with a trailing id (company_name_1234 vs company_name_5678). After Bugbot 1 we stripped that suffix so {{phone_number_0803}} could still resolve to phone_number. The strip was applied to both sides:
const wantedBase = stripTrailingIdSuffix(key); // company_name_1234 → company_name
if (exactSlugs.has(stripTrailingIdSuffix(dataKey))) { // company_name_5678 → company_name
return stringifyForTemplate(value); // silent cross-bind
}
That is a real silent data swap: page 2’s “Company Name” goes out as page 1’s answer.
Why Bugbot missed it: Bugbot 1 asked “does {{email}} steal email_marketing_opt_in?” We fixed prefix matching and added a phone-suffix happy path test ({{phone_number_0803}} → phone_number). We never tested two siblings that share a base. Codex asked the collision question.
Fix:
{{phone_number_0803}} → phone_number).// {{company_name_1234}} + { company_name_1234, company_name_5678 }
// → 1234 only
// {{company_name}} + two suffixed siblings
// → leave unresolved (ambiguous)
All three fail the “would this change production webhook bodies?” test.
"true" in email still ships junk after Bugbot signed off.data.name and the standard name key.None of these required a new product requirement. They are consequences of the contract we already claimed: send the real contact fields, don’t invent or steal values, don’t overwrite good data.
This was not a sloppy first review. The loop was doing its job:
The miss pattern is consistent:
| Pattern | What happened |
|---|---|
| Same bug, next hop | Validate source A (saved lead). Forget source B (inbound payload). Forget that "true" is as empty as "" for an email key. |
| Fix over-tightens the previous heuristic | includes("name") was wrong. Replacing it with two exact strings dropped valid labels. |
| Happy-path test for the last fix | Suffix strip made {{phone_number_0803}} work. Collision with a second company_name_* was untested. |
| Reviewer recency | After five rounds on this file, “contact validation” felt done. The apply-time guard still used a weaker predicate than extract-time. |
| Second reviewer, first look | Codex did not carry the “we already handled poisoning” narrative. It read the predicates as they stood. |
Bugbot is strong at the next adjacent hole in a recently touched function. It is weaker at re-deriving the full invariant after the function has been patched five times. Codex’s value here was that second derivation.
includes("name") removal needed a one-line inventory: Legal Name, Your Name, Applicant Name, Contact Name.lead.data email” made Bugbot 6 less likely to ask “is the inbound payload validated the same way?”Codex’s three comments were implemented and covered:
name without treating company or first/last labels as a full name.The pull request that Bugbot had cleared was not yet correct. The second reviewer was not repeating work. It closed the last hop of a bug class the first reviewer had been chasing for five rounds.
Inbound payload (current page + leftover aliases):
{
"email": "true",
"phone": "mobile",
"name": "true",
"company": "Northwind"
}
Saved lead (form on page 1):
{
"email": "[email protected]",
"phone": "2025550100",
"firstName": "Jane",
"lastName": "Smith",
"name": "Jane Smith"
}
After Bugbot 6 (clean): outbound email / phone / name stay "true" / "mobile" / "true".
After Codex P1: they become the saved contact values. Company is untouched.
That is the whole argument, in one payload.
Hey @David_Stredansky! Thanks a lot for this deatiled report. Excited to share with the team.
Quick clarifying question so we can be precise about each case: for the rounds where Bugbot came back clean before Codex found more, were those reviews running on the PR itself (automatic, or triggered with a comment on the PR), or run from inside the editor?
It matters because the two work differently: PR reviews use Incremental Review by default, so each follow-up round focuses on what changed since Bugbot’s last review, while reviews run from the editor re-read the whole selected diff every round.
thanks @Colin ; I run the bugbot from agent’s chat - I usually instruct him to do up to X round of bugbot reviews with the goal of having clean result. The codex review is triggered in github PR as a “codex review” comment - and its findings are then again fixed by cursor agents.
I do not instruct specifically which changes should bugbot review but from I gather it takes a look at the changes that are not in git branch ( so git diff ).
Thanks for looking at this - excited to see this moving forward! If there is a way to better utilize bugbot now I’d be glad to hear it