diff --git a/README.md b/README.md index 7f518ded..31421093 100644 --- a/README.md +++ b/README.md @@ -186,6 +186,7 @@ export ANTHROPIC_API_KEY=sk-ant-… | `--model` | `claude-haiku-4-5-20251001` | Anthropic model id. Use opus for stronger play at higher cost. | | `--turn-timeout` | `12s` | Per-turn LLM budget. The gateway default-actions the slot if exceeded. | | `--data-version` | `gen1-v1` | Must match the gateway's `DATA_VERSION` env. | +| `--name` | the `--model` id | Trainer name this slot's results post under on the leaderboard. | --- @@ -196,8 +197,8 @@ between "an arena where bots compete on a real leaderboard" and what runs today. | Area | Today | To make the headline true | |---|---|---| -| **Leaderboard identity** | Free-text name, no ownership; clients barely prompt | Prompt for a trainer/agent name everywhere a battle starts; surface the board in the SPA. (Optional later: claim-a-handle + secret to stop impersonation.) | -| **Leaderboard visibility** | Rating computed + stored, but not shown in the UI | A real standings page — wins/losses/Elo, sortable | +| **Leaderboard identity** | A joining slot declares its own name (`--name`, MCP `trainer_name`, `?name=` on the WS), and the battle's trainer is rebound to it — so a result posts under whoever actually played, not the placeholder the battle's creator typed. Names are **self-reported and unverified** | Claim-a-handle + secret, so a name on the board can't be taken by anyone who wants it | +| **Leaderboard visibility** | Standings page in the SPA (name + Elo), fed by `/api/leaderboard` | Sortable, with wins/losses alongside the rating — the store already returns them | | **Bot onboarding** | Two reference clients, MCP + CLI | A 5-minute "write your own bot" quickstart against a documented protocol | | **Provider coverage** | Benchmark (`cmd/bench`) runs Anthropic, OpenAI, Gemini, and local Ollama behind one `Client` interface, in `raw`/`cot` conditions; the live harness (`pokearena-agent`) is still Anthropic-only | Bring the remaining vendors to the live harness too | diff --git a/backlog/2026-08-11T18-30-identity-on-join.md b/backlog/2026-08-11T18-30-identity-on-join.md new file mode 100644 index 00000000..626d2f25 --- /dev/null +++ b/backlog/2026-08-11T18-30-identity-on-join.md @@ -0,0 +1,106 @@ +# Identity belongs to the joiner, not the creator + +The leaderboard has had working Elo since the beginning and has never +meant anything. Today's change is small — a name on the join — but it is +the one that makes a result attributable to whoever produced it. + +## The bug was in *when*, not *whether*, we asked for a name + +The README has said "free-text name, no ownership; clients barely prompt" +for months, and I read that as a UI gap: add a name field to more screens +and the board improves. That is not what was wrong. + +A `live_pvp` battle is created before its players arrive. The creator +POSTs `p1_name` / `p2_name`, both trainer FKs are bound *then*, and the +slots are filled later over the join WebSocket. So the names on a battle +are always the creator's guesses about who will show up — `"Opponent"`, +`"AI"`, `"Agent 2"`. There was no moment in the protocol at which the +actual occupant of a slot could say who it was. `pokearena-agent` had no +name flag, MCP `join_battle` had no name argument, and the SPA's +share-link join hardcoded `'Trainer'`. Not one of them was under-prompting; +there was nothing to prompt *into*. + +That reframes the fix. It isn't "collect the name in more places," it's +"make join a point at which identity can be declared," which needs a wire +field (`?name=` on the play URL), a message field (`LiveAction.Trainer`), +and a write (`RebindBattleTrainer`). Once those exist the three clients +are one line each. + +## The second track was blocked on the same field + +I went looking at PR #109 (decision-quality eval) in the same session and +its handoff doc says, in the gotchas: + +> **Postgres has NO model identity** — `p1_name` is always "Agent", +> `p2_name` "AI". Model attribution ONLY comes from the `bid=`→model +> mapping in the run dirs. The previous mapping (`/tmp/pk-agentic`) was +> wiped, which is why we re-ran. + +That is the same missing field, discovered independently, and it cost a +re-run of a full attributed batch. The benchmark had been reconstructing +in `/tmp` a fact the database should have carried. Worth naming because +the two symptoms look unrelated — "the leaderboard is meaningless" and +"we lost the model attribution for a batch" — and have one cause. + +## What I deliberately did not build + +The name is **self-reported**. Any holder of a slot token can claim any +name, including one already on the board. I considered gating it — a +secret per handle, first-writer-wins on a name — and stopped, because +that is a different feature (accounts) wearing this one's clothes, and +shipping half of it would produce a board that *looks* verified. + +So the honest split is: this change makes identity **expressible and +attributable**; it does not make it **verified**. Both the README status +row and `live-pvp.md` §3 now say that in those words, and §3 lists +impersonation under "not designing against" rather than leaving it +unmentioned. The README's "for fun, unverified" disclaimer stays exactly +as it was — it is still true, just for a narrower reason. + +Two things I did constrain, because they are cheap and the absence would +be a real bug rather than a known limitation: + +- **A name is only accepted after the slot claim succeeds.** Rebinding + before the claim would let anyone who can guess a battle id rewrite its + trainers without ever playing. +- **`RebindBattleTrainer` refuses a battle with a winner.** Elo is applied + from `p1_trainer` / `p2_trainer` at completion; a late or replayed join + that renamed a settled battle would move a rating that was already + computed against the old trainer. The `winner < 0` guard is the whole + defense and it is why that method has an integration test. + +## A concurrency seam I didn't expect + +`trainerName` was a plain `[2]string` on `Match`, written once at +construction and read by the coordinator goroutine. Accepting a name at +attach makes it a field written by the *action-pump* goroutine at an +arbitrary moment — a data race, even though nothing about a display name +can affect a turn's outcome. + +I gave it its own tiny mutex-guarded type (`trainerNames`) rather than +routing the name through the coordinator's channel, following the +`slotConns` precedent already in the package: a self-contained concurrency +unit, documented as touched from one goroutine and read from another. The +comment on it says out loud that a concurrent name arrival is benign in +behavior and a race only in the memory-model sense, because that's the +thing a future reader will otherwise have to re-derive before touching it. + +`set("")` is a no-op rather than a store. Without that, an anonymous +re-attach — which is exactly what a reconnect after a blip is — would +blank a name the slot declared on its first attach. + +## Where the default came from + +`pokearena-agent --name` defaults to the model id rather than to empty. +An unnamed agent inherits the creator's placeholder, which is the +behavior that caused all of this, so "stay anonymous" is the wrong +default for a bot: the useful thing a harness can do with no +configuration is say which model played. The SPA remembers its name in +`localStorage` so a returning player keeps their row instead of minting +"Challenger" every session. + +The one path still weaker than I'd like: a first-time share-link joiner +in the browser gets `"Challenger"`, because they never passed through the +setup form and I didn't want to put a modal in front of a battle +invitation. A name field on the picker screen is the obvious follow-up +and is UI work, not protocol work — the protocol side is done. diff --git a/backlog/2026-08-11T21-15-remeasuring-decision-quality.md b/backlog/2026-08-11T21-15-remeasuring-decision-quality.md new file mode 100644 index 00000000..631e26cf --- /dev/null +++ b/backlog/2026-08-11T21-15-remeasuring-decision-quality.md @@ -0,0 +1,195 @@ +# Re-measuring decision quality, and what wouldn't re-measure + +PR #109 had been open since 17 July and was 46 commits behind. The plan was +"rebase it and re-measure on library v2." The rebase was the easy half; the +re-measure turned out to be two different questions with two different answers, +and separating them is most of what this round produced. + +## The rebase was free, which is worth noting + +Eight commits, 1367 lines, 46 commits of drift — and exactly one file +overlapped with anything `main` had touched (`internal/ai/expectimax.go`, where +main changed the replace-phase handling and the PR appended `ScoreActions`). +Cherry-picked clean, built, and the whole suite went green with no edits. + +I had assumed a branch that stale would need real work, and it didn't, because +the PR is almost entirely *additive* — a new package file, a new command, a new +report section. That is a property of how it was scoped, not luck. Worth +remembering the next time a branch's age is used as an argument for abandoning +it: age is a proxy for conflict risk, and a purely additive branch barely +accumulates any. + +## The published table can't be re-measured, and saying so is the deliverable + +`docs/decision-quality.md` carried a four-model table — Gemini / Opus / Sonnet / +Haiku, blunder rate and median regret — measured in July on v1 neutral teams. +Library v2 landed 4 August. So those numbers describe a format that no longer +ships, which is precisely the failure the +[v2 re-sweep entry](2026-08-04T17-51-resweeping-expectimax-on-v2.md) was written +up to prevent, recurring in a different document three weeks later. + +The depth sweep could be re-measured because it runs offline. This one can't: +the battles lived in a Postgres on the author's machine and their model +attribution in `/tmp/pk-agentic-v2`, and the handoff doc records that `/tmp` +being wiped had *already* forced one re-run. Both are gone now. Reproducing the +table means paying for a fresh batch across four vendors — a decision about +money, not an afternoon of compute. + +So the table is labelled v1-era and unrepeatable, with the reason, next to the +v2 numbers rather than instead of them. Same treatment the pre-fix depth numbers +got in August. The thing I want to avoid is a reader assuming one run produced +everything on the page. + +## What *could* be measured, and the bias it exposed + +The metric itself had never been shown to rank correctly. It was validated on +"does recovery reproduce the stored state" — a data-integrity property — and +then pointed straight at four models, where nobody knows the true ordering. If +the metric were subtly inverted, nothing in the pipeline would have said so. + +`cmd/decision-sim` closes that: it plays deterministic policies offline and +writes the same export shape the live path persists, so the whole pipeline runs +from a checkout with no gateway, no database, and no API spend. 72 games on v2: + +| policy | win rate | blunder rate | median regret | +|---|---:|---:|---:| +| expectimax d2 | 44% | 3% | 0 | +| expectimax d1 | 61% | 11% | 12 | +| heuristic | 56% | 21% | 111 | +| random | 0% | 39% | 192 | + +Blunder rate is monotone in policy strength. Good — that's the soundness check, +and it's now a test rather than a hope. + +But look at the first two rows. **Expectimax d2 blunders least and wins least.** +It is not the best player in the table; it is the player most *similar to the +oracle*, which is expectimax d3. Its 68% match rate against the heuristic's 27% +is the same fact stated louder. + +That is a real limitation and it was hiding in plain sight. The doc's central +fairness argument is that every policy is scored from `ai.MakeView` — the +identical fog-of-war projection — so the oracle is "a better player looking at +the same information, not an omniscient one." That argument is correct and it is +about *information*. It says nothing about *algorithm*, and algorithm turns out +to matter: agreement with an expectimax oracle partly measures being an +expectimax. + +The uncomfortable part is what that does to the doc's headline finding. "Gemini +blunders least but wins less than Opus" is structurally identical to "d2 blunders +least but wins least," and in the case where I know the cause, the cause is +kinship with the yardstick. That doesn't explain the model result away — no LLM +is running expectimax — but it means the finding cannot be read as +"Gemini reasons more cleanly" without an argument that its style isn't simply +closer to the oracle's. Separating those needs a second oracle of a different +family, and raising the depth won't do it: a deeper oracle is a *more* +expectimax-shaped one. + +## A cross-check I didn't plan and am glad I ran + +Expectimax d2 came out at 44% against the heuristic. `docs/benchmark.md` §6 puts +it at 42.9% [36.8, 49.2] on v2. Those are the same number, produced by two +entirely separate code paths — the `bench` match runner there, `CaptureStored` +plus a fresh export/score pipeline here. + +I ran it as a sanity check on my harness. It is better than that: it is evidence +the offline capture reproduces live-shaped battles faithfully, which is the +assumption the whole `decision-sim` path rests on. Without it, "the metric works +offline" would have been an assertion about code I had just written. + +## Threshold calibration, briefly + +`BlunderThreshold = 300` was flagged in the original handoff as a first guess to +calibrate once the data existed. On v2: random's *median* regret is 192, below +the bar. So 300 is not "notices sloppiness" — it is a severe-tail cut, and even a +policy choosing uniformly at random sits under it half the time. That is +defensible for a headline metric, and it is exactly why median regret belongs in +the table next to it rather than behind it. Left the constant alone; the number +is fine once you know what it means, and now the doc says. + +## The ranking is now a test, not a table + +I first wrote this entry with "wire the ordering into CI" as the open item, then +noticed the objection to that: the monotonicity above is the metric's *only* +soundness property, and leaving it as a number in a document means it is checked +whenever someone re-reads the document. It is the thing most likely to break +silently if the oracle or the value function moves. + +`TestScoreDecisions_RanksAWorsePolicyAsBlunderingMore` pins it — random against +heuristic, one team, a depth-2 oracle, 0.8 seconds. Coarse on purpose: the +property is the *direction* of the ranking, and the widest available gap is the +one least likely to flake. The four-policy sweep stays in `decision-sim` where +it can afford the nine minutes. + +## The second oracle existed all along + +I closed the section above with "a second oracle of a different family would +quantify the bias, and doesn't exist." That was wrong within the hour. The +heuristic agent *is* a second family — depth-0, no lookahead, no opponent model +— and it already scores every legal action internally to pick its own move. +Exposing `ScoreActions` on it was ten lines. + +The lesson is not "look harder before declaring something missing," though that +too. It is that I had been thinking of the oracle as *the strongest available +player*, so the only candidates I considered were things stronger than +expectimax d3 — of which there are none here. The requirement is not strength, +it is **independence**. A weaker judge from a different family is far more +informative than a marginally stronger one from the same family, because the +question is whether two unrelated notions of "good move" agree. + +## What the second judge said + +Same 72 battles, scored twice: + +| policy | vs expectimax d3 | vs heuristic | +|---|---:|---:| +| expectimax d2 | 3% (best) | 19% (3rd) | +| expectimax d1 | 11% (2nd) | 13% (2nd) | +| heuristic | 21% (3rd) | 2% (best) | +| random | 39% (worst) | 22% (worst) | + +The three skilled policies rank in **exactly opposite order**. Each judge crowns +its own family. Match rate is the cleanest statement of it: the heuristic policy +agrees with the heuristic oracle 92% of the time and with expectimax 27% — same +player, same games, same fog-of-war projection. + +I expected the bias to be real and modest, something to note in a caveat. It is +total. On this evidence a single-oracle blunder rate does not rank skilled +policies at all; it reports proximity to the judge. The one finding that +survives both is that random is worst. + +That is a much harsher result for the metric than I went looking for, and it +lands directly on the doc's headline. "Gemini blunders least but wins less than +Opus" is structurally identical to "expectimax d2 blunders least but wins least," +and in the case where the cause is knowable, the cause is kinship. No LLM is an +expectimax, so this doesn't prove the model ordering wrong — it removes the +grounds for believing it. The one time the ordering could be checked against an +independent judge, it inverted. + +## The judge has to be strong enough to be a judge + +A second finding from the same runs, and one I nearly shipped a broken test +over. I wrote the soundness test with a depth-2 oracle because depth 3 was slow, +and it failed — random scored *better* than the heuristic. Not a flake: at depth +2 the expectimax judge gives random 35% and heuristic 33%, a two-point margin. +At depth 3, 43% vs 15%. + +So a depth-2 oracle cannot distinguish random play from competent play. Depth 3 +isn't a preference, it's the floor below which the metric measures nothing. + +The part worth keeping: this runs *against* the intuition from +[§6](../docs/benchmark.md), where expectimax wins fewer games as depth rises. +Playing well and judging well are different capabilities, and the depth that +hurts one is required by the other. I would have assumed the depth sweep's +conclusion transferred. It doesn't. + +The test now asserts the wide-margin, fast half (random vs heuristic under the +heuristic judge, 22% vs 3%, one second) and documents why the expectimax arm +lives in `decision-sim` instead. A two-point margin dressed up as a soundness +property would have been worse than no test. + +## Still open + +A third oracle that isn't hand-built. Expectimax and the heuristic are different +families but the same author and the same era of thinking about this game; they +could share blind spots. A trained policy would be the real test. Nothing in the +repo is close to that today. diff --git a/backlog/2026-08-12T01-40-fixing-the-baseline-the-metric-found.md b/backlog/2026-08-12T01-40-fixing-the-baseline-the-metric-found.md new file mode 100644 index 00000000..664244f6 --- /dev/null +++ b/backlog/2026-08-12T01-40-fixing-the-baseline-the-metric-found.md @@ -0,0 +1,101 @@ +# Fixing the baseline the metric found + +The verifiable-error metric was built the day before to measure *contestants*. +The first thing it measured was our own reference opponent, and it found the +heuristic spending turns on moves the rules make impossible: 27 boost-at-cap +turns across six games, and Thunder Wave re-applied to an already-paralysed +target for six consecutive turns. + +## Two bugs that look like one + +They are not the same mistake, and the difference is the interesting part. + +**The boost branch had no check at all.** It valued a self-boost at 55 while +healthy, never consulting `me.Stages`. At +6 the move cannot change anything and +it still scored 55, which beats most attacks. So the agent boosted, and boosted, +and boosted. + +**The status branch did have a check** — `return 0` with the comment "a status +move is wasted on an already-statused foe." Someone saw this exact case and +guarded it. The guard was just not strong enough: a damaging move that deals no +damage also scores 0, and `Decide` breaks ties toward the earliest legal action, +so a dead status move in an early move slot won the tie and was replayed every +single turn. + +That second one is the one worth remembering. The code contained the right +belief, correctly commented, and was still wrong — because 0 is not a neutral +value in a scoring function whose other outputs bottom out at 0. The fix is a +`deadMoveScore` constant well below anything a live option can score, including +switching, since giving up a turn to reposition genuinely beats spending it on a +guaranteed no-op. + +## Not the engine, and that distinction did the work + +Every one of these moves *fails correctly* in the engine. That is precisely why +they were detectable — the metric watches for actions that provably cannot +accomplish anything, and the engine's correct rejection of them is what makes +"provably" true. + +So the blast radius was bounded in a way worth stating: rules, replays, +determinism, fairness — all untouched. What changed is one *player*. But that +player is the opponent every published figure in §6 is measured against, so +"only the bot" still meant re-measuring the document's only load-bearing table. + +## Re-sweeping, and being wrong about what I expected + +I expected the fix to move the numbers a little and braced for having to +re-caveat §6. 240 games per depth, same methodology as before: + +| Depth | pre-fix | post-fix | +|---|---:|---:| +| 1 | 54.4% | 53.8% | +| 2 | 42.9% | 42.9% | +| 3 | 42.1% | 42.1% | + +Depths 2 and 3 are identical to the exact game — 103/240 and 101/240 both times. +Depth 1 moved by two games out of 240. + +The reason, once you look: the wasted turns clustered in already-decided stall +positions. The six-turn Thunder Wave loop was on the Bastion wall team at turns +72–77 — a game whose outcome was long settled. A bot wasting turns in a position +it has already won or lost does not change the result. Twenty-seven wasted turns +sounds like a lot until you notice which turns they were. + +Two things I'd have gotten wrong by reasoning instead of measuring: I would have +guessed the effect was larger, and if the numbers *had* moved I would have had +no way to tell whether the fix or ordinary variance did it. Re-running the whole +sweep on a deterministic offline harness costs 22 minutes of compute and no +money, which is a very cheap way to convert a guess into a fact. + +## The sweep is now committed + +It had been a scratch test. That was the same mistake as the last three rounds +in miniature — a published number whose derivation lives in a `/tmp` file. It +is now `TestDepthSweep`, gated behind `POKEARENA_DEPTH_SWEEP=1` so it never +runs in the suite, and §6 carries the command. The figures in the document can +be re-derived rather than trusted. + +## What I did not fix, and why that was the harder call + +The heuristic also under-values Rest — the move carries no effect block in the +dataset, so it never reaches the heal branch and lands on a flat score of 5. +Fixing that would make the baseline meaningfully stronger. + +I left it. The line I drew: **correct a provable waste, do not change a +strategy.** A move that cannot possibly work is a defect in any sense of the +word, and fixing it makes the bot do what it already meant to do. Teaching it to +use Rest well is a judgement about how the game should be played, and it would +change the baseline's strength — which is a decision about what the benchmark +measures, not a bug fix, and it should be made deliberately and announced rather +than smuggled in beside a correctness patch. + +The same line explains why the ability-immunity case is absent from the metric +itself: Levitate blanks a Ground move, but the attacker may not know the +ability, so charging it would penalise unknowable information. + +## The loop that closed + +Built a metric to judge outside contestants; it immediately indicted the +reference opponent. That is a good sign about the metric and an uncomfortable +one about the baseline, and the useful version of both is that the tool works on +its author. Worth expecting the next measurement instrument to do the same. diff --git a/cmd/bench-report/main.go b/cmd/bench-report/main.go index a7d9250c..bc72365a 100644 --- a/cmd/bench-report/main.go +++ b/cmd/bench-report/main.go @@ -37,12 +37,40 @@ func main() { ref := flag.String("ref", "heuristic", "benchmark mode: the one opponent every contestant is scored against") dataDir := flag.String("data", "data", "benchmark mode: dataset directory (for replay re-simulation)") teamsPath := flag.String("teams", "data/benchmark-teams.json", "benchmark mode: team library the baseline replays re-simulate on") + decisionQuality := flag.String("decision-quality", "", "benchmark mode: JSON of per-model decision-quality stats (decision-eval -json) to render as a section") + benchRun := flag.String("bench-run", "", "standings mode: a bench-run output directory; prints the entrant table and exits") + asJSON := flag.Bool("json", false, "standings mode: emit the table as JSON") flag.Parse() + // Standings mode: recompute a run's headline numbers from its result files + // alone. No gateway, no Postgres, no API access -- so the table under a + // published claim can always be re-derived from a directory small enough to + // commit alongside it. + if *benchRun != "" { + games, err := LoadRun(*benchRun) + if err != nil { + log.Fatalf("read run: %v", err) + } + if len(games) == 0 { + log.Fatalf("no game results in %s", *benchRun) + } + rows := Standings(games) + if *asJSON { + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + if err := enc.Encode(rows); err != nil { + log.Fatalf("encode: %v", err) + } + return + } + fmt.Print(FormatStandings(rows)) + return + } + // Benchmark mode: fold both arms into one "vs the reference" ladder and // render it through the standard report — same leaderboard, same replays. if *baseline != "" || *agentic != "" { - if err := renderBenchmark(*baseline, *agentic, *ref, *dataDir, *teamsPath, *outPath); err != nil { + if err := renderBenchmark(*baseline, *agentic, *ref, *dataDir, *teamsPath, *decisionQuality, *outPath); err != nil { log.Fatalf("%v", err) } return @@ -81,7 +109,7 @@ func main() { // renderBenchmark builds the vs-reference RunRecord from the two benchmark arms // and writes it through the standard report renderer. -func renderBenchmark(baselinePath, agenticDir, ref, dataDir, teamsPath, outPath string) error { +func renderBenchmark(baselinePath, agenticDir, ref, dataDir, teamsPath, dqPath, outPath string) error { dex, err := domain.LoadDex(dataDir, "bench") if err != nil { return fmt.Errorf("load dex: %w", err) @@ -94,9 +122,26 @@ func renderBenchmark(baselinePath, agenticDir, ref, dataDir, teamsPath, outPath // Reuse the trace's own header for provenance (engine revision, ruleset, // dataset), but null the round-robin game count — this report's game total is // per-contestant, shown in the leaderboard, not derivable from that formula. - header, err := readTraceHeader(baselinePath) - if err != nil { - return fmt.Errorf("read trace header: %w", err) + // The baseline is optional: with no trace (agentic arm alone, e.g. a + // decision-quality report) synthesize a minimal header from the loaded + // dataset so the masthead and ruleset pills still render. + var header eval.RunHeader + if baselinePath != "" { + if h, err := readTraceHeader(baselinePath); err == nil { + header = h + } else if !os.IsNotExist(err) { + return fmt.Errorf("read trace header: %w", err) + } else { + log.Printf("no baseline trace at %s — rendering the agentic arm alone", baselinePath) + } + } + if header.Ruleset == "" { + header.Ruleset = eval.Ruleset() + } + if len(header.Teams) == 0 { + for _, t := range lib.Teams { + header.Teams = append(header.Teams, t.Name) + } } header.GamesPerPairing = 0 @@ -105,6 +150,17 @@ func renderBenchmark(baselinePath, agenticDir, ref, dataDir, teamsPath, outPath return fmt.Errorf("build vs-reference record: %w", err) } + // Optional decision-quality section: precomputed offline (decision-eval + // scores stored turns against the oracle), loaded here so the report can show + // reasoning quality without re-scoring or touching a database. + if dqPath != "" { + dq, err := loadDecisionQuality(dqPath) + if err != nil { + return fmt.Errorf("load decision-quality: %w", err) + } + rec.DecisionQuality = dq + } + // Contestant names for the masthead count (the reference is already dropped). header.Contestants = header.Contestants[:0] for _, c := range rec.Contestants { @@ -150,6 +206,20 @@ func readTraceHeader(path string) (eval.RunHeader, error) { return h, nil } +// loadDecisionQuality reads the per-model decision-quality stats emitted by +// `decision-eval -manifest ... -json` (a JSON array of eval.ModelStats). +func loadDecisionQuality(path string) ([]eval.ModelStats, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("read %s: %w", path, err) + } + var stats []eval.ModelStats + if err := json.Unmarshal(data, &stats); err != nil { + return nil, fmt.Errorf("parse %s: %w", path, err) + } + return stats, nil +} + func loadRecord(path string) (eval.RunRecord, error) { data, err := os.ReadFile(path) if err != nil { diff --git a/cmd/bench-report/report_run.go b/cmd/bench-report/report_run.go new file mode 100644 index 00000000..52f25b28 --- /dev/null +++ b/cmd/bench-report/report_run.go @@ -0,0 +1,161 @@ +package main + +// Reporting for a bench-run output directory: pooled win rate with Wilson +// intervals per entrant, plus head-to-head-free standings. +// +// This reads only the per-game result files bench-run wrote. It deliberately +// does not need Postgres, the gateway, or any API access, so the numbers behind +// a published table can be recomputed from a directory that fits in a git +// commit. + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + + "pokearena/internal/eval" +) + +// runGame mirrors the record bench-run writes per game. Duplicated rather than +// shared so the two commands can be read independently; the json tags are the +// contract between them. +type runGame struct { + Label string `json:"label"` + Entrant string `json:"entrant"` + Harness string `json:"harness"` + Model string `json:"model"` + Team string `json:"team"` + BattleID string `json:"battle_id"` + Winner int `json:"winner"` + Status string `json:"status"` + Seconds int `json:"seconds"` +} + +// EntrantStanding is one row of the run's results table. +type EntrantStanding struct { + Entrant string `json:"entrant"` + Harness string `json:"harness"` + Model string `json:"model"` + Games int `json:"games"` + Wins int `json:"wins"` + Losses int `json:"losses"` + Unfinished int `json:"unfinished"` + WinRate float64 `json:"win_rate"` + CILow float64 `json:"ci_low"` + CIHigh float64 `json:"ci_high"` + MedianSecs int `json:"median_seconds"` +} + +// LoadRun reads every per-game result in a bench-run output directory. +func LoadRun(dir string) ([]runGame, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return nil, err + } + var out []runGame + for _, e := range entries { + name := e.Name() + if e.IsDir() || !strings.HasSuffix(name, ".json") || name == "config.json" { + continue + } + b, err := os.ReadFile(filepath.Join(dir, name)) + if err != nil { + return nil, err + } + var g runGame + if err := json.Unmarshal(b, &g); err != nil { + return nil, fmt.Errorf("%s: %w", name, err) + } + out = append(out, g) + } + sort.Slice(out, func(i, j int) bool { return out[i].Label < out[j].Label }) + return out, nil +} + +// Standings folds games into one row per entrant. +// +// Unfinished games (winner == -1: the agent hung, timed out, or abandoned) are +// counted and reported but excluded from the win-rate denominator. Counting +// them as losses would conflate "played badly" with "the harness fell over", +// which are different findings — and a harness that fails to finish games is +// itself a result worth seeing in its own column rather than hidden inside a +// win rate. +func Standings(games []runGame) []EntrantStanding { + type acc struct { + harness, model string + wins, losses, unfin int + secs []int + } + byID := map[string]*acc{} + for _, g := range games { + a := byID[g.Entrant] + if a == nil { + a = &acc{harness: g.Harness, model: g.Model} + byID[g.Entrant] = a + } + switch g.Winner { + case 0: + a.wins++ + case 1: + a.losses++ + default: + a.unfin++ + } + if g.Seconds > 0 { + a.secs = append(a.secs, g.Seconds) + } + } + + out := make([]EntrantStanding, 0, len(byID)) + for id, a := range byID { + decided := a.wins + a.losses + s := EntrantStanding{ + Entrant: id, Harness: a.harness, Model: a.model, + Games: decided + a.unfin, Wins: a.wins, Losses: a.losses, + Unfinished: a.unfin, MedianSecs: medianInt(a.secs), + } + if decided > 0 { + s.WinRate = float64(a.wins) / float64(decided) + s.CILow, s.CIHigh = eval.WilsonInterval(float64(a.wins), decided, 1.96) + } + out = append(out, s) + } + sort.Slice(out, func(i, j int) bool { + if out[i].WinRate != out[j].WinRate { + return out[i].WinRate > out[j].WinRate + } + return out[i].Entrant < out[j].Entrant + }) + return out +} + +func medianInt(xs []int) int { + if len(xs) == 0 { + return 0 + } + s := append([]int(nil), xs...) + sort.Ints(s) + n := len(s) + if n%2 == 1 { + return s[n/2] + } + return (s[n/2-1] + s[n/2]) / 2 +} + +// FormatStandings renders the table. The confidence interval is printed beside +// every win rate rather than offered as an option, because a win rate from a +// small run is the single easiest number in this project to over-read. +func FormatStandings(rows []EntrantStanding) string { + var b strings.Builder + fmt.Fprintf(&b, "%-24s %6s %5s %5s %6s %8s %-18s %7s\n", + "entrant", "games", "W", "L", "unfin", "win%", "95% CI", "med s") + for _, r := range rows { + ci := fmt.Sprintf("[%.0f%%, %.0f%%]", 100*r.CILow, 100*r.CIHigh) + fmt.Fprintf(&b, "%-24s %6d %5d %5d %6d %7.1f%% %-18s %7d\n", + r.Entrant, r.Games, r.Wins, r.Losses, r.Unfinished, 100*r.WinRate, ci, r.MedianSecs) + } + return b.String() +} diff --git a/cmd/bench-run/main.go b/cmd/bench-run/main.go new file mode 100644 index 00000000..5e837bb5 --- /dev/null +++ b/cmd/bench-run/main.go @@ -0,0 +1,287 @@ +// Command bench-run plays a whole benchmark matrix and can be re-run safely. +// +// One command, any size, resumable. It enumerates every game the config calls +// for, skips the ones already on disk, and plays the rest through the per-game +// runner (scripts/bench/play-live.sh), which drives an agent CLI over the +// PokéArena MCP tools. +// +// Three properties are the point: +// +// - **Resumable.** The plan is a pure function of the config and each game's +// result lands in its own file named from its coordinates. A run killed +// overnight resumes by rebuilding the same plan and skipping what exists — +// there is no central ledger to lose. +// - **Balanced under interruption.** Games are interleaved across entrants, +// so stopping early leaves every entrant with the same number of games +// rather than the first one complete and the last with none. +// - **Durable attribution.** Each game's entrant id is sent as the trainer +// name, so which agent played which battle is a fact in Postgres, not a +// mapping in a scratch directory. A previous batch was lost exactly that +// way (docs/decision-quality-eval-handoff.md). +// +// Usage: +// +// bench-run -config bench.json -out runs/2026-08-11 # play +// bench-run -config bench.json -out runs/2026-08-11 -dry-run # show the plan +package main + +import ( + "context" + "encoding/json" + "flag" + "fmt" + "log" + "os" + "os/exec" + "os/signal" + "path/filepath" + "strconv" + "strings" + "sync" + "syscall" + "time" + + "pokearena/internal/eval" +) + +// GameResult is the per-game record written to the output directory. Its +// presence is what marks a game complete for resume, so it is written only +// after the runner returns. +type GameResult struct { + Label string `json:"label"` + Entrant string `json:"entrant"` + Harness string `json:"harness"` + Model string `json:"model"` + Team string `json:"team"` + Index int `json:"index"` + BattleID string `json:"battle_id"` + Winner int `json:"winner"` // 0 = entrant, 1 = opponent, -1 = unfinished + Status string `json:"status"` + StartedAt string `json:"started_at"` + Seconds int `json:"seconds"` +} + +func main() { + log.SetFlags(log.Ltime) + log.SetPrefix("[bench-run] ") + + configPath := flag.String("config", "bench.json", "run configuration") + outDir := flag.String("out", "", "output directory for results (required)") + dryRun := flag.Bool("dry-run", false, "print the plan and what remains, then exit") + scriptPath := flag.String("runner", "scripts/bench/play-live.sh", "per-game runner script") + flag.Parse() + + if *outDir == "" { + log.Fatal("-out is required") + } + + cfg, err := loadConfig(*configPath) + if err != nil { + log.Fatalf("config: %v", err) + } + if err := cfg.Validate(); err != nil { + log.Fatalf("config: %v", err) + } + + plan := eval.Interleaved(eval.BuildPlan(cfg)) + if err := os.MkdirAll(*outDir, 0o755); err != nil { + log.Fatalf("mkdir %s: %v", *outDir, err) + } + done, err := completedLabels(*outDir) + if err != nil { + log.Fatalf("scan %s: %v", *outDir, err) + } + + s := eval.Summarize(plan, done) + log.Printf("%d entrants x %d teams x %d games = %d total (%d per entrant)", + s.Entrants, s.Teams, cfg.GamesPerTeam, s.Total, s.PerEntrant) + log.Printf("%d already complete, %d remaining", s.Total-s.Remaining, s.Remaining) + + todo := eval.Remaining(plan, done) + if *dryRun { + for _, g := range todo { + fmt.Printf("%s\t%s\t%s\t%s\n", g.Label, g.Entrant.Harness, g.Entrant.Model, g.Team) + } + return + } + if len(todo) == 0 { + log.Print("nothing to do") + return + } + + // Save the config next to the results so a published number can be traced + // to the matrix that produced it, even if the source config later changes. + if err := writeJSON(filepath.Join(*outDir, "config.json"), cfg); err != nil { + log.Fatalf("write config copy: %v", err) + } + + conc := cfg.Concurrency + if conc < 1 { + conc = 1 + } + log.Printf("playing %d games, concurrency %d", len(todo), conc) + run(todo, *outDir, *scriptPath, conc) +} + +func run(todo []eval.PlannedGame, outDir, script string, conc int) { + // Canceled only when run returns, so any child process still alive at + // teardown is killed rather than orphaned. Interrupt deliberately does NOT + // cancel it — see below, in-flight games are allowed to finish writing. + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Ctrl-C stops scheduling new games but lets in-flight ones finish writing, + // so an interrupted run leaves no half-written result file to be mistaken + // for a completed game on resume. + stop := make(chan os.Signal, 1) + signal.Notify(stop, os.Interrupt, syscall.SIGTERM) + stopped := make(chan struct{}) + go func() { + <-stop + log.Print("interrupt: finishing in-flight games, then stopping (re-run to resume)") + close(stopped) + }() + + sem := make(chan struct{}, conc) + var wg sync.WaitGroup + var mu sync.Mutex + played, failed := 0, 0 + + for _, g := range todo { + select { + case <-stopped: + wg.Wait() + log.Printf("stopped: %d played, %d failed this session", played, failed) + return + default: + } + + wg.Add(1) + sem <- struct{}{} + go func(g eval.PlannedGame) { + defer wg.Done() + defer func() { <-sem }() + + res, err := playGame(ctx, g, outDir, script) + mu.Lock() + defer mu.Unlock() + if err != nil { + failed++ + log.Printf("%s: FAILED: %v", g.Label, err) + return + } + played++ + log.Printf("%s: winner=%d (%s) in %ds", g.Label, res.Winner, res.Status, res.Seconds) + }(g) + } + wg.Wait() + log.Printf("done: %d played, %d failed this session", played, failed) +} + +// playGame runs one game and records it. The result file is written only on a +// clean run: a game that errored is left absent so re-running retries it rather +// than baking a failure into the dataset. +func playGame(ctx context.Context, g eval.PlannedGame, outDir, script string) (GameResult, error) { + started := time.Now() + // The entrant id travels as the trainer name, so attribution is recorded in + // the battle row rather than inferred later from a scratch file. + // + // No timeout on this context: the per-game wall-clock cap lives in the + // runner script, which also kills the CLI's own children. This context + // exists only so a straggler dies at teardown instead of being orphaned. + cmd := exec.CommandContext(ctx, "bash", script, + g.Entrant.Harness, g.Entrant.Model, g.Team, g.Label, outDir, g.Entrant.ID) + out, err := cmd.CombinedOutput() + if err != nil { + return GameResult{}, fmt.Errorf("%s: %w", lastLine(string(out)), err) + } + + bid, winner, status := parseRunnerOutput(string(out)) + if bid == "" { + return GameResult{}, fmt.Errorf("runner printed no bid=: %s", lastLine(string(out))) + } + + res := GameResult{ + Label: g.Label, Entrant: g.Entrant.ID, Harness: g.Entrant.Harness, + Model: g.Entrant.Model, Team: g.Team, Index: g.Index, + BattleID: bid, Winner: winner, Status: status, + StartedAt: started.UTC().Format(time.RFC3339), Seconds: int(time.Since(started).Seconds()), + } + return res, writeJSON(filepath.Join(outDir, g.Label+".json"), res) +} + +// parseRunnerOutput reads the runner's authoritative result line, which comes +// from the gateway rather than the agent's self-report: +// +//