Review PRs with a three-auditor panel and an arbitrator

[bockets]

Replaces the single synthesizing review pass with a panel: caspar, balthazar, and melchior each audit the change independently, running the same auditor prompt, and each produces its own verdict. An arbitrator then weighs all three and issues the one verdict that gets posted.

flowchart LR
  ctx["1 · context<br/>diff · PR · stack · ticket + AC · epic<br/>discussion · designs · manifest<br/>(no model)"]
  subgraph panel["2 · panel — one prompt, three independent jobs"]
    direction TB
    caspar["caspar<br/>openai/gpt-5.6-luna"]
    balthazar["balthazar<br/>google/gemini-3.7-flash"]
    melchior["melchior<br/>z-ai/glm-5.3-flash"]
  end
  arb["3 · arbitrate<br/>openai/gpt-5.6-sol"]
  posted(["the one verdict the PR sees"])
  ctx --> caspar
  ctx --> balthazar
  ctx --> melchior
  caspar --> arb
  balthazar --> arb
  melchior --> arb
  arb --> posted

Loading

Nothing reviews the diff ahead of the panel

Stage 1 runs no model. It gathers what the panel judges against and stops. The auditors are the first thing to form an opinion about the code.

A pre-pass that read the diff and handed the panel a findings list would get three seats mostly agreeing about one model's opinion, and the arbitrator would be weighing verdicts that shared an origin. CI asserts no prompt reads a pre-existing findings file and that the registry declares no prompt but the auditor's and the arbitrator's, so a pre-pass can't come back by accident.

What the panel is given

Stage 1 hands the panel one immutable artifact containing everything it judges against:

File	What it holds
context-manifest.json	What was gathered, and which sources were unavailable and why
pr.diff	The diff of the reviewed commit against its base
pr.json	Title, body, labels, commit messages, touched files with line counts
stack.json	The PRs this one is stacked on, nearest first, when its base isn't the default branch
ticket.json	The BIG-XXXXX ticket: summary, description, acceptance criteria, status, type, parent key
epic.json	That parent — where the wider goal lives when a ticket's own description reads as a fragment
conversations.json	Comments, inline review threads, prior reviews
designs.json	Design references found in all of the above: Figma and other design-tool links, screenshots, walkthrough videos, ticket attachments

Every source degrades to {"available": false, "reason": …} rather than failing the review — a PR with no ticket, no epic and no designs is still reviewable, just with less to judge intent against.

The manifest is the point of the stage. Without it an auditor can't tell "there is no epic for this work" from "the epic fetch failed", and both read as silence — which is how a review ends up judging intent against nothing and not saying so. The prompts now require an auditor to name the context it didn't have and what it couldn't judge without it.

Stacked PRs get walked. When a PR's base isn't the default branch, stage 1 follows base branch → the PR whose head is that branch, until the default branch, and records each ancestor's number, title, body, state, ticket key, commit headlines and touched paths.

This one was silently wrong before. pr.diff is measured against the base, so it holds only this PR's own changes while the branch beneath it carries work that is itself under review — and an auditor not told that misreads a stacked PR in both directions: blaming this PR for what an ancestor introduced, and reporting as missing the work an ancestor already did. The prompts now rule out both and point the panel at the questions that are actually this PR's: does it duplicate or contradict an ancestor, does it use an interface the ancestor doesn't provide, does it only make sense if an ancestor merges first without saying so. Ancestors usually carry a sibling ticket in the same epic, and satisfying that ticket isn't this PR's job.

The walk is capped at ten levels and won't revisit a branch, so a cycle left by a retargeted branch can't hang a review. A base that is neither the default branch nor any PR's head — a release branch — is recorded as not a stack, with the reason.

Design links are recorded, never followed. The panel has no Figma credentials, and a code review has no reason to pull design or member-facing content into a build artifact. What a recorded link buys an auditor is knowing that a design exists for this work — the difference between "no design was specified" and "a design was specified and I can't see it" — and the prompts forbid treating a reference it can't open as a defect.

