codereview-agent
A GitHub App that reviews pull requests with agents that investigate the code instead of skimming the diff.
- Status
- In progress: week 3 of 4. The review engine works end to end against real PRs.
- Role
- Sole engineer
- When
- Aug 2026 – now
- Stack
- Python 3.12 · FastAPI · LangGraph · Anthropic SDK (tool use, Claude Haiku 4.5) · GitHub App (JWT + installation tokens) · Pydantic · httpx · unidiff · ruff / oxlint / dotnet format · pytest + respx · Docker
- Size
- ~4,200 lines of source · 133 tests
How it got here
| Stage | What was built |
|---|---|
| Week 1 | FastAPI service structured as routers, services and schemas, with dependency injection. GitHub App install flow: the App's private key signs a JWT, which is exchanged for short-lived installation tokens cached server-side. The browser only holds a signed, opaque installation ID. |
| Week 2 | A hand-written tool-use loop with five tools: get_file_content, search_codebase, check_dependency_versions (Python, npm and NuGet manifests), run_linter (ruff, oxlint, dotnet format) and submit_review, which validates its output and feeds errors back to the model. Each run reports its token cost, including runs that fail. |
| Week 3 · 1 | A port of the loop to LangGraph that matches its behavior, selectable per request with ?engine= and kept equivalent by parity tests. The final iteration forces submit_review, so a run that hits its budget still returns a review. |
| Week 3 · 2 | The multi-agent workflow below: triage, parallel specialists, synthesis. |
| Next | An eval with a seeded-bug PR to measure recall. Then durable, asynchronous reviews: 202 Accepted, an SQS worker, LangGraph checkpoints in Postgres so a crashed review resumes instead of being paid for twice, an MCP server, and deployment to the VPS. |
The workflow
Each specialist runs as its own compiled subgraph with its own conversation. Only its findings and token counts come back to the parent. Every specialist sees the full file inventory but only its own files' diffs.
Triage: cut the input before paying for it
The first real run re-sent a 397 KB diff on every iteration, about 114k input tokens per call. Most of it was Markdown. The model then reviewed the docs and reported their contents as defects. Triage parses the diff with unidiff and skips files in this order: deleted, binary, lockfile, vendored, generated, documentation. It caps each remaining file's diff at 400 lines, with a pointer to get_file_content for the rest.
| PR | Files | Reviewable | Diff in | Diff out |
|---|---|---|---|---|
| #1 · large | 26 | 13 | 397 KB | 54 KB (−86%) |
| #2 · README | 1 | 0 | 339 B | 0 |
PR #2 now returns in 2.2 s with zero requests to the model API. The old loop took 5 model calls and $0.02 to reach the same answer. One judgement call: .txt is not treated as documentation, because skipping requirements.txt costs review coverage, while reviewing a notes file costs only a few cents.
Single loop vs workflow, same PR
| PR #1 · Claude Haiku 4.5 | Loop | Workflow |
|---|---|---|
| Findings | 3 | 10 |
| Findings about code (not docs) | 1 of 3 | 10 of 10 |
| Model calls | 3 | 15 |
| Input / output tokens | 342k / 1.8k | 399k / 3.5k |
| Cost | $0.351 | $0.417 |
| Wall-clock time | 27.7 s | 26.9 s |
The workflow cost more, not less, and my estimate ($0.15–0.40) was low. A fair comparison is $0.35 for a shallow review of the docs against $0.42 for a review of the code with about five times as many model calls, in the same time. The three specialists started within 3 ms of each other. Most of the input tokens were re-sent conversation history, which is what prompt caching discounts, so that's the next cost experiment.
Finding quality, assessed honestly: 4–5 of the 10 were useful, 3 were theoretical, and 2 were false positives caused by my own linter tool. It runs ruff with --isolated, so ruff flagged import order it couldn't resolve. The fix is to lint with the reviewed repo's own config. A seeded-bug PR comes next, so "useful" becomes a measured recall number rather than my judgement.
Decisions and tradeoffs
Split by concern, not by file
About four specialist runs per review, whatever the PR size, and reasoning across files stays inside each concern. If a very large PR ever needs file-group chunking, that's a change to the router function alone.
Isolate failures inside the node, after testing the framework's own option
I tested LangGraph's error_handler in throwaway scripts. It fires for nodes reached by a normal edge, but not for tasks started with Send, which is how specialists fan out (v1.2.12). So each specialist wraps its run in asyncio.timeout plus a narrow try/except that records timed_out or failed and lets the rest of the review complete.
Rejected: a catch-all except Exception, which would turn real bugs into quietly incomplete reviews.
Retries in exactly one layer
Anthropic calls retry inside the SDK, and GitHub calls through my own call_with_retry. There's no LangGraph retry_policy on top, because a node-level retry would re-run, and re-pay for, an entire specialist. Stacking retry layers multiplies the attempts.
Reject tool arguments the model invented
In a real run, the model called get_file_content with a lines argument that doesn't exist. The executor ignored it and returned the whole file without telling the model. Tool calls are now checked against the schema the model was shown, and against that agent's allowed tools. Unknown arguments come back as an error the model can correct.
Build the loop by hand first, then port it to LangGraph without changing behavior
Porting the loop exactly, with parity tests, made the LangGraph comparison like for like. Only after that did the graph get restructured into something a single loop can't do well: fan-out with isolated context per agent and checkpointing per subgraph. I confirmed checkpointing writes 22 checkpoints across five namespaces in one run, so a crash can resume inside a half-finished specialist.
Bugs that mocks couldn't catch
- Pricing lookup by exact model name. Real responses echo a model ID with a date suffix (
claude-haiku-4-5-20251001), which no mock reproduced. Without the fix, cost tracking would have silently failed in production. dotnet format analyzersdidn't catch what plaindotnet formatcatches. I only found this by running the command.- A test fixture serving JSON as "the diff".
respxmatches the first route registered, and the metadata route (same URL, differentAcceptheader) was registered first. Every week 2 test had been sending JSON to the fake model, which was invisible until triage actually parsed the diff.