2026-09-19/7 MIN READ

An Un-Tuned AI Reviewer Is a Stranger to Your Codebase

Two pull requests against Mastra, two confident Major findings from CodeRabbit. The first was wrong. The second was right, and its proposed fix would have hung the suite. Nothing in the comments told them apart — only running the tests did.

Last week I opened two pull requests against Mastra, the TypeScript agent framework that runs the agents behind Loma, an omnichannel agentic subscription solution for fresh produce in the Dominican Republic. Both PRs got reviewed by CodeRabbit, the AI reviewer the Mastra team runs on every PR. Both times it flagged a Major finding within minutes.

The first time it was wrong. The second time it was right, and its proposed fix was still wrong.

Nothing in the two comments told me which was which. They were equally confident, equally specific, equally well written. The only thing that separated them was a test suite. This post is about why that is the normal case, not the exception, and what you can do about it.

The setup

Mastra's maintainers run a tight loop. You file an issue with a pinned, keyless reproduction, their triage bot classifies it within hours, and once the triage label is gone you may open a PR. CodeRabbit reviews it on the assertive profile, then a human maintainer decides. It works: of the sixteen issues I filed on September 18, five were fixed by the maintainers before I woke up the next morning.

Two of the triaged issues were unclaimed, so I took them.

Issues filed

one day, each with a keyless repro

Fixed by maintainers overnight

before the next morning

Taken by me

both unclaimed after triage

Major findings from CodeRabbit

2 of 2

within minutes, both confident

PR 1: the reviewer was wrong

mastra-ai/mastra#24476 fixes issue #24443. When an output processor rejects a model's answer (Mastra calls it tripping the wire), one of the three processor hooks blanked result.text while the same prose was still sitting in result.steps[].text and result.response.messages. The other two hooks kept it. My fix made all three agree: report the rejection through result.tripwire, keep the text.

CodeRabbit read the diff and posted one Major finding: preserve text on rejected completed steps. It wanted me to go further and also delete this line, a few dozen lines away from my change:

packages/core/src/loop — the line CodeRabbit wanted deletedMajor · assertive profile

const stepText = stepTripwire ? '' : self.#bufferedByStep.text;

Read cold, it looks like the same inconsistency I was fixing. Text produced, then thrown away because of a tripwire. Why keep it?

I applied the suggestion verbatim and ran the suite. Three existing tests failed.

vitest · packages/core3 failed

✗ should retry with feedback when processor calls abort with retry: true
✗ should not include rejected assistant response in messages on retry
✗ should not include rejected assistant response in messages on retry (variant)

  expected: 'improved response'
  actual:   'bad response that needs improvementimproved response'

The failing assertion tells the whole story. The rejected attempt's text had been glued onto the front of the retry's text.

That line is not an inconsistency. It is a convention: when a processor rejects an attempt and asks for a retry, the rejected attempt's text must not leak into the final answer. The codebase pins it with three tests. CodeRabbit could not know that, because the rule lives nowhere a reviewer can read except the tests themselves, and reviewers read diffs.

I replied with the failing test names and the concatenated string, reverted, and CodeRabbit's check flipped to pass.

PR 2: the reviewer was right, and its fix would have hung the suite

mastra-ai/mastra#24479 fixes issue #24174. If you abort an agent run and tear down its subscription in the same tick, the aborted run stays registered as the thread's active run. The next subscription seeds itself from it and replays a phantom second run: a second agent_start, a second terminal event, after the session already said agent_end(aborted).

My first commit added one guard where a new subscription seeds from the active run: skip it if the run is in the aborted set. Tests written first, failing on main, passing on the branch. Clean.

CodeRabbit posted a Major: filter aborted runs in enqueueRun. Its point: my guard covered the seed path, but a retained backend such as Redis Streams replays the aborted run's run-registered event to any fresh subscriber, whose handler resolves the still-present local record and enqueues it anyway. A second door I had left open.

It was right. I wrote a new failing test for that path before touching anything, and it failed exactly as described.

Then I applied CodeRabbit's proposed fix, gating enqueueRun on the same aborted-run-id set, and ran the suite. Two persistence tests hung until the 120-second timeout.

