The whole design, in writing
Every step of the build above, written out: the problem each piece solves, the option that was taken and the ones that were not, the numbers, and how it fails in production.
The big idea
What does a code review bot actually need to get right?
The obvious version is "send the diff to an LLM, post whatever it says as PR comments." Try that at real scale and you get a bot developers mute within a week: it repeats itself on every push, comments confidently on things it doesn't have context for, buries the one real bug under nitpicks, and occasionally quotes a leaked API key straight back into a public comment thread.
Every one of those failures has a specific fix: run cheap, deterministic checks before any LLM call, scope review to the diff plus just enough context, retrieve repo-wide context the diff alone can't show, gate on confidence before posting, track what's already been said, and redact secrets. We'll build all of it.
What the new pieces do
- Developerclient
- Pushes a diff and expects useful, precise review comments — not 40 nitpicks and not silence on the one real bug.
Step 1 · Cheap checks first
Don't send everything to an LLM
A PR touches 30 files. Some issues — an unused import, a hardcoded credential pattern, a missing semicolon — are trivially, deterministically detectable. Should everything, including these, go through an LLM call?
An unused variable and a hardcoded secret pattern are both deterministically detectable. Route them through an LLM anyway?
Using a slow, costly, probabilistic tool for something a linter catches perfectly and instantly is a strictly worse trade on speed, cost, AND reliability — "simpler architecture" isn't worth being worse at the job.
Static analysis tools are precise and instantaneous for their class of problems (unused code, known-bad patterns, style). Running them first means the LLM never has to (and never should try to) rediscover what a linter already knows with certainty — its budget goes entirely toward the semantic reasoning only it can do.
An LLM asked to also catch every stylistic/lint-level issue is doing redundant, error-prone work on problems that have a perfectly reliable, instant, free existing solution — a wasted opportunity and a reliability regression on the "easy" findings.
Run static analyzers and linters first — deterministic, instant, and precise for their class of issues. Only what genuinely requires understanding intent (does this logic actually do what the PR claims, is this the right approach) needs to reach the LLM at all.
Why this piece earns its place
A linter answers a different question than review asks. It reports the state of a file; review asks what this PR introduced. So run the analyzers on the merge base as well as the head and post the difference — not the head's findings filtered by location. The second run is the part people skip, and it is what saves you when code moves: a function lifted wholesale into a new file arrives inside the diff carrying every long-standing violation it already had, and a location filter alone hands all of them to this author as new work. Everything the second run cancels out is a backlog item, not review. Two operational details follow. Use the repository's own analyzer config rather than your defaults, or the bot spends its first week arguing with a style the team already settled. And distinguish no findings from did not run: an analyzer that dies on a half-finished refactor exits with an empty result, which looks exactly like a clean file, and a whole PR silently loses its cheapest coverage with nothing in any dashboard to say so. Pin analyzer versions per repo too — a toolchain upgrade that rewrites the finding set overnight arrives as a flood of comments on PRs that changed nothing.
What the new pieces do
- Webhook / APIgateway
- Receives the PR webhook, kicks off review, and is the single point that decides what actually gets posted back.
- Static Analyzersservice
- Fast, deterministic, precise checks — unused variables, style, known-bad security patterns — that never need an LLM call.
Step 2 · Diff-scoped, not file-scoped
Review the change, with just enough context
A file is 800 lines; a PR changes 12 of them. Should the LLM see the whole file every time, or just the diff?
Scope review to the diff plus surrounding context — the changed lines, a reasonable window of unchanged code around them for coherence, but not the entire file (let alone the entire repo). This keeps every review focused on what actually changed (comments on 770 untouched lines waste tokens and, worse, invite the bot to comment on pre-existing code that wasn't part of this PR at all), and keeps the token/cost budget proportional to the size of the change, not the size of the file.
Why this piece earns its place
The window is the part that gets built wrong. A fixed number of lines either side of a hunk cuts the enclosing function in half, and a model handed half a function invents the other half — expand to syntactic boundaries instead, with the same parser you already want for chunking, so the unit is a whole function or class body. Include the removed lines, not only the new side of the diff: a review that sees just the new state cannot notice that a bounds check disappeared, and a deletion is often the most valuable thing a diff has to show. Two hunks inside the same function should be assembled once, together, or the bot reviews the same code twice and contradicts itself. The second reason this step matters is downstream. The hunk is the addressing scheme for everything after it — a comment has to be posted against a file, a side and a line the code host will accept, and the feedback loop attributes a reaction back to that same location. Assemble context loosely here and every one of those coordinates is approximate.
What the new pieces do
- Diff + Context Assemblerservice
- Builds exactly what the LLM needs: the changed lines, enough surrounding code to make sense of them, and nothing from the rest of the file that wasn't touched.
Step 3 · Context the diff alone can't show
What about the function this diff calls?
The diff modifies a function's call site, passing an argument in a new order. The LLM can't judge whether that's correct without knowing the CALLED function's actual signature — which isn't in the diff at all, because that function wasn't touched.
The diff calls a function whose definition isn't in the diff. How does the LLM get that context?
This is RAG applied to a codebase: rather than shipping the whole repo to the LLM (far too large, mostly irrelevant), retrieve exactly the symbols and context the diff touches — the called function's signature, a relevant type definition, the project's style guide — and include only that.
Most real repos are far larger than any practical context window, and even where they'd technically fit, the vast majority of that content is irrelevant to this specific diff — an expensive, unfocused way to get a small amount of needed information.
Ignoring cross-file context means the bot can never catch a whole class of real bugs — call-site mismatches, broken invariants across a call boundary — which are exactly the kind of subtle issue a good human reviewer would catch by checking the callee.
Maintain a Repo Embedding Index — an indexed, retrievable representation of the codebase's symbols, definitions, and conventions. When the diff references something outside itself (a called function, an imported type, a style guideline), the Context Assembler retrieves just that, giving the LLM the specific cross-file context it needs without shipping the entire repository.
Why this piece earns its place
Index at definition granularity — a function or type with its signature, its docstring and its path as one unit — rather than fixed windows of characters. A retrieved fragment of a function body is worse than retrieving nothing, because it looks like context and the model reasons from it with the same confidence. Then handle the miss, which is the failure nothing downstream will show you. Retrieval always returns its nearest candidates and never says it did not find the symbol you asked for, so a call site gets judged against a plausible wrong signature and the finding reads exactly as sure as one that is right. Return unresolved references explicitly, name them in the prompt, and let Step 4's gate treat a review built on a miss as a reason to hold the finding back. The other thing to say out loud is that this box is the one piece of standing state the design owns. Indexing a codebase is not a webhook-latency operation, so a repo's first PR arrives before its index does, and the index costs money on days nobody opens a PR at all. Decide up front that a missing or half-built index degrades to diff-only review rather than making a developer wait on it.
What the new pieces do
- Repo Embedding Indexdata
- A retrievable index of the whole codebase, so the LLM can pull in a called function's definition or a project convention it wasn't directly shown.
Step 4 · The semantic layer
What only an LLM can actually judge
With static checks done and context assembled, the LLM reviews for logic correctness, missed edge cases, naming, and whether the change actually does what the PR description claims. Should every single thing it flags get posted as a comment?
The LLM flags a dozen potential issues, from "this could theoretically be null" to a clear logic bug. Post all of them?
A dozen comments per PR, many of them low-confidence nitpicks, is exactly how developers learn to ignore (or mute) a review bot entirely — volume without precision destroys the trust the bot depends on to be useful at all.
Not every LLM-generated observation is worth interrupting a developer for. Attaching a confidence/severity score to each finding and gating what actually gets posted keeps the bot's signal-to-noise ratio high, which is the difference between a tool developers trust and one they mute.
Artificially capping at one comment would suppress genuine, distinct issues just as readily as it suppresses noise — the goal is filtering by QUALITY of the finding, not an arbitrary count.
Have the LLM attach a confidence and severity score to each finding, and gate posting on that score — clear logic bugs and security issues surface immediately; speculative or stylistic nitpicks are suppressed or batched into a single low-priority summary rather than posted as individual interruptions.
Why this piece earns its place
A model's self-reported confidence is not a probability. Ask for one and the numbers cluster in a narrow band and rank poorly across different prompts, so treat it as an ordering, set the cut against a labelled sample per category, and keep the threshold in per-repo config rather than in the prompt — a number you can move without re-testing how the model behaves. Cheaper corroboration beats asking the model twice. Require every finding to resolve, in two separate checks: the code it quotes has to appear somewhere in what was actually sent — the hunk, the window around it, or a retrieved definition — and the line it comments on has to be one this PR changed. Conflating those two throws away exactly the cross-file catches Step 3 exists to produce, because a finding about a callee's signature is evidence and the comment still lands on the call site. A finding quoting code nobody sent resolves to neither, and dropping it before scoring is cheaper than any threshold. Then log the suppressed findings anyway, with their scores and their fate. Without that shadow record the bar can only ever ratchet upward, because you have no evidence about what you are not posting — and the day someone asks whether the bot would have caught a bug that shipped, the answer is in that log or nowhere.
- confidence scoreper finding
- high-severitysurfaces immediately
- low-confidencesuppressed or batched
What the new pieces do
- Review LLMservice
- Reasons about logic, naming, and missed edge cases — the layer static analyzers structurally can't do, since it requires understanding intent.
Step 5 · Never repeat yourself
A re-push shouldn't repost old comments
A developer pushes a fix, then pushes again to address review feedback, then again for a typo. Each push re-triggers review. Should each run repost everything it finds, including issues already flagged (and possibly already fixed) on an earlier push?
A PR gets pushed 5 times. Should each push's review repost all previously-flagged issues again?
Reposting identical comments on every push is exactly the comment-spam pattern that erodes trust — "nothing missed" doesn't require repeating what was already clearly said once.
Stopping review after the first push means genuinely NEW issues introduced by later pushes (a fix that itself introduces a bug) go completely uncaught — the bot needs to keep reviewing, just not keep repeating itself.
A ledger of "what has already been said, about what specific piece of code" lets each new review run diff against its own prior findings: already-flagged-and-unresolved issues stay silent (they were already said once), resolved issues can even be acknowledged, and only genuinely new findings from the latest push get posted.
Maintain a Comment Ledger keyed by (finding, diff-hunk) that records exactly what's already been posted. Every new review run checks against it: a previously-flagged, still-unresolved issue isn't reposted; a resolved one can be quietly closed; only genuinely new findings from the latest push generate a new comment.
Why this piece earns its place
Everything here rests on the key, and the obvious keys are all wrong. Comment text changes between runs because the model rewords itself. Line numbers move when anything above them is edited. Commit SHAs disappear on a force-push, and a rebase — routine on a long-lived PR — rewrites all of them at once, so a ledger keyed on SHA forgets everything it knew and reposts the lot in one go. What survives is a fingerprint: category, file path, and a hash of the normalized flagged code with whitespace and formatting stripped, so the same issue on code that merely moved is still the same issue. Then let the write make the decision. Two pushes seconds apart start two review runs that both read an empty ledger and both post, which surfaces as an occasional duplicate nobody can reproduce on demand. A unique constraint on the key, claimed before the comment is sent, makes that race harmless. It buys cancellation too: a run that finds a newer push already recorded can stop rather than finish work no one will read.
What the new pieces do
- Comment Ledgerdata
- Tracks exactly which finding, on which diff-hunk, has already been posted — so a re-push only surfaces genuinely new issues.
Step 6 · Handle secrets carefully
Don't leak what you're flagging
A diff accidentally includes a real API key. The bot should flag it — but sending that raw secret up to an external LLM provider as part of "look what I found," and then quoting it back verbatim in a public PR comment, would make the leak WORSE, not better.
Detect likely secrets with a pattern-based scanner (part of the deterministic Step 1 layer) before anything reaches the LLM, and redact the actual value in both the LLM request and the posted comment — flag "a credential-shaped string was found on line 42" without ever repeating the credential itself anywhere in the pipeline, including in logs.
Why this piece earns its place
Redaction belongs where the scanner already is. The Static Analyzers are the first thing to touch a diff — before any context is assembled, before a byte of it leaves the building — so that is where the matched value gets swapped for a stable placeholder, once, in the text every later stage reads. Scrub the outbound comment instead and the value has already been through the prompt, the retry, the request trace and the crash dump, and now you are hunting it in four places at once. Two things follow that people learn the hard way. A detected secret is already leaked: it is in the git history, and deleting the line in the next commit does not unpublish it. The comment's real job is therefore rotate this credential, not remove this line, and saying the second one leaves the developer believing they have fixed something they have not. And detectors fire on test fixtures, base64 blobs and deliberately fake example keys, so each repo needs an allowlist — which costs more here than ordinary noise, because every redacted region is code the model then reviews blind. Over-redact a file and the bot is reasoning about a hole in it, with no drop in how sure it sounds.
Step 7 · Learn from developer reactions
Fewer repeat false positives over time
Developers thumbs-down a category of comment repeatedly — say, a particular style suggestion this team has explicitly decided against. Should the bot keep making the same suggestion forever?
Feed 👍/👎 reactions and explicit dismissals back into per-repo confidence thresholds — a category of finding that's consistently downvoted on a given repo gets its posting bar raised (or is suppressed outright) for that repo specifically, without affecting other repos where the same finding might be genuinely valued. This is a lightweight feedback loop, not a full retraining pipeline: threshold and prompt adjustments, scoped per repository.
Why this piece earns its place
The signal is sparse and biased, and treating it as clean data is how these loops go wrong. Silence is the normal outcome and it means nothing — the comment may have been right and quietly acted on. The stronger signal is free and already in the repository: did the flagged lines change before the PR merged? It is noisier than it sounds — lines move for reasons that have nothing to do with the comment — but it arrives for every comment rather than for the handful that earn a reaction, so pool it across a whole category and never read it as a verdict on one finding. Second, this loop can starve itself. Raise the bar on a category and you stop posting it, so you stop collecting evidence about it, and it can never earn its way back. Keep Step 4's suppressed-findings log as the offline record, or keep posting a thin sample below the bar, so there is something to re-evaluate against. Third, bound what feedback is allowed to move. Per-repo thresholds exist so one team's taste stays local — but a high-severity security category must not be tunable down to silence by a few irritated clicks on a Friday. Feedback adjusts the nitpick bar; it does not get a vote on a credential.
What the new pieces do
- Dev Feedbackevents
- Reactions on posted comments feed back into per-repo confidence thresholds, quietly reducing repeat false positives over time.
Step 8 · The sharp edges
The 5,000-line PR
Most PRs are small. Occasionally one isn't — a large refactor or a vendored dependency update touches thousands of lines. Reviewing all of it with full LLM depth would be slow and expensive, and most of a huge mechanical diff is genuinely low-risk.
Apply a budget and prioritization strategy for large PRs: chunk the diff, prioritize files by risk signal (source code over generated/vendored files, files touching auth/payments/security-sensitive paths over config), and apply full LLM-depth review to the highest-priority chunks first within a fixed budget — falling back to static-analysis-only coverage for the long tail rather than either reviewing everything at full depth (too slow/expensive) or skipping huge PRs entirely (too risky).
Why this piece earns its place
Enforce the budget where the work is spawned, not as a wall-clock timeout on the whole job. A review killed at the deadline has usually already posted part of itself, and a half-posted review is worse than a late one: the ledger now records findings from a pass that never finished, so the rest is never said and nothing ever retries it. Decide the chunk list up front, post at the end, and let the ledger act as the resume point when a run dies. Then say what you did not review. A bot that silently covers a fraction of a large refactor reads, to the author, as full coverage and a clean bill of health — one line naming which paths got full-depth review and which got static-only is the difference between a tool you can trust and one that lies by omission. Last, isolate it: a single huge PR at full depth will occupy a repo's workers and delay the small PRs queued behind it. A per-repo concurrency cap keeps that from being the thing people actually notice, because the median stays fine while everyone else waits.
You did it
You just designed an AI code review bot.
Everything you assembled, in order
- Cheap, deterministic static analyzers run first — the LLM is reserved for what genuinely requires understanding intent.
- Review is scoped to the diff plus just enough surrounding context, not the whole file or the whole repo.
- A Repo Embedding Index (RAG over the codebase) supplies cross-file context — a called function's signature, a project convention — the diff alone can't show.
- Every finding carries a confidence/severity score; only what clears the bar gets posted, keeping signal-to-noise high enough that developers don't mute the bot.
- A Comment Ledger tracks what's already been said per diff-hunk, so repeated pushes to a PR surface only genuinely new findings.
- Likely secrets are detected and redacted before anything reaches the LLM or a posted comment — the review itself must never become a new leak.
- Developer 👍/👎 feedback tunes per-repo confidence thresholds, so repeat false positives quietly decrease over a team's first few weeks.
- Large PRs get budgeted, risk-prioritized review rather than either unbounded cost or being skipped entirely.