No handoff file carries an author, assignee or reporter: the panel judges the change, not who wrote it, and leaving identities out keeps them off the artifacts and out of telemetry. The PR discussion is the one exception, where attribution is what makes an unresolved human objection legible.

Independence is structural, not requested in the prompt

Each wise man is a job of its own — caspar, balthazar, melchior — written out rather than fanned out from a matrix, so a seat is an independent unit end to end: its own runner, its own model, its own status check, re-runnable on its own. It reads only the stage-1 handoff and writes only its own audit-<seat>.json.

Everything seat-specific lives in the job's env, which leaves the three steps: blocks byte-identical. That's load-bearing: a split between seats is only evidence about the models if the harness around them is the same, and three copies of a block drift one seat at a time. CI diffs the blocks and fails if they stop matching, and fails if the set of seat jobs stops matching the roster in registry.json. The arbitration job is the first and only place in a run where more than one verdict exists together. So when two auditors disagree, they disagree because they judged the code differently — not because one saw the other's answer.

The seats run different models on byte-identical prompt text. Holding the prompt fixed is what makes a split between seats signal about the change; three runs of one model would agree trivially and leave the arbitrator nothing to weigh. CI asserts the auditor prompt names no seat, so this can't silently regress.

Each model is an OpenRouter slug overrideable by a repository variable (MAGI_MODEL_CASPAR, ARBITER_MODEL, …), so changing a seat's model never touches a prompt. There is no fallback model: a seat with no MAGI_MODEL_<SEAT> fails its own job with an explicit error rather than quietly doubling up on another seat's model, which would be a panel that lost a voice while still reporting three verdicts.

Arbitration weighs rather than tallies

A blocking finding the arbitrator can confirm in the code outweighs any number of approvals that missed it; one it can disprove is discarded however confidently raised; unanimity the evidence doesn't support is overruled with an explanation. It may confirm or discard what the panel raised but may not introduce a blocking issue no auditor found — it arbitrates instead of becoming a fourth reviewer.

The posted body is the arbitrator's summary verbatim, opening with a thesis sentence stating the verdict and closing with a Panel: line recording how the seats split and where the arbitrator overrode them.

Failure modes

A seat producing no valid verdict is recorded as failed, never counted as an approval. Arbitration requires a majority of seats to have reported and fails loudly below that, rather than quietly degrading to a one-reviewer review. Requiring only a majority keeps one rate-limited seat from failing a whole review — and is what makes a 1-1 tie reachable, which is tagged no_majority since there is no majority to follow or override. "Re-run failed jobs" re-runs only the failed seats, reusing the context stage that already succeeded.

⚠️ Breaking: job names for required status checks

The jobs are now context, caspar, balthazar, melchior, arbitrate. Two names changed: stage 1 was codex (it no longer runs Codex), and the seats were matrix legs reported as audit (caspar) and friends. Any repo with codex or audit (…) as a required status check must switch those rules, or branch protection will block merges forever on checks that never report.

A PR could approve itself

Found reviewing this branch, and worth calling out because it was the sharpest thing here. Every job runs with the reviewed PR's checkout as its working directory — the same directory the handoff downloads into, the same one the model writes verdict.json to, and the same one the audit-*.json globs read. Nothing distinguished a committed file from a model's output.

So a PR could commit verdict.json with {"verdict":"approve"}, and if the arbitrator's model exited without writing its own (max turns, a give-up, a write to another path — none of which fail the step), the submit step would post that approval as BiggiePockets. The same hole let a committed audit-anything.json inflate PANEL_SEATS_REPORTING, satisfy quorum with zero real verdicts, and inject seat tags into telemetry.

