diff --git a/.github/workflows/biggiepockets-review.yml b/.github/workflows/biggiepockets-review.yml index 4ca96ef..2f9a19c 100644 --- a/.github/workflows/biggiepockets-review.yml +++ b/.github/workflows/biggiepockets-review.yml @@ -1,8 +1,26 @@ name: BiggiePockets Code Review (reusable) -# Reusable two-stage AI review (Codex first pass, then Claude verifies and -# synthesizes) anchored on the PR's JIRA ticket, with the BiggiePockets service -# account submitting the resulting approve / request-changes decision. +# Reusable AI review panel, anchored on the PR's JIRA ticket, with the BiggiePockets +# service account submitting the resulting approve / request-changes decision. +# +# A review runs in three stages: +# 1. Context — gathers everything the panel judges against and uploads it as one +# immutable handoff: the diff, the PR itself (body, labels, commits, +# touched files), the PRs it is stacked on, the JIRA ticket and its +# acceptance criteria, that ticket's epic, the PR discussion, and any +# design references found in those texts. Whatever isn't available is recorded as unavailable, in +# context-manifest.json, so an auditor can tell "there is no epic" from +# "the epic fetch failed". No model reviews the code here: nothing +# reads the diff ahead of the panel, so no finding is planted in front +# of the auditors whose independence is the whole point. +# 2. Panel — three auditors (caspar, balthazar, melchior) each audit the change +# INDEPENDENTLY, each in a job of its own named for the seat, all +# running the SAME auditor prompt, and each writing its own verdict. +# No auditor can see another's verdict: a seat's output is uploaded per +# seat and downloaded only by stage 3. +# 3. Arbitration — one arbitrator reads all three verdicts, verifies the blocking +# findings against the code, and issues the ONE verdict that is posted. +# It is the only stage whose output reaches the pull request. # # Callers own the triggers and gating (e.g. fire on review_requested when # BiggiePockets is the requested reviewer) and pass the PR number as `pr`. @@ -11,23 +29,30 @@ name: BiggiePockets Code Review (reusable) # BIGGIEPOCKETS_PAT, DATADOG_API_KEY (optional; enables review-quality # metrics in Datadog LLM Obs). # -# Prompt versioning: every review-stage prompt lives in this repo's prompts/ -# registry (BiggerPockets/.github) as a versioned template, plus one shared rule -# block per concern under prompts/_shared/ injected via {{@path}}. -# Prompts are resolved by scripts/resolve-prompts.sh into step outputs; a derived -# prompt_version (content hash) identifies each prompt in Datadog LLM Obs. Each LLM -# span also carries its template and variables under meta.input.prompt, which is what -# Datadog Prompt Tracking reads to group runs by prompt and version. +# Models: every model-running stage's model is an OpenRouter slug pinned below and +# overrideable per repository without editing this workflow — set the matching Actions +# *variable* in the calling repo (Settings → Actions → Variables), which the `vars` +# context below reads. An unset or empty variable keeps the pinned default. +# MAGI_MODEL_CASPAR caspar (default openai/gpt-5.6-luna) +# MAGI_MODEL_BALTHAZAR balthazar (default google/gemini-3.7-flash) +# MAGI_MODEL_MELCHIOR melchior (default z-ai/glm-5.3-flash) +# ARBITER_MODEL arbitration (default openai/gpt-5.6-sol) +# Every seat carries its own model in its own job, and no seat can inherit another's: a +# seat quietly running the same model as another seat is a panel that has lost a voice +# while still reporting three verdicts. A roster entry with no job fails CI; a job whose +# model expression resolves to nothing fails at its `Confirm seat model` step. +# The three seats run different models on purpose: the auditor prompt is identical, so +# what a disagreement between seats measures is the models, and three runs of one model +# would mostly agree trivially and leave the arbitrator nothing to weigh. # -# Prompt A/B test (BIG-21717): prompts/registry.json declares the arms, the control -# arm, and what share of pull requests an experiment arm gets. Each review runs ONE -# Claude pass, using the arm that scripts/resolve-prompts.sh assigns to that PR by -# hashing ':' — so the arms are compared BETWEEN pull requests and a review -# never costs a second Claude pass. The assigned arm writes both the posted summary -# and the approve / request-changes decision, which means an experiment prompt does -# affect real outcomes on its share of PRs. Assignment is deterministic, so a re-run -# reviews with the same arm. Set experiment_split_percent to 0 in registry.json to -# route everything to the control arm; every consuming repo reads that one file. +# Prompt versioning: both prompts live in this repo's prompts/ registry +# (BiggerPockets/.github) as versioned templates, plus one shared rule block per +# concern under prompts/_shared/ injected via {{@path}}. Prompts are resolved by +# scripts/resolve-prompts.sh into step outputs; a derived prompt_version (content hash) +# identifies each prompt in Datadog LLM Obs. Each LLM span also carries its template and +# variables under meta.input.prompt, which is what Datadog Prompt Tracking reads to group +# runs by prompt and version. The panel seats all resolve the SAME auditor prompt, so one +# prompt_name/prompt_version covers all three audit spans. on: workflow_call: inputs: @@ -48,10 +73,20 @@ on: type: string jobs: - # Codex and Claude intentionally run in separate jobs. GitHub can only re-run a - # failed job (not an individual step), so this boundary lets a rate-limited Claude - # run be retried without paying for another Codex pass. - codex: + # Stage 1. Gathers every input the panel reads — the diff, the JIRA ticket, the PR + # discussion — and uploads the lot as one immutable handoff. No model runs here: the + # panel is the first thing that forms an opinion about the code. + # + # Separate from the panel jobs because GitHub can only re-run a failed JOB (not an + # individual step): this boundary lets a rate-limited auditor be retried without + # re-fetching the diff and ticket, and guarantees all three auditors judge the exact + # same commit and context. + # + # NOTE for callers: this job was previously named `codex`. A repo that lists it as a + # required status check must switch that check to `context`. The panel's checks were + # likewise `audit (caspar)` and friends when the seats were matrix legs; they are now + # `caspar`, `balthazar` and `melchior`. + context: runs-on: ubuntu-latest outputs: head_sha: ${{ steps.pr_head.outputs.sha }} @@ -59,20 +94,8 @@ jobs: permissions: contents: read pull-requests: read - id-token: write # claude-code-action authenticates via OIDC env: PR: ${{ inputs.pr }} - # Explicit model per stage, threaded into both the actual invocation below and the - # Datadog LLM Obs tags/spans at the bottom of this job. Empty means "let the tool - # use its own default" (today's behavior, unchanged). Set these once you want a - # specific model pinned and reliably visible in telemetry — e.g. when Codex is - # pointed at a non-default model — instead of an implicit, untracked default. - # The Codex stage runs through OpenRouter, so its model must be an OpenRouter - # model slug and must be set explicitly — Codex's own default model name is not - # one OpenRouter accepts. - CODEX_MODEL: "openai/gpt-5.6-sol" - CODEX_RESPONSES_ENDPOINT: "https://openrouter.ai/api/v1/responses" - CLAUDE_MODEL: "google/gemini-3.7-flash" steps: - name: Checkout PR head uses: actions/checkout@v4 @@ -80,8 +103,21 @@ jobs: ref: refs/pull/${{ inputs.pr }}/head fetch-depth: 0 - # Pin the Claude job to the exact commit Codex reviewed. This matters when a - # failed Claude job is retried after the PR has received another push. + # Same reason as the panel jobs below: this job's working directory is the PR's own + # checkout. Every file this stage publishes is written unconditionally, but the raw + # API responses are read back only when the fetch that produces them ran — so a PR + # that committed ticket-raw.json could otherwise inject attachments into the handoff. + - name: Clear reserved handoff filenames + run: | + rm -rf verdict.json audit-*.json audit-*.env \ + pr.diff pr.json stack.json ticket.json epic.json \ + conversations.json designs.json context-manifest.json review-context.env \ + pr-raw.json ticket-raw.json epic-raw.json urls.json attachments.json \ + stack-*.json + + # Pin every later job to the exact commit this context was gathered from. This + # matters when a failed audit or arbitration job is retried after the PR has + # received another push. - name: Record PR head SHA id: pr_head run: echo "sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" @@ -94,7 +130,7 @@ jobs: # in the CALLER's repo and so is not a valid ref here. # GITHUB_TOKEN is scoped to the caller's repo, so use the BiggiePockets PAT. # persist-credentials is off so the PAT is never left in registry/.git/config where - # the later Codex/Claude steps (which act on untrusted PR diffs and can read files/run + # the later model steps (which act on untrusted PR diffs and can read files/run # Bash) could read it — read-only prompts only need the checkout itself, not the # stored credentials. - name: Checkout prompt registry @@ -113,16 +149,40 @@ jobs: - name: Record review start time run: echo "REVIEW_START_NS=$(date +%s%N)" >> "$GITHUB_ENV" - # Resolve PR context (base ref and title) from the API. - - name: Resolve PR context + # Everything from here to the upload is context gathering. Each source writes one + # artifact file and nothing more: no step reads another's output to form a view of + # the code. A source that isn't there degrades to {"available": false, "reason": ...} + # instead of failing the review — a PR with no ticket, no epic and no designs is + # still reviewable, just with less to judge intent against. + # + # No artifact carries an author, assignee or reporter. The panel judges the change, + # not who wrote it, and leaving identities out keeps them off the handoff and out of + # telemetry. The PR discussion is the one exception: attribution is what makes + # "a human reviewer objected and it was never resolved" legible. + - name: Gather PR metadata env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - base=$(gh pr view "$PR" --repo "$GITHUB_REPOSITORY" --json baseRefName -q .baseRefName) - title=$(gh pr view "$PR" --repo "$GITHUB_REPOSITORY" --json title -q .title) + gh pr view "$PR" --repo "$GITHUB_REPOSITORY" \ + --json number,title,body,baseRefName,headRefName,isDraft,labels,additions,deletions,changedFiles,commits,files \ + > pr-raw.json + + jq '{ + available: true, + number, title, body, + base_ref: .baseRefName, + head_ref: .headRefName, + draft: .isDraft, + labels: [.labels[].name], + additions, deletions, + changed_files: .changedFiles, + commits: [.commits[] | {message: .messageHeadline, body: .messageBody}], + files: [.files[] | {path, additions, deletions}] + }' pr-raw.json > pr.json + + base=$(jq -r '.base_ref' pr.json) echo "BASE_REF=$base" >> "$GITHUB_ENV" - echo "PR_TITLE=$title" >> "$GITHUB_ENV" - echo "Reviewing PR #$PR (base $base): $title" + echo "Reviewing PR #$PR (base $base): $(jq -r '.title' pr.json)" - name: Compute PR diff run: | @@ -130,15 +190,105 @@ jobs: git diff "origin/$BASE_REF...HEAD" > pr.diff echo "PR diff is $(wc -l < pr.diff) lines" + # Walk down the stack. A PR whose base is not the default branch is usually stacked + # on top of another PR, and pr.diff above is computed against that base — so the + # diff is this PR's own changes only, and the branch it sits on contains work that + # is itself under review and can still change. + # + # An auditor not told that reads a stacked PR wrongly in both directions: it blames + # this PR for what the parent introduced, and it reports as missing the work the + # parent already did. Always writes stack.json. + - name: Walk the PR stack + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + default=$(gh repo view "$GITHUB_REPOSITORY" --json defaultBranchRef -q .defaultBranchRef.name) + base=$(jq -r '.base_ref' pr.json) + + note="pr.diff contains ONLY this PR's own changes, measured against its base branch." + if [ "$base" = "$default" ]; then + jq -n --arg default "$default" --arg base "$base" --arg note "$note" \ + '{available: true, stacked: false, reason: "the PR targets the default branch", default_branch: $default, base_ref: $base, depth: 0, ancestors: [], note: $note}' \ + > stack.json + echo "PR targets the default branch ($default); not a stack." + exit 0 + fi + + # Follow base -> the PR whose head is that base, until the default branch. The + # depth cap and the seen-list are guards, not expectations: a retargeted branch + # can leave a cycle behind, and a cycle here would hang the whole review. + ancestors='[]' + b="$base" + # Seeded with this PR's own head branch. A base chain that comes back around to + # it would otherwise match this very PR by branch name and report the panel that + # it is stacked on itself. + seen=" $(jq -r '.head_ref' pr.json) " + depth=0 + while [ "$b" != "$default" ] && [ "$depth" -lt 10 ]; do + case " $seen " in *" $b "*) echo "::warning::Stack walk revisited branch $b; stopping."; break;; esac + seen="$seen $b" + + # An open PR for the branch is the one being stacked on; fall back to the most + # recent closed one, which is how a just-merged parent still explains the base. + # Cross-repository PRs are excluded: --head matches on branch name alone, so a + # fork whose branch happens to be named like our base is not our ancestor. + num=$(gh pr list --repo "$GITHUB_REPOSITORY" --head "$b" --state all --limit 10 \ + --json number,state,isCrossRepository \ + -q '[.[] | select(.isCrossRepository == false)] + | ([.[] | select(.state == "OPEN")] + .) + | .[0].number // empty') + if [ -z "$num" ]; then + echo "No pull request has $b as its head; stopping the walk there." + break + fi + + gh pr view "$num" --repo "$GITHUB_REPOSITORY" \ + --json number,title,body,state,url,baseRefName,headRefName,additions,deletions,commits,files \ + > "stack-$num.json" + + ancestors=$(jq -n --argjson a "$ancestors" --slurpfile p "stack-$num.json" \ + '$a + [$p[0] | { + number, title, body, state, url, + base_ref: .baseRefName, + head_ref: .headRefName, + additions, deletions, + ticket_key: ((.title | capture("(?BIG-[0-9]+)").k) // null), + commits: [.commits[] | .messageHeadline], + files: [.files[] | .path] + }]') + + b=$(jq -r '.baseRefName' "stack-$num.json") + depth=$((depth + 1)) + done + + jq -n \ + --arg default "$default" \ + --arg base "$base" \ + --arg note "$note" \ + --argjson ancestors "$ancestors" \ + '{ + available: true, + stacked: (($ancestors | length) > 0), + reason: (if ($ancestors | length) > 0 then "the PR is stacked on unmerged work" + else "the base branch is not the default branch, but no pull request has it as its head" end), + default_branch: $default, + base_ref: $base, + depth: ($ancestors | length), + ancestors: $ancestors, + note: $note + }' > stack.json + echo "Stack: base $base, default $default, $(jq -r '.depth' stack.json) ancestor PR(s) $(jq -r '[.ancestors[].number] | map("#" + tostring) | join(" <- ")' stack.json)" + # Anchor the review on the work's intent. Always writes ticket.json: either the # ticket fields, or {"available": false} so a missing/unfetchable ticket degrades - # to a diff-based review instead of failing the review. + # to a diff-based review instead of failing the review. Also records the parent + # key, which is what the epic step below fetches. - name: Fetch JIRA ticket for intent env: JIRA_EMAIL: ${{ secrets.JIRA_EMAIL }} JIRA_API_TOKEN: ${{ secrets.JIRA_API_TOKEN }} run: | - key=$(printf '%s' "$PR_TITLE" | grep -oE 'BIG-[0-9]+' | head -1 || true) + key=$(jq -r '.title' pr.json | grep -oE 'BIG-[0-9]+' | head -1 || true) if [ -z "$key" ]; then echo '{"available": false, "reason": "no BIG-XXXXX key in PR title"}' > ticket.json echo "No JIRA ticket key in PR title; falling back to diff-based review." @@ -147,18 +297,72 @@ jobs: http=$(curl -sS -w '%{http_code}' -o ticket-raw.json \ -u "$JIRA_EMAIL:$JIRA_API_TOKEN" -H 'Accept: application/json' \ - "https://biggerpockets.atlassian.net/rest/api/3/issue/$key?fields=summary,description,customfield_11557") \ + "https://biggerpockets.atlassian.net/rest/api/3/issue/$key?fields=summary,description,customfield_11557,status,issuetype,labels,parent,attachment") \ || http="000" if [ "$http" = "200" ]; then - jq --arg key "$key" '{available: true, key: $key, summary: .fields.summary, description: .fields.description, acceptance_criteria: .fields.customfield_11557}' \ - ticket-raw.json > ticket.json + jq --arg key "$key" '{ + available: true, + key: $key, + summary: .fields.summary, + description: .fields.description, + acceptance_criteria: .fields.customfield_11557, + status: .fields.status.name, + issue_type: .fields.issuetype.name, + labels: .fields.labels, + parent: (if .fields.parent then {key: .fields.parent.key, summary: .fields.parent.fields.summary, type: .fields.parent.fields.issuetype.name} else null end) + }' ticket-raw.json > ticket.json + echo "PARENT_KEY=$(jq -r '.parent.key // ""' ticket.json)" >> "$GITHUB_ENV" echo "Wrote ticket.json for $key" else echo "{\"available\": false, \"reason\": \"fetch failed (HTTP $http) for $key\"}" > ticket.json echo "Could not fetch $key (HTTP $http); falling back to diff-based review." fi + # The epic is where the wider goal usually lives: a ticket's own description is + # often a slice of work whose purpose only makes sense against the epic it belongs + # to. Fetched separately because a ticket can have no parent, or a parent the token + # can't read, and neither is a reason to fail a review. + - name: Fetch the ticket's epic for wider intent + env: + JIRA_EMAIL: ${{ secrets.JIRA_EMAIL }} + JIRA_API_TOKEN: ${{ secrets.JIRA_API_TOKEN }} + run: | + # Distinguish the three ways there can be no epic. "No epic" and "I could not + # read the epic" are different pieces of evidence, and an auditor that cannot + # tell them apart cannot say which one it judged without. + if [ "$(jq -r '.available' ticket.json)" != "true" ]; then + echo '{"available": false, "reason": "no ticket was available, so no epic to fetch"}' > epic.json + echo "No ticket, so no epic to fetch." + exit 0 + fi + if [ -z "${PARENT_KEY:-}" ]; then + echo '{"available": false, "reason": "ticket has no parent epic"}' > epic.json + echo "No parent epic to fetch." + exit 0 + fi + + http=$(curl -sS -w '%{http_code}' -o epic-raw.json \ + -u "$JIRA_EMAIL:$JIRA_API_TOKEN" -H 'Accept: application/json' \ + "https://biggerpockets.atlassian.net/rest/api/3/issue/$PARENT_KEY?fields=summary,description,issuetype,status,labels") \ + || http="000" + + if [ "$http" = "200" ]; then + jq --arg key "$PARENT_KEY" '{ + available: true, + key: $key, + summary: .fields.summary, + description: .fields.description, + issue_type: .fields.issuetype.name, + status: .fields.status.name, + labels: .fields.labels + }' epic-raw.json > epic.json + echo "Wrote epic.json for $PARENT_KEY" + else + echo "{\"available\": false, \"reason\": \"fetch failed (HTTP $http) for $PARENT_KEY\"}" > epic.json + echo "Could not fetch parent $PARENT_KEY (HTTP $http)." + fi + # Gather the existing PR discussion so the review accounts for context that lives # in conversation rather than the diff: general comments, inline review threads, and # prior review bodies. Always writes conversations.json (empty arrays if none). @@ -184,96 +388,443 @@ jobs: > conversations.json echo "Wrote conversations.json ($(jq '.issue_comments | length' conversations.json) comments, $(jq '.review_comments | length' conversations.json) review comments, $(jq '.reviews | length' conversations.json) reviews)" - # Resolve the review-stage prompts from the registry: the Codex prompt, plus the - # prompt of the single arm this PR is assigned to, each with its shared-rule blocks - # expanded and a content-derived prompt_version. Also emits the assigned arm's key - # and the bucket it was assigned from. + # Design references, harvested from the text already gathered above — the PR body + # and commit messages, the ticket and epic (description and acceptance criteria + # included, since a Figma link is usually pasted into one of them), and the + # discussion — plus the ticket's own attachments. + # + # Links are RECORDED, never followed. The panel has no Figma credentials, and + # pulling design or member-facing content into a review artifact is not something a + # code review needs. What a recorded link buys the auditors is the knowledge that a + # design exists for this work, which is the difference between "no design was + # specified" and "a design was specified and I can't see it". + - name: Collect design references + run: | + # Every URL appearing anywhere in one of the context files. `..|strings` walks + # the whole document, which is what reaches Atlassian Document Format nodes + # without having to know their shape. + # `|| true` because "this file contains no URLs" is the ordinary case, and grep + # exits 1 on no match — without it a ticket with no links would fail the step. + urls_from() { # file source-label + [ -f "$1" ] || return 0 + jq -r '[.. | strings] | .[]' "$1" 2>/dev/null \ + | grep -oE 'https?://[^][ )"'"'"'<>,]+' \ + | sed 's/[.,;:)]*$//' \ + | sort -u \ + | jq -R --arg source "$2" '{url: ., source: $source}' || true + } + + { urls_from pr.json pr + urls_from stack.json stack + urls_from ticket.json ticket + urls_from epic.json epic + urls_from conversations.json discussion + } | jq -s '.' > urls.json + + # Read the raw ticket only when the fetch actually succeeded, and treat an + # unparseable body as no attachments. `curl -o` writes the response body whatever + # the status, so a 401 or a proxy's HTML error page lands in ticket-raw.json — + # and a jq failure here would fail the context job and take down a review that + # is supposed to degrade to a diff-based one. + if [ "$(jq -r '.available' ticket.json)" = "true" ] && [ -f ticket-raw.json ]; then + jq '[.fields.attachment // [] | .[] | {filename, mime_type: .mimeType, size, url: .content, source: "ticket"}]' \ + ticket-raw.json > attachments.json 2>/dev/null || echo '[]' > attachments.json + else + echo '[]' > attachments.json + fi + + # Keep only URLs that are plausibly a design artifact. An unclassified link is + # dropped rather than listed: a designs.json full of Jira and CI links would + # bury the one Figma URL that matters. + jq -n \ + --slurpfile urls urls.json \ + --slurpfile attachments attachments.json \ + ' + def kind: + if test("figma\\.com") then "figma" + elif test("zeplin\\.io|invisionapp\\.com|sketch\\.com|miro\\.com|framer\\.com|abstract\\.com") then "design_tool" + elif test("loom\\.com|youtube\\.com|youtu\\.be|vimeo\\.com") then "walkthrough_video" + elif test("user-attachments|user-images\\.githubusercontent\\.com") then "screenshot" + elif test("\\.(png|jpe?g|gif|webp|svg|pdf)([?#]|$)") then "image" + else empty end; + # One entry per URL, listing every place it appeared: the same Figma link is + # usually in the ticket AND the PR body AND the discussion, and three copies + # of one link say nothing that one link with three sources does not. + ($urls[0] + | map(. + {kind: (.url | kind)}) + | group_by(.url) + | map({url: .[0].url, kind: .[0].kind, sources: (map(.source) | unique)}) + ) as $links + | ($attachments[0]) as $att + | { + available: ((($links | length) + ($att | length)) > 0), + links: $links, + attachments: $att, + note: "Links and attachments are recorded, not fetched: this review has no credentials for design tools. Treat a reference as evidence that a design exists for this work, not as something you can open, and never treat an unopenable reference as a defect." + }' > designs.json + echo "Wrote designs.json ($(jq '.links | length' designs.json) link(s), $(jq '.attachments | length' designs.json) attachment(s))" + + # One file that says what the panel actually got. Without it an auditor cannot 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. + - name: Write the context manifest + run: | + jq -n \ + --arg head_sha "${{ steps.pr_head.outputs.sha }}" \ + --arg base_ref "$BASE_REF" \ + --argjson diff_lines "$(wc -l < pr.diff | tr -d " ")" \ + --slurpfile pr pr.json \ + --slurpfile stack stack.json \ + --slurpfile ticket ticket.json \ + --slurpfile epic epic.json \ + --slurpfile conversations conversations.json \ + --slurpfile designs designs.json \ + '{ + head_sha: $head_sha, + base_ref: $base_ref, + diff: {file: "pr.diff", lines: $diff_lines}, + pr: {file: "pr.json", available: true, commits: ($pr[0].commits | length), files: ($pr[0].files | length)}, + stack: {file: "stack.json", stacked: $stack[0].stacked, depth: $stack[0].depth, default_branch: $stack[0].default_branch, ancestors: [$stack[0].ancestors[].number]}, + ticket: {file: "ticket.json", available: $ticket[0].available, key: ($ticket[0].key // null), reason: ($ticket[0].reason // null), has_acceptance_criteria: ((($ticket[0].acceptance_criteria // {}) | [.. | objects | select(.type == "text") | .text] | length) > 0)}, + epic: {file: "epic.json", available: $epic[0].available, key: ($epic[0].key // null), reason: ($epic[0].reason // null)}, + conversations: {file: "conversations.json", issue_comments: ($conversations[0].issue_comments | length), review_comments: ($conversations[0].review_comments | length), reviews: ($conversations[0].reviews | length)}, + designs: {file: "designs.json", available: $designs[0].available, links: ($designs[0].links | length), attachments: ($designs[0].attachments | length)} + }' > context-manifest.json + cat context-manifest.json + + # Resolve the prompts from the registry — the single auditor prompt every seat runs + # and the arbitrator prompt — each with its shared-rule blocks expanded and a + # content-derived prompt_version. Also emits the panel seats, which the arbitrator + # uses to know how many verdicts to expect. - name: Resolve review prompts id: resolve env: REGISTRY_DIR: registry run: registry/scripts/resolve-prompts.sh - # Stage 1: Codex reviews the diff against the ticket intent and writes findings to - # codex-findings.md (the `output-file`, which captures Codex's final message). - # - # Codex reviews the diff directly here rather than shelling out to the `codex review` - # subcommand. `codex review` starts its own in-process app-server that needs both a - # writable runtime home AND a localhost connection to the action's Responses API proxy. - # When it was launched as a child of `codex exec`, that child inherited the exec sandbox, - # which grants neither by default: under `sandbox: read-only` it died writing runtime - # state ("failed to initialize in-process app-server client: Read-only file system - # (os error 30)"), and under a writable profile it then died reaching the proxy - # ("stream disconnected before completion: error sending request for url - # http://127.0.0.1.../v1/responses"), because that profile grants filesystem writes but - # no network. Either way Codex produced no findings and every review silently degraded to - # a Claude-only pass. Reviewing inline via this step's own `codex exec` model call avoids - # the nested subprocess entirely: it needs only to read files, so a read-only sandbox is - # sufficient and there is no child process to sandbox. - # - # continue-on-error keeps a Codex failure (e.g. an OpenAI "Quota exceeded" error) from - # blocking the review: Stage 2 proceeds on its own when codex-findings.md is missing or - # empty. The next step still surfaces the failure as a warning so a genuinely broken Codex - # (e.g. a bad key) doesn't silently disappear. - - name: Record codex start time - run: echo "CODEX_START_NS=$(date +%s%N)" >> "$GITHUB_ENV" - - - name: Codex first-pass review - id: codex - continue-on-error: true - uses: openai/codex-action@v1 - with: - openai-api-key: ${{ secrets.OPENROUTER_API_KEY }} - responses-api-endpoint: ${{ env.CODEX_RESPONSES_ENDPOINT }} - model: ${{ env.CODEX_MODEL }} - sandbox: read-only - output-file: codex-findings.md - prompt: ${{ steps.resolve.outputs.codex_prompt }} - - # If Codex failed (it exits nonzero and writes no codex-findings.md — e.g. on a quota - # error), leave an empty findings file so Stage 2 cleanly takes its no-findings path, - # and record a warning for observability. - - name: Note skipped Codex first pass - if: steps.codex.outcome == 'failure' - run: | - echo "::warning::Codex first-pass review failed (commonly an OpenAI quota error); continuing with a Claude-only review." - : > codex-findings.md - - # Persist the handoff even when Codex failed. Claude consumes the same diff, - # ticket/discussion context, and (possibly empty) findings on every retry. - - name: Record Codex completion metadata - if: always() + # Persist the handoff. Every auditor consumes the same diff, ticket, epic, + # discussion and design references, on every retry. + # No `if: always()` on this or the upload: a stage that failed part-way through + # must not hand the panel a half-built context. Fail here and the whole review + # stops, which is the correct outcome. + - name: Record context metadata run: | - codex_end_ns=$(date +%s%N) - echo "CODEX_END_NS=$codex_end_ns" >> "$GITHUB_ENV" { echo "BASE_REF=$BASE_REF" echo "REVIEW_START_NS=$REVIEW_START_NS" - echo "CODEX_START_NS=$CODEX_START_NS" - echo "CODEX_END_NS=$codex_end_ns" - echo "CODEX_STATUS=${{ steps.codex.outcome }}" + echo "CONTEXT_END_NS=$(date +%s%N)" } > review-context.env - touch codex-findings.md - - name: Upload Codex review handoff - if: always() + # The raw API responses (and the per-ancestor stack-.json the walk fetched) are + # deliberately NOT uploaded: they carry fields nobody reviews (assignee, reporter, + # watchers) and the reshaped files above are what the prompts are written against. + - name: Upload review handoff uses: actions/upload-artifact@v4 with: - name: codex-review-${{ github.run_id }} + name: review-context-${{ github.run_id }} path: | pr.diff + pr.json + stack.json ticket.json + epic.json conversations.json - codex-findings.md + designs.json + context-manifest.json review-context.env if-no-files-found: error retention-days: 7 overwrite: true - # A failed/rate-limited Claude run can be retried with "Re-run failed jobs". The - # successful Codex job is not re-executed; this job downloads its immutable handoff. - claude: - needs: codex + # Stage 2. Three wise men, three jobs, one prompt. + # + # Each seat is its own job, 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 + # (`caspar`, `balthazar`, `melchior`), re-runnable on its own without touching the + # others. The only thing a seat can read is stage 1's handoff — nothing available to an + # auditor contains another auditor's output, which is what makes the three verdicts + # independent rather than merely concurrent. + # + # The three `steps:` blocks below are BYTE-IDENTICAL on purpose. Everything that differs + # between seats lives in the `env:` above them, so what a disagreement between seats + # measures is the models — a difference in harness would corrupt that comparison, and + # three copies of a block is exactly the kind of thing that drifts. Validation of that + # invariant is not left to reviewers: .github/workflows/validate-prompts.yml fails if + # the blocks stop matching, or if the set of seat jobs stops matching the registry. + # + # The model step is continue-on-error so a rate-limited seat still records a status for + # the arbitrator instead of taking the whole review down with it. + caspar: + needs: context + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + id-token: write # claude-code-action authenticates via OIDC + env: + PR: ${{ inputs.pr }} + AUDITOR: caspar + AUDITOR_MODEL: ${{ vars.MAGI_MODEL_CASPAR || 'openai/gpt-5.6-luna' }} + steps: + - name: Checkout reviewed PR head + uses: actions/checkout@v4 + with: + ref: ${{ needs.context.outputs.head_sha }} + fetch-depth: 0 + + # A reviewed PR is untrusted input, and this job's working directory IS the PR's + # checkout — the same directory the handoff downloads into, the same one the model + # writes its verdict to, and the same one the globs downstream read. A PR that + # committed verdict.json, or any audit-*.json, would otherwise hand this stage a + # verdict it wrote itself: nothing further down can tell a committed file from a + # model's output. Clear those names before anything reads them. + # + # A PR that legitimately contains one of these paths loses it from the working tree + # for the length of the review. It is still in pr.diff, which is what gets reviewed. + - name: Clear reserved handoff filenames + run: | + rm -rf verdict.json audit-*.json audit-*.env \ + review-context.env context-manifest.json \ + pr.diff pr.json stack.json ticket.json epic.json \ + conversations.json designs.json + + - name: Checkout prompt registry + uses: actions/checkout@v4 + with: + repository: BiggerPockets/.github + ref: ${{ needs.context.outputs.registry_sha }} + token: ${{ secrets.BIGGIEPOCKETS_PAT }} + persist-credentials: false + path: registry + + # The whole of stage 1: the diff, the PR, the ticket, its epic, the discussion, the + # design references, and the manifest saying which of those were available. + - name: Download review handoff + uses: actions/download-artifact@v4 + with: + name: review-context-${{ github.run_id }} + path: . + + - name: Restore review context + run: cat review-context.env >> "$GITHUB_ENV" + + # Resolve again from the same pinned registry ref. Prompt text stays out of job + # outputs (which are size-limited), while the artifact carries all model inputs. + - name: Resolve review prompts + id: resolve + env: + REGISTRY_DIR: registry + run: registry/scripts/resolve-prompts.sh + + # A seat with no model is a lost voice, not a fallback — fail before the panel can + # report a verdict short of a seat it claimed to have. + - name: Confirm seat model + run: | + if [ -z "$AUDITOR_MODEL" ]; then + echo "::error::Auditor '$AUDITOR' has no model: set MAGI_MODEL_$(printf '%s' "$AUDITOR" | tr '[:lower:]-' '[:upper:]_') as a repository variable, or give this job a default." >&2 + exit 1 + fi + echo "Auditor $AUDITOR runs $AUDITOR_MODEL" + + - name: Record audit start time + run: echo "AUDIT_START_NS=$(date +%s%N)" >> "$GITHUB_ENV" + + - name: Audit independently + id: audit + continue-on-error: true + uses: anthropics/claude-code-action@v1 + with: + # The action requires this input for launch validation. ANTHROPIC_AUTH_TOKEN + # below is what makes Claude Code send the OpenRouter key as a bearer token. + anthropic_api_key: ${{ secrets.OPENROUTER_API_KEY }} + # Allow the `claude` bot to trigger this review. Without this, PRs opened + # by the claude bot are rejected with "Workflow initiated by non-human + # actor: claude (type: Bot). Add bot to allowed_bots list." + allowed_bots: 'claude' + claude_args: '--allowedTools "Read,Grep,Glob,Bash,Write" --model ${{ env.AUDITOR_MODEL }}' + prompt: ${{ steps.resolve.outputs.auditor_prompt }} + env: + ANTHROPIC_BASE_URL: "https://openrouter.ai/api" + ANTHROPIC_AUTH_TOKEN: ${{ secrets.OPENROUTER_API_KEY }} + + # Name the verdict for its seat before it leaves this job. The arbitrator reads + # audit-.json, and a seat that produced no usable verdict must be visible as + # such rather than absent: the recorded status is what stops a failed seat from + # being mistaken for a silent approval. + - name: Record seat verdict + if: always() + run: | + audit_end_ns=$(date +%s%N) + status="ok" + [ "${{ steps.audit.outcome }}" != "success" ] && status="error" + + if [ -f verdict.json ] && jq -e '.verdict == "approve" or .verdict == "request_changes"' verdict.json >/dev/null 2>&1; then + mv verdict.json "audit-$AUDITOR.json" + echo "$AUDITOR: $(jq -r '.verdict' "audit-$AUDITOR.json") (confidence $(jq -r '.confidence // "unstated"' "audit-$AUDITOR.json"), $(jq -r '.blocking_findings // [] | length' "audit-$AUDITOR.json") blocking finding(s))" + else + status="error" + rm -f verdict.json + echo "::warning::Auditor $AUDITOR produced no valid verdict.json; the arbitrator will arbitrate without this seat." + fi + + # Shell-safe seat name so the arbitrator can source every seat's file at once. + key=$(printf '%s' "$AUDITOR" | tr '[:lower:]-' '[:upper:]_') + { + echo "AUDIT_${key}_START_NS=${AUDIT_START_NS:-$audit_end_ns}" + echo "AUDIT_${key}_END_NS=$audit_end_ns" + echo "AUDIT_${key}_STATUS=$status" + echo "AUDIT_${key}_MODEL=$AUDITOR_MODEL" + } > "audit-$AUDITOR.env" + + - name: Upload seat verdict + if: always() + uses: actions/upload-artifact@v4 + with: + name: audit-${{ env.AUDITOR }}-${{ github.run_id }} + path: | + audit-${{ env.AUDITOR }}.json + audit-${{ env.AUDITOR }}.env + if-no-files-found: error + retention-days: 7 + overwrite: true + + balthazar: + needs: context + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + id-token: write # claude-code-action authenticates via OIDC + env: + PR: ${{ inputs.pr }} + AUDITOR: balthazar + AUDITOR_MODEL: ${{ vars.MAGI_MODEL_BALTHAZAR || 'google/gemini-3.7-flash' }} + steps: + - name: Checkout reviewed PR head + uses: actions/checkout@v4 + with: + ref: ${{ needs.context.outputs.head_sha }} + fetch-depth: 0 + + # A reviewed PR is untrusted input, and this job's working directory IS the PR's + # checkout — the same directory the handoff downloads into, the same one the model + # writes its verdict to, and the same one the globs downstream read. A PR that + # committed verdict.json, or any audit-*.json, would otherwise hand this stage a + # verdict it wrote itself: nothing further down can tell a committed file from a + # model's output. Clear those names before anything reads them. + # + # A PR that legitimately contains one of these paths loses it from the working tree + # for the length of the review. It is still in pr.diff, which is what gets reviewed. + - name: Clear reserved handoff filenames + run: | + rm -rf verdict.json audit-*.json audit-*.env \ + review-context.env context-manifest.json \ + pr.diff pr.json stack.json ticket.json epic.json \ + conversations.json designs.json + + - name: Checkout prompt registry + uses: actions/checkout@v4 + with: + repository: BiggerPockets/.github + ref: ${{ needs.context.outputs.registry_sha }} + token: ${{ secrets.BIGGIEPOCKETS_PAT }} + persist-credentials: false + path: registry + + # The whole of stage 1: the diff, the PR, the ticket, its epic, the discussion, the + # design references, and the manifest saying which of those were available. + - name: Download review handoff + uses: actions/download-artifact@v4 + with: + name: review-context-${{ github.run_id }} + path: . + + - name: Restore review context + run: cat review-context.env >> "$GITHUB_ENV" + + # Resolve again from the same pinned registry ref. Prompt text stays out of job + # outputs (which are size-limited), while the artifact carries all model inputs. + - name: Resolve review prompts + id: resolve + env: + REGISTRY_DIR: registry + run: registry/scripts/resolve-prompts.sh + + # A seat with no model is a lost voice, not a fallback — fail before the panel can + # report a verdict short of a seat it claimed to have. + - name: Confirm seat model + run: | + if [ -z "$AUDITOR_MODEL" ]; then + echo "::error::Auditor '$AUDITOR' has no model: set MAGI_MODEL_$(printf '%s' "$AUDITOR" | tr '[:lower:]-' '[:upper:]_') as a repository variable, or give this job a default." >&2 + exit 1 + fi + echo "Auditor $AUDITOR runs $AUDITOR_MODEL" + + - name: Record audit start time + run: echo "AUDIT_START_NS=$(date +%s%N)" >> "$GITHUB_ENV" + + - name: Audit independently + id: audit + continue-on-error: true + uses: anthropics/claude-code-action@v1 + with: + # The action requires this input for launch validation. ANTHROPIC_AUTH_TOKEN + # below is what makes Claude Code send the OpenRouter key as a bearer token. + anthropic_api_key: ${{ secrets.OPENROUTER_API_KEY }} + # Allow the `claude` bot to trigger this review. Without this, PRs opened + # by the claude bot are rejected with "Workflow initiated by non-human + # actor: claude (type: Bot). Add bot to allowed_bots list." + allowed_bots: 'claude' + claude_args: '--allowedTools "Read,Grep,Glob,Bash,Write" --model ${{ env.AUDITOR_MODEL }}' + prompt: ${{ steps.resolve.outputs.auditor_prompt }} + env: + ANTHROPIC_BASE_URL: "https://openrouter.ai/api" + ANTHROPIC_AUTH_TOKEN: ${{ secrets.OPENROUTER_API_KEY }} + + # Name the verdict for its seat before it leaves this job. The arbitrator reads + # audit-.json, and a seat that produced no usable verdict must be visible as + # such rather than absent: the recorded status is what stops a failed seat from + # being mistaken for a silent approval. + - name: Record seat verdict + if: always() + run: | + audit_end_ns=$(date +%s%N) + status="ok" + [ "${{ steps.audit.outcome }}" != "success" ] && status="error" + + if [ -f verdict.json ] && jq -e '.verdict == "approve" or .verdict == "request_changes"' verdict.json >/dev/null 2>&1; then + mv verdict.json "audit-$AUDITOR.json" + echo "$AUDITOR: $(jq -r '.verdict' "audit-$AUDITOR.json") (confidence $(jq -r '.confidence // "unstated"' "audit-$AUDITOR.json"), $(jq -r '.blocking_findings // [] | length' "audit-$AUDITOR.json") blocking finding(s))" + else + status="error" + rm -f verdict.json + echo "::warning::Auditor $AUDITOR produced no valid verdict.json; the arbitrator will arbitrate without this seat." + fi + + # Shell-safe seat name so the arbitrator can source every seat's file at once. + key=$(printf '%s' "$AUDITOR" | tr '[:lower:]-' '[:upper:]_') + { + echo "AUDIT_${key}_START_NS=${AUDIT_START_NS:-$audit_end_ns}" + echo "AUDIT_${key}_END_NS=$audit_end_ns" + echo "AUDIT_${key}_STATUS=$status" + echo "AUDIT_${key}_MODEL=$AUDITOR_MODEL" + } > "audit-$AUDITOR.env" + + - name: Upload seat verdict + if: always() + uses: actions/upload-artifact@v4 + with: + name: audit-${{ env.AUDITOR }}-${{ github.run_id }} + path: | + audit-${{ env.AUDITOR }}.json + audit-${{ env.AUDITOR }}.env + if-no-files-found: error + retention-days: 7 + overwrite: true + + melchior: + needs: context runs-on: ubuntu-latest permissions: contents: read @@ -281,28 +832,46 @@ jobs: id-token: write # claude-code-action authenticates via OIDC env: PR: ${{ inputs.pr }} - CODEX_MODEL: "openai/gpt-5.6-sol" - CLAUDE_MODEL: "google/gemini-3.7-flash" + AUDITOR: melchior + AUDITOR_MODEL: ${{ vars.MAGI_MODEL_MELCHIOR || 'z-ai/glm-5.3-flash' }} steps: - name: Checkout reviewed PR head uses: actions/checkout@v4 with: - ref: ${{ needs.codex.outputs.head_sha }} + ref: ${{ needs.context.outputs.head_sha }} fetch-depth: 0 + # A reviewed PR is untrusted input, and this job's working directory IS the PR's + # checkout — the same directory the handoff downloads into, the same one the model + # writes its verdict to, and the same one the globs downstream read. A PR that + # committed verdict.json, or any audit-*.json, would otherwise hand this stage a + # verdict it wrote itself: nothing further down can tell a committed file from a + # model's output. Clear those names before anything reads them. + # + # A PR that legitimately contains one of these paths loses it from the working tree + # for the length of the review. It is still in pr.diff, which is what gets reviewed. + - name: Clear reserved handoff filenames + run: | + rm -rf verdict.json audit-*.json audit-*.env \ + review-context.env context-manifest.json \ + pr.diff pr.json stack.json ticket.json epic.json \ + conversations.json designs.json + - name: Checkout prompt registry uses: actions/checkout@v4 with: repository: BiggerPockets/.github - ref: ${{ needs.codex.outputs.registry_sha }} + ref: ${{ needs.context.outputs.registry_sha }} token: ${{ secrets.BIGGIEPOCKETS_PAT }} persist-credentials: false path: registry - - name: Download Codex review handoff + # The whole of stage 1: the diff, the PR, the ticket, its epic, the discussion, the + # design references, and the manifest saying which of those were available. + - name: Download review handoff uses: actions/download-artifact@v4 with: - name: codex-review-${{ github.run_id }} + name: review-context-${{ github.run_id }} path: . - name: Restore review context @@ -316,14 +885,22 @@ jobs: REGISTRY_DIR: registry run: registry/scripts/resolve-prompts.sh - # Stage 2: Claude verifies Codex's findings, reviews independently, and decides. - # This is the only Claude pass, running whichever arm's prompt was assigned to - # this PR. Its verdict is the review. - - name: Record Claude start time - run: echo "CLAUDE_START_NS=$(date +%s%N)" >> "$GITHUB_ENV" + # A seat with no model is a lost voice, not a fallback — fail before the panel can + # report a verdict short of a seat it claimed to have. + - name: Confirm seat model + run: | + if [ -z "$AUDITOR_MODEL" ]; then + echo "::error::Auditor '$AUDITOR' has no model: set MAGI_MODEL_$(printf '%s' "$AUDITOR" | tr '[:lower:]-' '[:upper:]_') as a repository variable, or give this job a default." >&2 + exit 1 + fi + echo "Auditor $AUDITOR runs $AUDITOR_MODEL" - - name: Claude verify and synthesize - id: claude + - name: Record audit start time + run: echo "AUDIT_START_NS=$(date +%s%N)" >> "$GITHUB_ENV" + + - name: Audit independently + id: audit + continue-on-error: true uses: anthropics/claude-code-action@v1 with: # The action requires this input for launch validation. ANTHROPIC_AUTH_TOKEN @@ -333,36 +910,244 @@ jobs: # by the claude bot are rejected with "Workflow initiated by non-human # actor: claude (type: Bot). Add bot to allowed_bots list." allowed_bots: 'claude' - claude_args: ${{ env.CLAUDE_MODEL != '' && format('--allowedTools "Read,Grep,Glob,Bash,Write" --model {0}', env.CLAUDE_MODEL) || '--allowedTools "Read,Grep,Glob,Bash,Write"' }} - prompt: ${{ steps.resolve.outputs.arm_prompt }} + claude_args: '--allowedTools "Read,Grep,Glob,Bash,Write" --model ${{ env.AUDITOR_MODEL }}' + prompt: ${{ steps.resolve.outputs.auditor_prompt }} env: ANTHROPIC_BASE_URL: "https://openrouter.ai/api" ANTHROPIC_AUTH_TOKEN: ${{ secrets.OPENROUTER_API_KEY }} - - name: Record Claude end time + # Name the verdict for its seat before it leaves this job. The arbitrator reads + # audit-.json, and a seat that produced no usable verdict must be visible as + # such rather than absent: the recorded status is what stops a failed seat from + # being mistaken for a silent approval. + - name: Record seat verdict if: always() - run: echo "CLAUDE_END_NS=$(date +%s%N)" >> "$GITHUB_ENV" + run: | + audit_end_ns=$(date +%s%N) + status="ok" + [ "${{ steps.audit.outcome }}" != "success" ] && status="error" + + if [ -f verdict.json ] && jq -e '.verdict == "approve" or .verdict == "request_changes"' verdict.json >/dev/null 2>&1; then + mv verdict.json "audit-$AUDITOR.json" + echo "$AUDITOR: $(jq -r '.verdict' "audit-$AUDITOR.json") (confidence $(jq -r '.confidence // "unstated"' "audit-$AUDITOR.json"), $(jq -r '.blocking_findings // [] | length' "audit-$AUDITOR.json") blocking finding(s))" + else + status="error" + rm -f verdict.json + echo "::warning::Auditor $AUDITOR produced no valid verdict.json; the arbitrator will arbitrate without this seat." + fi + + # Shell-safe seat name so the arbitrator can source every seat's file at once. + key=$(printf '%s' "$AUDITOR" | tr '[:lower:]-' '[:upper:]_') + { + echo "AUDIT_${key}_START_NS=${AUDIT_START_NS:-$audit_end_ns}" + echo "AUDIT_${key}_END_NS=$audit_end_ns" + echo "AUDIT_${key}_STATUS=$status" + echo "AUDIT_${key}_MODEL=$AUDITOR_MODEL" + } > "audit-$AUDITOR.env" + + - name: Upload seat verdict + if: always() + uses: actions/upload-artifact@v4 + with: + name: audit-${{ env.AUDITOR }}-${{ github.run_id }} + path: | + audit-${{ env.AUDITOR }}.json + audit-${{ env.AUDITOR }}.env + if-no-files-found: error + retention-days: 7 + overwrite: true + + # Stage 3. Weighs the three verdicts and issues the one that gets posted. + # + # Runs whenever stage 1 succeeded, even if a seat job failed outright: a seat that + # never started (no model configured, runner lost) would otherwise skip arbitration and + # leave the PR with no review at all. Whether a panel short a seat is still a panel is + # decided by the quorum check below, which says so out loud, rather than by GitHub + # silently skipping this job. + arbitrate: + needs: [context, caspar, balthazar, melchior] + if: ${{ !cancelled() && needs.context.result == 'success' }} + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + id-token: write # claude-code-action authenticates via OIDC + env: + PR: ${{ inputs.pr }} + ARBITER_MODEL: ${{ vars.ARBITER_MODEL || 'openai/gpt-5.6-sol' }} + steps: + - name: Checkout reviewed PR head + uses: actions/checkout@v4 + with: + ref: ${{ needs.context.outputs.head_sha }} + fetch-depth: 0 + + # A reviewed PR is untrusted input, and this job's working directory IS the PR's + # checkout — the same directory the handoff downloads into, the same one the model + # writes its verdict to, and the same one the globs downstream read. A PR that + # committed verdict.json, or any audit-*.json, would otherwise hand this stage a + # verdict it wrote itself: nothing further down can tell a committed file from a + # model's output. Clear those names before anything reads them. + # + # A PR that legitimately contains one of these paths loses it from the working tree + # for the length of the review. It is still in pr.diff, which is what gets reviewed. + - name: Clear reserved handoff filenames + run: | + rm -rf verdict.json audit-*.json audit-*.env \ + review-context.env context-manifest.json \ + pr.diff pr.json stack.json ticket.json epic.json \ + conversations.json designs.json + + - name: Checkout prompt registry + uses: actions/checkout@v4 + with: + repository: BiggerPockets/.github + ref: ${{ needs.context.outputs.registry_sha }} + token: ${{ secrets.BIGGIEPOCKETS_PAT }} + persist-credentials: false + path: registry + + - name: Download review handoff + uses: actions/download-artifact@v4 + with: + name: review-context-${{ github.run_id }} + path: . + + # Every seat's artifact flattened into the working directory. Filenames carry the + # seat name, so merging can't collide, and this is the first and only place in the + # run where more than one auditor's verdict exists together. + - name: Download panel verdicts + uses: actions/download-artifact@v4 + with: + pattern: audit-*-${{ github.run_id }} + merge-multiple: true + path: . + + - name: Restore review context + run: | + cat review-context.env >> "$GITHUB_ENV" + for f in audit-*.env; do [ -f "$f" ] && cat "$f" >> "$GITHUB_ENV"; done + true + + - name: Resolve review prompts + id: resolve + env: + REGISTRY_DIR: registry + run: registry/scripts/resolve-prompts.sh + + # A panel that lost most of its seats isn't a panel. Arbitrating over one verdict + # would quietly turn a three-auditor review into a single-reviewer one, so require + # a majority of seats to have produced a verdict and fail loudly otherwise — the + # failed audit jobs can then be re-run without repeating stage 1. + - name: Check panel quorum + env: + AUDITOR_COUNT: ${{ steps.resolve.outputs.auditor_count }} + run: | + seats=$(ls audit-*.json 2>/dev/null | wc -l | tr -d ' ') + quorum=$(( AUDITOR_COUNT / 2 + 1 )) + echo "PANEL_SEATS_REPORTING=$seats" >> "$GITHUB_ENV" + echo "PANEL_SEATS_DECLARED=$AUDITOR_COUNT" >> "$GITHUB_ENV" + if [ "$seats" -lt "$quorum" ]; then + echo "::error::Only $seats of $AUDITOR_COUNT auditors produced a verdict (quorum $quorum); re-run the failed audit jobs." >&2 + exit 1 + fi + if [ "$seats" -lt "$AUDITOR_COUNT" ]; then + echo "::warning::Arbitrating over $seats of $AUDITOR_COUNT auditors." + fi + for f in audit-*.json; do + echo "$f -> $(jq -r '.verdict' "$f") (confidence $(jq -r '.confidence // "unstated"' "$f"))" + done + + - name: Record arbitration start time + run: echo "ARBITER_START_NS=$(date +%s%N)" >> "$GITHUB_ENV" + + - name: Arbitrate the panel + id: arbiter + uses: anthropics/claude-code-action@v1 + with: + anthropic_api_key: ${{ secrets.OPENROUTER_API_KEY }} + allowed_bots: 'claude' + claude_args: '--allowedTools "Read,Grep,Glob,Bash,Write" --model ${{ env.ARBITER_MODEL }}' + prompt: ${{ steps.resolve.outputs.arbiter_prompt }} + env: + ANTHROPIC_BASE_URL: "https://openrouter.ai/api" + ANTHROPIC_AUTH_TOKEN: ${{ secrets.OPENROUTER_API_KEY }} + + - name: Record arbitration end time + if: always() + run: echo "ARBITER_END_NS=$(date +%s%N)" >> "$GITHUB_ENV" + + # Summarize how the panel split and whether arbitration went with it. This is the + # number worth watching: an arbitrator that never departs from the majority is a + # vote counter, and one that always does is ignoring the panel. + - name: Summarize panel outcome + if: always() + run: | + # Count with jq, not grep: the verdict is read from the parsed document, so a + # seat whose JSON is pretty-printed differently still counts. + approve=0; changes=0; seat_verdicts="" + for f in audit-*.json; do + [ -f "$f" ] || continue + seat="${f#audit-}"; seat="${seat%.json}" + v=$(jq -r '.verdict' "$f") + c=$(jq -r '.confidence // "unstated"' "$f") + case "$v" in + approve) approve=$((approve + 1)) ;; + request_changes) changes=$((changes + 1)) ;; + esac + seat_verdicts="${seat_verdicts:+$seat_verdicts,}$seat=$v/$c" + done + + echo "PANEL_APPROVE=$approve" >> "$GITHUB_ENV" + echo "PANEL_REQUEST_CHANGES=$changes" >> "$GITHUB_ENV" + # The seat-by-seat record, so a disagreement can be read back from telemetry + # without opening the run. + echo "PANEL_SEAT_VERDICTS=${seat_verdicts:-none}" >> "$GITHUB_ENV" + + if [ "$approve" -gt 0 ] && [ "$changes" -gt 0 ]; then + agreement="split" + elif [ "$changes" -gt 0 ]; then + agreement="unanimous_request_changes" + elif [ "$approve" -gt 0 ]; then + agreement="unanimous_approve" + else + agreement="none" + fi + echo "PANEL_AGREEMENT=$agreement" >> "$GITHUB_ENV" + echo "Panel: $approve approve, $changes request changes ($agreement) — ${seat_verdicts:-none}" - # Submit the review under the arm this PR was assigned. The arm is not disclosed - # in the comment: a reviewer who knows which prompt wrote a summary can't rate it - # blind, and the whole point of the split is an unbiased read on the two prompts. - name: Submit review as BiggiePockets id: submit env: # PAT authenticating as the BiggiePockets service account. A bot cannot # approve its own PR, so this never gates BiggiePockets-authored PRs. GH_TOKEN: ${{ secrets.BIGGIEPOCKETS_PAT }} - ASSIGNED_ARM: ${{ steps.resolve.outputs.assigned_arm }} run: | if [ ! -f verdict.json ]; then - echo "::error::Claude review run did not produce verdict.json" >&2 + echo "::error::Arbitration did not produce verdict.json" >&2 + exit 1 + fi + + # Validate before posting, the same way each seat validates its own verdict. This + # is the only text that reaches the pull request, and `jq -r` on a missing key + # yields the string "null" — which would post a review whose entire body is the + # word null rather than failing and saying why. + if ! jq -e '(.verdict == "approve" or .verdict == "request_changes") + and (.summary | type == "string") and (.summary | length > 0)' \ + verdict.json >/dev/null 2>&1; then + echo "::error::Arbitration wrote a verdict.json that is not a usable verdict:" >&2 + jq -c '{verdict, summary_type: (.summary | type), summary_length: (.summary | tostring | length)}' verdict.json >&2 2>/dev/null \ + || echo "(unparseable JSON)" >&2 exit 1 fi verdict=$(jq -r '.verdict' verdict.json) summary=$(jq -r '.summary' verdict.json) - printf '## BiggiePockets assisted review\n\n%s\n' "$summary" > review-body.md + # The review body is the arbitrator's summary verbatim — no heading. GitHub + # already attributes the review to BiggiePockets in its own chrome, so a title + # restating that just pushes the verdict below the fold. + printf '%s\n' "$summary" > review-body.md case "$verdict" in approve) @@ -372,31 +1157,57 @@ jobs: gh pr review "$PR" --request-changes --body-file review-body.md ;; *) - echo "::error::Unexpected verdict from $ASSIGNED_ARM arm review: '$verdict'" >&2 + echo "::error::Unexpected verdict from arbitration: '$verdict'" >&2 exit 1 ;; esac echo "VERDICT=$verdict" >> "$GITHUB_ENV" - # Reports ONE LLM OBS TRACE per review, tagged with the arm this PR was assigned, - # and joined by a stable run_id (repo + pr + review attempt) so offline evals and - # panel ratings line up per review. Never blocks/fails/delays the review itself. - # - # `arm`/`prompt_name`/`prompt_version` are payload-level tags, which is where a tag - # key resolves to one value — one arm per review, so one value each. + # Whether arbitration followed the panel majority, computed here so the tag + # reflects the verdict actually posted. A tie has no majority to follow or + # depart from — which is exactly when the arbitrator is doing the deciding — so + # record that as its own value rather than scoring it as an override. + approve="${PANEL_APPROVE:-0}" + changes="${PANEL_REQUEST_CHANGES:-0}" + if [ "$changes" -gt "$approve" ]; then + majority="request_changes" + elif [ "$approve" -gt "$changes" ]; then + majority="approve" + else + majority="none" + fi + + if [ "$majority" = "none" ]; then + followed="no_majority" + elif [ "$verdict" = "$majority" ]; then + followed="true" + else + followed="false" + fi + echo "ARBITER_FOLLOWED_PANEL=$followed" >> "$GITHUB_ENV" + echo "Panel majority: $majority; arbitrated verdict: $verdict (followed=$followed)" + + # Reports ONE LLM OBS TRACE per review, joined by a stable run_id (repo + pr + + # review attempt) so offline evals and panel ratings line up per review. Never + # blocks/fails/delays the review itself. # - # root biggiepockets.review -> codex.review, claude.synthesize + # root biggiepockets.review + # ├── magi.audit. (one span per auditor) + # └── magi.arbitrate # - # Compare arms by grouping on `arm` across pull requests (verdict rate, latency, - # tokens). + # The span tree is where a seat's individual verdict lives; the payload tags carry + # the panel-level outcome (agreement, split, whether arbitration followed the + # majority), which is what makes "is the panel earning its cost?" a query rather + # than a manual read of run logs. - name: Report review metrics to Datadog LLM Obs if: always() continue-on-error: true env: DD_API_KEY: ${{ secrets.DATADOG_API_KEY }} - CODEX_PROMPT_TEMPLATE: ${{ steps.resolve.outputs.codex_prompt_template }} - ARM_PROMPT_TEMPLATE: ${{ steps.resolve.outputs.arm_prompt_template }} + AUDITOR_PROMPT_TEMPLATE: ${{ steps.resolve.outputs.auditor_prompt_template }} + ARBITER_PROMPT_TEMPLATE: ${{ steps.resolve.outputs.arbiter_prompt_template }} + AUDITORS_JSON: ${{ steps.resolve.outputs.auditors }} run: | if [ -z "$DD_API_KEY" ]; then echo "Skipping Datadog LLM Obs submission: DATADOG_API_KEY not configured" @@ -405,34 +1216,18 @@ jobs: now_ns=$(date +%s%N) review_start_ns="${REVIEW_START_NS:-$now_ns}" - codex_start_ns="${CODEX_START_NS:-$review_start_ns}" - codex_end_ns="${CODEX_END_NS:-$codex_start_ns}" - claude_start_ns="${CLAUDE_START_NS:-$codex_end_ns}" - claude_end_ns="${CLAUDE_END_NS:-$claude_start_ns}" + context_end_ns="${CONTEXT_END_NS:-$review_start_ns}" + arbiter_start_ns="${ARBITER_START_NS:-$context_end_ns}" + arbiter_end_ns="${ARBITER_END_NS:-$arbiter_start_ns}" verdict="${VERDICT:-unknown}" - codex_prompt_name="${{ steps.resolve.outputs.codex_prompt_name }}" - codex_prompt_version="${{ steps.resolve.outputs.codex_prompt_version }}" - arm="${{ steps.resolve.outputs.assigned_arm }}" - arm_prompt_name="${{ steps.resolve.outputs.arm_prompt_name }}" - arm_prompt_version="${{ steps.resolve.outputs.arm_prompt_version }}" - control_arm="${{ steps.resolve.outputs.control_arm }}" - # Which side of the split this review is on, plus the inputs that produced it. - # Recording the bucket and the split percentage makes an assignment auditable - # after the fact: a later Apply that changes the split can't silently rewrite - # what an already-recorded review was assigned under. - assignment_bucket="${{ steps.resolve.outputs.assignment_bucket }}" - split_percent="${{ steps.resolve.outputs.experiment_split_percent }}" - arm_role="control" - [ "$arm" != "$control_arm" ] && arm_role="experiment" - - codex_status="ok" - [ "${CODEX_STATUS:-failure}" != "success" ] && codex_status="error" - codex_findings=$(cat codex-findings.md 2>/dev/null || true) - codex_findings_lines=$(wc -l < codex-findings.md 2>/dev/null || echo 0) - - claude_status="ok" - [ "${{ steps.claude.outcome }}" != "success" ] && claude_status="error" + auditor_prompt_name="${{ steps.resolve.outputs.auditor_prompt_name }}" + auditor_prompt_version="${{ steps.resolve.outputs.auditor_prompt_version }}" + arbiter_prompt_name="${{ steps.resolve.outputs.arbiter_prompt_name }}" + arbiter_prompt_version="${{ steps.resolve.outputs.arbiter_prompt_version }}" + + arbiter_status="ok" + [ "${{ steps.arbiter.outcome }}" != "success" ] && arbiter_status="error" root_status="ok" [ "${{ steps.submit.outcome }}" != "success" ] && root_status="error" @@ -440,9 +1235,9 @@ jobs: summary="" [ -f verdict.json ] && summary=$(jq -r '.summary // ""' verdict.json 2>/dev/null) || summary="" - # Cap free-text fields sent to Datadog so one outsized findings report can't - # blow past the LLM Obs payload limit; the untruncated text is always still in - # this step's own job logs. + # Cap free-text fields sent to Datadog so one outsized summary can't blow past + # the LLM Obs payload limit; the untruncated text is always still in this step's + # own job logs. truncate() { local text="$1" max=4000 if [ "${#text}" -gt "$max" ]; then @@ -451,21 +1246,22 @@ jobs: printf '%s' "$text" fi } - codex_findings_excerpt=$(truncate "$codex_findings") summary_excerpt=$(truncate "$summary") # Prompt Tracking: each LLM span carries the registry template that produced it, # under meta.input.prompt. The template keeps its {{PR}}/{{PROMPT_NAME}}/ - # {{PROMPT_VERSION}} placeholders and the substituted values ride along as - # `variables`, so every PR's run lands on the same prompt in Datadog instead of - # registering a new template per PR. `version` is our content-derived hash, so a - # prompt edit (a Roll) is what starts a new version. + # {{PROMPT_VERSION}}/{{AUDITORS}} placeholders and the substituted values ride + # along as `variables`, so every PR's run lands on the same prompt in Datadog + # instead of registering a new template per PR. `version` is our content-derived + # hash, so a prompt edit (a Roll) is what starts a new version. + auditor_list=$(printf '%s' "$AUDITORS_JSON" | jq -r 'join(", ")') prompt_json() { # name version template -> Prompt object jq -n \ --arg name "$1" \ --arg version "$2" \ --arg template "$3" \ --arg pr "$PR" \ + --arg auditors "$auditor_list" \ '{ id: $name, name: $name, @@ -474,47 +1270,105 @@ jobs: # Report only the placeholders this template actually uses, so the # variables list matches what a Playground replay needs to render it. variables: ( - {PR: $pr, PROMPT_NAME: $name, PROMPT_VERSION: $version} + {PR: $pr, PROMPT_NAME: $name, PROMPT_VERSION: $version, AUDITORS: $auditors} | with_entries(select(.key as $k | $template | contains("{{" + $k + "}}"))) ) }' } - codex_prompt_obj=$(prompt_json "$codex_prompt_name" "$codex_prompt_version" "$CODEX_PROMPT_TEMPLATE") - arm_prompt_obj=$(prompt_json "$arm_prompt_name" "$arm_prompt_version" "$ARM_PROMPT_TEMPLATE") + auditor_prompt_obj=$(prompt_json "$auditor_prompt_name" "$auditor_prompt_version" "$AUDITOR_PROMPT_TEMPLATE") + arbiter_prompt_obj=$(prompt_json "$arbiter_prompt_name" "$arbiter_prompt_version" "$ARBITER_PROMPT_TEMPLATE") - codex_model_tag="${CODEX_MODEL:-unspecified}" - claude_model_tag="${CLAUDE_MODEL:-unspecified}" + arbiter_model_tag="${ARBITER_MODEL:-unspecified}" - run_id="$GITHUB_REPOSITORY-pr$PR-${{ github.run_id }}" + # Includes the attempt: "Re-run failed jobs" is a documented path, and two + # reviews of the same PR sharing a join key would collide in every eval that + # groups by it. + run_id="$GITHUB_REPOSITORY-pr$PR-${{ github.run_id }}-${{ github.run_attempt }}" trace_id=$(openssl rand -hex 16) root_span_id=$(openssl rand -hex 8) - codex_span_id=$(openssl rand -hex 8) - claude_span_id=$(openssl rand -hex 8) + arbiter_span_id=$(openssl rand -hex 8) - spans=$(jq -n \ + # One span per seat, built from that seat's own recorded verdict and timing, so + # a disagreement is inspectable per auditor in the trace instead of collapsed + # into the arbitrated result. + audit_spans="[]" + seat_tags="[]" + for f in audit-*.json; do + [ -f "$f" ] || continue + seat="${f#audit-}"; seat="${seat%.json}" + key=$(printf '%s' "$seat" | tr '[:lower:]-' '[:upper:]_') + eval "s_start=\${AUDIT_${key}_START_NS:-$context_end_ns}" + eval "s_end=\${AUDIT_${key}_END_NS:-\$s_start}" + eval "s_status=\${AUDIT_${key}_STATUS:-error}" + eval "s_model=\${AUDIT_${key}_MODEL:-unspecified}" + [ "$s_status" = "ok" ] || s_status="error" + s_verdict=$(jq -r '.verdict // "unknown"' "$f") + s_confidence=$(jq -r '.confidence // "unstated"' "$f") + s_findings=$(jq -r '.blocking_findings // [] | length' "$f") + s_summary=$(truncate "$(jq -r '.summary // ""' "$f")") + + audit_spans=$(jq -n \ + --argjson spans "$audit_spans" \ + --arg trace_id "$trace_id" \ + --arg parent_id "$root_span_id" \ + --arg span_id "$(openssl rand -hex 8)" \ + --arg seat "$seat" \ + --arg model "$s_model" \ + --arg status "$s_status" \ + --arg summary "$s_summary" \ + --arg pr "$PR" \ + --argjson prompt "$auditor_prompt_obj" \ + --argjson start_ns "$s_start" \ + --argjson duration "$((s_end - s_start))" \ + '$spans + [{ + parent_id: $parent_id, + trace_id: $trace_id, + span_id: $span_id, + name: "magi.audit.\($seat)", + start_ns: $start_ns, + duration: $duration, + status: $status, + meta: { + kind: "llm", + model_name: $model, + model_provider: "openrouter", + input: { + messages: [{role: "user", content: "Audit pr:\($pr) independently as \($seat)"}], + prompt: $prompt + }, + output: {messages: [{role: "assistant", content: $summary}]} + } + }]') + + seat_tags=$(jq -n --argjson t "$seat_tags" \ + --arg seat "$seat" --arg v "$s_verdict" --arg c "$s_confidence" \ + --arg m "$s_model" --arg n "$s_findings" \ + '$t + [ + "auditor_verdict.\($seat):\($v)", + "auditor_confidence.\($seat):\($c)", + "auditor_model.\($seat):\($m)", + "auditor_blocking_findings.\($seat):\($n)" + ]') + done + + fixed_spans=$(jq -n \ --arg trace_id "$trace_id" \ --arg root_span_id "$root_span_id" \ - --arg codex_span_id "$codex_span_id" \ - --arg claude_span_id "$claude_span_id" \ + --arg arbiter_span_id "$arbiter_span_id" \ --arg repo "$GITHUB_REPOSITORY" \ --arg pr "$PR" \ --arg base_ref "${BASE_REF:-unknown}" \ - --arg codex_model "$codex_model_tag" \ - --arg claude_model "$claude_model_tag" \ + --arg arbiter_model "$arbiter_model_tag" \ --argjson review_start_ns "$review_start_ns" \ --argjson root_duration "$((now_ns - review_start_ns))" \ - --argjson codex_start_ns "$codex_start_ns" \ - --argjson codex_duration "$((codex_end_ns - codex_start_ns))" \ - --argjson claude_start_ns "$claude_start_ns" \ - --argjson claude_duration "$((claude_end_ns - claude_start_ns))" \ - --arg codex_status "$codex_status" \ - --arg claude_status "$claude_status" \ + --argjson arbiter_start_ns "$arbiter_start_ns" \ + --argjson arbiter_duration "$((arbiter_end_ns - arbiter_start_ns))" \ + --arg arbiter_status "$arbiter_status" \ --arg root_status "$root_status" \ - --arg codex_findings_excerpt "$codex_findings_excerpt" \ --arg summary_excerpt "$summary_excerpt" \ - --argjson codex_prompt "$codex_prompt_obj" \ - --argjson arm_prompt "$arm_prompt_obj" \ + --arg seat_verdicts "${PANEL_SEAT_VERDICTS:-none}" \ + --argjson arbiter_prompt "$arbiter_prompt_obj" \ '[ { parent_id: "undefined", @@ -533,85 +1387,67 @@ jobs: { parent_id: $root_span_id, trace_id: $trace_id, - span_id: $codex_span_id, - name: "codex.review", - start_ns: $codex_start_ns, - duration: $codex_duration, - status: $codex_status, - meta: { - kind: "llm", - model_name: $codex_model, - model_provider: "openrouter", - input: { - messages: [{role: "user", content: "Review pr:\($pr) against ticket.json/pr.diff"}], - prompt: $codex_prompt - }, - output: {messages: [{role: "assistant", content: $codex_findings_excerpt}]} - } - }, - { - parent_id: $root_span_id, - trace_id: $trace_id, - span_id: $claude_span_id, - name: "claude.synthesize", - start_ns: $claude_start_ns, - duration: $claude_duration, - status: $claude_status, + span_id: $arbiter_span_id, + name: "magi.arbitrate", + start_ns: $arbiter_start_ns, + duration: $arbiter_duration, + status: $arbiter_status, meta: { kind: "llm", - model_name: $claude_model, + model_name: $arbiter_model, model_provider: "openrouter", input: { - messages: [{role: "user", content: $codex_findings_excerpt}], - prompt: $arm_prompt + messages: [{role: "user", content: "Panel verdicts: \($seat_verdicts)"}], + prompt: $arbiter_prompt }, output: {messages: [{role: "assistant", content: $summary_excerpt}]} } } ]') + spans=$(jq -n --argjson fixed "$fixed_spans" --argjson audits "$audit_spans" '$fixed + $audits') + payload=$(jq -n \ --arg ml_app "biggiepockets-review" \ --arg repo "$GITHUB_REPOSITORY" \ --arg pr "$PR" \ --arg run_id "$run_id" \ --arg verdict "$verdict" \ - --arg arm "$arm" \ - --arg arm_role "$arm_role" \ - --arg prompt_name "$arm_prompt_name" \ - --arg prompt_version "$arm_prompt_version" \ - --arg control_arm "$control_arm" \ - --arg assignment_bucket "$assignment_bucket" \ - --arg split_percent "$split_percent" \ - --arg codex_model "$codex_model_tag" \ - --arg claude_model "$claude_model_tag" \ - --arg codex_prompt_name "$codex_prompt_name" \ - --arg codex_prompt_version "$codex_prompt_version" \ - --argjson codex_findings_lines "$codex_findings_lines" \ + --arg agreement "${PANEL_AGREEMENT:-unknown}" \ + --arg followed "${ARBITER_FOLLOWED_PANEL:-unknown}" \ + --arg approve "${PANEL_APPROVE:-0}" \ + --arg changes "${PANEL_REQUEST_CHANGES:-0}" \ + --arg reporting "${PANEL_SEATS_REPORTING:-0}" \ + --arg declared "${PANEL_SEATS_DECLARED:-0}" \ + --arg auditor_prompt_name "$auditor_prompt_name" \ + --arg auditor_prompt_version "$auditor_prompt_version" \ + --arg arbiter_prompt_name "$arbiter_prompt_name" \ + --arg arbiter_prompt_version "$arbiter_prompt_version" \ + --arg arbiter_model "$arbiter_model_tag" \ + --argjson seat_tags "$seat_tags" \ --argjson spans "$spans" \ '{ data: { type: "span", attributes: { ml_app: $ml_app, - tags: [ + tags: ([ "repo:\($repo)", "pr:\($pr)", "run_id:\($run_id)", - "arm:\($arm)", - "arm_role:\($arm_role)", - "prompt_name:\($prompt_name)", - "prompt_version:\($prompt_version)", "verdict:\($verdict)", - "control_arm:\($control_arm)", - "assignment_bucket:\($assignment_bucket)", - "experiment_split_percent:\($split_percent)", - "codex_model:\($codex_model)", - "claude_model:\($claude_model)", - "codex_findings_lines:\($codex_findings_lines)", - "codex_prompt_name:\($codex_prompt_name)", - "codex_prompt_version:\($codex_prompt_version)" - ], + "panel_agreement:\($agreement)", + "arbiter_followed_panel:\($followed)", + "panel_approve:\($approve)", + "panel_request_changes:\($changes)", + "panel_seats_reporting:\($reporting)", + "panel_seats_declared:\($declared)", + "auditor_prompt_name:\($auditor_prompt_name)", + "auditor_prompt_version:\($auditor_prompt_version)", + "arbiter_prompt_name:\($arbiter_prompt_name)", + "arbiter_prompt_version:\($arbiter_prompt_version)", + "arbiter_model:\($arbiter_model)" + ] + $seat_tags), spans: $spans } } @@ -626,5 +1462,5 @@ jobs: if [ "$http_code" -ge 300 ] 2>/dev/null || [ "$http_code" = "000" ]; then echo "::warning::Datadog LLM Obs submission failed (HTTP $http_code): $(cat /tmp/dd_response.json 2>/dev/null)" else - echo "Reported ${arm} arm to Datadog LLM Obs (trace ${trace_id}, prompt ${arm_prompt_name}@${arm_prompt_version}, run_id ${run_id})" + echo "Reported panel to Datadog LLM Obs (trace ${trace_id}, agreement ${PANEL_AGREEMENT:-unknown}, verdict ${verdict}, run_id ${run_id})" fi diff --git a/.github/workflows/validate-prompts.yml b/.github/workflows/validate-prompts.yml index 2131fa6..c719e8b 100644 --- a/.github/workflows/validate-prompts.yml +++ b/.github/workflows/validate-prompts.yml @@ -3,8 +3,8 @@ name: Validate prompt registry # Guard rails for the BiggiePockets review prompt registry (prompts/). Prompt # versions are derived from content, so this prevents the failure modes that a # hand-bumped version can't: a broken template, a dangling {{@include}}, a -# registry.json that references a prompt that doesn't exist, or a resolver that -# is not deterministic for a fixed PR number. +# registry.json that references a prompt that doesn't exist, a panel that can't be +# fanned out, or a resolver that is not deterministic for a fixed PR number. on: pull_request: @@ -17,37 +17,44 @@ on: jobs: validate: runs-on: ubuntu-latest + env: + REGISTRY_DIR: ${{ github.workspace }} steps: - name: Checkout uses: actions/checkout@v4 - name: Validate registry config run: | - jq -e '.version == 2' prompts/registry.json >/dev/null - jq -e '.codex_prompt == "codex-first-pass"' prompts/registry.json >/dev/null - jq -e '.control_arm == "control"' prompts/registry.json >/dev/null - # An out-of-range split would route reviews to an arm that doesn't exist, or - # silently pin every review to one side of a test that looks live. - jq -e '.experiment_split_percent | type == "number" and . >= 0 and . <= 100' \ - prompts/registry.json >/dev/null - for arm in $(jq -r '.arms | keys[]' prompts/registry.json); do - prompt=$(jq -r --arg a "$arm" '.arms[$a]' prompts/registry.json) - test -f "prompts/$prompt.md" || { echo "prompts/$prompt.md missing for arm $arm"; exit 1; } + jq -e '.version == 4' prompts/registry.json >/dev/null + for field in auditor_prompt arbiter_prompt; do + name=$(jq -r --arg f "$field" '.[$f] // empty' prompts/registry.json) + test -n "$name" || { echo "registry.json missing $field"; exit 1; } + test -f "prompts/$name.md" || { echo "prompts/$name.md missing for $field"; exit 1; } done - echo "registry.json OK" + # A one-seat "panel" is a single reviewer wearing a panel's costume: the + # arbitrator would have nothing to weigh and the extra stage would be pure cost. + count=$(jq -r '.auditors | length' prompts/registry.json) + test "$count" -ge 2 || { echo "registry.json declares $count auditor(s); need >= 2"; exit 1; } + distinct=$(jq -r '.auditors | unique | length' prompts/registry.json) + test "$count" = "$distinct" || { echo "duplicate auditor seat names"; exit 1; } + # Seat names become artifact names, file names, and env-var suffixes downstream. + for seat in $(jq -r '.auditors[]' prompts/registry.json); do + printf '%s' "$seat" | grep -qE '^[a-z][a-z0-9-]*$' \ + || { echo "auditor seat '$seat' is not lowercase alphanumeric/dashes"; exit 1; } + done + echo "registry.json OK ($count auditors)" - name: Resolver resolves all prompts for e2e PR env: - REGISTRY_DIR: ${{ github.workspace }} PR: '12345' run: | : > /tmp/o1.txt GITHUB_OUTPUT=/tmp/o1.txt scripts/resolve-prompts.sh >/dev/null + test -s /tmp/o1.txt || { echo "resolver wrote no output" >&2; exit 1; } echo "resolve OK" - name: Resolver is deterministic for a fixed PR env: - REGISTRY_DIR: ${{ github.workspace }} PR: '12345' run: | : > /tmp/o2.txt @@ -61,7 +68,6 @@ jobs: - name: No unresolved template markers env: - REGISTRY_DIR: ${{ github.workspace }} PR: '12345' run: | : > /tmp/o4.txt @@ -72,10 +78,11 @@ jobs: # the runner overrides that one with its own per-step file. test -s /tmp/o4.txt || { echo "resolver wrote no output" >&2; exit 1; } # Only the *resolved* prompts must be marker-free. The *_prompt_template - # outputs keep their {{PR}}/{{PROMPT_NAME}}/{{PROMPT_VERSION}} placeholders on - # purpose — that is the template Datadog Prompt Tracking records. - for out in codex_prompt arm_prompt; do + # outputs keep their {{PR}}/{{PROMPT_NAME}}/{{PROMPT_VERSION}}/{{AUDITORS}} + # placeholders on purpose — that is the template Datadog Prompt Tracking records. + for out in auditor_prompt arbiter_prompt; do resolved=$(sed -n "/^$out<&2; exit 1; } if printf '%s' "$resolved" | grep -qE '\{\{'; then echo "Unresolved template markers remain in $out:" >&2 printf '%s' "$resolved" | grep -nE '\{\{' >&2 @@ -86,7 +93,6 @@ jobs: - name: Prompt templates are include-expanded and non-empty env: - REGISTRY_DIR: ${{ github.workspace }} PR: '12345' run: | : > /tmp/o5.txt @@ -96,7 +102,7 @@ jobs: # template. They keep {{PR}}-style placeholders, but a shared-rule include # must already be expanded — an unexpanded {{@...}} would ship a template # that no longer matches the text the model actually saw. - for out in codex_prompt_template arm_prompt_template; do + for out in auditor_prompt_template arbiter_prompt_template; do tmpl=$(sed -n "/^$out<&2; exit 1; } if printf '%s' "$tmpl" | grep -qE '\{\{@'; then @@ -107,76 +113,192 @@ jobs: done echo "templates include-expanded" - - name: Arms are distinct prompts + # Independence is only real if the auditors are the FIRST thing to read the diff. + # A pre-pass that hands the panel a list of findings makes three auditors agree + # about one model's opinion, which is the failure the panel exists to avoid. Assert + # the registry declares no prompt other than the auditor's and the arbitrator's, and + # that no prompt tells a model to read a pre-existing findings file. + - name: Nothing reviews the diff ahead of the panel run: | - # Two arms pointing at the same prompt file would report an A/B result for a - # test that has no variable in it. - total=$(jq -r '.arms | keys | length' prompts/registry.json) - distinct=$(jq -r '.arms | [.[]] | unique | length' prompts/registry.json) - test "$total" = "$distinct" || { echo "two arms resolve to the same prompt"; exit 1; } - echo "arms distinct ($total)" - - # Only the ASSIGNED arm's prompt is resolved per run, so a broken variant prompt - # would stay invisible until a PR happened to hash into it. Sweep PR numbers until - # every declared arm has been assigned at least once, and resolve each one. - - name: Every arm resolves for some PR + extra=$(jq -r 'to_entries + | map(select(.key | test("_prompt$"))) + | map(.key) + | map(select(. != "auditor_prompt" and . != "arbiter_prompt")) + | join(", ")' prompts/registry.json) + test -z "$extra" \ + || { echo "registry.json declares a pre-panel prompt: $extra" >&2; exit 1; } + if grep -rniE 'codex-findings|first[- ]pass|pre-?pass' prompts/; then + echo "a prompt references a pre-panel review pass" >&2 + exit 1 + fi + echo "no pre-panel review stage" + + # The whole point of the panel is three INDEPENDENT judgments of the same question. + # If the resolver ever parameterized the auditor prompt by seat, the seats would be + # answering subtly different questions and a disagreement would no longer mean the + # auditors disagreed. Assert the emitted auditor prompt is one fixed text. + - name: Auditor prompt is identical for every seat env: - REGISTRY_DIR: ${{ github.workspace }} - GITHUB_REPOSITORY: BiggerPockets/validate + PR: '12345' run: | - declared=$(jq -r '.arms | keys | sort | join(" ")' prompts/registry.json) - want=$(jq -r '.arms | keys | length' prompts/registry.json) - split=$(jq -r '.experiment_split_percent' prompts/registry.json) - seen=""; count=0 - # Stop as soon as every arm has been covered — each resolve costs ~0.15s, and a - # sane split hits both arms within the first handful of PR numbers. - for pr in $(seq 1 500); do - [ "$count" -eq "$want" ] && break - out=/tmp/arm-$pr.txt - # The resolver APPENDS to GITHUB_OUTPUT, so start from an empty file or a - # second resolve doubles every key and the extracted value spans both. - : > "$out" - PR=$pr GITHUB_OUTPUT=$out scripts/resolve-prompts.sh >/dev/null - arm=$(sed -n '/^assigned_arm<&2; exit 1; } - printf '%s' "$resolved" | grep -qE '\{\{' && { echo "markers left in arm $arm" >&2; exit 1; } - echo "arm $arm resolves (first seen at pr $pr)" - done - got=$(printf '%s\n' $seen | sort | tr '\n' ' ' | sed 's/ $//') - # A 0 or 100 split deliberately routes every review to one arm, so only demand - # full coverage when the split actually sends traffic both ways. - if [ "$split" -gt 0 ] && [ "$split" -lt 100 ]; then - test "$got" = "$declared" || { - echo "arms assigned over 500 PRs: '$got'; declared: '$declared'" >&2 + : > /tmp/o6.txt + GITHUB_OUTPUT=/tmp/o6.txt scripts/resolve-prompts.sh >/dev/null + # Exactly one auditor_prompt block is emitted, and it names no individual seat. + blocks=$(grep -c '^auditor_prompt<<' /tmp/o6.txt) + test "$blocks" = "1" || { echo "expected 1 auditor_prompt block, got $blocks" >&2; exit 1; } + prompt=$(sed -n '/^auditor_prompt<&2 exit 1 - } - fi - echo "arm coverage OK ($got)" + fi + done + echo "auditor prompt is seat-agnostic" - - name: Assignment is stable for a fixed PR + # The arbitrator has to know which seats to expect, or a seat that failed to + # produce a verdict reads as an absence rather than a gap. + - name: Arbitrator prompt names the panel env: - REGISTRY_DIR: ${{ github.workspace }} - GITHUB_REPOSITORY: BiggerPockets/validate PR: '12345' run: | - # A PR that changed arms between reviews would post two different review styles - # on the same PR and split its telemetry across both arms. - for i in 1 2 3; do - : > /tmp/assign-$i.txt - GITHUB_OUTPUT=/tmp/assign-$i.txt scripts/resolve-prompts.sh >/dev/null - sed -n '/^assigned_arm< /tmp/arm-only-$i.txt + : > /tmp/o7.txt + GITHUB_OUTPUT=/tmp/o7.txt scripts/resolve-prompts.sh >/dev/null + tmpl=$(sed -n '/^arbiter_prompt_template<&2; exit 1; } + resolved=$(sed -n '/^arbiter_prompt<&2; exit 1; } done - diff /tmp/arm-only-1.txt /tmp/arm-only-2.txt - diff /tmp/arm-only-2.txt /tmp/arm-only-3.txt - echo "assignment stable ($(cat /tmp/arm-only-1.txt))" + echo "arbiter prompt names every seat" + + # The arbitrator is told the panel roster through these outputs, and the quorum + # check counts against auditor_count. A malformed roster surfaces mid-review, after + # stage 1 and three audits have already been paid for. + - name: Auditors output is a well-formed panel roster + env: + PR: '12345' + run: | + : > /tmp/o8.txt + GITHUB_OUTPUT=/tmp/o8.txt scripts/resolve-prompts.sh >/dev/null + auditors=$(sed -n '/^auditors<= 2 and all(.[]; type == "string")' >/dev/null \ + || { echo "auditors output is not a JSON array of >=2 strings: $auditors" >&2; exit 1; } + count=$(sed -n '/^auditor_count<&2; exit 1; } + # Single line, so it survives being interpolated into a workflow expression. + test "$(printf '%s' "$auditors" | wc -l | tr -d ' ')" = "0" \ + || { echo "auditors output must be compact single-line JSON" >&2; exit 1; } + echo "auditors roster well-formed ($auditors)" + + # Each seat is a job of its own, so the panel's shape now lives in two files. These + # are the two ways that can go wrong, and both are invisible on inspection: + # + # - The roster and the jobs disagree. A seat in registry.json with no job never + # runs, and the arbitrator's quorum check reports a panel one seat short of the + # one it was promised. A job with no roster entry runs an audit nobody counts. + # - The seat jobs drift apart. What a disagreement between seats measures is the + # models, so the harness around them has to be identical; three copies of a + # steps block is exactly the kind of thing that drifts one seat at a time. + # Everything seat-specific belongs in the job's `env`, which is why the + # `steps:` blocks can be — and must stay — byte-identical. + - name: Seat jobs match the roster and each other + run: | + wf=.github/workflows/biggiepockets-review.yml + seats=$(jq -r '.auditors[]' prompts/registry.json) + + # A job block runs from ` :` to the next line at that indent. + job_block() { awk -v start=" $1:" ' + index($0, start) == 1 { inside = 1; next } + inside && /^ [^ ]/ { exit } + inside { print }' "$wf"; } + + for seat in $seats; do + grep -qE "^ $seat:$" "$wf" \ + || { echo "registry seat '$seat' has no job in $wf" >&2; exit 1; } + key=$(printf '%s' "$seat" | tr '[:lower:]-' '[:upper:]_') + job_block "$seat" | grep -q "MAGI_MODEL_$key" \ + || { echo "job '$seat' does not read MAGI_MODEL_$key" >&2; exit 1; } + # Byte-identical means identical including the seat name, so nothing is + # normalized away here: the seat name must not appear in the steps at all. + job_block "$seat" | sed -n '/^ steps:$/,$p' > "/tmp/steps-$seat.txt" + test -s "/tmp/steps-$seat.txt" \ + || { echo "job '$seat' has no steps block" >&2; exit 1; } + done + + first=$(printf '%s\n' $seats | head -1) + for seat in $seats; do + diff -u "/tmp/steps-$first.txt" "/tmp/steps-$seat.txt" \ + || { echo "seat job '$seat' has drifted from '$first'" >&2; exit 1; } + done + + # And the other direction: every seat job the workflow defines is on the roster. + declared=$(grep -oE 'MAGI_MODEL_[A-Z0-9_]+' "$wf" | sort -u) + for var in $declared; do + seat=$(printf '%s' "${var#MAGI_MODEL_}" | tr '[:upper:]_' '[:lower:]-') + printf '%s\n' $seats | grep -qx "$seat" \ + || { echo "$wf declares $var but '$seat' is not on the roster" >&2; exit 1; } + done + echo "seat jobs match the roster ($(printf '%s\n' $seats | wc -l | tr -d ' ') seats) and each other" + + # A reviewed PR is untrusted input, and every job that checks out the PR head has that + # checkout as its working directory — the same directory verdict.json and the + # audit-*.json globs are read from. Without a step that clears those names first, a PR + # can commit a file the review mistakes for a model's verdict and approve itself. This + # is the kind of invariant that disappears when someone adds a job by copying another + # one and trimming it, so it is asserted rather than reviewed. + - name: Every PR checkout clears the reserved handoff names + run: | + wf=.github/workflows/biggiepockets-review.yml + job_block() { awk -v start=" $1:" ' + index($0, start) == 1 { inside = 1; next } + inside && /^ [^ ]/ { exit } + inside { print }' "$wf"; } + + checked=0 + for job in $(grep -oE '^ [a-z][a-z0-9_-]*:' "$wf" | tr -d ' :'); do + block=$(job_block "$job") + printf '%s' "$block" | grep -qE 'name: Checkout (reviewed )?PR head' || continue + checked=$((checked + 1)) + printf '%s' "$block" | grep -q 'name: Clear reserved handoff filenames' \ + || { echo "job '$job' checks out the PR head without clearing reserved handoff names" >&2; exit 1; } + for reserved in verdict.json 'audit-\*.json' pr.diff ticket.json; do + printf '%s' "$block" | grep -q -- "$reserved" \ + || { echo "job '$job' does not clear $reserved" >&2; exit 1; } + done + done + test "$checked" -ge 4 \ + || { echo "expected at least 4 jobs checking out the PR head, found $checked" >&2; exit 1; } + echo "all $checked PR checkouts clear the reserved handoff names" + + - name: Resolver rejects a broken registry + run: | + # Each of these misconfigurations would otherwise surface as a confusing + # mid-review failure (or, for a one-seat panel, as a silently degraded review). + cp prompts/registry.json /tmp/registry.bak + restore() { cp /tmp/registry.bak prompts/registry.json; } + expect_fail() { + local label="$1" + if PR=1 GITHUB_OUTPUT=/tmp/reject.txt scripts/resolve-prompts.sh >/dev/null 2>&1; then + restore; echo "resolver accepted $label" >&2; exit 1 + fi + restore; echo "rejected: $label" + } + jq '.auditor_prompt = "does-not-exist"' /tmp/registry.bak > prompts/registry.json + expect_fail "auditor_prompt pointing at a missing file" + jq '.arbiter_prompt = null' /tmp/registry.bak > prompts/registry.json + expect_fail "a null arbiter_prompt" + jq '.auditors = ["caspar"]' /tmp/registry.bak > prompts/registry.json + expect_fail "a one-seat panel" + jq '.auditors = ["caspar", "caspar", "melchior"]' /tmp/registry.bak > prompts/registry.json + expect_fail "duplicate seat names" + jq '.auditors = ["Caspar", "melchior"]' /tmp/registry.bak > prompts/registry.json + expect_fail "a seat name that is not shell/artifact safe" - name: Roll bumps prompt versions (content-derived provenance) env: - REGISTRY_DIR: ${{ github.workspace }} PR: '1' run: | : > /tmp/before.txt @@ -187,10 +309,10 @@ jobs: for f in prompts/_shared/*.md; do git checkout -- "$f"; done test -s /tmp/before.txt && test -s /tmp/after.txt \ || { echo "resolver wrote no output" >&2; exit 1; } - before=$(sed -n '/^arm_prompt_version<&2 exit 1 fi - echo "shared-rule Roll bumps versions ($before -> $after)" \ No newline at end of file + echo "shared-rule Roll bumps versions ($before -> $after)" diff --git a/README.md b/README.md index 07c51b3..aaa3da0 100644 --- a/README.md +++ b/README.md @@ -4,24 +4,169 @@ Org-wide GitHub defaults and shared reusable workflows. ## Reviews by BiggiePockets -`.github/workflows/biggiepockets-review.yml` is a **reusable** workflow that runs a -two-stage AI code review on a pull request: - -1. **Codex first pass** — reviews the diff against the PR's JIRA ticket and writes findings. -2. **Claude verify & synthesize** — validates Codex's findings, reviews the diff - independently (grepping for callers/tests, factoring in the existing PR discussion), - checks the change against the ticket's acceptance criteria, and decides a single verdict. - -The **BiggiePockets** service account then submits the resulting `approve` / -`request_changes` review on the PR. If the PR has no `BIG-XXXXX` key in its title (or the -ticket can't be fetched), the review degrades gracefully to a diff-based review instead of -failing. +`.github/workflows/biggiepockets-review.yml` is a **reusable** workflow that reviews a pull +request with a **panel** of independent auditors and an arbitrator: + +1. **Context** — gathers everything the panel judges against and uploads it as one + immutable handoff: the diff, the PR itself, the **PRs it is stacked on**, the JIRA ticket + and its acceptance criteria, that ticket's **epic**, the PR's existing discussion, and any + **design references** found in those texts — plus a manifest saying which of them were + actually available. **No model reviews the code here.** +2. **Panel** — three auditors named **caspar**, **balthazar**, and **melchior** each audit + the change *independently*, running the *same* auditor prompt, and each writes its own + verdict (`approve` / `request_changes`, plus a stated confidence and its blocking + findings). Each reads the diff cold, greps for callers and tests, and factors in the + existing PR discussion and the ticket's intent. +3. **Arbitration** — one arbitrator reads all three verdicts, verifies each blocking finding + against the code, weighs the panel, and issues the single verdict that gets posted. + +**Nothing reviews the diff ahead of the panel.** The auditors are the first thing to form an +opinion about the code, and stage 1 is deliberately model-free. A pre-pass that handed the +panel a list of findings would get three auditors agreeing about one model's opinion, which +is exactly the failure a panel exists to avoid — so CI asserts no prompt reads a +pre-existing findings file and the registry declares no prompt but the auditor's and the +arbitrator's. + +```mermaid +flowchart LR + ctx["1 · context
diff · PR · stack · ticket + AC · epic
discussion · designs · manifest
(no model)"] + subgraph panel["2 · panel — one prompt, three independent jobs"] + direction TB + caspar["caspar
openai/gpt-5.6-luna"] + balthazar["balthazar
google/gemini-3.7-flash"] + melchior["melchior
z-ai/glm-5.3-flash"] + end + arb["3 · arbitrate
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 +``` -Codex and Claude run as separate GitHub Actions jobs. Codex uploads the reviewed commit's -diff, ticket/discussion context, and findings as a short-lived artifact; Claude downloads -that immutable handoff. If Claude is rate-limited, use **Re-run failed jobs** on the workflow -run. GitHub reruns only the Claude job, reusing the completed Codex pass instead of invoking -Codex again. +The **BiggiePockets** service account then submits that `approve` / `request_changes` review +on the PR. If the PR has no `BIG-XXXXX` key in its title (or the ticket can't be fetched), +the review degrades gracefully to a diff-based review instead of failing. + +**What makes the audits independent.** Each seat is a job of its own — `caspar`, +`balthazar`, `melchior`, each named for the seat and each written out in the workflow rather +than fanned out from a matrix. A seat runs on its own runner, can read only the stage-1 +handoff, and never another seat's output. It writes its verdict to `audit-.json` and +uploads it; the arbitration job is the first and only place in the 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. + +Everything that differs between the three jobs lives in the job's `env` — the seat name and +its model — which leaves their `steps:` blocks byte-identical. That is 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 too if the set of seat jobs stops matching the roster in +`prompts/registry.json`. + +The seats run **different models** on the identical prompt (see *Models* below). That is the +point: with the prompt held fixed, a split between seats is signal about the change, whereas +three runs of one model would mostly agree trivially and leave the arbitrator nothing to +weigh. + +**Arbitration weighs, it does not tally.** 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 it was raised; and unanimity that the evidence doesn't support is +overruled with an explanation. The arbitrator may confirm or discard what the panel raised +but may not introduce a blocking issue no auditor found — it arbitrates rather than becoming +a fourth reviewer. The posted summary closes with a `Panel:` line recording how the seats +split and where the arbitrator overrode them. + +**Retries.** Stage 1 uploads the reviewed commit's whole context as one short-lived +artifact; every later job downloads that immutable handoff, so all three auditors +judge the exact same commit even if the PR is pushed to mid-review. If a seat is rate-limited, +use **Re-run failed jobs** on the workflow run: GitHub re-runs only the failed seats, reusing +the context stage that already succeeded. A seat that produces +no valid verdict is recorded as failed rather than counted as an approval, and arbitration +requires a **majority of seats** to have reported — below that it fails loudly instead of +quietly downgrading to a one-reviewer review. + +Requiring only a majority is what makes a tie possible: three binary verdicts can only land +3-0, 2-1, 1-2, or 0-3, but if one seat fails, arbitration runs over the two that reported and +those two can split 1-1. That is the case where the arbitrator decides alone, so it is tagged +`arbiter_followed_panel:no_majority` rather than scored as overriding a majority that never +existed. The alternative — demanding all three seats — would let one rate-limited seat fail +an entire review. + +### What the panel is given + +Stage 1 gathers context and nothing else — it never reads the diff to form a view of it. Each +source becomes one file in the handoff every seat downloads: + +| 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, and the 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 from the PR title: summary, description, acceptance criteria, status, type, and its parent's key | +| `epic.json` | That parent — where the wider goal usually lives, when a ticket's own description reads as a fragment | +| `conversations.json` | The PR's existing discussion: 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 | + +A source that isn't there is recorded as `{"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 exists so an auditor can tell *"there is no epic +for this work"* from *"the epic fetch failed"* — both otherwise read as silence, which is how +a review ends up judging intent against nothing and not saying so. The prompts require an +auditor to name context it did not have and what it could not judge without it. + +**Stacked PRs.** When a PR's base isn't the default branch, stage 1 walks down the stack — +base branch to the PR whose head is that branch, until it reaches the default branch — and +records each ancestor's number, title, body, state, ticket key, commit headlines, and touched +paths. This matters because `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 still under review. +An auditor not told that misreads a stacked PR in both directions: it blames this PR for what +an ancestor introduced, and it reports as missing the work an ancestor already did. The +prompts rule both out, and instead direct the panel at the questions that are 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 often carry a +sibling ticket in the same epic, and satisfying that ticket is not this PR's job. + +The walk is capped at ten levels and refuses to revisit a branch, so a cycle left behind by a +retargeted branch can't hang a review. A base that is neither the default branch nor any PR's +head — a long-lived 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 the knowledge 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, because attribution is what makes *"a human reviewer +objected and it was never resolved"* legible to the panel. + +### Models + +Every model is an OpenRouter slug with a default pinned in the workflow, overrideable per repo +by setting the matching **Actions variable** (Settings → Actions → Variables) in the calling +repo. An unset or empty variable keeps the default. Stage 1 runs no model, so it has no entry. + +| Stage | Variable | Default | +| --- | --- | --- | +| caspar | `MAGI_MODEL_CASPAR` | `openai/gpt-5.6-luna` | +| balthazar | `MAGI_MODEL_BALTHAZAR` | `google/gemini-3.7-flash` | +| melchior | `MAGI_MODEL_MELCHIOR` | `z-ai/glm-5.3-flash` | +| Arbitration | `ARBITER_MODEL` | `openai/gpt-5.6-sol` | + +There is no fallback model, and no seat can inherit another's. Adding a seat is deliberately +a three-part change — roster entry, job, model — with each part failing on its own terms if +you skip it: a roster entry with no job fails CI, and a job whose model expression resolves to +nothing fails that job at its `Confirm seat model` step. What none of them do is let the seat +run, because a seat quietly sharing another seat's model is a panel that has lost a voice +while still reporting three verdicts. + +Changing a seat's model never touches the prompt — the auditor prompt is seat-agnostic by +construction, and CI asserts it. The review logic lives centrally in this repo. Each consuming repo only adds a thin **caller** workflow that owns the triggers and gating and delegates to this one. @@ -32,11 +177,12 @@ Do this once per repo you want BiggiePockets to review. #### 1. Install the Claude GitHub app -The Claude verification stage uses [`anthropics/claude-code-action`](https://github.com/anthropics/claude-code-action). +The auditor and arbitration stages use [`anthropics/claude-code-action`](https://github.com/anthropics/claude-code-action) +as their harness, pointed at OpenRouter model slugs. Install the official [Claude GitHub app](https://github.com/apps/claude) for the organization -once; you do **not** need to reinstall it per repo. Claude's model requests authenticate -through OpenRouter using the shared `OPENROUTER_API_KEY` described below, so no personal -Claude Code OAuth token is required. +once; you do **not** need to reinstall it per repo. Model requests authenticate through +OpenRouter using the shared `OPENROUTER_API_KEY` described below, so no personal Claude Code +OAuth token is required. #### 2. Add the caller workflow @@ -83,19 +229,19 @@ jobs: #### 3. Make the secrets available The reusable workflow consumes several secrets via `secrets: inherit`: credentials for the -the AI review provider (`OPENROUTER_API_KEY`, shared by both the Codex and Claude stages), -an Atlassian email + API token to fetch the PR's JIRA ticket for intent, and a personal access -token for the BiggiePockets service account that submits the review. Configure them as -**organization secrets** (recommended — set once, available to every repo) or as per-repo -secrets if you prefer to scope them. +AI review provider (`OPENROUTER_API_KEY`, shared by all three seats and arbitration), an +Atlassian email + API token to fetch the PR's JIRA ticket for +intent, and a personal access token for the BiggiePockets service account that submits the +review. Configure them as **organization secrets** (recommended — set once, available to +every repo) or as per-repo secrets if you prefer to scope them. It also reports per-review traces to the `biggiepockets-review` app in Datadog LLM -Observability via `secrets.DATADOG_API_KEY`: verdict, timing, prompt template and version -(tracked as prompts, see below), the model each -stage ran (`CODEX_MODEL`/`CLAUDE_MODEL` env vars in the workflow — both are OpenRouter -model slugs and must be set), and the actual findings text from Codex and the summary Claude wrote, -so review quality is inspectable, not just counted. This secret is optional — reviews still -run and post normally without it, but no metrics are reported. +Observability via `secrets.DATADOG_API_KEY`: the arbitrated verdict, how the panel split, +each seat's own verdict and confidence, timing per stage, prompt template and version +(tracked as prompts, see below), the model each stage ran, and the summary the arbitrator +wrote — so review quality is inspectable, not just counted. This +secret is optional; reviews still run and post normally without it, but no metrics are +reported. The exact secret names each step expects are visible in the `env:` and `with:` blocks of [`.github/workflows/biggiepockets-review.yml`](.github/workflows/biggiepockets-review.yml). @@ -122,6 +268,12 @@ write access. ### Using it +If your repo lists a review job as a **required status check**, the job names are `context`, +one per seat (`caspar`, `balthazar`, `melchior`), and `arbitrate`. Two of those changed: stage +1 was once named `codex`, and the seats were once matrix legs reported as `audit (caspar)` and +friends. A branch protection rule still requiring `codex` or `audit (…)` will block merges +forever, since no job by those names runs any more. + Once installed, trigger a review either way: - **Request a review** — add **BiggiePockets** as a reviewer on the PR. The workflow fires @@ -131,43 +283,40 @@ Once installed, trigger a review either way: workflow_dispatch** and pass the PR number. (Available once the caller file is on the repo's default branch.) -### Prompt registry and the prompt A/B test +### Prompt registry -The review-stage prompts are not inline in the workflow. They live in this repo under -`prompts/` and are resolved at runtime by `scripts/resolve-prompts.sh`: +The prompts are not inline in the workflow. They live in this repo under `prompts/` and are +resolved at runtime by `scripts/resolve-prompts.sh`: ``` prompts/ - registry.json # arms + control arm + split + codex prompt - codex-first-pass.md # Stage 1 prompt (template) - claude-synthesize.md # Stage 2 control arm (template) - claude-synthesize-thesis-first.md # Stage 2 thesis-first arm (template) + registry.json # which prompt runs at each stage + the panel seats + magi-audit.md # stage 2 prompt — every seat runs this one text (template) + magi-arbitrate.md # stage 3 prompt (template) _shared/{completeness,privacy,migration-data,perf,parsing,navigation}-rules.md # shared rule blocks ``` - **Templates + shared blocks.** Each prompt references the shared rule blocks via - `{{@prompts/_shared/.md}}`, so the Codex and Claude prompts can never drift out of - sync. Prompts resolve `{{PR}}`, `{{PROMPT_NAME}}`, `{{PROMPT_VERSION}}` too. + `{{@prompts/_shared/.md}}`, so one edit updates every prompt that enforces that + rule. Prompts also resolve `{{PR}}`, `{{PROMPT_NAME}}`, `{{PROMPT_VERSION}}`, and + `{{AUDITORS}}` (the seat list, which the arbitrator needs so a missing verdict reads as a + gap rather than an absence). +- **One auditor prompt, three seats.** The resolver emits `auditor_prompt` **once** and + every seat runs that same text — nothing in it is parameterized by seat. That is what + makes a disagreement between seats meaningful: the seats are answering the same question, + so the difference is the model and the judgment, not the wording. CI asserts the auditor + prompt names no seat. - **Content-derived versions.** `prompt_version` is a content hash of the template plus the shared blocks it includes — it changes only when that prompt's text changes, not per PR - or per arm, so Datadog LLM Obs can attribute quality to the exact prompt text that ran. -- **One arm per pull request.** Two Claude prompts sit in the registry — `control` and - `thesis-first` — and each review runs exactly one of them. `scripts/resolve-prompts.sh` - hashes `:` into a bucket 0-99 and assigns the PR to the experiment arm when - that bucket falls under `experiment_split_percent` (50 today), so the arms split traffic - evenly and every review costs a single Claude pass. Assignment is a pure function of - repo and PR number: re-running a review reuses the same arm, and one PR never sees two - review styles. -- **The assigned arm decides.** Whichever arm a PR draws writes the posted summary *and* - the approve / request-changes verdict. No separate control gate holds the decision back, - which is the tradeoff for one pass per review: an experiment prompt affects real review - outcomes on its share of PRs. Set `experiment_split_percent` to `0` to route every - review to `control_arm` without removing the arm. The arm is never named in the review - comment — a reviewer who knows which prompt wrote a summary can't judge it blind. -- **Comparison is between PRs, not within one.** No PR is reviewed twice, so there is no - paired A/B to diff on a single PR. Compare the arms by grouping Datadog on `arm` across - many reviews — verdict rate, latency, tokens. That needs volume before it means - anything; a gap in approve rate over a dozen PRs is noise. + and not per seat. One `auditor_prompt_version` therefore covers all three audit spans, and + Datadog LLM Obs can attribute quality to the exact prompt text that ran. +- **Panel seats.** `registry.json` lists the seats under `auditors`. That roster is what + tells the arbitrator how many verdicts to expect, so a seat that failed reads as a gap + rather than an absence. Each seat also has a job of its own in the workflow, which means + adding or removing one is a change in both files — CI fails if they disagree. The resolver + rejects a panel of fewer than two seats, duplicate seat names, and seat names that aren't + safe as artifact names, file names, and env-var suffixes. Seat models are set in the + workflow, never here (see *Models* above). - **Prompt Tracking.** Every LLM span carries the prompt that produced it under `meta.input.prompt` — the registry template with its `{{PR}}`-style placeholders intact, plus the values that filled them as `variables`, plus `id`/`name`/`version`. Keeping the @@ -176,38 +325,46 @@ prompts/ volume, latency, tokens, and a version diff per prompt, and any span can be replayed in the Playground with its exact template and variables. A version starts when the prompt text changes (a Roll), since `version` is the same content hash reported as a tag. -- **Datadog.** Each review is one trace, tagged with the arm that ran it: - - ``` - biggiepockets.review → codex.review, claude.synthesize - ``` - - A tag key resolves to one value per submitted payload, so an `arm` tag is only - trustworthy while a payload carries a single arm — which it does by construction now - that one arm runs per review. Tags include `arm`, `arm_role` (`control`/`experiment`), - `prompt_name`, `prompt_version`, `verdict`, `assignment_bucket`, and - `experiment_split_percent`. Recording the bucket and the split keeps an assignment - auditable: changing the split later can't rewrite what an already-recorded review ran - under. A stable `run_id` (`repo-pr-runid`) joins offline evals and panel ratings to the - exact review. - - Spans carrying an `arm_agreement`, `experiment_verdict`, or `label_assignment` tag came - from an earlier setup and are not comparable to these; exclude them when grouping by - arm. - -**Registry operations** (kept distinct so a formatting experiment can't silently change the -production prompt): +- **Datadog.** Each review is one trace: a `biggiepockets.review` root with one + `magi.audit.` child per seat and one `magi.arbitrate` child — the shape of the + diagram above, minus stage 1, which runs no model and so has no LLM span. + + A seat's own verdict, confidence, model, and blocking-finding count live on that seat's + span and on `auditor_verdict.` / `auditor_confidence.` / + `auditor_model.` / `auditor_blocking_findings.` tags, so a disagreement is + queryable without opening the run. Panel-level tags carry the outcome: `verdict` (what was + posted), `panel_agreement` (`unanimous_approve` / `unanimous_request_changes` / `split`), + `panel_approve`, `panel_request_changes`, `panel_seats_reporting` / + `panel_seats_declared`, and `arbiter_followed_panel` (`true` / `false` / `no_majority`). + A stable `run_id` (`repo-pr-runid`) joins offline evals and panel ratings to the exact + review. + + **The two numbers worth watching** are `panel_agreement` and `arbiter_followed_panel`. + If the panel is nearly always unanimous, the seats aren't adding independent information + and the extra seats aren't earning their cost — diversify the models. If the arbitrator + never departs from the majority it is a vote counter, and if it always does it is ignoring + the panel; either way the arbitration prompt is the thing to fix. + + Spans carrying `arm`, `arm_role`, `arm_agreement`, `experiment_verdict`, or + `label_assignment` tags came from the earlier A/B setup and are not comparable to these; + neither are spans carrying `codex_model` or a `codex.review` child, which came from a + pre-panel first pass that no longer runs. Exclude both when querying. + +**Registry operations:** - **Roll** — edit a prompt or shared-rule file; its content-derived `prompt_version` bumps. -- **Apply** — point `control_arm` or an arm at a different stored prompt in - `registry.json`, or change `experiment_split_percent` (no version change). Callers tracking `@main` pick the change up on their next run. A - caller that pins `uses:` to a tag or SHA needs BOTH that `@ref` and its `registry_ref` - input bumped in lockstep — Apply owns that ref-bump explicitly. -- **Split** — add a new arm entry in `registry.json` + its prompt file, and give it a - share of traffic via `experiment_split_percent`. -- **Merge** — fold a variant's content into another prompt and remove the arm. +- **Apply** — point `auditor_prompt` or `arbiter_prompt` at a different + stored prompt (no version change). Callers tracking `@main` pick the change up on their + next run. A caller that pins `uses:` to a tag or SHA needs BOTH that `@ref` and its + `registry_ref` input bumped in lockstep — Apply owns that ref-bump explicitly. +- **Seat** — add or remove a name in `auditors`, **and** the matching job in + `.github/workflows/biggiepockets-review.yml` with its `MAGI_MODEL_`. CI fails if the + roster and the seat jobs disagree, or if the new job's `steps:` block isn't byte-identical + to the others. A `validate-prompts.yml` workflow guards the registry: it fails a PR if a template has -dangling includes, `registry.json` references a missing prompt, two arms point at the -same prompt, an arm's prompt fails to resolve, arm assignment isn't stable for a fixed PR, -the resolver isn't deterministic, or a shared-rule edit doesn't bump versions. +dangling includes, `registry.json` references a missing prompt, the auditor prompt names a +seat, the arbitrator prompt doesn't name every seat, the roster isn't a well-formed list of +at least two safe unique names, the seat jobs don't match the roster and each other, a broken +registry resolves instead of erroring, the resolver isn't deterministic for a fixed PR, or a +shared-rule edit doesn't bump versions. diff --git a/prompts/claude-synthesize-thesis-first.md b/prompts/claude-synthesize-thesis-first.md deleted file mode 100644 index ed4557a..0000000 --- a/prompts/claude-synthesize-thesis-first.md +++ /dev/null @@ -1,64 +0,0 @@ -Synthesize a single review decision for pull request #{{PR}}. - -1. Read ticket.json. If "available" is true, treat its summary and description as the - intended behavior, and its acceptance_criteria (Atlassian Document Format JSON) as - GUIDELINES rather than a hard contract. The AC describe the minimum the ticket set out to - achieve, not the ceiling on what counts as correct: don't reject a change merely for not - matching them verbatim. Judge whether the change satisfies the ticket's intent, and weigh - git history and the PR discussion (steps 3-4) as more authoritative evidence of what was - actually meant. -2. Read Codex's first-pass findings in codex-findings.md (if missing or empty, proceed - with your own review). -3. Read conversations.json: the PR's existing discussion (issue_comments, review_comments - with file/line, and prior reviews). Factor this context into your review: respect - decisions already settled in the thread, don't re-raise concerns the author has already - addressed or that were explicitly accepted, and weigh unresolved objections raised by - human reviewers. Treat the conversation as more reliable than the ticket's literal - acceptance criteria: if the thread shows the work was refined or explicitly accepted - beyond the AC in a sound way, honor that. -4. Independently review the diff in pr.diff. Do NOT rely on the hunks alone: use `git log` / - `git show` on the branch to inspect commit history and grep for callers and related tests - to judge impact. - When the diff adds or changes image assets, check that they are provided at retina - quality (e.g. a 2x asset or an `srcset`/`@2x` variant); flag raster images that ship - only at 1x where a high-DPI version is expected. {{@prompts/_shared/migration-data-rule.md}} -5. Enforce task completeness and reject incomplete tasks, half-measures, placeholders, or - deferred work per these rules as blocking issues: - {{@prompts/_shared/completeness-rules.md}} -6. Enforce these BiggerPockets member-privacy rules and treat a genuine violation as a - blocking issue: - {{@prompts/_shared/privacy-rules.md}} -7. Check for batch/task performance hot spots per these rules — an N+1 or O(n^2)+ - pattern on a full-table #process/#collection is a genuine finding, not a nitpick; - flag it with a suggested fix: - {{@prompts/_shared/perf-rules.md}} -8. Check how the diff parses structured values, per these rules: - {{@prompts/_shared/parsing-rules.md}} -9. Check that in-app navigational links use React Router's Link rather than a raw `` - tag, per these rules: - {{@prompts/_shared/navigation-rules.md}} -10. Validate which of Codex's findings are real (discard false positives), add any genuine - issues Codex missed, and (when a ticket is available) judge genuine misses of the ticket's - intent or clear scope creep — but give credit when the author went beyond the literal - acceptance criteria in a sound way rather than flagging it as non-compliant. -11. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one - genuine, blocking issue (such as a bug, regression, privacy violation, half-finished task, - placeholder, or deferred work); otherwise "approve". A change that exceeds the AC without - breaking the ticket's intent is a reason to approve, not to block. - -Write a file named verdict.json in the current working directory with EXACTLY this shape: - {"verdict": "approve" | "request_changes", "summary": ""} - -Write the summary in a THESIS-FIRST structure: - -- OPEN with a 1-3 sentence thesis paragraph stating the verdict and the single most - important reason, e.g. "Requesting changes: the schema migration also updates data and - should be a one-off rake task — see details below." -- Follow with ONE short supporting paragraph per finding. Each paragraph explains, in a - sentence or two: the file/line, why it matters, and the suggested direction. If there is - nothing to change, say so in the thesis paragraph and briefly note you have no blocking - findings. -- Cover the same substance a normal review would (which Codex findings you confirmed, - anything you added, how the change measures against the ticket, how the existing PR - discussion informed your decision) but prioritized and compressed: the thesis first, then - details. Target roughly 150-200 words total, always leading with the verdict. \ No newline at end of file diff --git a/prompts/claude-synthesize.md b/prompts/claude-synthesize.md deleted file mode 100644 index 14ea29e..0000000 --- a/prompts/claude-synthesize.md +++ /dev/null @@ -1,53 +0,0 @@ -Synthesize a single review decision for pull request #{{PR}}. - -1. Read ticket.json. If "available" is true, treat its summary and description as the - intended behavior, and its acceptance_criteria (Atlassian Document Format JSON) as - GUIDELINES rather than a hard contract. The AC describe the minimum the ticket set out to - achieve, not the ceiling on what counts as correct: don't reject a change merely for not - matching them verbatim. Judge whether the change satisfies the ticket's intent, and weigh - git history and the PR discussion (steps 3-4) as more authoritative evidence of what was - actually meant. -2. Read Codex's first-pass findings in codex-findings.md (if missing or empty, proceed - with your own review). -3. Read conversations.json: the PR's existing discussion (issue_comments, review_comments - with file/line, and prior reviews). Factor this context into your review: respect - decisions already settled in the thread, don't re-raise concerns the author has already - addressed or that were explicitly accepted, and weigh unresolved objections raised by - human reviewers. Treat the conversation as more reliable than the ticket's literal - acceptance criteria: if the thread shows the work was refined or explicitly accepted - beyond the AC in a sound way, honor that. -4. Independently review the diff in pr.diff. Do NOT rely on the hunks alone: use `git log` / - `git show` on the branch to inspect commit history and grep for callers and related tests - to judge impact. - When the diff adds or changes image assets, check that they are provided at retina - quality (e.g. a 2x asset or an `srcset`/`@2x` variant); flag raster images that ship - only at 1x where a high-DPI version is expected. {{@prompts/_shared/migration-data-rule.md}} -5. Enforce task completeness and reject incomplete tasks, half-measures, placeholders, or - deferred work per these rules as blocking issues: - {{@prompts/_shared/completeness-rules.md}} -6. Enforce these BiggerPockets member-privacy rules and treat a genuine violation as a - blocking issue: - {{@prompts/_shared/privacy-rules.md}} -7. Check for batch/task performance hot spots per these rules — an N+1 or O(n^2)+ - pattern on a full-table #process/#collection is a genuine finding, not a nitpick; - flag it with a suggested fix: - {{@prompts/_shared/perf-rules.md}} -8. Check how the diff parses structured values, per these rules: - {{@prompts/_shared/parsing-rules.md}} -9. Check that in-app navigational links use React Router's Link rather than a raw `` - tag, per these rules: - {{@prompts/_shared/navigation-rules.md}} -10. Validate which of Codex's findings are real (discard false positives), add any genuine - issues Codex missed, and (when a ticket is available) judge genuine misses of the ticket's - intent or clear scope creep — but give credit when the author went beyond the literal - acceptance criteria in a sound way rather than flagging it as non-compliant. -11. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one - genuine, blocking issue (such as a bug, regression, privacy violation, half-finished task, - placeholder, or deferred work); otherwise "approve". A change that exceeds the AC without - breaking the ticket's intent is a reason to approve, not to block. - -Write a file named verdict.json in the current working directory with EXACTLY this shape: - {"verdict": "approve" | "request_changes", "summary": ""} -In the summary, note which of Codex's findings you confirmed, anything you added, how the -change measures against the ticket, how the existing PR discussion informed your decision, -and the rationale for the decision. \ No newline at end of file diff --git a/prompts/codex-first-pass.md b/prompts/codex-first-pass.md deleted file mode 100644 index a490f53..0000000 --- a/prompts/codex-first-pass.md +++ /dev/null @@ -1,38 +0,0 @@ -Perform a first-pass code review of this pull request and output a concise -markdown findings report. Your final message IS the report — it is captured -verbatim and handed to a second reviewer, so do not add conversational preamble. - -1. Read ticket.json. If "available" is true, treat its summary as the intended behavior and - its acceptance_criteria as GUIDELINES, not a hard contract — they describe the minimum the - ticket set out to achieve, not the ceiling on what counts as correct. Do not flag a change - just because it doesn't match the acceptance criteria verbatim: first judge whether the - change actually satisfies the ticket's intent, and give particular credit when the author - went beyond the literal criteria (see step 2 for how to weigh that against history and - context). Only treat a deviation as a blocker if it genuinely fails the ticket's purpose. - If the ticket is not available, do a diff-based review. -2. Review the diff in pr.diff. Do NOT rely on the hunks alone: use `git log` / `git show` on - the branch and grep the repository for callers and related tests to judge impact and - intent. Git history and the PR's context are usually more reliable evidence of what was - meant than the ticket's literal acceptance criteria: when a commit, the diff, or the - conversation shows the work was refined or extended beyond the AC in a sound way, treat - that as an improvement rather than scope creep. -3. Enforce task completeness and reject incomplete tasks, half-measures, placeholders, or - deferred work per these rules as blocking issues: - {{@prompts/_shared/completeness-rules.md}} -4. Enforce these BiggerPockets member-privacy rules and flag any violation with a - file/line reference: - {{@prompts/_shared/privacy-rules.md}} -5. {{@prompts/_shared/migration-data-rule.md}} -6. Check the diff for batch/task performance hot spots per these rules, and flag each - genuine one with a file/line reference and a suggested fix: - {{@prompts/_shared/perf-rules.md}} -7. Check how the diff parses structured values, per these rules: - {{@prompts/_shared/parsing-rules.md}} -8. Check that in-app navigational links use React Router's Link rather than a raw `` - tag, per these rules: - {{@prompts/_shared/navigation-rules.md}} -9. Report concrete issues — bugs, regressions, security problems, member-privacy - violations, incomplete tasks/half-measures/placeholders/deferred work, and genuine misses - of the ticket's intent or clear scope creep — each with a file/line reference and a brief - rationale. Do not list "doesn't match acceptance criteria" as an issue by itself; only - raise it when the deviation harms the intent. If nothing is blocking, say so briefly. \ No newline at end of file diff --git a/prompts/magi-arbitrate.md b/prompts/magi-arbitrate.md new file mode 100644 index 0000000..5a64414 --- /dev/null +++ b/prompts/magi-arbitrate.md @@ -0,0 +1,59 @@ +You are the arbitrator for the review of pull request #{{PR}}. Three auditors — +{{AUDITORS}} — audited this change independently, without seeing each other's work. Each +wrote a verdict. Your job is to weigh those verdicts against the evidence and issue the ONE +verdict that gets posted as the review. Yours is the only output that reaches the pull +request. + +1. Read every audit verdict: for each auditor in {{AUDITORS}} there is a file + `audit-.json` in the current working directory, with this shape: + {"verdict", "confidence", "blocking_findings": [{"file", "line", "issue"}], "summary"} + If one of those files is missing or unparseable, say so explicitly in your summary and + arbitrate over the verdicts you do have — never silently treat a missing auditor as an + approval. +2. Read the evidence the auditors worked from, all of it in the current working directory: + context-manifest.json (what was gathered and what was unavailable), pr.diff, pr.json, + stack.json, ticket.json (its acceptance_criteria are guidelines, not a hard contract), + epic.json, conversations.json and designs.json (references only — they are not fetched, so + a design you cannot open is never itself a finding). When stack.json says "stacked" is + true, pr.diff is this PR's own changes on top of unmerged ancestor PRs: discard a finding + that belongs to an ancestor's code, and discard "this is missing" when an ancestor's files + or commits already did it. +3. Verify each distinct blocking finding against the code yourself. Use `git log` / `git + show` on the branch and grep for callers and tests. Findings often overlap: treat two + auditors describing the same defect at the same location as ONE finding raised twice, not + two findings. +4. Weigh, do not tally. A verdict's weight comes from its evidence, not from how many + auditors share it: + - A single blocking finding you can confirm in the code outweighs any number of approvals + that simply did not notice it. + - A blocking finding you can disprove — the auditor misread the code, the concern was + already settled in the PR discussion, or the "missing" work is present elsewhere in the + diff — is discarded no matter how confidently it was raised. + - A stated confidence of "low" is a reason to verify that auditor's reasoning yourself + before it moves your verdict, in either direction. + - Unanimity is not proof. If all three agree and the evidence does not support them, follow + the evidence and explain why you departed. +5. Decide ONE verdict. Use "request_changes" only when at least one blocking issue survives + your verification; otherwise "approve". Be pragmatic: a change that exceeds the ticket's + acceptance criteria without breaking its intent is a reason to approve, not to block. +6. Arbitrate; do not re-review. You may confirm or discard what the auditors raised, but do + not introduce a blocking issue no auditor raised. If you spot one, record it in the summary + as a non-blocking observation for the author and leave the verdict to the findings the + panel actually surfaced. + +Write a file named verdict.json in the current working directory with EXACTLY this shape: + {"verdict": "approve" | "request_changes", "summary": ""} + +Structure the summary for the pull request author: + +- OPEN with a 1-3 sentence thesis stating the verdict and the single most important reason. +- Then one short paragraph per surviving blocking finding: the file/line, why it matters, and + the suggested direction. If nothing survived, say so. +- CLOSE with a short "Panel" line recording how the auditors split and where you overrode + them, e.g. "Panel: 2 approve, 1 request changes. Sustained melchior's finding on the + unbatched query; discarded caspar's nil-guard concern, which the diff already handles at + app/models/foo.rb:41." +- Cover how the change measures against the ticket and how the existing PR discussion + informed the outcome. Aim for roughly 200-250 words. + +Do not name which model sat in which seat, and do not write any other file. diff --git a/prompts/magi-audit.md b/prompts/magi-audit.md new file mode 100644 index 0000000..2065cae --- /dev/null +++ b/prompts/magi-audit.md @@ -0,0 +1,107 @@ +You are one of three independent auditors reviewing pull request #{{PR}}. Audit the +change on its own merits and reach your own verdict. You have no visibility into the +other auditors and must not speculate about what they concluded — a separate arbitrator +weighs all three verdicts afterwards. Your job is an honest, self-contained judgment, +not a guess at consensus. No one has reviewed this diff before you: every issue in your +verdict is one you found yourself. + +Everything you need was gathered for you and sits in the current working directory. + +1. Start with context-manifest.json. It names every file that was gathered and which + sources were unavailable and why. Read it before anything else so you know what you are + working with: a source that isn't there is a gap in the evidence, never a defect in the + change. Do not fetch anything yourself to fill a gap — say in your summary that you + judged without it. +2. Read ticket.json. If "available" is true, treat its summary and description as the + intended behavior, and its acceptance_criteria (Atlassian Document Format JSON) as + GUIDELINES rather than a hard contract. The AC describe the minimum the ticket set out to + achieve, not the ceiling on what counts as correct: don't reject a change merely for not + matching them verbatim. Judge whether the change satisfies the ticket's intent, and weigh + git history and the PR discussion (steps 4-5) as more authoritative evidence of what was + actually meant. +3. Read epic.json: the parent the ticket belongs to, where the wider goal usually lives. + Use it to judge whether the change moves that goal forward, and to make sense of a + ticket whose own description reads as a fragment. An epic is context, not a contract — + never block a change for work that plainly belongs to a sibling ticket. +4. Read pr.json: the PR's title and body, labels, commit messages, and the files it + touches with their line counts. The body is usually where the author states the scope + they took on and any follow-up they deliberately left out; weigh that statement against + the ticket rather than ignoring it. +5. Read stack.json. If "stacked" is true, this PR sits on top of one or more pull requests + that are themselves unmerged, listed nearest-first in "ancestors". This changes what you + are judging: pr.diff is ONLY this PR's own changes, measured against its base branch, so + the base already contains the ancestors' work. + - Never raise a finding about code an ancestor introduced. It is under review in its own + PR, and the same objection filed twice against two PRs is noise. + - Never report as missing something an ancestor already did. Check its "files" and + commit headlines before concluding a piece of the ticket was skipped. + - Do judge this PR's fit with the stack: work that duplicates or contradicts an ancestor, + an interface used here that the ancestor does not provide, or a change that only makes + sense if an ancestor is merged first and does not say so. + - Judge this PR against ITS ticket. Ancestors often carry a different ticket in the same + epic ("ticket_key"), and satisfying a sibling ticket is not this PR's job. +6. Read conversations.json: the PR's existing discussion (issue_comments, review_comments + with file/line, and prior reviews). Factor this context into your review: respect + decisions already settled in the thread, don't re-raise concerns the author has already + addressed or that were explicitly accepted, and weigh unresolved objections raised by + human reviewers. Treat the conversation as more reliable than the ticket's literal + acceptance criteria: if the thread shows the work was refined or explicitly accepted + beyond the AC in a sound way, honor that. +7. Read designs.json: design references (Figma and other design tools, screenshots, + walkthrough videos, ticket attachments) harvested from those texts. They are recorded, + NOT fetched — you cannot open them and must not try. What they tell you is that a design + exists for this work, which matters when the diff changes UI: judge the change against + what the ticket and discussion say about that design, and never treat a reference you + cannot open as a defect. +8. Review the diff in pr.diff. Do NOT rely on the hunks alone: use `git log` / + `git show` on the branch to inspect commit history and grep for callers and related tests + to judge impact. + When the diff adds or changes image assets, check that they are provided at retina + quality (e.g. a 2x asset or an `srcset`/`@2x` variant); flag raster images that ship + only at 1x where a high-DPI version is expected. {{@prompts/_shared/migration-data-rule.md}} +9. Enforce task completeness and reject incomplete tasks, half-measures, placeholders, or + deferred work per these rules as blocking issues: + {{@prompts/_shared/completeness-rules.md}} +10. Enforce these BiggerPockets member-privacy rules and treat a genuine violation as a + blocking issue: + {{@prompts/_shared/privacy-rules.md}} +11. Check for batch/task performance hot spots per these rules — an N+1 or O(n^2)+ + pattern on a full-table #process/#collection is a genuine finding, not a nitpick; + flag it with a suggested fix: + {{@prompts/_shared/perf-rules.md}} +12. Check how the diff parses structured values, per these rules: + {{@prompts/_shared/parsing-rules.md}} +13. Check that in-app navigational links use React Router's Link rather than a raw `` + tag, per these rules: + {{@prompts/_shared/navigation-rules.md}} +14. When a ticket is available, judge genuine misses of the ticket's intent or clear scope + creep — but give credit when the author went beyond the literal acceptance criteria in a + sound way rather than flagging it as non-compliant. +15. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one + genuine, blocking issue (such as a bug, regression, privacy violation, half-finished task, + placeholder, or deferred work); otherwise "approve". A change that exceeds the AC without + breaking the ticket's intent is a reason to approve, not to block. + +Write a file named verdict.json in the current working directory with EXACTLY this shape: + + { + "verdict": "approve" | "request_changes", + "confidence": "high" | "medium" | "low", + "blocking_findings": [ + {"file": "", "line": "", "issue": ""} + ], + "summary": "" + } + +- "blocking_findings" lists ONLY the issues you consider blocking, and must be empty when + your verdict is "approve". Non-blocking observations belong in the summary. +- "confidence" is how sure you are of your own verdict. Say "low" when the diff's impact + turned on code you could not inspect, and reserve "high" for a verdict you could defend + line by line. The arbitrator weighs this, so an inflated confidence corrupts the panel. +- In the summary, state your rationale, how the change measures against the ticket and its + epic, and how the existing PR discussion informed your decision. Name any context that + was unavailable and what you could not judge without it. + +Write the file even if you found nothing: an approve with an empty blocking_findings list is +a complete verdict. Do not write any other file, and do not post a review or comment — the +arbitrator, not you, speaks to the pull request. diff --git a/prompts/registry.json b/prompts/registry.json index 4424aef..8c0ba60 100644 --- a/prompts/registry.json +++ b/prompts/registry.json @@ -1,36 +1,41 @@ { "//": "Prompt registry config for the BiggiePockets review workflow.", "//": "", - "//": "Each review runs ONE Claude synthesize pass. Which arm's prompt it runs is", - "//": "decided per pull request: scripts/resolve-prompts.sh hashes ':' into a", - "//": "bucket 0-99, and a PR below experiment_split_percent is assigned the experiment", - "//": "arm instead of control_arm. The assigned arm writes the summary that gets posted", - "//": "AND the approve / request-changes decision, so an experiment arm's prompt does", - "//": "affect real review outcomes on its share of PRs. Set experiment_split_percent to", - "//": "0 to route every review to control_arm without deleting the arm.", + "//": "A review is a panel. Three auditors — caspar, balthazar, and melchior — each", + "//": "audit the change independently, in separate GitHub Actions jobs, running the", + "//": "SAME auditor prompt and each producing its own verdict. No auditor can see", + "//": "another's verdict: the audit outputs are uploaded per seat and are only ever", + "//": "downloaded by the arbitrator job that runs after all three finish.", "//": "", - "//": "Assignment is a pure function of repo and PR number, so re-running a review", - "//": "reuses the same arm and a PR never sees two different review styles.", + "//": "Nothing reviews the diff before the panel does. The only work that happens ahead", + "//": "of the auditors is gathering context — the diff, the PR, the JIRA ticket and its", + "//": "acceptance criteria, that ticket's epic, the PR discussion, and any design", + "//": "references found in them — so no finding is planted in front of three auditors", + "//": "whose independence is the point.", "//": "", - "//": "Comparison is BETWEEN pull requests: Datadog tags each review with its assigned", - "//": "arm, so verdict rates and latency group by arm across PRs. No PR is ever reviewed", - "//": "by two arms, which is what keeps Claude token spend at one pass per review.", + "//": "The arbitrator then reads all three verdicts, weighs them, and issues the ONE", + "//": "verdict that gets posted as the approve / request-changes review. It is the only", + "//": "stage whose output reaches the pull request.", + "//": "", + "//": "auditors lists the seats. Each seat is a job of its own in", + "//": ".github/workflows/biggiepockets-review.yml, named for the seat; this list is what", + "//": "tells the arbitrator which seats to expect, so a seat that failed to produce a", + "//": "verdict is a hard error rather than a silently smaller panel. Adding or removing a", + "//": "seat is therefore a two-part change — this list plus that workflow's job and its", + "//": "MAGI_MODEL_ — and CI fails if the two disagree. Every seat runs", + "//": "auditor_prompt verbatim: seats differ only in which model they run, which the", + "//": "workflow decides, never this file.", "//": "", "//": "prompts/.md are versioned templates; shared rule blocks live in", "//": "prompts/_shared/ and are injected via {{@path}} markers. Prompt versions are", "//": "derived from file content by scripts/resolve-prompts.sh, never listed here.", "//": "", "//": "Operations: Roll = edit a prompt file (its derived version bumps). Apply = point", - "//": "control_arm or an arm at a different prompt file (no version change). Split = add", - "//": "an arm entry + prompt file, and give it a share via experiment_split_percent.", - "//": "Merge = fold a variant into another prompt file and remove the arm. Every", - "//": "consuming repo resolves this one file, so a change here changes them all at once.", - "version": 2, - "codex_prompt": "codex-first-pass", - "control_arm": "control", - "experiment_split_percent": 50, - "arms": { - "control": "claude-synthesize", - "thesis-first": "claude-synthesize-thesis-first" - } + "//": "auditor_prompt or arbiter_prompt at a different prompt file (no version change).", + "//": "Every consuming repo resolves this one file, so a change here changes them all at", + "//": "once.", + "version": 4, + "auditor_prompt": "magi-audit", + "arbiter_prompt": "magi-arbitrate", + "auditors": ["caspar", "balthazar", "melchior"] } diff --git a/scripts/resolve-prompts.sh b/scripts/resolve-prompts.sh index b41ecfc..445f928 100755 --- a/scripts/resolve-prompts.sh +++ b/scripts/resolve-prompts.sh @@ -1,38 +1,39 @@ #!/usr/bin/env bash # resolve-prompts.sh — resolve the BiggiePockets review prompts from the registry. # -# prompts/registry.json declares the codex prompt, one or more arms (each a stored -# prompt template), the control arm, and what percentage of pull requests an -# experiment arm gets. This script picks ONE arm per review — hash(':') -# mod 100, below experiment_split_percent means the experiment arm — and emits only -# that arm's prompt. The assigned arm both posts its summary and decides the review, -# so arms are compared BETWEEN pull requests and each review costs one Claude pass. +# prompts/registry.json declares the auditor prompt, the arbitrator prompt, and the +# panel seats. A review runs the auditor prompt once per seat (caspar, balthazar, +# melchior) in independent jobs, then the arbitrator prompt once over the three verdicts +# those jobs produced. Those are the only two prompts a review runs — nothing reviews the +# diff ahead of the panel. This script emits both; the workflow decides which model runs +# in which seat. +# +# The auditor prompt is emitted ONCE and every seat runs that same text. Nothing in it +# is parameterized by seat, so the three audits differ only by model and by the +# independent judgment of the run — never by wording. # # Shared rule blocks (privacy, migration-data, perf) are stored once in # prompts/_shared/ and injected into every prompt that references them via {{@path}} -# markers, so the Codex and Claude prompts can never drift out of sync. +# markers, so one edit updates every prompt that enforces that rule. # # The resolved text substitutes late-binding placeholders used for provenance only: -# {{PR}}, {{PROMPT_NAME}}, {{PROMPT_VERSION}} +# {{PR}}, {{PROMPT_NAME}}, {{PROMPT_VERSION}}, {{AUDITORS}} # # Alongside each resolved text, the unsubstituted template (shared blocks expanded, -# those three placeholders left intact) is emitted as _prompt_template. It is -# what Datadog LLM Obs Prompt Tracking records as the prompt template, with the -# substituted values reported as its variables, so runs of one prompt group together -# instead of splitting into a new template per PR. +# those placeholders left intact) is emitted as _prompt_template. It is what +# Datadog LLM Obs Prompt Tracking records as the prompt template, with the substituted +# values reported as its variables, so runs of one prompt group together instead of +# splitting into a new template per PR. # # prompt_version is DERIVED from content, never hand-set: a hash of the template file # plus every shared block it includes. It changes if and only if that prompt's text -# changes (a Roll), stays stable across PRs, and never encodes an arm — the arm is -# reported separately so Datadog LLM Obs can hold both (prompt_name/version = source -# of truth for the text; the arm = the test condition). +# changes (a Roll) and stays stable across PRs, so Datadog can attribute a change in +# review quality to the exact prompt text that produced it. # -# Inputs (env): REGISTRY_DIR, PR, GITHUB_REPOSITORY (falls back to "unknown", which -# only changes which arm a PR lands in, never whether assignment is deterministic). -# Outputs: codex_prompt{,_name,_version,_template}; arm_prompt{,_name,_version,_template} -# for the ASSIGNED arm; assigned_arm (its key); control_arm; experiment_split_percent; -# assignment_bucket (0-99, so an assignment can be recomputed and audited from the tag -# alone). +# Inputs (env): REGISTRY_DIR, PR. +# Outputs: auditor_prompt{,_name,_version,_template}; arbiter_prompt{,_name,_version,_template}; +# auditors (JSON array of seat names — the panel roster the arbitrator is told to expect, +# one job per seat in the workflow); auditor_count. set -euo pipefail @@ -46,42 +47,49 @@ if [ ! -f "$REGISTRY_FILE" ]; then fi MAP_VERSION=$(jq -r '.version // empty' "$REGISTRY_FILE") -CODEX_NAME=$(jq -r '.codex_prompt // empty' "$REGISTRY_FILE") -CONTROL_ARM=$(jq -r '.control_arm // empty' "$REGISTRY_FILE") -SPLIT_PERCENT=$(jq -r '.experiment_split_percent // 0' "$REGISTRY_FILE") -ARMS=() -while IFS= read -r arm; do ARMS+=("$arm"); done < <(jq -r '.arms | keys[]' "$REGISTRY_FILE") +AUDITOR_NAME=$(jq -r '.auditor_prompt // empty' "$REGISTRY_FILE") +ARBITER_NAME=$(jq -r '.arbiter_prompt // empty' "$REGISTRY_FILE") -for name in "$CODEX_NAME" "$CONTROL_ARM"; do +# Validate the config so a bad edit is caught at review time instead of silently +# running a stale prompt or an undersized panel. +: "${MAP_VERSION:?registry.json missing version}" +for field in auditor_prompt arbiter_prompt; do + name=$(jq -r --arg f "$field" '.[$f] // empty' "$REGISTRY_FILE") if [ -z "$name" ]; then - echo "::error::registry.json is missing codex_prompt or control_arm" >&2 + echo "::error::registry.json is missing $field" >&2 + exit 1 + fi + if [ ! -f "$REGISTRY_DIR/prompts/$name.md" ]; then + echo "::error::$field is '$name' but prompts/$name.md does not exist" >&2 exit 1 fi done -if ! printf '%s' "$SPLIT_PERCENT" | grep -qE '^[0-9]+$' || [ "$SPLIT_PERCENT" -gt 100 ]; then - echo "::error::experiment_split_percent must be an integer 0-100, got '$SPLIT_PERCENT'" >&2 + +if ! jq -e '.auditors | type == "array"' "$REGISTRY_FILE" >/dev/null; then + echo "::error::registry.json .auditors must be an array of seat names" >&2 exit 1 fi - -# Validate the config so a bad edit is caught at review time instead of silently -# running a stale/duplicate prompt. -: "${MAP_VERSION:?registry.json missing version}" -for arm in "${ARMS[@]}"; do - prompt=$(jq -r --arg a "$arm" '.arms[$a] // empty' "$REGISTRY_FILE") - if [ -z "$prompt" ]; then - echo "::error::Arm '$arm' has no prompt mapping in prompts/registry.json" >&2 - exit 1 - fi - if [ ! -f "$REGISTRY_DIR/prompts/$prompt.md" ]; then - echo "::error::Arm '$arm' maps to '$prompt' but prompts/$prompt.md does not exist" >&2 +AUDITORS=() +while IFS= read -r seat; do AUDITORS+=("$seat"); done < <(jq -r '.auditors[]' "$REGISTRY_FILE") +if [ "${#AUDITORS[@]}" -lt 2 ]; then + echo "::error::registry.json declares ${#AUDITORS[@]} auditor(s); a panel needs at least 2" >&2 + exit 1 +fi +# Seat names become artifact names and shell identifiers downstream, and a duplicate +# would have two jobs overwrite one seat's verdict — shrinking the panel invisibly. +for seat in "${AUDITORS[@]}"; do + if ! printf '%s' "$seat" | grep -qE '^[a-z][a-z0-9-]*$'; then + echo "::error::auditor seat '$seat' must be lowercase alphanumeric/dashes" >&2 exit 1 fi done -if ! printf '%s\n' "${ARMS[@]}" | grep -qx -- "$CONTROL_ARM"; then - echo "::error::control_arm '$CONTROL_ARM' is not a declared arm" >&2 +if [ "$(printf '%s\n' "${AUDITORS[@]}" | sort -u | wc -l)" -ne "${#AUDITORS[@]}" ]; then + echo "::error::registry.json .auditors contains duplicate seat names" >&2 exit 1 fi +AUDITOR_LIST=$(printf '%s, ' "${AUDITORS[@]}"); AUDITOR_LIST="${AUDITOR_LIST%, }" + # Expand {{@path}} includes (path relative to REGISTRY_DIR), recording each included # file on _INCLUDED (de-duplicated). A marker may appear inline within a line: text # before/after the marker is preserved and the included content is always followed by @@ -90,7 +98,7 @@ fi # recursive resolver. _INCLUDED=() expand_template() { - local file="$1" out="" line rest pre rel post + local file="$1" out="" line rest pre rel while IFS= read -r line || [ -n "$line" ]; do rest="$line" while [[ "$rest" == *'{{@'* ]]; do @@ -102,9 +110,11 @@ expand_template() { exit 1 fi seen=0 - for f in "${_INCLUDED[@]:-}"; do - if [ "$f" = "$incfile" ]; then seen=1; break; fi - done + if [ "${#_INCLUDED[@]}" -gt 0 ]; then + for f in "${_INCLUDED[@]}"; do + if [ "$f" = "$incfile" ]; then seen=1; break; fi + done + fi if [ "$seen" -eq 0 ]; then _INCLUDED+=("$incfile"); fi out+="$pre" out+="$(cat "$incfile")" @@ -122,7 +132,12 @@ prompt_version() { # name -> content-derived version (template + shared includes expand_template "$REGISTRY_DIR/prompts/$name.md" > /dev/null { cat "$REGISTRY_DIR/prompts/$name.md" - for f in "${_INCLUDED[@]:-}"; do cat "$f"; done + # Test the length, not "${_INCLUDED[@]:-}": that form expands an empty array to one + # empty string, and a prompt with no {{@}} includes (the arbitrator has none) would + # `cat ""` and abort the resolve under `set -e`. + if [ "${#_INCLUDED[@]}" -gt 0 ]; then + for f in "${_INCLUDED[@]}"; do cat "$f"; done + fi } | sha256sum | cut -c1-12 } @@ -139,6 +154,7 @@ prompt_text() { # name version -> resolved text text="${text//\{\{PR\}\}/$PR}" text="${text//\{\{PROMPT_NAME\}\}/$name}" text="${text//\{\{PROMPT_VERSION\}\}/$version}" + text="${text//\{\{AUDITORS\}\}/$AUDITOR_LIST}" printf '%s' "$text" } @@ -157,31 +173,12 @@ write_output() { # name value — multiline-safe via GITHUB_OUTPUT heredoc; fixe printf '%s<<_BIGGIEPOCKETS_PROMPT_EOF_\n%s\n_BIGGIEPOCKETS_PROMPT_EOF_\n' "$name" "$value" >> "$GITHUB_OUTPUT" } -emit_prompt "codex" "$CODEX_NAME" - -# Experiment arm = the first arm that is not the control arm; there may be none (a -# single-arm registry, e.g. after the experiment is merged away). -EXPERIMENT_ARM="" -for arm in "${ARMS[@]}"; do - if [ "$arm" != "$CONTROL_ARM" ]; then EXPERIMENT_ARM="$arm"; break; fi -done - -# Assign this PR to one arm. sha256 over ":" rather than the PR number alone, -# so two repos don't hand the same arm to their matching PR numbers, and the low 32 bits -# mod 100 give the bucket. Deterministic by construction: a re-review of the same PR -# resolves the same arm, and the bucket is emitted so any assignment can be re-derived -# from telemetry without rerunning this script. -ASSIGN_KEY="${GITHUB_REPOSITORY:-unknown}:$PR" -BUCKET=$(( 16#$(printf '%s' "$ASSIGN_KEY" | sha256sum | cut -c1-8) % 100 )) -ASSIGNED_ARM="$CONTROL_ARM" -if [ -n "$EXPERIMENT_ARM" ] && [ "$BUCKET" -lt "$SPLIT_PERCENT" ]; then - ASSIGNED_ARM="$EXPERIMENT_ARM" -fi +emit_prompt "auditor" "$AUDITOR_NAME" +emit_prompt "arbiter" "$ARBITER_NAME" -write_output "control_arm" "$CONTROL_ARM" -write_output "experiment_split_percent" "$SPLIT_PERCENT" -write_output "assignment_bucket" "$BUCKET" -write_output "assigned_arm" "$ASSIGNED_ARM" -emit_prompt "arm" "$(jq -r --arg a "$ASSIGNED_ARM" '.arms[$a]' "$REGISTRY_FILE")" +# Emitted as compact JSON, on one line, so it survives interpolation into a workflow +# expression. +write_output "auditors" "$(jq -c '.auditors' "$REGISTRY_FILE")" +write_output "auditor_count" "${#AUDITORS[@]}" -echo "Resolved $MAP_VERSION prompts: codex=$CODEX_NAME assigned=$ASSIGNED_ARM (bucket $BUCKET, split ${SPLIT_PERCENT}% to ${EXPERIMENT_ARM:-none})" +echo "Resolved v$MAP_VERSION prompts: auditor=$AUDITOR_NAME arbiter=$ARBITER_NAME panel=[$AUDITOR_LIST]"