The reason took a while to see. A run keeps its id across a suspension. When a run is parked on a tool approval and the abort deadline fires, abortRun adds the id to the aborted set, then declines to release the approval-gated run. The run is later legitimately resumed under the same id. CodeRabbit's gate saw the id in the aborted set and dropped the resumed segment's registration. The run never came back.

Fig. 01Why gating on the run id hangs a resumed run

RENDERING DIAGRAM…

.mermaid-status{display:none}sequenceDiagram participant S as Suspended R participant A as abortRun participant Set as abortedRunIds participant E as enqueueRun participant Res as Resumed R A->>Set: add R A-->>S: approval-gated, not released Note over S,Res: later, the run is legitimately resumed Res->>E: register R alt proposed gate E->>Set: is R aborted? Set-->>E: yes E--xRes: dropped. Run never returns. else what shipped E->>E: is THIS record marked aborted? E-->>Res: no. Enqueued. end

Sequence diagram of a suspended run with id R. abortRun adds R to the aborted-run-id set, then leaves the run parked because it is approval-gated. Later the run is legitimately resumed under the same id R and its resumed segment asks enqueueRun to register it. Under CodeRabbit's proposed gate, enqueueRun asks the set whether R is aborted, the set says yes, and the registration is dropped: the run never returns and the suite hangs. Under what shipped, enqueueRun instead checks whether this specific record is marked aborted, it is not, and the segment is enqueued.

The correct fix follows from that: an abort is a property of the segment, not the run id. abortRun marks the specific record it stopped, enqueueRun drops marked records, a resumed segment registers its own unmarked record. One mechanism, one choke point, no new state to sweep. The reply on the PR has the details.

What the two cases have in common

Fig. 02The loop that told the two findings apart

RENDERING DIAGRAM…

.mermaid-status{display:none}flowchart TD A["AI reviewer posts a finding"] B["Apply it on a branch"] C["Run the full suite"] D{"Green?"} E["Accept. Credit the reviewer."] F["Read the failing tests"] G{"Is the finding still valid?"} H["Decline with the test names"] I["Write a failing test for the real problem"] J["Design the fix from the failure, not the suggestion"] A --> B B --> C C --> D D -->|"yes"| E D -->|"no"| F F --> G G -->|"no: it broke a pinned convention"| H G -->|"yes: the problem is real, the fix is not"| I I --> J J --> C classDef accept fill:#0e2b31,stroke:#22d3ee,color:#f8fafc classDef decline fill:#2a1013,stroke:#fb7185,color:#fecdd3 classDef work fill:#2a1a05,stroke:#d97706,color:#facc15 class E accept class H decline class I,J work

Flowchart of how to handle an AI reviewer's finding. The reviewer posts a finding. Apply it on a branch and run the full suite. If the suite is green, accept it and credit the reviewer. If not, read the failing tests and ask whether the finding is still valid. If it broke a pinned convention, decline with the test names. If the problem is real but the fix is not, write a failing test for the real problem, design the fix from that failure rather than from the suggestion, and run the suite again.

In both PRs the reviewer's problem statement was plausible and its fix was a diff-local edit. In PR 1 the problem was not real. In PR 2 it was real, but the fix ignored a lifecycle that only shows up when you run the persistence suite.

Neither miss was a reasoning failure. Both were missing-context failures. The reviewer knew the diff and enough of the surrounding file to sound authoritative. It did not know the conventions maintainers apply by hand, and it did not run the tests.

PR	Problem statement	Proposed fix	What told them apart
#24476	Wrong. The line it flagged was a pinned convention, not an inconsistency.	Wrong. Deleting it leaked rejected text into the final answer.	Three existing tests failed on the branch.
#24479	Right. A retained backend replays the aborted run through a second door.	Wrong. Gating on the run id dropped every legitimately resumed run.	Two persistence tests hung until the 120-second timeout.

Why it happens: the reviewer only has one of four layers

Fig. 03What a maintainer knows versus what the reviewer is given

RENDERING DIAGRAM…

