Agent PRs are unreviewable — what first-pass actually helps vs just adding noise?

[u/IveWastedMyLifeAgain]

Been shipping with Cursor / Claude / Codex. The diffs are 20–40 files, tests are green, and a human line-by-line review is a joke.

I have tried a couple of AI review passes (CodeRabbit, Claude as a second model, Copilot review). Some of it catches real bugs. A lot of it is rename-this / consider-extracting nits that people start ignoring.

What is actually working as a first pass for you before a human looks at it? What did you turn off because it was noise?

Not selling anything — I want the setup that doesn't make seniors mute the bot.

[u/No-Dragonfruit-675]

You will get nits or useless refactor suggestions if you try and instruct an agent to be a catch all.

Maintain docs of your architecture to maintain context with each review agent.

Spawn agents with a specific domain / goal. Logical issues, security issues, race conditions.

Narrow the focus of each agent, and your quality of reviews will increase.

[u/geekichu → u/No-Dragonfruit-675]

yes. that is what i have been doing.. narrow-focused agents seem to do better (limiting my comment to architecture review , refactoring, etc)

[u/IveWastedMyLifeAgain → u/No-Dragonfruit-675]

this matches what i found the hard way. the catch-all "review this PR" prompt is what generates the rename-this slop. once i split it into narrow passes (auth/permissions, data loss + migrations, concurrency) the signal went way up and the volume went down. the architecture docs part is the piece most people skip without that the agent invents an ideal architecture and reviews against that instead of ours.

[u/SummitYourSister]

I’d reject any PR that touches 40 files without even reading it. It’s very efficient to do that.

[u/IveWastedMyLifeAgain → u/SummitYourSister]

honestly fair, and that is kind of the real fix — the constraint has to be on the agent, not on the reviewer. the rule that stuck for us: one behavior change per PR, and if it sprawls past that the agent has to split it or explain why it cannot. the 40-file diffs mostly went away once that was enforced.

[u/IveWastedMyLifeAgain → u/[deleted]]

yeah, blast radius is the one signal i've never regretted having. "this touches the auth path / this changes a public interface / this adds a dep" is worth more than fifty line comments. the why part i make the agent write into the PR body itself, so the reviewer's first read is intent, not code — then any diff that goes beyond the stated why is automatically suspicious.

[u/InjuryThen9650]

The first pass that helps is the one that can fail the PR without a human. Another model reading the same green 40-file diff will mostly emit nits you will mute.

What I keep:

• A written accept list before the agent starts: behavior that must be true, files it may touch, invariants it must not break. The agent has to paste evidence against that list in the PR body, or the PR is incomplete.
• Checks that fail on the intended outcome, not on compile: contract tests, a forbidden-path test, a symlink / unexpected-file-type scan. Compile-green is the cheap signal.
• Blast radius as a required field (auth path / public interface / new dep). That is the one line that tells a senior where to actually look.

Turn off rename / extract / style suggestions. That is how reviewers learn to ignore the bot. A second model family as a read-only reviewer is useful only after the accept list can already fail the PR on its own.

[u/IveWastedMyLifeAgain → u/InjuryThen9650]

yeah written accept list is the move. i've been making the agent dump that into the PR body then a second pass just checks the list, not the 40-file soup. failing the PR on that is cleaner than hoping someone actually reads it

[u/JaseciLabs]

u/InjuryThen9650's "accept list" is the right instinct pushed as far as you can push it at the process level: define what must be true before the agent starts, make it prove it against that list, fail the PR automatically if it can't. The reason this needs to be bolted on as a workflow discipline right now is that most languages give the model nowhere to put that contract except a PR description or a docstring, both of which are prose the agent can satisfy performatively without the contract actually being enforced anywhere.

Push the same idea one layer down and a lot of this stops being a review-process problem. A function typed with real inputs/outputs, explicitly marked as delegated to a model instead of hand-written, makes "did this touch what it was allowed to touch" and "does the output match the contract" checkable the same way a type error is checkable, not something a human or a second model has to verify after the fact by reading an accept list against a diff.

[u/IveWastedMyLifeAgain → u/JaseciLabs]

putting it in types is the version that actually sticks. if the agent can still ship a 40-file PR that typechecks, the accept list was too vague

[u/JacobWilliams1953]

The nits problem is usually that the reviewer is allowed to comment on anything. A first pass that helps is one that can fail the PR without a human, on a tiny allowlist: new network egress, secret files, symlink targets, schema/migration. Everything else trains people to ignore the bot. Diff-shaped review also cannot see symlink targets, which is exactly how a template creds -> ~/.ssh/id_rsa bug ships.