Every job that checks out the PR head now clears the reserved names before anything reads them, and CI asserts it does — this is exactly the invariant that vanishes when a job is added by copying another one and trimming it. Four smaller fixes from the same pass: arbitration's verdict.json is validated before the review is posted (a missing summary would have posted the word null as the review body); a JIRA 401 whose body is an HTML login page no longer fails the context job and take down a review that's documented to degrade instead; the Datadog run_id includes run_attempt, so a re-run no longer collides with the run it replaced; and the stack walk can't report a PR as stacked on itself or pick up a fork's identically-named branch.

Telemetry

One span per seat, plus each seat's verdict/confidence/model/finding-count as tags. The two panel-level tags worth watching:

• panel_agreement — near-total unanimity means the extra seats aren't adding independent information, and aren't earning their cost.
• arbiter_followed_panel — an arbitrator that never departs from the majority is a vote counter; one that always does is ignoring the panel.

Spans with a codex.review child or a codex_model tag came from the pre-panel first pass and aren't comparable; same for the arm/experiment_verdict tags from the older A/B setup. Exclude both when querying.

Removed: the dual-arm A/B apparatus

arms, control_arm, experiment_split_percent, and <repo>:<pr> bucket assignment are gone. The structure can't express "three seats, one prompt", and the experiment had almost no usable data — seven runs, all predating model pinning, on since-rolled prompt versions. Git history keeps it recoverable.

Also

Fixes a latent resolver bug the arbitrator prompt exposes: prompt_version ran cat over an empty-array expansion, aborting the resolve for any prompt with no {{@}} includes. magi-arbitrate.md is the first such prompt.

Verification

validate-prompts.yml was rewritten for the new structure and all 12 checks pass locally, including new ones asserting nothing reviews the diff ahead of the panel, the auditor prompt is seat-agnostic, the arbitrator names every seat, the roster is well-formed, the seat jobs match the roster and each other, and a broken registry is rejected rather than resolved. The pre-panel check was negative-tested both ways — a re-added pre-pass prompt and a prompt referencing a findings file are each caught — and the seat-job check three ways: a roster seat with no job, one seat job growing an extra step, and a job declaring a model for a seat that isn't on the roster.

Stage 1's new context gathering was run end to end against stub gh and curl responses across five shapes: a rich PR (ticket with a parent epic, attachments, Figma/Loom/screenshot links across the body, ticket and comments), a bare one (no ticket key, empty body, no comments, no links), a one-deep stack, a two-deep stack, and a PR based on a release branch that no PR heads. The stack cases confirm the ancestors come back nearest-first with their ticket keys, and that an ancestor's Figma link reaches designs.json tagged to the stack source. A sixth shape — JIRA answering 401 with an HTML login page — confirms the context job survives it (the pre-fix jq exits 5 on that body).

The self-approval fix was verified by running the clear step over a directory seeded with verdict.json, audit-fake.json, audit-fake.env, pr.diff, ticket.json and an unrelated keep-me.rb: all reserved names go, the unrelated file stays. Arbitration's new validation was driven over six verdict.json shapes — valid, missing summary, null summary, empty summary, an unknown verdict string, and malformed JSON — posting only for the valid one and failing with a diagnostic for the rest. Both new CI checks were negative-tested by removing a clear step and by dropping a seat's. Three bugs surfaced and were fixed there: a grep with no match aborting the design harvest under pipefail, an unparenthesized jq comparison, and has_acceptance_criteria reporting true for an empty ADF document.

The arbitration job's bash was driven locally against a split panel with a failed seat — quorum, tally, per-seat spans, and seat tags all behave — the majority/override logic was checked across all eight approve/changes/tie combinations, the Datadog payload was captured and inspected to confirm it carries no codex remnants, and the seat-model lookup was exercised for both a known seat and a seat with no model.

Not yet exercised end-to-end against a live PR; z-ai/glm-5.3-flash in particular is a slug worth confirming against OpenRouter before merge (it's a repository variable, so it's a settings change rather than a code change if wrong).