A good review agent needs a bias toward silence, a validation pass against its own findings, a context budget, and a definition of "correct" that comes from the repo instead of from the industry. This walks through rules I encoded in a review agent skill, with the prompt examples, and why each one exists.
The twenty-comment review
Here is what happens when you point a competent model at a diff and ask it to review.
It comes back with twenty comments. Two are real: a race in the retry path, a check that got deleted along with the code it was guarding. Four are true but irrelevant, a magic number in a test fixture, a function that could be split. Six are opinions dressed as findings, "consider extracting this to a constant." Five are about code the PR never touched. Three are wrong, because the model didn't read the caller that already validates the input.
Every one of them is formatted beautifully. Severity label, file and line, a code snippet, a suggested fix.
The author now has to read twenty comments to find two. Next PR, they skim. The one after that, they stop reading. The agent's precision problem has become a trust problem, and once a reviewer is untrusted, its recall doesn't matter at all. A human staff engineer reviewing that same diff leaves two comments and approves. That is not laziness. That is the job.
So the interesting design question for a review agent is not "how do we find more." It's "how do we find less, and be right."
Everything below comes out of building klaussy-agents, an MIT-licensed CLI that scaffolds review skills into a repo. The prompt excerpts are the real ones, rewrapped to fit and with em-dashes normalized to commas, from skills/review/SKILL.md and sub-agents.md.
1. Make silence a valid outcome
A model asked to review code assumes it was asked to produce findings. Zero findings reads to it like failure, so it pads. The fix is to mark this as a valid and wanted outcome:
**Precision over recall.** Default to *not* reporting. If no finding is one a
competent author would clearly want to fix, return an empty review and say so,
an empty review is a valid, good outcome, not a failure. Do not invent findings
or pad to look thorough.
Enter fullscreen mode Exit fullscreen mode
"Be concise" gets interpreted as "write shorter comments," and you get twenty terse comments instead of twenty long ones. "An empty review is a valid, good outcome" changes the target instead of the word count.
Pair it with a rule that makes padding expensive:
**Every finding must name a concrete trigger.** State the specific input, state,
or execution path that makes it go wrong. If you cannot describe how the problem
is reached, you have not proven it, drop it.
Enter fullscreen mode Exit fullscreen mode
Most padding cannot survive that. "Consider extracting this constant" has no input that triggers it. "This N+1 fires once per row in the results list, so a 500-row page makes 500 queries" does. Requiring the trigger is a filter the model applies to itself, and it costs you almost nothing in real findings.
One thing I banned: self-assigned confidence scores.
**Don't self-assign confidence scores.** A number you make up is noise; the
trigger path above is the real evidence.
Enter fullscreen mode Exit fullscreen mode
"Confidence: 85%" is a number the model invented. It looks like calibration and isn't. Worse, it lets a weak finding ship with a hedge attached instead of being dropped. Make the model produce evidence, not a probability.
2. Trace every path the change touches, then narrow again
Focus is not the same as shallowness. The dangerous bugs in a PR are usually not visible in the diff hunk. They live one call up.
Two mechanisms handle this without widening the review.
Read whole files, never just hunks. The diff tells you what changed. It does not tell you what the change means. So the context phase reads the full text of every reviewable changed file, in one parallel batch, before any analysis happens.
Audit what was deleted. This one catches more regressions than any other rule in the skill:
**Removed-behavior audit:** for every deleted or replaced line in the diff, name
the invariant, guard, or behavior it enforced, then confirm the new code
re-establishes it (or that dropping it is intentional and safe). Silently
removed checks are a top source of regressions.
Enter fullscreen mode Exit fullscreen mode
Additions get scrutinized because they're new. Deletions get skimmed, because a red line looks like cleanup. A null check that vanished during a refactor is invisible in review and obvious in production.
Recalibrate by creating a steelman. Every finding, before it ships, goes through this:
**Argue the author's side, then refute it.** For each finding, write the
strongest one-line case that it is *not* a real problem (the input can't occur,
a caller already guards it, the framework handles it). Then either refute that
case with specific code evidence, or, if you can't, drop the finding as a likely
false positive. A finding you can't defend against its own counterargument
doesn't ship.
Enter fullscreen mode Exit fullscreen mode
Nothing else in the skill removes as many bad findings. Models are agreeable by default, and an agreeable reviewer confirms its own first impression. Forcing an explicit steelman of the author, then requiring code evidence to knock it down, kills most false positives at the source. The validator reads callers and callees to do it, which is the tracing work that made the finding provable or not.
The two rules work as a pair. Trace wide to find it. Steelman hard to keep it.
3. Treat context as a budget
A large PR will not fit usefully in one window, and even when it fits, attention degrades across a long context. Three moves, in increasing order of cleverness.
Delete the noise before the model sees it. Most of a big diff is not reviewable by anyone. Lockfiles, dist/ and node_modules/, minified bundles, generated protobuf, pure renames. A deterministic pre-pass strips them, which is a job for code, not for a model:
LOCKFILES = frozenset({"package-lock.json", "yarn.lock", "poetry.lock",
"uv.lock", "Cargo.lock", "go.sum", ...})
NOISE_DIR_PARTS = frozenset({"node_modules", "vendor", "dist", "build",
"out", ".next", "__generated__", ...})
Enter fullscreen mode Exit fullscreen mode
A PR that reads as 4,000 changed lines is routinely 600 after trimming, and those 600 are the ones a human would have read.
The important detail is what happens to the rest. It does not disappear. The trimmed diff is followed by an explicit Excluded from review manifest listing every dropped file and the reason, because silent truncation reads as "I covered everything" when it didn't. If a finding in real code points at a generated file, the reviewer can go look.
Split the remaining context by lens, not by chunk. Above 150 changed lines, the review fans out to parallel sub-agents, each with the same diff and one job:
You are a senior engineer reviewing a pull request. Your ONLY focus is the lens
described below. Other concerns (correctness, architecture, security, scope,
etc.) are handled by parallel reviewers, ignore them.
Enter fullscreen mode Exit fullscreen mode
Correctness and concurrency. Architecture, performance, reliability, dependencies. Security, readability, tests. Scope and conventions. Two more activate conditionally: an agentic and evals lens when the diff touches AI code, and an architecture-decision lens when it contains an ADR or design doc.
Chunk-splitting means no agent ever sees the whole change, and cross-file bugs go missing. Lens-splitting means every agent sees the whole change and is only responsible for one kind of question, which is also how a good human review rotation works.
Fan out the slow part too. Validation is the one phase that stays serial by default, and it's the most expensive one, because every finding means reading files and tracing paths. So above six findings, validators run in parallel batches of four to six, grouped by file so each validator reads a given file once.
There's also a cost lever. Not every lens needs the same model. Scope and conventions is largely pattern matching, so it runs on a cheap fast model, while correctness, security, and architecture keep the expensive one. In a parallel fan-out this saves money more than time.
4. The repo's conventions outrank the industry's
This is where general-purpose review agents consistently annoy people. The model knows what modern code looks like. Your repo is eleven years old. Those disagree, and the model is wrong to resolve it in favor of the industry.
If the codebase uses callbacks throughout, a comment recommending async/await is not a review finding. It's a proposal for a refactor in a thread about something else. Same for a custom Result type instead of exceptions, a hand-rolled DI container, a naming scheme from 2015. Consistency with the surrounding code is worth more than proximity to current best practice, because consistency is what lets the next person read the file.
So the conventions lens gets its standards from the repo, not from the model's priors:
### Project Conventions
{{REPO_SPECIFIC_CHECKS}}
If no repo-specific checks are listed above, read CLAUDE.md and any matching
`.claude/rules/*.md` for the area being changed, and verify the PR adheres to
the conventions and known pitfalls listed there.
Enter fullscreen mode Exit fullscreen mode
{{REPO_SPECIFIC_CHECKS}} is filled in at scaffold time by parsing the repo's own CLAUDE.md and path-scoped rule files into check items. The agent is checking the PR against the house style as written down, not against a general notion of good code. And convention findings have to cite the rule they're breaking:
For convention violations, reference the specific convention (file path or
section in CLAUDE.md / `.claude/rules/`).
Enter fullscreen mode Exit fullscreen mode
A citation requirement is a good forcing function. If the agent can't point at where a convention is written down, what it has is a preference, and preferences don't go in review comments.
An agent that wants to modernize your codebase is not reviewing your PR, it's proposing a different one.
5. Thorough and excessive are different axes
Thoroughness and brevity are not a tradeoff you have to make. Thoroughness is about how many paths you check. Excessiveness is about how many of your observations you report. You want maximum on the first and heavy filtering on the second.
Three filters do that work.
Severity has to mean something. Blocker, High, Medium, Low, Warn, Nit. If everything lands at Medium, the scale is decorative. The validation pass explicitly downgrades: a race condition that only affects a debug-only code path is a Low, not a High, and calling it High trains the author to distrust every High.
Out-of-scope changes get flagged, not fixed. Unrelated changes get Warn severity, which says "you probably meant to do this, confirm," not "change it." That's a different move from a finding, and it's the right one for drive-by refactors bundled into a feature PR.
Unrelated optimizations you happen to notice do not go in the review. This is the hardest one to hold. The agent read three files and it found an inefficient loop in one of them. Untouched by this PR. It is a real inefficiency. It still does not belong in this review, because a review comment is a request for the author to act, and asking them to fix code they didn't write in a PR about something else is how a two-day PR becomes a two-week PR. File it, mention it in chat, open a separate issue. Do not put it in the thread.
The nit question resolves the same way. A trailing-whitespace nit is not wrong. It's just not worth what it costs: it consumes the author's attention, and attention is the scarce resource in a review. Ten nits stacked above one Blocker is a formatting choice that hides a bug.
Thorough means the agent traced the deletion, read the callers, and checked the error path. Excessive means it told you about all of it.
6. Make it sound like a person, because tone changes whether it's read
You can be completely right and still get ignored. Feedback that reads as generated, or as a lint report with a scolding attached, gets skimmed.
Two layers handle this, and both are needed.
The prompt side asks for human prose. No em-dashes, the single biggest tell. No filler openers (It's worth noting that). No chatbot scaffolding (Hope this helps!). No emoji, no ALL-CAPS scolding, no LLM lexicon (delve, robust, seamless, leverage). No ranking praise, which is its own tell: great catch on a review the agent itself wrote is nonsense.
Collaborative is the default:
- Assume the author had a reason; acknowledge it when it helps ("I see why this
routes through X, one risk is …"). Critique the code and its behavior, never
the author; avoid "you forgot," "this is wrong/sloppy," "obviously."
- Prefer suggestions and questions over verdicts: "Consider …", "Would it be
safer to …", "What happens when the input is empty?"
- Agreeable is not padded: warmth lives in the framing, not in filler praise.
Enter fullscreen mode Exit fullscreen mode
Blunt mode exists for people who want it: lead with the problem and the fix, no hedging, no softening. Still professional, still critiquing the code and not the author.
Maintain substance:
**Tone must not dilute substance.** Every comment keeps its severity, its
`file:line` + verbatim code quote, its concrete trigger / failure scenario, and
its specific suggested fix. A note that hides a real Blocker, downgrades
severity, or drops the detail has failed.
Enter fullscreen mode Exit fullscreen mode
Politeness that turns a Blocker into "you might want to look at this sometime" is worse than bluntness. Tone is the wrapper, never the content.
And because a prompt asks but does not enforce, there's a deterministic pass afterward that strips the hard tells with regex, preserving fenced and inline code untouched. Prompt compliance is soft, regex is not.
What this buys you
A review that comes back with two comments, both real, both traceable to a specific input, both citing a convention or a failure path, gets read. The author fixes both and merges.
A review with twenty comments gets skimmed once and ignored afterward, and the two real findings inside it die with the other eighteen.
The rules that get you the first one are mostly rules about not reporting: default to silence, require a trigger, steelman the author before you ship, cite the repo instead of the industry, keep unrelated observations out of the thread. The thoroughness still happens.
If you want to read the full skill, it's in klaussy-agents under templates/skills/review/, MIT licensed. pip install klaussy-agents and klaussy init will scaffold a version wired to your repo's own conventions.
What's the worst false positive your review bot has confidently reported?
Written by Stephanie Dover, Software Engineer 10+ YOE, ex GitHub, Twitch, Microsoft. Creator of Klaussy.
0 Comments
Log in to join the conversation.No comments yet. Be the first to share your thoughts.