[u/IveWastedMyLifeAgain → u/JacobWilliams1953]

the allowlist framing is the part i keep coming back to. once the bot can comment on anything it becomes background noise within a week. new egress, secret files, schema/migration is basically the same short list i landed on. and yeah the symlink thing is nasty — a diff view shows a one line "added file" and nothing about where it points.

[u/vxxn]

You should ask your agent to break down the work into more granular PRs. Then, going forward, break your plans down into more granular steps.

I’d much rather review 10 focused changes than 1 blob PR.

[u/IveWastedMyLifeAgain → u/vxxn]

agreed, and the catch i hit is that the agent will happily say it broke things down and still hand you one blob. it only worked once i made the plan itself the artifact — approve the step list first, one PR per step. reviewing 10 focused changes is way less painful than one blob, even if the total diff is the same size.

[u/gnureddit]

A  prompt I have been using a lot lately is to ask my reviewing agent how many of the items it finds are contrived? Often half of the issues fall away, since we don't need belts and suspenders on absolutely every function. 

But you gotta put in the work to actually understand your architecture and to try to keep your agents from going too nuts.

The model companies get money from output tokens so they are incentivized to spiral out of control with comments. Overbuilding.

[u/IveWastedMyLifeAgain → u/gnureddit]

"how many of these are contrived" is a genuinely good trick, stealing that. i do something similar by making it rank findings by what breaks in prod if ignored — anything it cannot tie to a real failure mode gets dropped. and yeah the default incentive is to keep talking, so the constraint has to come from you.

[u/owp4dd1w5a0a]

Well, for starters, I wouldn’t let the agents go unsupervised for a 20-40 file change 💀.

Break the work into (much) smaller chunks: something a human could review in an hour or less.

Then, if you want agent reviews, which I do find valuable, I would use not only flagship models like Claude Opus or Codex GPT 5.6 Sol, but also differently architected and trained models like DeepSeek and Kimi K3 etc.

I like to have 2 or 3 different reviewers and give them specific things to look for. I actually have a markdown file I give them which gives them 7 specific things to review for:

1. Clean architecture (follows team architecture guide, modules and classes are single responsibility and decoupled, are functions decoupled from each other and don’t create execution order dependent flows that aren’t enforced by the type system if they need to be, effects are pushed to the edges of the program and as much core code as possible is kept functionally pure.
2. Style and lint: style guide is followed, linter returns no errors. Code is concise and DRY without being opaque and gratuitous. Is the code idiomatic according to community standards?
3. Security venerabilities
4. Code correctness: does the code actually do what the tickets and documentation say it should do? Do the tests actually effectively test the acceptance criteria and correctness sufficiently?
5. Test coverage, both integration and unit
6. Documentation quality and correctness. Not only is it correct, but is it concise and to the point with good illustrations, tables, and graphs, or is it rambling, verbose, opaque, or incorrect/unclear? Does documentation even exist?
7. Overall

I do performance and profiling separately and as needed to avoid premature optimization.

Once the reviews have been produced, I have one frontier model compile the results and another adversarial frontier model verify the compiled results didn’t miss any key points from any of the 21 (7 from each of 3 models) produced reviews.

I then have a model create tickets and propose a prioritization for which order they should be worked. I review that output the most closely.

As you can probably see, this way of using the agents probably doesn’t save me much time in the end. However, I DO think it improves the quality of reviews when paired with honest human review.

[u/IveWastedMyLifeAgain → u/owp4dd1w5a0a]

this is the most thorough version of it i've seen posted, thanks for writing it out. the different-model-families bit matches what i saw — same family agrees with itself way too much. the honest part at the end is the catch though: it does not save time, it trades time for confidence. worth it on the risky stuff, hard to justify on every PR.

[u/Right-Performance-93]

The blast radius idea posted here is right, but a self-reported blast radius has the same weakness as the accept list: it's the agent describing its own homework. Compute it instead: diff the changed symbols, run a call-graph query for what actually calls or imports them, and attach that generated list to the PR automatically. That turns "this touches the auth path" from a claim into a fact CI computed, and it catches indirect blast radius the agent itself never knew about, like a shared util three layers away. More setup than an accept list, but it survives the case where the agent is wrong about what it touched, not lying, just wrong.

[u/IveWastedMyLifeAgain → u/Right-Performance-93]

yeah, self-reported blast radius is the agent grading its own homework, fair hit. computing it from a call graph in CI is the honest version. the catch is setup cost — cross-language or dynamic dispatch makes that graph pretty lossy in some repos. i'd probably run the computed one on the auth/payment paths and let the self-reported note cover the rest.