.mermaid-status{display:none}flowchart LR subgraph K["What a maintainer knows"] direction TB L1["1 · The diff"] L2["2 · The surrounding code"] L3["3 · Conventions rejected attempts are withheld · aborted runs never leak into later lifecycles · use the logger, not console · keep the error cause"] L4["4 · The test suite, run"] end R["Un-tuned AI reviewer"] R -->|"full"| L1 R -.->|"partial"| L2 R -.-x|"none"| L3 R -.-x|"none"| L4 classDef has fill:#0e2b31,stroke:#22d3ee,color:#f8fafc classDef part fill:#151b27,stroke:#67e8f9,color:#94a3b8 classDef none fill:#2a1013,stroke:#fb7185,color:#fecdd3 classDef reviewer fill:#2a1a05,stroke:#d97706,color:#facc15 class L1 has class L2 part class L3,L4 none class R reviewer

Diagram of the four layers a maintainer knows: the diff, the surrounding code, the conventions (rejected attempts are withheld, aborted runs never leak into later lifecycles, use the logger not console, keep the error cause), and the test suite, run. An un-tuned AI reviewer has the diff in full, the surrounding code partially, and none of the conventions or the test run.

I went and looked at how Mastra configures CodeRabbit. The .coderabbit.yaml runs the assertive profile and carries path instructions for exactly two things: how to write changeset files, and validate in inputSchema and outputSchema, not inline in execute. Both are good rules. Neither says anything about the stream, loop or processor code where both of my PRs lived.

So on that code the reviewer is a stranger. A smart one. Still a stranger.

Tuning for signal, not silence

The tempting fix is to lower the profile from assertive to chill. That would have muted the wrong finding on PR 1 and probably the right one on PR 2 too. Fewer words, less signal. Not a trade I would take.

The better fix is to give the reviewer the third layer. I left a note for the maintainers on PR 1 proposing exactly that:

01Path instructions for the hot directories.A dozen lines for packages/core/src/{stream,loop,processors}/** stating the contracts in plain language: a rejected retry attempt's text is withheld; an aborted run must not reappear in a later lifecycle; log through the configured logger, never console; wrap errors without dropping cause. The reviewer reads these before every review of those paths.
02Learnings from replies.CodeRabbit stores per-repository learnings when a human replies to a finding with a reason. Every declined-with-a-reason makes the next review slightly better. It is slow and per-thread, but it compounds.
03A single conventions document.A short section in DEVELOPMENT.md, linked from the config as a knowledge-base entry, so the rules live in one place instead of in reviewers' heads and scattered comments.

What none of this fixes is the second kind of miss. On PR 2 the problem statement was correct and the fix was still wrong in a way only a running test could show. No amount of tuning gives a reviewer the fourth layer. You have to run the tests yourself, every time, before you accept an edit.

The lesson, in one paragraph

An un-tuned AI reviewer is a stranger to your codebase. It reads the diff, sounds like a senior engineer, and has never seen your conventions or run your tests.

Treat its findings as hypotheses: apply on a branch, run the suite, read the failures, then decide. Write your conventions where the tool can read them so the first kind of miss gets rarer. Accept that the second kind never goes away. On the two PRs above the reviewer went one for two on problems and zero for two on fixes, and it was still worth having in the loop, because the one real catch was a bug I had not seen.

Problems, correct

1 / 2

Fixes, correct

0 / 2

Real bugs I had missed

still worth having in the loop

Core unit suite after both PRs

16,189

passing, zero failures, exit-code parity with main

Receipts

mastra-ai/mastra#24476 — PR 1, the wrong finding ↗Fixes #24443. Keeps result.text on a tripwire across all three processor hooks.
Evidence reply on PR 1 ↗The three failing test names and the concatenated string that declined the suggestion.
mastra-ai/mastra#24479 — PR 2, the right finding with the wrong fix ↗Fixes #24174. Marks the aborted segment instead of gating on the run id.
Mechanism reply on PR 2 ↗Why the proposed enqueueRun gate hung two persistence tests, and what shipped instead.
The tuning proposal to the maintainers ↗Path instructions, learnings from replies, and a single conventions document.
pradelgorithm/mastra-repros ↗Keyless reproductions for every issue in the batch.

Disclosure: the fixes, tests and evidence replies were built with an AI coding agent doing the legwork under my direction. It ran the suite before accepting either suggestion. That was the whole point.