System design — autonomous code-review agent

Cursor / Cognition (Devin) / Sourcegraph (Amp) / GitHub Copilot / Greptile-shaped question. "Design an agent that reviews PRs on its own — comments on bugs, flags style issues, optionally opens a fix PR." Tests whether you can compose tools + planning + memory + evals into a system that runs without a human at the wheel. Harder than the copilot — the failure modes are nastier.

First, the vocabulary — every moving part, defined once

Before the architecture, lock down the words. The interviewer will use these casually and expect you to volley back. None of them are hard once you stop treating them as jargon.

• Agent vs. pipeline. A pipeline is a fixed sequence of steps you wrote by hand: do A, then B, then C, always in that order. An agent is an LLM that decides for itself what to do next, in a loop, choosing from a menu of tools — it might read three files, then grep, then read one more, depending on what it finds. Pipelines are predictable and cheap; agents are flexible but can wander. The senior move is to use a pipeline for the simple 80% and reserve the agent for the gnarly 20% where you genuinely can't predict the steps in advance.
• Tool calling. The mechanism that turns an LLM from a text-generator into something that can act. You hand the model a list of functions it's allowed to call (each with a name, a description, and typed arguments — e.g. read_file(path, range)). The model replies not with prose but with a structured request: "call read_file with path='app/auth.py'." Your code runs the real function, returns the result as text, and the model continues. The LLM never touches your systems directly — it only asks, and your code decides whether and how to honor each request. That gap is your safety boundary.
• The trigger (webhook on PR). A webhook is GitHub calling you: when a PR is opened or pushed, GitHub sends an HTTP POST to a URL you registered, carrying the event details. You don't poll ("any new PRs yet? ...now? ...now?"); you get pinged. You verify the payload signature (so a stranger can't fake the ping), then immediately drop the job on a queue and return 200 OK fast — webhook senders time out and retry if you're slow, which would double-trigger your review.
• Context gathering (diff + repo). The diff is the patch — exactly which lines were added (+) and removed (-). It's necessary but never sufficient: a diff shows you what changed, not what depends on it. So you also pull repo context — the surrounding function, the file, and crucially the callers and callees of anything the diff touched. "Context gathering" is the act of assembling that bundle before reasoning, because an LLM reviewing only the diff is reviewing in a vacuum.
• Chunking large diffs. LLMs have a context window — a hard ceiling on how much text fits in one call. A 4,000-line diff blows past it (and even when it fits, quality drops as the model drowns in tokens). Chunking means slicing the work into reviewable pieces — ideally one logical unit per chunk (a file, or a single function and its immediate context) rather than blind 500-line slabs — reviewing each separately, then merging findings. Smart chunking keeps related code together so the model never judges half a function.
• The review prompt. The instructions you give the reviewing model: its role ("you are a careful senior reviewer"), what to look for (bugs, security, perf — scoped to the PR's intent), the output format (a list of {file, line, severity, message} objects, not freeform prose), and the rules of restraint ("only flag issues you are confident about; if unsure, stay silent"). A vague prompt produces a chatty, nitpicky reviewer; a tight prompt with explicit severity and a confidence bar produces a quiet, trusted one.
• Posting comments via API. GitHub's REST/GraphQL API lets you attach a comment to a specific line of a specific file in the PR's review (the "inline comment" endpoint). You map each finding to its exact line and post it. Two subtleties bite here: the line numbers must be relative to the diff GitHub knows about (not the raw file), and you should batch findings into one review rather than firing N separate comment calls (cleaner UI, fewer notifications, fewer rate-limit hits).
• Guardrails / false-positive control. Everything that keeps the agent from being wrong in public: the critic pass, a confidence threshold below which findings are dropped, allow/deny lists (never comment on generated or vendored files), and severity gating (only surface high-severity items at first). A "false positive" is a comment about a bug that isn't real; controlling them is the single highest-leverage thing you do, because trust is asymmetric — one dumb comment erases the credit from ten good ones.
• Idempotency (don't double-comment). Idempotent means running the same operation twice has the same effect as running it once. Webhooks fire multiple times for one PR (every push, plus retries), so a naive agent re-reviews and re-posts the same comment on every push — spam. You make posting idempotent by keying each finding (e.g. a hash of file + line + message) and checking "did I already post this?" before posting, and by only reviewing the incremental diff since your last review rather than the whole PR again.
• Cost / latency budget. Each tool call and model call costs tokens (money) and wall-clock time. A budget is the hard ceiling per PR — e.g. max 30 tool calls, max 10 minutes, max $1 — enforced in code, not hoped for in the prompt. Without it, one pathological diff sends the agent into an expensive loop. Reviews aren't realtime, so you trade latency for cost freely; what you cannot do is let either run unbounded.
• Eval of review quality. An eval set is your test suite for the agent: a curated collection of past PRs where you already know the ground truth — which were real bugs, which comments humans accepted. You re-run the agent over this set after any change and measure precision (of the comments it made, how many were real) and recall (of the real bugs present, how many it caught). This is the only way to know whether a prompt tweak helped or hurt instead of guessing.
• Human override. The escape hatch and the feedback signal in one. Humans can dismiss any comment, resolve a thread, or thumbs-down a finding — and the agent must (a) never re-post a dismissed comment and (b) feed those reactions back into the eval set and tuning. The human is always the final authority; the agent's job is to be useful enough that overriding it is rare.

Why these five and not "what's the QPS?" — this is a quality-and-trust product, not a throughput product. The volume is low (a team merges tens to low-hundreds of PRs a day, not millions of requests a second), so the interesting constraints are about correctness and permission, not scale. Pin down read-only-vs-write first because it changes the entire risk profile: a commenting agent that's wrong wastes someone's attention; a merging agent that's wrong ships a bug to production. Pin down scope because "find security bugs" and "enforce import ordering" want completely different tools and prompts. Pin down the success metric early so that every later decision can be justified against it.

Walking the flow: trigger → fetch → chunk → analyze → post → learn

The reference box lists the parts; this section traces a single PR through them so you can narrate the whole journey in the interview without losing the thread. Picture a developer pushing a 600-line PR titled "fix: handle empty cart in checkout."

1. Trigger — the ping that starts it all

The push fires a webhook to your endpoint. Your handler does almost nothing on purpose: verify the signature, extract the PR id and head SHA (the commit fingerprint), enqueue a job, and return 200 in milliseconds. All the slow work happens off the queue, by a worker. This split is what makes the system robust — if a worker crashes mid-review, the job is still on the queue to retry, and the webhook sender already got its fast acknowledgment.

# webhook handler — fast, dumb, idempotent
def on_pr_event(req):
    verify_signature(req)              # reject forged pings
    job = {"pr": req.pr_id, "sha": req.head_sha}
    queue.enqueue(job, dedup_key=req.head_sha)  # same SHA twice -> one job
    return 200                          # return now; worker does the review later

Note the dedup_key: GitHub may deliver the same event twice, and a fast typist pushes three times in a minute. Keying the job by head SHA means redundant pings collapse into one review — the first brush with idempotency, before you've even posted anything.

2. Fetch diff + context — assemble the evidence

The worker pulls the diff via the API, then enriches it. For each changed symbol it consults the symbol index (built ahead of time, refreshed incrementally on every push) to find callers and callees. "Symbol index" means: instead of treating the repo as a pile of files, you've parsed it with a tool like Treesitter into functions and classes, so "who calls checkout()?" is a lookup, not a brute-force text search. This is the difference between a reviewer who reads only the changed page and one who flips to every page that references it.

Why index by symbol and not just grep on demand? Because grep finds the string checkout everywhere — in comments, in unrelated variables — while a symbol index knows the definition and its true call sites. And why incremental (content-hash each file, re-index only what changed)? Because re-parsing a million-line monorepo on every push is wasteful; you skip the 99.9% that didn't change.

3. Chunk — make it fit and keep it coherent

A 600-line diff across eight files won't review well as one blob. You split along natural seams: group the hunks by file, and within a large file by function, so each chunk is a self-contained unit the model can judge fully. Each chunk carries its gathered context (the changed code plus its callers) so the model never reasons about a function in isolation. The cardinal sin is splitting mid-function — half a function looks buggy in a hundred harmless ways.

4. Analyze — plan, then the ReAct loop

First a cheap LLM call classifies intent ("this is a bugfix for empty-cart checkout") so the reviewer knows what "done right" looks like. Then the planner writes a short, concrete to-do list — "verify the empty-cart branch, check the two callers of checkout(), confirm a test covers the empty case." Then the agent runs the ReAct loop: it reasons, calls a tool, reads the result, reasons again — under the budget. Crucially, where a question has a deterministic answer, the agent calls the real tool: it runs the actual linter and the actual test suite rather than imagining their output.

# ReAct loop, bounded — pseudocode
budget = Budget(tool_calls=30, seconds=600, dollars=1.0)
while not agent.done and budget.ok():
    thought, action = model.step(plan, history)   # reason -> pick a tool
    result = tools.run(action)                  # real linter, real tests, real grep
    history.append(thought, action, result)
    budget.charge(result.cost)
findings = agent.collect_findings()             # {file, line, severity, msg}

The budget is enforced here, in code — not requested in the prompt, because a model under a confusing diff will happily ignore a polite "please be brief." When the budget runs out, the agent stops and posts what it has (with a note), rather than failing silently.

5. Critic pass + post — the senior check, then publish

Before anything is posted, a second model re-reads each proposed finding against the real diff and asks one question: "is this a genuine issue here, or is it hallucinated / out of scope / nitpicky?" It drops the failures. This single step removes 30–50% of false positives — far cheaper and more reliable than trying to make the first reviewer perfect. Survivors are filtered again by severity and by allow/deny lists (skip generated files), then posted via the API as one batched review, each comment pinned to its diff line.

Posting is made idempotent: each finding is hashed (file + line + normalized message); before posting, you check the PR's existing bot comments and skip any hash already present. On the next push you review only the incremental diff and only post net-new findings — so the developer who pushes five times doesn't get the same comment five times.

6. Learn / eval — close the loop

When humans react — accept a suggestion, dismiss a comment, resolve a thread — those signals flow back. Dismissed comments train the deny side; accepted ones validate the reviewer. Periodically you fold notable PRs into the eval set, the curated 200+ historical PRs with known answers, and re-run it on every prompt or model change to measure precision and recall. This is what turns the agent from a static script into something that gets measurably better — and what stops a "harmless" prompt edit from silently doubling your false-positive rate.

The tradeoffs you must say out loud

A design answer is only senior if you name what you're giving up. Two tensions sit at the heart of this system.

The pitfalls that sink real deployments

Each of these has killed a code-review agent in the wild. Name the pitfall and its mitigation — that pairing is the signal.

• Noisy comments. Too many low-value nits (style, naming, "consider extracting this") bury the one comment that mattered, and devs stop reading. Mitigate: severity gating, a confidence threshold, and the critic pass; surface only high-severity findings until trust is earned.
• Hallucinated issues. The model confidently flags a bug that doesn't exist — often by misreading context it never fetched. Mitigate: the critic pass re-checking each finding against the real diff, plus requiring the model to cite the exact lines its claim rests on (an uncitable claim gets dropped).
• Re-reviewing on every push. Without idempotency the agent re-runs the full review and re-posts every comment each time the dev pushes — instant spam, instant distrust. Mitigate: review only the incremental diff since the last reviewed SHA, hash each finding, and skip anything already posted or already dismissed.
• Runaway loops / cost blowout. A huge or adversarial diff sends the agent into 200 tool calls. Mitigate: hard budgets (tool-call, time, dollar) enforced in code; stop and post-partial when exceeded.
• Comment spam on generated files. Reviewing package-lock.json or generated protobufs wastes the budget and annoys everyone. Mitigate: a deny-list of paths/globs skipped before analysis.
• Wrong context retrieved. The agent fetches a similarly-named-but-unrelated function and reasons about the wrong code. Mitigate: symbol-level (not text-level) retrieval, and feeding the agent the resolved definition rather than a fuzzy search hit.