Technical design
Code review matched to change risk
Two comments about naming. None about the default that flipped to allow.
Company Quillstack is a hypothetical company, written for this reference design; the details below are its constraints. Every figure is illustrative, computed from stated assumptions. No client work or measured result is claimed. The story explains the problem and follows one item through the system; this page is the engineering detail behind it.
Scope and assumptions
The system runs on pull-request events. It classifies changes, gathers context, runs model reviews, and posts findings. It never approves, merges or blocks merges itself; branch protection and CODEOWNERS do that.
Illustrative figures assume 60 pull requests a day: 22 route to no model review (docs, generated code, bumps handled by the dependency check), 32 to standard review, 6 to deep review. The old bot averaged 4 comments per pull request (60 × 4 = 240); I assume routed reviews average 1.5 comments on standard and 3.5 on deep (32 × 1.5 + 6 × 3.5 = 69).
Architecture
pull request event
│
▼
diff analyzer ── path rules · CODEOWNERS · symbol index · lockfile parser
│
▼
router ── policy floors (authz, migrations, secrets, restricted paths)
│
┌───┼──────────────┬──────────────────┬───────────────────┐
▼ ▼ ▼ ▼ ▼
none dependency standard review deep review human-only
check (diff + callers) (callers, tests, (no external
policy, checklist) model)
│ │
└────────┬─────────┘
▼
findings bound to head commit
│
freshness check before posting
▼
review comments / status
Routing rules
| Route | Triggered by | Can the model change it? |
|---|---|---|
| None | Only *.md, docs folder, generated clients |
Can raise to standard |
| Dependency check | Manifest or lockfile changes only | Can raise to standard |
| Standard | Application code outside floor paths | Can raise to deep |
| Deep (floor) | auth/, permissions, policy files, migrations, crypto usage, CI config |
No lowering |
| Human-only (floor) | Restricted directories (signing keys, billing internals) | No lowering, no model at all |
Rules live in a versioned YAML file owned by the platform team. Every routing decision records the rule version and which signals fired.
Context assembly
Standard review sends the changed hunks plus the full changed functions. Deep review adds: callers of changed symbols (from a symbol index built with tree-sitter or the language server), the tests touching those symbols, the relevant policy document, and a route-specific checklist. For authorization: "enumerate inputs where the old behavior denied; confirm the new behavior denies them". For migrations: "is it reversible; does it lock a large table; is the application compatible with both schemas during deploy".
Context is capped by token budget per route. When the cap is hit, the review says what it left out.
Finding format and freshness
Each finding: repository, base commit, analyzed head commit, path, line range, symbol, severity, the evidence (quoted lines), and what context was missing. Before posting, the poster compares the analyzed head with the pull request's current head. If they differ, the finding is withheld and a rerun is queued. Resolved threads are workflow state; they are never used as a label of correctness.
Failure handling
| Failure | Response |
|---|---|
| Head moved before posting | Withhold, rerun on new head |
| Model timeout or error | Post "model review unavailable" status; human reviews unaffected |
| Diff too large for route budget | Review what fits, list skipped files |
| Restricted path detected mid-diff | Whole pull request goes human-only |
| Routing rules file invalid | Fall back to deep for all, alert platform team |
The last row matters: when the router is unsure, it fails toward more review, never less.
Evaluation
Use 200 historical merged pull requests with outcomes attached (reverts, incidents, security findings). Measure:
- high-impact under-routing: high-impact changes not routed deep (target zero; each one is investigated);
- over-routing: low-impact changes routed deep (cost);
- finding usefulness on deep reviews, labelled by the owning engineers as useful, wrong or noise;
- stale findings prevented, counted from the freshness check;
- latency and cost per route.
Rerun the set when the routing rules, prompts or model change.
Trade-offs I considered
- One strong model on everything. Simplest to build and the thing that failed. Attention is the scarce resource, for people and for budgets.
- Model-based routing only. Flexible, but a model deciding that authorization code is "probably fine" is exactly the failure to prevent. Floors belong in code.
- Static analysis rules for authorization. Valuable, and I would add them. They catch known patterns; the deep review with a checklist catches the changed default that no rule anticipated.
Stack
A GitHub App (or the equivalent for the team's git host) receiving pull-request events, a Python service for analysis and routing, tree-sitter for symbol extraction, a hosted model with a zero-retention agreement for standard and deep reviews, and the host's review-comment API. Findings and routing decisions are stored in Postgres for evaluation.
Security and operations
Source code is sensitive. Only non-restricted paths are sent to the model provider, and only the context the route needs. Secrets scanning runs before any context is assembled; a detected secret blocks model review for that pull request. Weekly, the platform team reviews the under-routing list and the rule file's change history.