Research: shipping CSS, and a ReviewBench harness for g1t's reviewer
- docs/research/css-shipping.md: our CSS is already static and single-file; the gain left is loading it before the module preloads. - docs/research/reviewbench.md and bench/reviewbench: how the benchmark scores, where our reviewer is likely weak, and a cost-capped harness (not run).
10 files+1162−00/10 viewed
| 1 | + | .cache/ |
| 1 | + | # syntax=docker/dockerfile:1.7 | |
| 2 | + | # | |
| 3 | + | # g1t's reviewer as a ReviewBench agent image: the g1t sandbox's base image | |
| 4 | + | # (the Claude Code CLI and toolchains production reviews run with) plus the | |
| 5 | + | # contract adapter in agent/. Built by `node bench/reviewbench/run.mjs build`, | |
| 6 | + | # which passes BASE from services/runner/base.json and regenerates | |
| 7 | + | # agent/instructions.txt from crates/runner/src/review.rs first. | |
| 8 | + | # | |
| 9 | + | # linux/amd64, as the contract requires. | |
| 10 | + | ||
| 11 | + | ARG BASE | |
| 12 | + | FROM --platform=linux/amd64 ${BASE} | |
| 13 | + | COPY agent/ /bench/ | |
| 14 | + | # /work/change.diff and /work/review.json are where the production prompt | |
| 15 | + | # tells the agent to read and write; the benchmark mounts the rest of /work. | |
| 16 | + | RUN mkdir -p /work && chmod 0777 /work | |
| 17 | + | ENTRYPOINT ["node", "/bench/agent.mjs"] |
| 1 | + | // g1t's pull request reviewer behind the ReviewBench agent contract | |
| 2 | + | // (https://github.com/review-bench/ReviewBench/blob/main/AGENT_CONTRACT.md). | |
| 3 | + | // | |
| 4 | + | // Runs inside the image built from bench/reviewbench/Dockerfile, which is | |
| 5 | + | // the g1t sandbox's base image (same Claude Code CLI, same toolchains). | |
| 6 | + | // It does what crates/runner/src/review.rs does after its clone: write the | |
| 7 | + | // change to /work/change.diff, run Claude Code headless with the same | |
| 8 | + | // flags as crates/runner/src/harness.rs, read /work/review.json, and apply | |
| 9 | + | // the same filters services/work/src/reviews.rs applies when it records a | |
| 10 | + | // review. Then it writes the review's line comments as findings. | |
| 11 | + | // | |
| 12 | + | // Configuration (all optional): | |
| 13 | + | // ANTHROPIC_MODEL or RB_CONFIG_MODEL model id (production review route: claude-sonnet-5-5) | |
| 14 | + | // ANTHROPIC_BASE_URL g1t's model proxy (MODELS_URL/anthropic) or a gateway | |
| 15 | + | // ANTHROPIC_API_KEY provider key, or a model-proxy session token | |
| 16 | + | // RB_CONFIG_BUDGET_USD per-PR cost cap (--max-budget-usd), default 5 | |
| 17 | + | // RB_CONFIG_VARIANT prompt variant: "production" (default) or a file in variants/ | |
| 18 | + | ||
| 19 | + | import { spawn } from "node:child_process"; | |
| 20 | + | import { copyFileSync, existsSync, readFileSync, writeFileSync } from "node:fs"; | |
| 21 | + | ||
| 22 | + | import { buildPrompt } from "./prompt-lib.mjs"; | |
| 23 | + | ||
| 24 | + | const REPO = "/work/repo"; | |
| 25 | + | const DIFF_FILE = "/work/change.diff"; | |
| 26 | + | const REVIEW_FILE = "/work/review.json"; | |
| 27 | + | const MAX_TURNS = "80"; // harness.rs | |
| 28 | + | const MAX_REVIEW_COMMENTS = 30; // reviews.rs | |
| 29 | + | const MAX_REVIEW_CHARS = 20_000; // reviews.rs | |
| 30 | + | ||
| 31 | + | const env = (name, fallback) => process.env[name] ?? fallback; | |
| 32 | + | const out = env("RB_OUT", "/work/out/findings.json"); | |
| 33 | + | const agent = env("RB_AGENT", "g1t-agent"); | |
| 34 | + | const pr = JSON.parse(readFileSync(env("RB_PR_JSON", "/work/pr/pr.json"), "utf8")); | |
| 35 | + | const model = env("RB_CONFIG_MODEL", env("ANTHROPIC_MODEL", "claude-sonnet-5-5")); | |
| 36 | + | const budget = Number(env("RB_CONFIG_BUDGET_USD", "5")); | |
| 37 | + | const variant = env("RB_CONFIG_VARIANT", "production"); | |
| 38 | + | ||
| 39 | + | // The run's settings, printed so ReviewBench can see each label took effect. | |
| 40 | + | console.log(`model=${model} budget_usd=${budget} variant=${variant}`); | |
| 41 | + | ||
| 42 | + | function write(findings, usage) { | |
| 43 | + | const body = { | |
| 44 | + | pr: { repo: pr.repo ?? `https://github.com/${env("RB_NWO")}`, pr_number: Number(env("RB_PR_NUMBER", pr.pr_number)), base: env("RB_BASE", pr.base), head: env("RB_HEAD", pr.head) }, | |
| 45 | + | agent, | |
| 46 | + | findings, | |
| 47 | + | usage, | |
| 48 | + | }; | |
| 49 | + | writeFileSync(out, JSON.stringify(body, null, 2)); | |
| 50 | + | } | |
| 51 | + | ||
| 52 | + | /** Paths the change touches, as the work service knows them (pull.files). */ | |
| 53 | + | function changedPaths(diff) { | |
| 54 | + | const paths = new Set(); | |
| 55 | + | for (const line of diff.split("\n")) { | |
| 56 | + | const m = line.match(/^\+\+\+ b\/(.+)$/) ?? line.match(/^--- a\/(.+)$/); | |
| 57 | + | if (m && m[1] !== "/dev/null") paths.add(m[1]); | |
| 58 | + | } | |
| 59 | + | return paths; | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | function runClaude(prompt) { | |
| 63 | + | return new Promise((resolve, reject) => { | |
| 64 | + | const args = ["--print", prompt, "--output-format", "stream-json", "--verbose", "--max-turns", MAX_TURNS, "--dangerously-skip-permissions", "--model", model]; | |
| 65 | + | if (budget > 0) args.push("--max-budget-usd", budget.toFixed(2)); | |
| 66 | + | const child = spawn("claude", args, { cwd: REPO, stdio: ["ignore", "pipe", "inherit"], env: { ...process.env, ANTHROPIC_MODEL: model } }); | |
| 67 | + | let result = null; | |
| 68 | + | let buffer = ""; | |
| 69 | + | child.stdout.on("data", (chunk) => { | |
| 70 | + | buffer += chunk; | |
| 71 | + | let at; | |
| 72 | + | while ((at = buffer.indexOf("\n")) >= 0) { | |
| 73 | + | const line = buffer.slice(0, at); | |
| 74 | + | buffer = buffer.slice(at + 1); | |
| 75 | + | try { | |
| 76 | + | const event = JSON.parse(line); | |
| 77 | + | if (event.type === "result") result = event; | |
| 78 | + | } catch {} | |
| 79 | + | } | |
| 80 | + | }); | |
| 81 | + | child.on("error", reject); | |
| 82 | + | child.on("close", (code) => (result ? resolve(result) : reject(new Error(`claude exited ${code} without a result`)))); | |
| 83 | + | }); | |
| 84 | + | } | |
| 85 | + | ||
| 86 | + | const started = Date.now(); | |
| 87 | + | copyFileSync(env("RB_DIFF", "/work/pr/diff.patch"), DIFF_FILE); | |
| 88 | + | const diff = readFileSync(DIFF_FILE, "utf8"); | |
| 89 | + | if (!diff.trim()) { | |
| 90 | + | // review.rs bails on an empty change; the benchmark still wants a file. | |
| 91 | + | write([], { time_in_ms: Date.now() - started }); | |
| 92 | + | process.exit(0); | |
| 93 | + | } | |
| 94 | + | ||
| 95 | + | const instructionsFile = variant === "production" ? new URL("./instructions.txt", import.meta.url) : new URL(`./variants/${variant}.txt`, import.meta.url); | |
| 96 | + | const prompt = buildPrompt(pr, readFileSync(instructionsFile, "utf8")); | |
| 97 | + | const result = await runClaude(prompt); | |
| 98 | + | ||
| 99 | + | let review = null; | |
| 100 | + | if (existsSync(REVIEW_FILE)) { | |
| 101 | + | try { | |
| 102 | + | review = JSON.parse(readFileSync(REVIEW_FILE, "utf8")); | |
| 103 | + | } catch {} | |
| 104 | + | } | |
| 105 | + | // review.rs: no file means the agent's answer is the review, with no line comments. | |
| 106 | + | const comments = Array.isArray(review?.comments) ? review.comments : []; | |
| 107 | + | const known = changedPaths(diff); | |
| 108 | + | const findings = comments | |
| 109 | + | .filter((c) => typeof c?.body === "string" && c.body.trim() && typeof c.path === "string") | |
| 110 | + | // reviews.rs: a line in a file the pull request does not change is dropped. | |
| 111 | + | .filter((c) => known.size === 0 || known.has(c.path.replace(/^\.\//, ""))) | |
| 112 | + | .slice(0, MAX_REVIEW_COMMENTS) | |
| 113 | + | .map((c) => { | |
| 114 | + | const line = Number.isInteger(c.line) && c.line > 0 ? c.line : 1; | |
| 115 | + | const end = Number.isInteger(c.end_line) && c.end_line >= line ? c.end_line : line; | |
| 116 | + | return { file: c.path.replace(/^\.\//, ""), start_line: line, end_line: end, message: c.body.trim().slice(0, MAX_REVIEW_CHARS), producer: agent }; | |
| 117 | + | }); | |
| 118 | + | ||
| 119 | + | // Kept beside the findings for analysis: what production would also have | |
| 120 | + | // posted (verdict, summary) and what the run cost. | |
| 121 | + | writeFileSync(out.replace(/\.json$/, ".g1t.json"), JSON.stringify({ verdict: review?.verdict ?? null, body: review?.body ?? result.result ?? "", dropped: comments.length - findings.length, cost_usd: result.total_cost_usd, turns: result.num_turns, model }, null, 2)); | |
| 122 | + | write(findings, { time_in_ms: Date.now() - started, ...(typeof result.total_cost_usd === "number" ? { cost_usd: result.total_cost_usd } : {}) }); |
| 1 | + | You are reviewing a pull request. The repository is checked out in the current directory at the pull request's head. The change under review is in /work/change.diff; read it first, then read the surrounding code as needed. You may run the project's tests. Do not modify the repository. | |
| 2 | + | ||
| 3 | + | Judge whether the change does what it is for, whether it is correct, and whether it would break anything. Be specific and brief. Comment only on real problems or things a maintainer would want to know; do not praise, and do not restate the diff. | |
| 4 | + | ||
| 5 | + | Write your review to /work/review.json as JSON with exactly this shape: | |
| 6 | + | ||
| 7 | + | { | |
| 8 | + | "verdict": "approve" or "request_changes", | |
| 9 | + | "body": "A summary in Markdown: what you checked and your conclusion.", | |
| 10 | + | "comments": [ | |
| 11 | + | { "path": "path/in/the/repository", "line": 12, "body": "What is wrong on this line and what to do about it." } | |
| 12 | + | ] | |
| 13 | + | } | |
| 14 | + | ||
| 15 | + | `line` is the line number in the file as it is after the change. `comments` may be empty. Use "request_changes" only if something must be fixed before merging. Then finish. |
| 1 | + | // The part of ../prompt.mjs the image needs, without the path to the Rust | |
| 2 | + | // source (the image only carries instructions.txt, generated from it). | |
| 3 | + | ||
| 4 | + | /** What production says about the pull request, then the instructions. */ | |
| 5 | + | export function buildPrompt(pr, instructions) { | |
| 6 | + | const about = [`Pull request #${pr.pr_number}: ${pr.title ?? ""}`, pr.body ?? ""].filter((part) => part.trim()).join("\n\n"); | |
| 7 | + | return `${about}\n\n${instructions}`; | |
| 8 | + | } |
| 1 | + | You are reviewing a pull request. The repository is checked out in the current directory at the pull request's head. The change under review is in /work/change.diff. Do not modify the repository. | |
| 2 | + | ||
| 3 | + | Work in three passes. | |
| 4 | + | ||
| 5 | + | 1. Map the change. Read the diff and list every file it touches. For each changed function, type or config, find its callers and the code it calls (grep for the names) and read them. Read the tests that cover the change, and the tests that should and do not. | |
| 6 | + | ||
| 7 | + | 2. Look for problems, file by file, in this order: correctness (wrong logic, off-by-one, null and error paths, broken invariants, concurrency, resource leaks), security (input reaching queries, shells, paths, HTML or regexes; secrets; authorization), reliability (missing error handling, retries, timeouts, migrations that cannot run twice), API and compatibility (callers this change breaks, changed behavior without a version bump), tests (missing or ineffective tests for new behavior), performance (work in loops, unbounded growth), documentation that the change makes wrong, and accessibility in UI code. Where you can, run the project's tests or a quick check to confirm a suspicion. | |
| 8 | + | ||
| 9 | + | 3. Check each candidate before keeping it. Re-read the exact lines. Drop it if it is not true of the code at head, if it is pure style, or if the change does not make it worse. Keep every distinct problem that survives; do not stop at the first few. | |
| 10 | + | ||
| 11 | + | Write your review to /work/review.json as JSON with exactly this shape: | |
| 12 | + | ||
| 13 | + | { | |
| 14 | + | "verdict": "approve" or "request_changes", | |
| 15 | + | "body": "A summary in Markdown: what you checked and your conclusion.", | |
| 16 | + | "comments": [ | |
| 17 | + | { "path": "path/in/the/repository", "line": 12, "end_line": 14, "severity": "high" | "medium" | "low", "body": "What is wrong, why it matters, and what to do about it." } | |
| 18 | + | ] | |
| 19 | + | } | |
| 20 | + | ||
| 21 | + | `line` and `end_line` are line numbers in the file as it is after the change, spanning the problem. One comment per problem. Every problem you mention in the summary must also be a comment on its lines. `comments` may be empty. Use "request_changes" only if something must be fixed before merging. Then finish. |
| 1 | + | // The review prompt, taken from the runner's own source so the benchmark | |
| 2 | + | // measures what production runs. crates/runner/src/review.rs holds the | |
| 3 | + | // instructions as a Rust string constant; services/runner/src/index.ts | |
| 4 | + | // (startReviewRun) puts "what the pull request is" in front of them. | |
| 5 | + | // | |
| 6 | + | // Production also appends repository instructions (AGENTS.md, CLAUDE.md, | |
| 7 | + | // .g1t/review.md), workspace memory and the context hub. Benchmark repos | |
| 8 | + | // have none of g1t's memory, so only the repository files apply; Claude | |
| 9 | + | // Code reads CLAUDE.md from the checkout by itself, as it does in a | |
| 10 | + | // trusted production run. | |
| 11 | + | ||
| 12 | + | import { readFileSync } from "node:fs"; | |
| 13 | + | import { fileURLToPath } from "node:url"; | |
| 14 | + | ||
| 15 | + | const REVIEW_RS = fileURLToPath(new URL("../../crates/runner/src/review.rs", import.meta.url)); | |
| 16 | + | ||
| 17 | + | /** The `INSTRUCTIONS` constant of review.rs, unescaped as rustc would. */ | |
| 18 | + | export function instructionsFromRust(source = readFileSync(REVIEW_RS, "utf8")) { | |
| 19 | + | const match = source.match(/const INSTRUCTIONS: &str = "([\s\S]*?)";\n/); | |
| 20 | + | if (!match) throw new Error("INSTRUCTIONS not found in review.rs; update bench/reviewbench/prompt.mjs"); | |
| 21 | + | return match[1] | |
| 22 | + | // A backslash at the end of a line continues the string and eats the | |
| 23 | + | // next line's leading whitespace. | |
| 24 | + | .replace(/\\\n\s*/g, "") | |
| 25 | + | .replace(/\\"/g, '"') | |
| 26 | + | .replace(/\\n/g, "\n") | |
| 27 | + | .replace(/\\\\/g, "\\"); | |
| 28 | + | } | |
| 29 | + | ||
| 30 | + | export { buildPrompt } from "./agent/prompt-lib.mjs"; | |
| 31 | + | ||
| 32 | + | // `node prompt.mjs` writes instructions.txt for the image build, and | |
| 33 | + | // `node prompt.mjs --check` fails when it has drifted from review.rs. | |
| 34 | + | if (process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]) { | |
| 35 | + | const out = fileURLToPath(new URL("./agent/instructions.txt", import.meta.url)); | |
| 36 | + | const text = instructionsFromRust(); | |
| 37 | + | if (process.argv.includes("--check")) { | |
| 38 | + | let current = ""; | |
| 39 | + | try { | |
| 40 | + | current = readFileSync(out, "utf8"); | |
| 41 | + | } catch {} | |
| 42 | + | if (current !== text) { | |
| 43 | + | console.error("bench/reviewbench/agent/instructions.txt is out of date; run node bench/reviewbench/prompt.mjs"); | |
| 44 | + | process.exit(1); | |
| 45 | + | } | |
| 46 | + | } else { | |
| 47 | + | const { writeFileSync } = await import("node:fs"); | |
| 48 | + | writeFileSync(out, text); | |
| 49 | + | console.log(`wrote ${out} (${text.length} chars)`); | |
| 50 | + | } | |
| 51 | + | } |
| 1 | + | #!/usr/bin/env node | |
| 2 | + | // Runs g1t's pull request reviewer over ReviewBench and scores it with the | |
| 3 | + | // benchmark's own judge. docs/research/reviewbench.md explains the design. | |
| 4 | + | // | |
| 5 | + | // node bench/reviewbench/run.mjs setup clone ReviewBench (pinned) and install its judge | |
| 6 | + | // node bench/reviewbench/run.mjs build build the agent image from the runner base image | |
| 7 | + | // node bench/reviewbench/run.mjs estimate --set sample:50 | |
| 8 | + | // node bench/reviewbench/run.mjs review --set sample:50 [--seed 1] [--variant production] [--model claude-sonnet-5-5] --yes | |
| 9 | + | // node bench/reviewbench/run.mjs judge --run <id> [--judge-model claude-sonnet-5] --yes | |
| 10 | + | // node bench/reviewbench/run.mjs report --run <id> [--json] | |
| 11 | + | // | |
| 12 | + | // --set is test (the benchmark's 25), full (all 219) or sample:N (N of the | |
| 13 | + | // 219, stratified by change size, reproducible with --seed). | |
| 14 | + | // | |
| 15 | + | // Model access, from the environment, passed into each container: | |
| 16 | + | // ANTHROPIC_API_KEY a provider key, or | |
| 17 | + | // ANTHROPIC_BASE_URL + ANTHROPIC_API_KEY g1t's model proxy (MODELS_URL/anthropic and a session token) | |
| 18 | + | // The judge reads ANTHROPIC_API_KEY itself (through ReviewBench's pi registry). | |
| 19 | + | // | |
| 20 | + | // Nothing that spends money runs without --yes. Each review is capped by | |
| 21 | + | // --budget-usd (default 5), and a run stops once --max-total-usd is spent. | |
| 22 | + | // | |
| 23 | + | // Needs docker, git, jq and bash (Git Bash on Windows) and Node 20+. | |
| 24 | + | ||
| 25 | + | import { execFileSync, spawnSync } from "node:child_process"; | |
| 26 | + | import { existsSync, mkdirSync, readFileSync, readdirSync, writeFileSync } from "node:fs"; | |
| 27 | + | import { dirname, join } from "node:path"; | |
| 28 | + | import { fileURLToPath } from "node:url"; | |
| 29 | + | ||
| 30 | + | const HERE = dirname(fileURLToPath(import.meta.url)); | |
| 31 | + | const ROOT = join(HERE, "..", ".."); | |
| 32 | + | const CACHE = join(HERE, ".cache"); | |
| 33 | + | const RB = join(CACHE, "ReviewBench"); | |
| 34 | + | const RUNS = join(CACHE, "runs"); | |
| 35 | + | // The ReviewBench commit these numbers are comparable against. | |
| 36 | + | const RB_COMMIT = "ceb0794a3768da6ef4a56e5311dfb4afd29e5dee"; | |
| 37 | + | const IMAGE = "g1t-reviewbench:dev"; | |
| 38 | + | // The model the official leaderboard is judged with. | |
| 39 | + | const DEFAULT_JUDGE = "claude-sonnet-5"; | |
| 40 | + | // services/runner/wrangler.jsonc AGENT_ROUTES.review | |
| 41 | + | const DEFAULT_MODEL = "claude-sonnet-5-5"; | |
| 42 | + | ||
| 43 | + | // $ per million tokens: input, output, cache read, cache write (5 minute). | |
| 44 | + | const PRICES = { | |
| 45 | + | "claude-sonnet-5-5": [2, 10, 0.2, 2.5], | |
| 46 | + | "claude-sonnet-5": [2, 10, 0.2, 2.5], | |
| 47 | + | "claude-opus-5-5": [4, 20, 0.2, 5], | |
| 48 | + | "claude-haiku-4-5": [1, 5, 0.1, 1.25], | |
| 49 | + | }; | |
| 50 | + | ||
| 51 | + | function args() { | |
| 52 | + | const [command, ...rest] = process.argv.slice(2); | |
| 53 | + | const flags = {}; | |
| 54 | + | for (let i = 0; i < rest.length; i++) { | |
| 55 | + | const name = rest[i].replace(/^--/, ""); | |
| 56 | + | const next = rest[i + 1]; | |
| 57 | + | if (next === undefined || next.startsWith("--")) flags[name] = true; | |
| 58 | + | else flags[name] = rest[++i]; | |
| 59 | + | } | |
| 60 | + | return { command, flags }; | |
| 61 | + | } | |
| 62 | + | ||
| 63 | + | const run = (cmd, argv, opts = {}) => { | |
| 64 | + | const result = spawnSync(cmd, argv, { stdio: "inherit", ...opts }); | |
| 65 | + | if (result.status !== 0) throw new Error(`${cmd} ${argv.join(" ")} exited ${result.status}`); | |
| 66 | + | }; | |
| 67 | + | ||
| 68 | + | function manifest() { | |
| 69 | + | return JSON.parse(readFileSync(join(RB, "corpus", "manifest.json"), "utf8")); | |
| 70 | + | } | |
| 71 | + | ||
| 72 | + | /** A seeded shuffle, so a sample is the same sample next week. */ | |
| 73 | + | function shuffle(items, seed) { | |
| 74 | + | let state = seed >>> 0 || 1; | |
| 75 | + | const random = () => ((state = (state * 1664525 + 1013904223) >>> 0) / 2 ** 32); | |
| 76 | + | const copy = [...items]; | |
| 77 | + | for (let i = copy.length - 1; i > 0; i--) { | |
| 78 | + | const j = Math.floor(random() * (i + 1)); | |
| 79 | + | [copy[i], copy[j]] = [copy[j], copy[i]]; | |
| 80 | + | } | |
| 81 | + | return copy; | |
| 82 | + | } | |
| 83 | + | ||
| 84 | + | /** Indices into the full manifest for --set. */ | |
| 85 | + | function select(set, seed) { | |
| 86 | + | const all = manifest(); | |
| 87 | + | if (set === "full") return all.map((_, i) => i); | |
| 88 | + | if (set === "test") { | |
| 89 | + | const test = JSON.parse(readFileSync(join(RB, "corpus", "test", "test.json"), "utf8")); | |
| 90 | + | const keys = new Set(test.map((p) => `${p.nwo}#${p.pr_number}`)); | |
| 91 | + | return all.flatMap((p, i) => (keys.has(`${p.nwo}#${p.pr_number}`) ? [i] : [])); | |
| 92 | + | } | |
| 93 | + | const m = /^sample:(\d+)$/.exec(set); | |
| 94 | + | if (!m) throw new Error(`--set is test, full or sample:N, not ${set}`); | |
| 95 | + | const n = Number(m[1]); | |
| 96 | + | // Stratified by change size, in the corpus's proportions, so a sample is | |
| 97 | + | // not all small or all huge. | |
| 98 | + | const bucket = (p) => { | |
| 99 | + | const lines = p.lines_added + p.lines_removed; | |
| 100 | + | return lines <= 200 ? 0 : lines <= 1000 ? 1 : 2; | |
| 101 | + | }; | |
| 102 | + | const strata = [[], [], []]; | |
| 103 | + | all.forEach((p, i) => strata[bucket(p)].push(i)); | |
| 104 | + | const picked = []; | |
| 105 | + | for (const stratum of strata) { | |
| 106 | + | const share = Math.round((stratum.length / all.length) * n); | |
| 107 | + | picked.push(...shuffle(stratum, seed).slice(0, share)); | |
| 108 | + | } | |
| 109 | + | return picked.slice(0, n).sort((a, b) => a - b); | |
| 110 | + | } | |
| 111 | + | ||
| 112 | + | /** | |
| 113 | + | * What one review is expected to cost. An agentic review re-reads its | |
| 114 | + | * growing context every turn, mostly from cache: turns and context grow | |
| 115 | + | * with the size of the change. Calibrate against real review runs (the | |
| 116 | + | * billing ledger records each run's cost) before trusting the absolute | |
| 117 | + | * numbers; see docs/research/reviewbench.md. | |
| 118 | + | */ | |
| 119 | + | function estimateOne(p, model) { | |
| 120 | + | const [, output, cacheRead, cacheWrite] = PRICES[model] ?? PRICES[DEFAULT_MODEL]; | |
| 121 | + | const lines = p.lines_added + p.lines_removed; | |
| 122 | + | const turns = Math.min(80, 12 + Math.round(Math.sqrt(lines) * 0.8) + Math.min(p.files_changed, 40) * 0.4); | |
| 123 | + | const startContext = 18_000 + Math.min(lines, 6_000) * 12; // prompt, tools, the diff when read | |
| 124 | + | const endContext = Math.min(startContext + turns * 2_500, 180_000); // files read along the way | |
| 125 | + | const meanContext = (startContext + endContext) / 2; | |
| 126 | + | // Each turn re-reads the context from cache and writes only what it added. | |
| 127 | + | const fresh = endContext; | |
| 128 | + | const cachedReads = Math.max(0, turns * meanContext - fresh); | |
| 129 | + | const outTokens = turns * 450 + 2_500; | |
| 130 | + | return (cachedReads * cacheRead + fresh * cacheWrite + outTokens * output) / 1e6; | |
| 131 | + | } | |
| 132 | + | ||
| 133 | + | /** | |
| 134 | + | * The judge: matching per file chunk plus classifying every unmatched | |
| 135 | + | * finding with read access to the repository (a few tool calls each). | |
| 136 | + | */ | |
| 137 | + | function estimateJudge(p, model, findingsPerPr = 6) { | |
| 138 | + | const [input, output, cacheRead] = PRICES[model] ?? PRICES[DEFAULT_JUDGE]; | |
| 139 | + | const matchCalls = Math.max(1, Math.ceil(findingsPerPr / 3)); | |
| 140 | + | const unmatched = findingsPerPr * 0.5; | |
| 141 | + | const tokensIn = matchCalls * 6_000 + unmatched * 5 * 12_000; | |
| 142 | + | const tokensOut = matchCalls * 1_500 + unmatched * 5 * 600; | |
| 143 | + | return (tokensIn * 0.5 * input + tokensIn * 0.5 * cacheRead + tokensOut * output) / 1e6; | |
| 144 | + | } | |
| 145 | + | ||
| 146 | + | function estimate(indices, model, judgeModel, rounds = 1) { | |
| 147 | + | const all = manifest(); | |
| 148 | + | let review = 0; | |
| 149 | + | let judge = 0; | |
| 150 | + | for (const i of indices) { | |
| 151 | + | review += estimateOne(all[i], model); | |
| 152 | + | judge += estimateJudge(all[i], judgeModel); | |
| 153 | + | } | |
| 154 | + | return { prs: indices.length, rounds, review_usd: review * rounds, judge_usd: judge * rounds, total_usd: (review + judge) * rounds }; | |
| 155 | + | } | |
| 156 | + | ||
| 157 | + | function setup() { | |
| 158 | + | mkdirSync(CACHE, { recursive: true }); | |
| 159 | + | if (!existsSync(join(RB, ".git"))) run("git", ["clone", "https://github.com/review-bench/ReviewBench.git", RB]); | |
| 160 | + | run("git", ["-C", RB, "fetch", "-q", "origin", RB_COMMIT]); | |
| 161 | + | run("git", ["-C", RB, "checkout", "-q", "--detach", RB_COMMIT]); | |
| 162 | + | run("npm", ["ci", "--no-audit", "--no-fund"], { cwd: RB, shell: process.platform === "win32" }); | |
| 163 | + | } | |
| 164 | + | ||
| 165 | + | function build() { | |
| 166 | + | run(process.execPath, [join(HERE, "prompt.mjs")]); | |
| 167 | + | const base = JSON.parse(readFileSync(join(ROOT, "services", "runner", "base.json"), "utf8")); | |
| 168 | + | run("docker", ["build", "--platform", "linux/amd64", "--build-arg", `BASE=${base.image}@${base.digest}`, "-t", IMAGE, HERE]); | |
| 169 | + | } | |
| 170 | + | ||
| 171 | + | function review(flags) { | |
| 172 | + | if (!flags.yes) throw new Error("review spends model credit; run estimate first, then pass --yes"); | |
| 173 | + | const set = flags.set ?? "sample:50"; | |
| 174 | + | const seed = Number(flags.seed ?? 1); | |
| 175 | + | const model = flags.model ?? DEFAULT_MODEL; | |
| 176 | + | const variant = flags.variant ?? "production"; | |
| 177 | + | const budget = Number(flags["budget-usd"] ?? 5); | |
| 178 | + | const maxTotal = Number(flags["max-total-usd"] ?? 150); | |
| 179 | + | const indices = select(set, seed); | |
| 180 | + | const id = flags.run ?? `${new Date().toISOString().slice(0, 10)}-${set.replace(":", "")}-${variant}-${model}`; | |
| 181 | + | const dir = join(RUNS, id); | |
| 182 | + | mkdirSync(dir, { recursive: true }); | |
| 183 | + | writeFileSync(join(dir, "run.json"), JSON.stringify({ id, set, seed, model, variant, budget_usd: budget, indices, reviewbench: RB_COMMIT, g1t: execFileSync("git", ["-C", ROOT, "rev-parse", "HEAD"]).toString().trim(), started_at: new Date().toISOString() }, null, 2)); | |
| 184 | + | const env = { ...process.env, RB_CONFIG_MODEL: model, RB_CONFIG_VARIANT: variant, RB_CONFIG_BUDGET_USD: String(budget), TRY_AGENT_WORK: join(dir, ".work") }; | |
| 185 | + | const pass = ["-e", "RB_CONFIG_MODEL", "-e", "RB_CONFIG_VARIANT", "-e", "RB_CONFIG_BUDGET_USD", "-e", "ANTHROPIC_API_KEY"]; | |
| 186 | + | if (process.env.ANTHROPIC_BASE_URL) pass.push("-e", "ANTHROPIC_BASE_URL"); | |
| 187 | + | let spent = 0; | |
| 188 | + | for (const i of indices) { | |
| 189 | + | if (spent >= maxTotal) { | |
| 190 | + | console.error(`stopped: spent $${spent.toFixed(2)} of --max-total-usd ${maxTotal}`); | |
| 191 | + | break; | |
| 192 | + | } | |
| 193 | + | spawnSync("bash", [join(RB, "scripts", "try-agent.sh"), IMAGE, "--set", "full", "--pr", String(i), ...pass], { cwd: dir, env, stdio: "inherit" }); | |
| 194 | + | spent = sidecars(dir).reduce((sum, s) => sum + (s.cost_usd ?? 0), 0); | |
| 195 | + | } | |
| 196 | + | console.log(`run ${id}: ${readdirSync(join(dir, "findings")).length} of ${indices.length} reviewed, $${spent.toFixed(2)} spent`); | |
| 197 | + | } | |
| 198 | + | ||
| 199 | + | /** What production would have posted beside the findings, and the cost. */ | |
| 200 | + | function sidecars(dir) { | |
| 201 | + | const out = join(dir, ".work", "out"); | |
| 202 | + | if (!existsSync(out)) return []; | |
| 203 | + | return readdirSync(out).flatMap((key) => { | |
| 204 | + | const file = join(out, key, "findings.g1t.json"); | |
| 205 | + | return existsSync(file) ? [{ key, ...JSON.parse(readFileSync(file, "utf8")) }] : []; | |
| 206 | + | }); | |
| 207 | + | } | |
| 208 | + | ||
| 209 | + | function judge(flags) { | |
| 210 | + | if (!flags.yes) throw new Error("judging spends model credit; pass --yes"); | |
| 211 | + | const dir = join(RUNS, flags.run); | |
| 212 | + | const judgeModel = flags["judge-model"] ?? DEFAULT_JUDGE; | |
| 213 | + | run("npm", ["run", "judge", "--", "--candidate", join(dir, "findings"), "--golden", join(RB, "golden"), "--manifest", join(RB, "corpus", "manifest.json"), "--provider", "anthropic", "--model", judgeModel, "--output", join(dir, "scoring", "results.json"), "--repo-dir", join(CACHE, "judge-repos"), "--concurrency", String(flags.concurrency ?? 4)], { cwd: RB, shell: process.platform === "win32" }); | |
| 214 | + | } | |
| 215 | + | ||
| 216 | + | function report(flags) { | |
| 217 | + | const dir = join(RUNS, flags.run); | |
| 218 | + | const meta = JSON.parse(readFileSync(join(dir, "run.json"), "utf8")); | |
| 219 | + | const results = JSON.parse(readFileSync(join(dir, "scoring", "results.json"), "utf8")); | |
| 220 | + | const side = sidecars(dir); | |
| 221 | + | const cost = side.reduce((sum, s) => sum + (s.cost_usd ?? 0), 0); | |
| 222 | + | const summary = { | |
| 223 | + | run: meta.id, | |
| 224 | + | date: meta.started_at.slice(0, 10), | |
| 225 | + | g1t: meta.g1t, | |
| 226 | + | model: meta.model, | |
| 227 | + | variant: meta.variant, | |
| 228 | + | set: meta.set, | |
| 229 | + | prs: side.length, | |
| 230 | + | findings: readdirSync(join(dir, "findings")).reduce((sum, f) => sum + JSON.parse(readFileSync(join(dir, "findings", f), "utf8")).findings.length, 0), | |
| 231 | + | review_cost_usd: Number(cost.toFixed(2)), | |
| 232 | + | request_changes: side.filter((s) => s.verdict === "request_changes").length, | |
| 233 | + | dropped_outside_change: side.reduce((sum, s) => sum + (s.dropped ?? 0), 0), | |
| 234 | + | // The judge's own aggregate block, as ReviewBench writes it. | |
| 235 | + | metrics: results.metrics ?? results.summary ?? results, | |
| 236 | + | }; | |
| 237 | + | if (flags.json) { | |
| 238 | + | console.log(JSON.stringify(summary)); | |
| 239 | + | return; | |
| 240 | + | } | |
| 241 | + | console.log(`## ReviewBench: ${summary.run}\n`); | |
| 242 | + | console.log(`g1t ${summary.g1t.slice(0, 8)}, ${summary.model}, prompt ${summary.variant}, ${summary.set} (${summary.prs} PRs), review cost $${summary.review_cost_usd}\n`); | |
| 243 | + | console.log("```json\n" + JSON.stringify(summary.metrics, null, 2) + "\n```"); | |
| 244 | + | } | |
| 245 | + | ||
| 246 | + | const { command, flags } = args(); | |
| 247 | + | try { | |
| 248 | + | switch (command) { | |
| 249 | + | case "setup": | |
| 250 | + | setup(); | |
| 251 | + | break; | |
| 252 | + | case "build": | |
| 253 | + | build(); | |
| 254 | + | break; | |
| 255 | + | case "estimate": { | |
| 256 | + | const indices = select(flags.set ?? "sample:50", Number(flags.seed ?? 1)); | |
| 257 | + | const model = flags.model ?? DEFAULT_MODEL; | |
| 258 | + | const judgeModel = flags["judge-model"] ?? DEFAULT_JUDGE; | |
| 259 | + | console.log(JSON.stringify({ set: flags.set ?? "sample:50", model, judge: judgeModel, ...estimate(indices, model, judgeModel, Number(flags.rounds ?? 1)) }, (k, v) => (typeof v === "number" ? Number(v.toFixed(2)) : v), 2)); | |
| 260 | + | break; | |
| 261 | + | } | |
| 262 | + | case "review": | |
| 263 | + | review(flags); | |
| 264 | + | break; | |
| 265 | + | case "judge": | |
| 266 | + | judge(flags); | |
| 267 | + | break; | |
| 268 | + | case "report": | |
| 269 | + | report(flags); | |
| 270 | + | break; | |
| 271 | + | default: | |
| 272 | + | console.error(readFileSync(fileURLToPath(import.meta.url), "utf8").split("\n").slice(1, 22).join("\n")); | |
| 273 | + | process.exit(2); | |
| 274 | + | } | |
| 275 | + | } catch (error) { | |
| 276 | + | console.error(String(error.message ?? error)); | |
| 277 | + | process.exit(1); | |
| 278 | + | } |
| 1 | + | # How g1t.sh ships CSS | |
| 2 | + | ||
| 3 | + | Internal research, 2026-10-06. Prompted by GitHub's "Improving site | |
| 4 | + | performance by shipping more CSS" (github.blog, engineering). Read-only: | |
| 5 | + | nothing here is applied yet. The measurements can be repeated with the | |
| 6 | + | commands at the end. | |
| 7 | + | ||
| 8 | + | ## Verdict | |
| 9 | + | ||
| 10 | + | - **The article's technique does not apply to us. We already ship CSS the | |
| 11 | + | way GitHub moved to.** GitHub replaced runtime CSS-in-JS | |
| 12 | + | (styled-components) with static CSS Modules and got faster server | |
| 13 | + | rendering. apps/web has no CSS-in-JS. Tailwind v4 compiles to one static | |
| 14 | + | stylesheet at build time. Nothing runs at render time to produce styles, | |
| 15 | + | and no `<style>` tags are injected. | |
| 16 | + | - **We do have a CSS delivery problem, and it is a small fix.** The one | |
| 17 | + | stylesheet is the **last** element in `<head>`. It comes after 2 font | |
| 18 | + | preloads and 38 to 58 `modulepreload` links. On a slow mobile link it | |
| 19 | + | shares bandwidth with roughly 400 KB of JavaScript and fonts, and first | |
| 20 | + | paint waits for it. Under real (DevTools) slow-4G throttling, the | |
| 21 | + | stylesheet finished at 2.9 to 3.3 s. First paint followed at 3.5 s. TBT | |
| 22 | + | was 0 and the document had arrived at 0.7 s. | |
| 23 | + | - **Recommended:** (1) emit the stylesheet first in `<head>`, ahead of the | |
| 24 | + | module preloads. (2) Send it as a `Link: rel=preload` response header, so | |
| 25 | + | Cloudflare Early Hints can start it during server think time. Expected | |
| 26 | + | gain: roughly 0.3 to 0.4 s off FCP/LCP on mobile in the Lighthouse model, | |
| 27 | + | and up to the CSS's whole load time (1 s or more) on pages with slow | |
| 28 | + | loaders, for first visits. Repeat visits gain nothing, because the CSS is | |
| 29 | + | cached immutable for a year. Risk is low; the details are under | |
| 30 | + | [Patches](#patches). | |
| 31 | + | - **Not recommended:** per-route CSS splitting, critical-CSS inlining and | |
| 32 | + | CSS Modules. They would add requests, HTML weight or complexity for about | |
| 33 | + | 15 KB (brotli) that is already cached across every page. | |
| 34 | + | ||
| 35 | + | ## The article | |
| 36 | + | ||
| 37 | + | GitHub's Primer design system and github.com styled React components with | |
| 38 | + | styled-components (CSS-in-JS). That had three costs: | |
| 39 | + | ||
| 40 | + | - Styles were computed and injected at runtime, on the server during SSR | |
| 41 | + | and on the client during hydration. | |
| 42 | + | - SSR spent time collecting styles. | |
| 43 | + | - The overhead grew with the number of components on a page. | |
| 44 | + | ||
| 45 | + | The fix was CSS Modules: plain `.module.css` files colocated with | |
| 46 | + | components and compiled to static stylesheets "sent as part of the HTML | |
| 47 | + | for a page", with no client or server runtime. The title's "shipping more | |
| 48 | + | CSS" means more static CSS bytes in exchange for no runtime styling work. | |
| 49 | + | ||
| 50 | + | How they did it: | |
| 51 | + | ||
| 52 | + | 1. **Primer, 2023 to 2024.** CSS Module files beside each component, | |
| 53 | + | feature-flagged old/new styles, visual regression tests, rolled out to | |
| 54 | + | the team, then staff, then everyone. | |
| 55 | + | 2. **github.com, 2025 to 2026.** A `@primer/styled-react` bridge kept the | |
| 56 | + | legacy `sx` prop working. They migrated about 7,760 `sx` props: | |
| 57 | + | - 6,419 by May 2026, with a VS Code extension (sx-to-css) and a codemod; | |
| 58 | + | - the last 895 by Copilot agents in three weeks. | |
| 59 | + | 3. **Theming, 2026.** They moved off styled-components theme utilities to | |
| 60 | + | CSS variables (`@primer/css`). | |
| 61 | + | ||
| 62 | + | Measured results (server-side only; the post gives no client metrics such | |
| 63 | + | as LCP, INP or CLS, and no byte counts): | |
| 64 | + | ||
| 65 | + | - Primer migration (Dec 2024): **55% less time to server-render a page** | |
| 66 | + | and **25% less time for components to initialize**. | |
| 67 | + | - github.com migration: SSR improvements **from 1% up to about 22%** per | |
| 68 | + | page; the best controller improved by 21.97%. | |
| 69 | + | - 100% CSS Modules as of June 2026. | |
| 70 | + | ||
| 71 | + | What we take from it: the expensive thing was *runtime* styling. GitHub's | |
| 72 | + | end state is static CSS in a stylesheet, which is where apps/web already | |
| 73 | + | is. | |
| 74 | + | ||
| 75 | + | ## How apps/web ships CSS today | |
| 76 | + | ||
| 77 | + | | Question | Answer | | |
| 78 | + | |---|---| | |
| 79 | + | | Styling system | Tailwind v4 via `@tailwindcss/vite`. Tokens and fonts come from `@g1t/theme`. Custom CSS (keyframes, `art-*` drawings, markdown) is in `app/app.css`. | | |
| 80 | + | | CSS-in-JS | None: no styled-components, emotion, stitches or runtime `<style>` injection. `style=""` attributes are CSS variables on drawings (114 on `/`, 1 to 38 elsewhere) and Shiki token colors in code views. | | |
| 81 | + | | Files | **One** stylesheet, `assets/root-<hash>.css`, imported once in `app/root.tsx` (`import "./app.css"`). No route imports CSS, so the manifest has CSS on the root route only. | | |
| 82 | + | | Size | **120,058 B raw, 18,758 B gzip -9, 15,058 B brotli -q11**. On the wire from Cloudflare: about 19.9 KB. Breakdown: Tailwind utilities 93 KB; `@property` registrations 2 KB (in a 4.5 KB block); theme 2.6 KB; base 4 KB; custom CSS outside layers about 18 KB; 7 `@font-face`, 23 `@keyframes`. | | |
| 83 | + | | Caching | `cache-control: public, max-age=31536000, immutable` (hashed name). Every page shares the same file, so after the first page it costs nothing. | | |
| 84 | + | | How it's loaded | A render-blocking `<link rel="stylesheet">`, which is correct. **But it is the last tag in `<head>`**: icons, then 2 font preloads, then 38 (`/pricing`) to 58 (`/flagon-io/g1t`) `modulepreload`s, then the stylesheet. There is no `preload` for it, no `Link` header, no Early Hints, and no inlined critical CSS. | | |
| 85 | + | | Fonts | Hanken (22 KB) and Bricolage (66 KB) are preloaded and use `font-display: swap`, with metric-matched fallbacks (`size-adjust` and overrides), so swapping does not shift layout. Plex Mono loads on demand. | | |
| 86 | + | | React Router | `<Links/>` renders the route CSS from the manifest. On client navigation, React Router loads the next routes' stylesheets before committing. With a single shared file that is already cached, navigation never waits on CSS and there is no late-loaded route stylesheet. | | |
| 87 | + | | Animations | Infinite animations exist only in the landing drawings. They are paused off-screen (`[data-paused]`), promoted to their own layer (`will-change` on `[data-live]`), and stopped under `prefers-reduced-motion`. | | |
| 88 | + | ||
| 89 | + | The order matters because the browser asks for resources in document | |
| 90 | + | order and the CSS is asked for last. In the DevTools-throttled run of | |
| 91 | + | `/flagon-io/g1t/pull/1` (slow 4G, 150 ms RTT, about 1.6 Mbps), all 61 | |
| 92 | + | subresources were requested between 645 and 664 ms. The stylesheet was the | |
| 93 | + | 61st. The bandwidth was then shared: | |
| 94 | + | ||
| 95 | + | | Resource | Bytes | Requested (ms) | Finished (ms) | | |
| 96 | + | |---|---:|---:|---:| | |
| 97 | + | | HTML | 16,555 | 0 | 699 | | |
| 98 | + | | Hanken font (preload) | 22,082 | 645 | 3,282 | | |
| 99 | + | | Bricolage font (preload) | 66,554 | 646 | 4,271 | | |
| 100 | + | | entry.client.js | 68,516 | 649 | 4,310 | | |
| 101 | + | | 56 other modulepreloads | ~300 KB | 649 to 664 | 1,268 to 4,135 | | |
| 102 | + | | **root.css** | **19,890** | **664** | **3,271** | | |
| 103 | + | | First contentful paint | | | **3,515** | | |
| 104 | + | ||
| 105 | + | Chrome gives the stylesheet top priority. Cloudflare's HTTP/3 prioritization | |
| 106 | + | should favor it on a real connection more than DevTools throttling does, | |
| 107 | + | so these absolute numbers are pessimistic. The direction holds either way: | |
| 108 | + | nothing paints until the CSS arrives, and the CSS is queued behind about | |
| 109 | + | 450 KB that is not needed for first paint. | |
| 110 | + | ||
| 111 | + | ## Measurements | |
| 112 | + | ||
| 113 | + | The build is `npm run build` in apps/web, run locally on 2026-10-06. Its | |
| 114 | + | CSS hash `KP0DOluu` matches production. CSS requests are counted from the | |
| 115 | + | live HTML. | |
| 116 | + | ||
| 117 | + | | Page | HTML (decoded) | CSS requests | modulepreloads | TTFB (curl, server-timing) | | |
| 118 | + | |---|---:|---:|---:|---| | |
| 119 | + | | `/` | 194 KB | 1 | 42 | 462 ms (server 33 ms) | | |
| 120 | + | | `/pricing` | 64 KB | 1 | 38 | 254 ms (server 180 ms) | | |
| 121 | + | | `/flagon-io/g1t` | 118 KB | 1 | 58 | **1,403 ms** (server 1,311 ms, 22 service calls) | | |
| 122 | + | | `/flagon-io/g1t/pull/1` | 76 KB | 1 | 57 | 234 ms (server 74 ms) | | |
| 123 | + | ||
| 124 | + | Lighthouse 13.5.0, headless Chrome, signed out, performance only. The | |
| 125 | + | default mode is simulated throttling. "CSS est." is Lighthouse's | |
| 126 | + | render-blocking estimate for the stylesheet. | |
| 127 | + | ||
| 128 | + | | Page | Mode | Score | FCP | LCP | TBT | CLS | Style+layout | CSS est. | | |
| 129 | + | |---|---|---:|---:|---:|---:|---:|---:|---:| | |
| 130 | + | | `/` | desktop | 82 | 1.83 s | 1.93 s | 0 | 0.000 | 145 ms | 110 ms | | |
| 131 | + | | `/` | mobile | 71 | 4.30 s | 5.06 s | 0 | 0.000 | 476 ms | 420 ms | | |
| 132 | + | | `/` | mobile, run 2 | 71 | 4.24 s | 5.01 s | 0 | 0.000 | | 290 ms | | |
| 133 | + | | `/` | mobile, DevTools throttling | 83 | 3.51 s | 3.51 s | 0 | 0.000 | | 100 ms | | |
| 134 | + | | `/pricing` | desktop | 95 | 1.08 s | 1.19 s | 0 | 0.000 | 63 ms | 50 ms | | |
| 135 | + | | `/pricing` | mobile | 76 | 3.85 s | 4.40 s | 0 | 0.000 | 189 ms | 330 ms | | |
| 136 | + | | `/flagon-io/g1t` | desktop | 86 | 1.61 s | 1.69 s | 0 | 0.000 | 36 ms | 80 ms | | |
| 137 | + | | `/flagon-io/g1t` | mobile | 64 | 5.43 s | 6.21 s | 0 | 0.000 | 131 ms | 410 ms | | |
| 138 | + | | `/flagon-io/g1t/pull/1` | desktop | 89 | 1.46 s | 1.57 s | 0 | 0.001 | 28 ms | 40 ms | | |
| 139 | + | | `/flagon-io/g1t/pull/1` | mobile | 64 | 5.44 s | 6.30 s | 0 | 0.000 | 151 ms | 420 ms | | |
| 140 | + | | `/flagon-io/g1t/pull/1` | mobile, run 2 | 65 | 5.32 s | 6.05 s | 0 | 0.001 | | 430 ms | | |
| 141 | + | | `/flagon-io/g1t/pull/1` | mobile, DevTools throttling | 83 | 3.52 s | 3.52 s | 1 | 0.001 | | 170 ms | | |
| 142 | + | ||
| 143 | + | What the numbers say: | |
| 144 | + | ||
| 145 | + | - **Layout shift is solved.** CLS is 0.000 to 0.001 everywhere. The only | |
| 146 | + | shifts recorded were a `<time>` element and a muted caption on the pull | |
| 147 | + | request page, both under 0.001. | |
| 148 | + | - **No main-thread problem.** TBT is 0 and no task is over 50 ms on any | |
| 149 | + | page. A lab tool can't measure INP, but with no long tasks and small DOMs | |
| 150 | + | (358 to 1,474 nodes) nothing points to an INP issue. Style recalculation | |
| 151 | + | and layout cost 28 to 151 ms per load. The exception is `/` on mobile at | |
| 152 | + | 476 ms under 4x CPU slowdown, which comes from DOM size (1,474 nodes) | |
| 153 | + | and the landing drawings, not from selector cost: Tailwind selectors are | |
| 154 | + | single classes. Paint on `/` (765 ms simulated mobile) is the animated | |
| 155 | + | drawings, already layered and paused off-screen. | |
| 156 | + | - **Unused CSS passes.** Lighthouse's unused-CSS audit passes on all four | |
| 157 | + | pages. A 15 KB brotli file is not worth splitting. | |
| 158 | + | - **The lost time is network and ordering.** Mobile FCP is 3.5 to 5.4 s | |
| 159 | + | while the document arrives in about 0.1 to 0.7 s and the main thread is | |
| 160 | + | idle. Lighthouse's render-blocking estimate for the stylesheet is 290 to | |
| 161 | + | 430 ms on mobile (simulated), and the waterfall above shows why. | |
| 162 | + | - The simulated mobile LCP has an extra 1.1 s of "render delay" on the | |
| 163 | + | repo and pull request pages. It did not reproduce under DevTools | |
| 164 | + | throttling (FCP = LCP = 3.5 s), so it looks like a simulation artifact of | |
| 165 | + | the long modulepreload list rather than a real delay. It is worth | |
| 166 | + | re-checking after the patches. | |
| 167 | + | ||
| 168 | + | ## Patches | |
| 169 | + | ||
| 170 | + | Neither patch is applied. Both are small. | |
| 171 | + | ||
| 172 | + | ### 1. Stylesheet first in `<head>` (apps/web/app/root.tsx) | |
| 173 | + | ||
| 174 | + | Import the stylesheet as a URL and make it the first link, with a React 19 | |
| 175 | + | `precedence`. React then hoists it into the stylesheet section of the | |
| 176 | + | head, which it writes before bulk preloads such as `modulepreload`. Nothing | |
| 177 | + | else changes, and React dedupes it on the client. | |
| 178 | + | ||
| 179 | + | ```diff | |
| 180 | + | -import "./app.css"; | |
| 181 | + | +import stylesheet from "./app.css?url"; | |
| 182 | + | ... | |
| 183 | + | export const links: Route.LinksFunction = () => [ | |
| 184 | + | + // First, so it is asked for before the fonts and the module preloads: | |
| 185 | + | + // nothing on the page paints until it arrives. | |
| 186 | + | + { rel: "stylesheet", href: stylesheet, precedence: "default" }, | |
| 187 | + | { rel: "icon", href: "/favicon.ico", sizes: "32x32" }, | |
| 188 | + | ``` | |
| 189 | + | ||
| 190 | + | Verify before shipping: | |
| 191 | + | ||
| 192 | + | 1. Run `npm run build`. Then check that `build/client/assets` still has | |
| 193 | + | exactly one CSS file, and that the server-rendered `<head>` has the | |
| 194 | + | stylesheet before the first `modulepreload` (curl a local `wrangler dev` | |
| 195 | + | or a preview). | |
| 196 | + | 2. Check that Tailwind's Vite plugin still processes `app.css` imported | |
| 197 | + | with `?url` in dev. React Router's own templates use this pattern; if | |
| 198 | + | HMR for styles regresses in dev, keep the side-effect import for dev | |
| 199 | + | only. | |
| 200 | + | 3. If React does not hoist it as expected, a fallback gets the same | |
| 201 | + | ordering: drop `precedence` and render | |
| 202 | + | `<link rel="stylesheet" href={stylesheet} />` directly in `Layout` | |
| 203 | + | before `<Meta />`. | |
| 204 | + | ||
| 205 | + | Expected gain: the stylesheet goes from the 61st request to the 1st. On a | |
| 206 | + | bandwidth-limited first visit it then finishes before most of the JS | |
| 207 | + | rather than among it. That means about 0.3 to 0.4 s off mobile FCP/LCP | |
| 208 | + | (Lighthouse's render-blocking estimate), and more on slower real | |
| 209 | + | connections. There is no change on desktop broadband or repeat visits. | |
| 210 | + | ||
| 211 | + | Risk: low. The same file, the same blocking semantics, a different | |
| 212 | + | position. The only real risk is the dev-mode `?url` behaviour above. | |
| 213 | + | ||
| 214 | + | ### 2. `Link` preload header for Early Hints (apps/web/app/entry.server.tsx) | |
| 215 | + | ||
| 216 | + | In `handleRequest`, before the response is built: | |
| 217 | + | ||
| 218 | + | ```ts | |
| 219 | + | // The stylesheet, announced in the headers: with Early Hints on, the | |
| 220 | + | // browser fetches it while loaders are still running. | |
| 221 | + | const css = routerContext.manifest.routes.root?.css ?? []; | |
| 222 | + | if (css.length > 0) { | |
| 223 | + | responseHeaders.append("Link", css.map((href) => `<${href}>; rel=preload; as=style`).join(", ")); | |
| 224 | + | } | |
| 225 | + | ``` | |
| 226 | + | ||
| 227 | + | Turn on **Early Hints** for the g1t.sh zone (Speed > Optimization > | |
| 228 | + | Content Optimization, or `early_hints` in the zone settings API). It needs | |
| 229 | + | a line in `scripts/cloudflare-setup.py` and a note in | |
| 230 | + | docs/SELF_HOSTING.md: self-hosted installs ignore it harmlessly, because a | |
| 231 | + | `Link` header is just a header. | |
| 232 | + | ||
| 233 | + | Cloudflare remembers `Link: rel=preload` headers from HTML responses and | |
| 234 | + | sends them as a `103` before the Worker answers the next request for that | |
| 235 | + | URL. Today g1t.sh sends no `Link` header (checked 2026-10-06). The gain is | |
| 236 | + | largest exactly where we are slowest: `/flagon-io/g1t` spent 1.3 s in | |
| 237 | + | loaders, all of which could overlap with fetching the CSS (and, if we | |
| 238 | + | choose, the two preloaded fonts) on a first visit. Even without a 103, the | |
| 239 | + | header lets the browser start the CSS as soon as the response headers | |
| 240 | + | arrive, before parsing any HTML. | |
| 241 | + | ||
| 242 | + | Risk: low. | |
| 243 | + | ||
| 244 | + | - The asset names are content-hashed, so a cached hint can never point at | |
| 245 | + | a stale file. At worst, right after a deploy, a hint names an old hash | |
| 246 | + | that still exists in the asset store until it rotates. | |
| 247 | + | - Early Hints apply only over HTTP/2 and HTTP/3. | |
| 248 | + | ||
| 249 | + | ### 3. Optional follow-ups | |
| 250 | + | ||
| 251 | + | - **Trim what competes with first paint.** The two preloaded fonts (88 KB) | |
| 252 | + | are requested before the CSS. With `font-display: swap` and | |
| 253 | + | metric-matched fallbacks they never block paint, so their preloads could | |
| 254 | + | move after the stylesheet (patch 1 does this). Alternatively, preload | |
| 255 | + | only Hanken and let Bricolage (66 KB, headlines only) load from the CSS. | |
| 256 | + | - **Fewer modulepreloads.** 38 to 58 per page, many under 1 KB (`dist-*`, | |
| 257 | + | `access-*`, `skeleton-*`). Grouping small shared chunks in | |
| 258 | + | `vite.config.ts` (as `icons` already is) would cut request overhead on | |
| 259 | + | first visits. This is a JS question and outside this note. | |
| 260 | + | ||
| 261 | + | ## Repeat the measurements | |
| 262 | + | ||
| 263 | + | ```sh | |
| 264 | + | cd apps/web && npm run build # CSS size: build/client/assets/*.css | |
| 265 | + | npx lighthouse https://g1t.sh/<page> --preset=desktop --only-categories=performance --output=json | |
| 266 | + | npx lighthouse https://g1t.sh/<page> --only-categories=performance --output=json # mobile, simulated | |
| 267 | + | npx lighthouse https://g1t.sh/<page> --throttling-method=devtools --only-categories=performance --output=json # mobile, real throttling | |
| 268 | + | ``` | |
| 269 | + | ||
| 270 | + | The waterfall is `audits["network-requests"]` in the JSON. The | |
| 271 | + | render-blocking estimate is `audits["render-blocking-insight"]`. |
| 1 | + | # Measuring g1t's reviewer with ReviewBench | |
| 2 | + | ||
| 3 | + | Internal research, 2026-10-06. Prompted by GitHub's "ReviewBench: an open | |
| 4 | + | benchmark for AI code review" (github.blog). It covers: | |
| 5 | + | ||
| 6 | + | - what the benchmark is; | |
| 7 | + | - how g1t's review agent maps onto it; | |
| 8 | + | - a harness in `bench/reviewbench/`, written but not run, because running | |
| 9 | + | it costs model credit; | |
| 10 | + | - the cost of running it; | |
| 11 | + | - where our reviewer is likely weak, and what to try; | |
| 12 | + | - how to make review quality a tracked number. | |
| 13 | + | ||
| 14 | + | Internal only. User-facing copy never names other review products or | |
| 15 | + | leaderboard positions. | |
| 16 | + | ||
| 17 | + | ## Verdict | |
| 18 | + | ||
| 19 | + | - **Worth doing.** ReviewBench is open (the code and the 219-PR corpus with | |
| 20 | + | labeled findings are MIT, in one repository). It scores the output shape | |
| 21 | + | our reviewer already produces: file, line, message. Its judge runs | |
| 22 | + | locally with our own key. It gives us the first quality number for | |
| 23 | + | `@g1t-agent review` that is not anecdote. | |
| 24 | + | - **Harness status.** The code skeleton is complete in | |
| 25 | + | `bench/reviewbench/`. The image builds from the production sandbox base | |
| 26 | + | image. The prompt is generated from `crates/runner/src/review.rs`, and | |
| 27 | + | `prompt.mjs --check` passes. Nothing has been run against a model. | |
| 28 | + | - **Cost, Sonnet 5.5 reviewer and Sonnet 5 judge.** These estimates come | |
| 29 | + | from a token model and are plus or minus 50%. Calibrate them against the | |
| 30 | + | billing ledger's real review runs. | |
| 31 | + | ||
| 32 | + | | Run | PRs | Review | Judge | Total | | |
| 33 | + | |---|---:|---:|---:|---:| | |
| 34 | + | | 50-PR sample | 50 | about $62 | about $17 | **about $80** | | |
| 35 | + | | Test set | 25 | | | about $38 | | |
| 36 | + | | Full set, one round | 219 | about $274 | about $73 | **about $350** | | |
| 37 | + | | Full set, three rounds (leaderboard protocol) | 657 | | | **about $1,040** | | |
| 38 | + | ||
| 39 | + | The 50-PR sample on Opus 5.5 is about $103. | |
| 40 | + | - **Expected result as the prompt stands:** high precision and low recall. | |
| 41 | + | The prompt asks for brevity and "only real problems". Production drops | |
| 42 | + | comments outside changed files and anchors each comment to a single | |
| 43 | + | line. The benchmark rewards several distinct, located findings per PR: | |
| 44 | + | the golden set averages 12 true positives per PR, and 59% of them are | |
| 45 | + | low severity. The first changes to try are in | |
| 46 | + | [Improvements](#improvements-to-try-in-order). | |
| 47 | + | ||
| 48 | + | ## The benchmark | |
| 49 | + | ||
| 50 | + | | | | | |
| 51 | + | |---|---| | |
| 52 | + | | Announcement | github.blog, "ReviewBench: an open benchmark for AI code review" | | |
| 53 | + | | Site, leaderboard | https://review-bench.ai (the site text is CC BY-NC 4.0) | | |
| 54 | + | | Repository | https://github.com/review-bench/ReviewBench, **MIT**; pinned at `ceb0794a` in the harness | | |
| 55 | + | | Data in the repository | `corpus/manifest.json` (219 PRs), `corpus/test/test.json` (the 25-PR test set), `golden/<pr_key>.json` (labeled findings, 7.8 MB). Each source repository is mirrored at `github.com/review-bench/<owner>_<repo>` with the base and head commits. | | |
| 56 | + | | Corpus | 219 PRs from 187 repositories, 19 languages: TypeScript 31%, Python 19%, C# 11%, Go 9%, JS 7%. Sizes: 36% over 1,000 changed lines; median 562 lines and 9 files. Types: 36% features, 27% bug fixes. | | |
| 57 | + | | Golden set | 4,632 findings, **2,623 TP** and 2,009 FP. The labeled FPs are kept so that a reviewer repeating a known false alarm is penalized. | | |
| 58 | + | | TP labels | Severity: high 181, medium 891, low **1,551**. Categories: correctness 1,068, maintainability 365, reliability 362, documentation 306, testing 223, security 98, api-architecture 81, performance 63, accessibility 57. Scope: introduced-by-pr 2,391, pre-existing 173, exacerbated 59. Context needed: diff-only 940, diff plus related files 1,529, broader project 154. 78% of TPs span several lines (median span 4 lines). | | |
| 59 | + | | Producers of the TPs | LLM reviewers 1,391; Copilot code review 1,185; human reviewers 39; deterministic tools 8. This is a source of bias, discussed below. | | |
| 60 | + | | Agent contract | One container per PR, `linux/amd64`, 15 minutes. Mounts: `/work/repo` (checkout at head, frozen, shallow, no network to GitHub), `/work/pr/diff.patch` (three-dot diff), `/work/pr/pr.json` (title and body). The agent writes `RB_OUT` as `{pr, agent, findings: [{file, start_line, end_line, message, producer}]}`. Egress is allowlisted. | | |
| 61 | + | | Judge | An LLM matcher compares candidate and golden findings per file, in chunks, semantically, many-to-many. Unmatched findings are then classified TP/FP with the same rubric used to build the golden set. Official judge: Claude Sonnet 5. Local judging uses your own provider key (`npm run judge -- --provider anthropic --model …`). | | |
| 62 | + | | Metrics | Grounded precision (matched TPs over matched) and grounded recall (golden TPs covered); the comparable pair. Augmented precision and recall, which also credit or penalize unmatched findings by the classifier. Novel TP count. F-beta with an adjustable β. Results can be split by severity and category, and reported macro and micro. Duration is reported but not scored. Leaderboard rows are the mean of 3 rounds on the full set. | | |
| 63 | + | | Leaderboard snapshot (2026-10-01) | Top rows: grounded precision 84 to 90%, **grounded recall 16 to 26%**, augmented F1 33 to 50, with 490 to 1,190 findings over 219 PRs. Precision is about the same for everyone; rank follows recall, and recall follows how many distinct, correct findings a reviewer emits. Every row is a vendor's product. | | |
| 64 | + | | Validity notes (from the docs) | The golden set is the union of what its producers found, so issues none of them found are invisible. The labels depend on the classifier (96.6% agreement with independent senior engineers). Augmented recall's denominator differs per agent; compare agents on grounded recall. | | |
| 65 | + | ||
| 66 | + | The bias matters for reading our score. Almost half the golden TPs were | |
| 67 | + | found by one commercial reviewer and most of the rest by LLM reviewers. A | |
| 68 | + | reviewer phrasing issues the way those producers do will match more | |
| 69 | + | easily. A finding nobody in the producer set made can still score through | |
| 70 | + | augmented metrics, but not grounded ones. Read our grounded recall as | |
| 71 | + | "agreement with the producer set", not as absolute coverage. | |
| 72 | + | ||
| 73 | + | ## What g1t's reviewer does today | |
| 74 | + | ||
| 75 | + | Sources: | |
| 76 | + | ||
| 77 | + | - `services/runner/src/index.ts` (`review`, `startReviewRun`, `withMemory`, | |
| 78 | + | `guidance`, `modelEnv`); | |
| 79 | + | - `crates/runner/src/review.rs` and `harness.rs`; | |
| 80 | + | - `services/work/src/reviews.rs` and `confidence.rs`; | |
| 81 | + | - `services/runner/wrangler.jsonc`. | |
| 82 | + | ||
| 83 | + | **Trigger.** A person asks for a review (`@g1t-agent review`, or the API). | |
| 84 | + | g1t also starts one by itself, from lifecycle and wait queues (`index.ts` | |
| 85 | + | around line 1870). Admission and guardrails apply. The default time cap | |
| 86 | + | for a review is 30 minutes, the cost cap comes from the workspace plan, and | |
| 87 | + | Claude Code enforces `--max-budget-usd`. | |
| 88 | + | ||
| 89 | + | **Inputs (the prompt, in order):** | |
| 90 | + | ||
| 91 | + | 1. `Pull request #N: <title>`, then the description. | |
| 92 | + | 2. The linked issue's title and body, if there is one. | |
| 93 | + | 3. What people have said on the PR (`peopleSaid`: its comments). | |
| 94 | + | 4. Repository instructions (`instructionsFor`, task `review`): `AGENTS.md`, | |
| 95 | + | `CLAUDE.md`, `.g1t/review.md`, and the same files in directories the | |
| 96 | + | change touches. Up to 8,000 characters per file and 24,000 in total, | |
| 97 | + | read from the head only for same-repository branches, never from forks. | |
| 98 | + | 5. Workspace and project memory (`memoryContext`), and the context hub | |
| 99 | + | (`hubContext`: catalog, relevant memory, recent decisions). | |
| 100 | + | 6. The fixed `INSTRUCTIONS` in `review.rs`: read `/work/change.diff`, | |
| 101 | + | then surrounding code, run tests if you like, don't modify anything; | |
| 102 | + | judge whether it does what it's for, whether it is correct, and whether | |
| 103 | + | it would break anything; "Be specific and brief. Comment only on real | |
| 104 | + | problems…"; write | |
| 105 | + | `{verdict, body, comments: [{path, line, body}]}` to | |
| 106 | + | `/work/review.json`. | |
| 107 | + | ||
| 108 | + | **The sandbox.** | |
| 109 | + | ||
| 110 | + | - Full clone at head, merge-base with the target branch, `git diff base | |
| 111 | + | HEAD` (equivalent to ReviewBench's three-dot diff). | |
| 112 | + | - Claude Code headless (`--print`, `stream-json`, `--max-turns 80`, | |
| 113 | + | `--dangerously-skip-permissions`) with the repository's toolchains, so it | |
| 114 | + | can run tests. | |
| 115 | + | - Fork checkouts get `UNTRUSTED_FLAGS`, so the repository's own Claude | |
| 116 | + | settings are not loaded. | |
| 117 | + | - A review gets no g1t MCP tools and no steer hooks, because no | |
| 118 | + | `G1T_AGENT_TOKEN` is set. | |
| 119 | + | ||
| 120 | + | **What it does not see:** | |
| 121 | + | ||
| 122 | + | - Required checks and their results (the review env has no `CHECKS`). | |
| 123 | + | - The CI status of the head. | |
| 124 | + | - Other open PRs touching the same files, although the work service | |
| 125 | + | computes them (`overlaps`). | |
| 126 | + | - Previous reviews of the same PR, beyond comments via `peopleSaid`. | |
| 127 | + | ||
| 128 | + | **Model.** The production route for `review` is **Claude Sonnet 5.5** | |
| 129 | + | (`claude-sonnet-5-5`, `AGENT_ROUTES` in `services/runner/wrangler.jsonc`). | |
| 130 | + | A workspace can route reviews to its own provider through the model proxy | |
| 131 | + | (`openModelSession`). No effort level is set, so it uses the Claude Code | |
| 132 | + | default. The model-env test uses Opus 5.5 for review as a fixture only. | |
| 133 | + | ||
| 134 | + | **Outputs and post-processing** (`reviews.rs`, `report_review`): | |
| 135 | + | ||
| 136 | + | - Verdict `approve` / `request_changes`, or none if invalid. | |
| 137 | + | - A summary capped at 20,000 characters, signed with the model name. | |
| 138 | + | - Line comments: empty bodies dropped; **comments on files the PR does | |
| 139 | + | not change are dropped**; at most **30** comments; one `line` (no | |
| 140 | + | ranges). | |
| 141 | + | - Posted as `g1t-agent`, with a `review.completed` event. | |
| 142 | + | - Confidence (`confidence.rs`) uses the result: `request_changes` sinks | |
| 143 | + | the change's confidence; `approve` with 3 or more comments costs a | |
| 144 | + | point; no review costs a point. | |
| 145 | + | - The reviewer reports no confidence or severity of its own. `confidence::ASK` | |
| 146 | + | is added to implement and revise runs, not to reviews. | |
| 147 | + | ||
| 148 | + | ### How it would be scored | |
| 149 | + | ||
| 150 | + | The adapter (`bench/reviewbench/agent/agent.mjs`) maps each surviving line | |
| 151 | + | comment to `{file: path, start_line: line, end_line: line, message: body}`. | |
| 152 | + | The verdict and summary are kept beside the findings for analysis | |
| 153 | + | (`findings.g1t.json`) but not scored. ReviewBench scores only located | |
| 154 | + | findings. | |
| 155 | + | ||
| 156 | + | So: | |
| 157 | + | ||
| 158 | + | - A problem described only in the summary scores nothing. | |
| 159 | + | - An empty comment list on a PR with golden TPs costs recall and nothing | |
| 160 | + | else. | |
| 161 | + | - Every comment counts toward precision. A comment matching a golden *FP* | |
| 162 | + | counts against grounded precision; an unmatched one is classified by the | |
| 163 | + | judge. | |
| 164 | + | ||
| 165 | + | Benchmark repositories have no g1t memory, issue, comments or | |
| 166 | + | `.g1t/review.md`. The prompt is title, body and `INSTRUCTIONS`, plus | |
| 167 | + | whatever `CLAUDE.md` or `AGENTS.md` the repository carries (Claude Code | |
| 168 | + | reads `CLAUDE.md` itself). This measures the reviewer's core, which is | |
| 169 | + | what we want. Memory and instructions are product features to measure | |
| 170 | + | online. | |
| 171 | + | ||
| 172 | + | ## The harness (`bench/reviewbench/`) | |
| 173 | + | ||
| 174 | + | | File | What it does | | |
| 175 | + | |---|---| | |
| 176 | + | | `run.mjs` | The driver. `setup` clones ReviewBench at the pinned commit and installs its judge. `build` builds the image. `estimate` prices a run without spending. `review` runs our reviewer through ReviewBench's own `scripts/try-agent.sh`, one fresh container per PR, with the benchmark's mounts and validation. `judge` runs ReviewBench's judge with our key. `report` prints a summary or one JSON line. Nothing that spends runs without `--yes`. There is a per-PR cap (`--budget-usd`, default $5, passed to `--max-budget-usd`) and a per-run cap (`--max-total-usd`, default $150). | | |
| 177 | + | | `Dockerfile` | `FROM` the production sandbox base image named in `services/runner/base.json` (pinned by digest, same Claude Code CLI version), plus the adapter. | | |
| 178 | + | | `agent/agent.mjs` | The contract adapter: what `review.rs` does after its clone (writes `/work/change.diff`, runs Claude Code with `harness.rs`'s flags, reads `/work/review.json`), then the filters `reviews.rs` applies (changed files only, at most 30, 20,000 characters), then findings. It also writes a `findings.g1t.json` sidecar: verdict, summary, cost, turns, and comments dropped. | | |
| 179 | + | | `agent/instructions.txt` | Generated from `review.rs` by `prompt.mjs`. `node bench/reviewbench/prompt.mjs --check` fails if production's prompt has moved, which can run in CI. | | |
| 180 | + | | `agent/variants/*.txt` | Alternative instructions to A/B against `production` (`--variant`). `findings-first.txt` is the first candidate (see below). | | |
| 181 | + | | `.cache/` | Gitignored: the ReviewBench clone, repository mirrors, runs, scores. | | |
| 182 | + | ||
| 183 | + | Model access goes into each container from the environment: | |
| 184 | + | ||
| 185 | + | - `ANTHROPIC_API_KEY` alone sends requests straight to the provider. | |
| 186 | + | - `ANTHROPIC_BASE_URL=<MODELS_URL>/anthropic` plus a model-proxy session | |
| 187 | + | token sends them through g1t's model proxy. The spend then lands on a | |
| 188 | + | workspace like any run: use an internal workspace such as flagon-io so | |
| 189 | + | it is visible in billing. | |
| 190 | + | ||
| 191 | + | The judge needs `ANTHROPIC_API_KEY` (or another provider ReviewBench's | |
| 192 | + | `pi` registry supports). | |
| 193 | + | ||
| 194 | + | ```sh | |
| 195 | + | node bench/reviewbench/run.mjs setup | |
| 196 | + | node bench/reviewbench/run.mjs build | |
| 197 | + | node bench/reviewbench/run.mjs estimate --set sample:50 | |
| 198 | + | node bench/reviewbench/run.mjs review --set sample:50 --seed 1 --yes # production prompt | |
| 199 | + | node bench/reviewbench/run.mjs review --set sample:50 --seed 1 --variant findings-first --yes | |
| 200 | + | node bench/reviewbench/run.mjs judge --run <id> --yes | |
| 201 | + | node bench/reviewbench/run.mjs report --run <id> | |
| 202 | + | ``` | |
| 203 | + | ||
| 204 | + | `sample:N` is stratified by change size in the corpus's proportions | |
| 205 | + | (≤200 / 201 to 1,000 / >1,000 lines) and fixed by `--seed`, so week-over-week | |
| 206 | + | runs see the same PRs. The leaderboard's own protocol is `--set full`, three | |
| 207 | + | times. | |
| 208 | + | ||
| 209 | + | Before the first paid run: | |
| 210 | + | ||
| 211 | + | 1. Run `build` and one PR (`try-agent.sh … --pr 0`) with a capped key, to | |
| 212 | + | check that the adapter writes valid findings. This costs about $1. | |
| 213 | + | 2. Check that the image runs as a user that can write `/work`. The base | |
| 214 | + | image's default user is used. | |
| 215 | + | 3. Calibrate `estimateOne` in `run.mjs` against the billing ledger's real | |
| 216 | + | `review` runs. Every run reports `total_cost_usd`, and spend is kept per | |
| 217 | + | pull request. | |
| 218 | + | ||
| 219 | + | The adapter re-implements about 40 lines of `review.rs` rather than | |
| 220 | + | calling it, because `review.rs` clones from g1t and posts back to the API. | |
| 221 | + | To remove that duplication, a small product change would add | |
| 222 | + | `MODE=review-local` to `g1t-runner`: read `/work/pr/*`, skip the clone and | |
| 223 | + | the report, and write `RB_OUT`. The adapter would then be `exec g1t-runner`. | |
| 224 | + | That is worth doing once the benchmark is in regular use. | |
| 225 | + | ||
| 226 | + | ### Cost model | |
| 227 | + | ||
| 228 | + | Per PR, an agentic review re-reads a growing context from cache every turn. | |
| 229 | + | `run.mjs` models it as follows: | |
| 230 | + | ||
| 231 | + | - turns: 12 + 0.8·√lines + 0.4·files, capped at 80; | |
| 232 | + | - context: from about 18K tokens plus the diff, growing about 2.5K tokens | |
| 233 | + | per turn, capped at 180K; | |
| 234 | + | - 90%+ of input served as cache reads ($0.20/M on Sonnet 5.5); | |
| 235 | + | - new context written once ($2.50/M); | |
| 236 | + | - about 450 output tokens per turn ($10/M). | |
| 237 | + | ||
| 238 | + | This gives about $0.90 for a median PR and about $1.25 averaged over the | |
| 239 | + | corpus, which is skewed by the 36% of PRs over 1,000 lines. The judge is | |
| 240 | + | estimated at about $0.33 per PR: matching calls plus a tool-using | |
| 241 | + | classification of each unmatched finding. | |
| 242 | + | ||
| 243 | + | ## Likely weaknesses against the benchmark | |
| 244 | + | ||
| 245 | + | These come from reading the prompt and code; none is measured yet. | |
| 246 | + | ||
| 247 | + | 1. **Recall is capped by the instructions.** "Be specific and brief. | |
| 248 | + | Comment only on real problems" and nothing asking for coverage. The | |
| 249 | + | golden set has 12 TPs per PR (median 10) and 59% are low severity, which | |
| 250 | + | the benchmark counts as worth fixing: small correctness slips, missing | |
| 251 | + | tests, stale docs. The best reviewers emit about 5 findings per PR. We | |
| 252 | + | probably emit 0 to 3. Expect grounded recall well under 15%. | |
| 253 | + | 2. **Category blind spots.** The instructions frame the review as | |
| 254 | + | "correct, does what it's for, doesn't break anything". That covers | |
| 255 | + | correctness (41% of TPs) and reliability (14%). It gives no prompt for | |
| 256 | + | maintainability (14%), documentation (12%), testing (9%), security (4%), | |
| 257 | + | API design, performance or accessibility. | |
| 258 | + | 3. **Problems that live only in the summary.** Nothing requires every | |
| 259 | + | issue in `body` to also be a line comment. Those issues score zero, and | |
| 260 | + | in the product they are not anchored where an author fixes them. | |
| 261 | + | 4. **Comments outside the changed files are dropped.** `reviews.rs` | |
| 262 | + | discards them. The benchmark anchors some TPs in untouched files, for | |
| 263 | + | example a missing dependency in `pyproject.toml`, a caller the change | |
| 264 | + | breaks, or 173 pre-existing-scope TPs. That is a product decision (the | |
| 265 | + | UI has nowhere to put them), and it costs recall. | |
| 266 | + | 5. **Single-line anchors.** 78% of golden TPs span several lines. Matching | |
| 267 | + | is semantic within a file, so this matters less than the wrong file | |
| 268 | + | would. A range still helps the matcher and helps a person. | |
| 269 | + | 6. **Large changes.** 36% of the corpus is over 1,000 lines (median 9 | |
| 270 | + | files, mean 28). One agent with 80 turns, a 30-minute cap and a | |
| 271 | + | 30-comment cap will read the first files carefully and skim the rest. | |
| 272 | + | No fan-out by file group. | |
| 273 | + | 7. **No verification or calibration.** There is no second pass that checks | |
| 274 | + | each candidate against the code, so precision rests on the "brief" | |
| 275 | + | instruction, which also caps recall. There is no per-comment severity or | |
| 276 | + | confidence, so we cannot pick an operating point (β), and | |
| 277 | + | `confidence.rs` can only count comments. | |
| 278 | + | 8. **Context the product has but the reviewer does not get.** Required | |
| 279 | + | checks and CI results, overlapping PRs, and earlier reviews. These do | |
| 280 | + | not matter on the benchmark but matter online. | |
| 281 | + | ||
| 282 | + | ## Improvements to try, in order | |
| 283 | + | ||
| 284 | + | Each is one `--variant` (or a flag) on the same 50-PR sample with the same | |
| 285 | + | seed. The comparison is grounded precision and recall, by severity and | |
| 286 | + | category. | |
| 287 | + | ||
| 288 | + | 1. **Prompt: findings first** (`agent/variants/findings-first.txt`, | |
| 289 | + | written). It works in three passes: | |
| 290 | + | - map the change and its callers; | |
| 291 | + | - hunt by an explicit category checklist; | |
| 292 | + | - verify each candidate and drop what isn't true at head. | |
| 293 | + | ||
| 294 | + | It asks for every distinct problem rather than "brief", requires every | |
| 295 | + | summary issue as a line comment, and adds `end_line` and `severity`. | |
| 296 | + | The adapter already reads `end_line`. Expect the largest recall gain | |
| 297 | + | for little cost. | |
| 298 | + | 2. **Self-critique as a separate pass.** A second, cheaper call (Sonnet | |
| 299 | + | 5.5 at low effort, or Haiku 4.5) gets each candidate plus the exact | |
| 300 | + | code lines and answers "true at head? worth the author's time?". Keep | |
| 301 | + | the survivors. This lets the first pass be generous (recall) while | |
| 302 | + | holding precision. It costs about 10 to 20% more. | |
| 303 | + | 3. **Context retrieval.** Before the agent starts, compute the changed | |
| 304 | + | symbols (from diff hunks) and their references (`git grep`, or the | |
| 305 | + | context service where indexed), and put the list in the prompt. Point | |
| 306 | + | it at the tests that cover the touched files. This targets the 1,529 | |
| 307 | + | TPs needing "diff plus related files". | |
| 308 | + | 4. **Fan-out on large diffs.** Over about 800 lines or 15 files, split by | |
| 309 | + | directory or file group. Run parallel reviewers (Claude Code subagents, | |
| 310 | + | or separate runs) with the shared PR context, then merge and dedupe by | |
| 311 | + | file, line and meaning. This targets item 6, the long tail where recall | |
| 312 | + | collapses. | |
| 313 | + | 5. **Confidence calibration.** Have each comment carry `severity` and | |
| 314 | + | `confidence`. On the benchmark, sweep a threshold to draw the | |
| 315 | + | precision/recall curve and pick the product's operating point, for | |
| 316 | + | example post only medium and above inline and fold the low items into | |
| 317 | + | the summary. Feed the same fields to `confidence.rs`, so that one | |
| 318 | + | high-severity comment counts for more than three nits. | |
| 319 | + | 6. **Model and effort.** Run the same sample on Sonnet 5.5 at | |
| 320 | + | `medium`/`high` effort and on Opus 5.5. The review route can change in | |
| 321 | + | `AGENT_ROUTES` without a code change. Opus 5.5 is about 1.4x the cost | |
| 322 | + | per review at our token profile. | |
| 323 | + | 7. **Product-side follow-ups** (not benchmark-visible): | |
| 324 | + | - allow file-level comments on unchanged files when the change breaks | |
| 325 | + | them, instead of dropping them; | |
| 326 | + | - add required-check results and overlapping PRs to the review prompt. | |
| 327 | + | ||
| 328 | + | ## Making review quality a tracked metric | |
| 329 | + | ||
| 330 | + | **Offline (ReviewBench).** | |
| 331 | + | ||
| 332 | + | - A weekly g1t Actions workflow, `.g1t/workflows/reviewbench.yml`, runs | |
| 333 | + | `review --set sample:50 --seed 1` with the production variant and the | |
| 334 | + | production model, then `judge` and `report --json`. | |
| 335 | + | - The runner base image already has the toolchains. The job needs Docker, | |
| 336 | + | or, more simply, it can run `agent.mjs` directly in a sandbox that *is* | |
| 337 | + | the same image, with `/work` laid out by a small wrapper instead of | |
| 338 | + | `try-agent.sh`. | |
| 339 | + | - Budget: about $80 a week (about $350 a month), billed to the flagon-io | |
| 340 | + | workspace through the model proxy so it shows in spend. | |
| 341 | + | - Also run on demand for any change to `review.rs`'s instructions or to | |
| 342 | + | the review route. `prompt.mjs --check` in CI flags such a change. | |
| 343 | + | - Do a full-set run (about $350) monthly, or before a model switch. | |
| 344 | + | ||
| 345 | + | **Where results go.** | |
| 346 | + | ||
| 347 | + | - Append each `report --json` line to an internal results store: a D1 | |
| 348 | + | table in the work service, or a JSON file committed to `docs/research/`. | |
| 349 | + | - Show it on a **sudo "Review quality"** page, with a trend of: | |
| 350 | + | - grounded precision and recall; | |
| 351 | + | - recall by severity (high and medium matter most) and by category; | |
| 352 | + | - findings per PR; | |
| 353 | + | - cost per PR. | |
| 354 | + | ||
| 355 | + | Mark the g1t commit and model on each point. | |
| 356 | + | - Not on public docs: our own numbers are fine internally, but | |
| 357 | + | user-facing copy compares to no one. | |
| 358 | + | ||
| 359 | + | **Online, the metric that matters.** Track the *addressed rate* of | |
| 360 | + | `g1t-agent` line comments: the share followed by a commit touching those | |
| 361 | + | lines before merge, or resolved by a person. Also track the share of | |
| 362 | + | `request_changes` verdicts that led to a revision. g1t has the comments, | |
| 363 | + | the commits and the review runs, so this is a query, not a model call. Put | |
| 364 | + | it next to the offline number on the same sudo page. The article's lesson | |
| 365 | + | is that the offline number earns trust only while it moves the same way as | |
| 366 | + | the online one. | |
| 367 | + | ||
| 368 | + | ## Open questions | |
| 369 | + | ||
| 370 | + | - Run once on the public leaderboard? Onboarding is self-service (a GHCR | |
| 371 | + | image and our key). The final run is three rounds on 219 PRs at our | |
| 372 | + | inference cost (about $820) with their judge free. A ranking is | |
| 373 | + | marketing-adjacent, and the no-comparisons rule applies to anything we | |
| 374 | + | say about it. | |
| 375 | + | - The judge is a Claude model and so is our reviewer. The ReviewBench docs | |
| 376 | + | report human agreement for the labels but no per-family judge bias. | |
| 377 | + | Treat small differences (under 2 points, inside the leaderboard's | |
| 378 | + | round-to-round deviation of about 0.5 to 2) as noise. |