Skip to content

feat(bench): add the public audit benchmark - #109

Open
drewstone wants to merge 3 commits into
mainfrom
feat/bench-audit
Open

feat(bench): add the public audit benchmark#109
drewstone wants to merge 3 commits into
mainfrom
feat/bench-audit

Conversation

@drewstone

@drewstone drewstone commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

What this is

Step 0 of the traces audit hill-climb: the public benchmark the rest of the track is scored against, under bench/audit/. It answers one question — on a coding-agent session, does an audit arm get the facts right, and at what cost — with a fleet of general subagents over the raw files as the arm to beat.

Nothing in here touches the shipped package. bench/ is not in files, so the npm tarball is unchanged.

Failure classes it measures

Each question carries the brief's failure classes in its probes field, and the fixtures plant a trap for each:

  • F1 ordered human turns told apart from injected text — an AGENTS.md block, environment_context blocks, and subagent_notification blocks, two of them after the last human turn, which is a three-character follow-up (ya?) to a substantive question.
  • F2 enumeration over a session longer than one read — 523 labctl status calls (two of them naming a run that does not exist), 16 spawn_agent calls with one failing on a thread limit, launches relaunched after a failure, cancels of a run id that does not exist.
  • F3 timestamps of specific records — every timestamp the gold scores is further than the scorer's 1 s tolerance from any other record time in its file, so the tolerance can never accept a neighbouring record's time.
  • F4 facts linked across spans and sessions — three PRs opened three ways (inside an exec script, straight from exec_command, and one whose URL appears only in a later write_stdin poll of a backgrounded process) and merged the same three ways, plus one merge the harness refused; and a forked Codex child whose inherited history is rewritten to the fork timestamp.
  • F5 files a session changed — apply_patch both as its own call and nested inside an exec script.
  • F6 verbatim tool output — Claude Bash results carrying is_error, in the main transcript and inside three Task sidechains.
  • F9 output past a truncation boundary — one tool output over 16 KiB whose answer is its last line.
  • F11 derived fields — a repeated token_count with an unchanged cumulative total (summing deltas double-counts), and one question whose correct answer is an empty list.

How the answer key stays honest

The generator records each planted fact while it writes the record that carries it, so the gold comes from the plan and never passes through src/adapters/. An adapter defect therefore shows up as a wrong answer rather than a quietly rewritten answer key. Scoring is exact match with no model judge: counts and paths by equality, times within 1 s, sets as sets, quotes by verbatim text plus a citation that resolves to the gold record (<file>:<line>, a span id, or a trace:// URI — span ids resolve through the source-record offsets the adapters attach, so a span id counts only when the span really came from that record).

fixtures.test.ts closes the other half of the loop by reading the generated JSONL back. Sixteen of the nineteen answers are re-derived from the bytes that way. Three are asserted by construction and the README says so: op.corrections rests on a judgement the generator makes (which human messages are corrections — the test checks each quoted correction is the verbatim text of the record it cites, not that the set is complete), and op.local-copy and child.spawned are literals.

Three things are reported rather than averaged away: a leaf answered null where the gold has a value is counted separately as a false "not in trace"; an unreported cost stays missing instead of becoming zero; and the two aggregate rows are scored over every wording the benchmark asks, not over the attempts made, so skipping the hard questions shows up in the cell a reader compares.

Tests (in pnpm test)

  • bench/audit/fixtures.test.ts — the generator is byte-identical across runs and writes exactly the manifest files; every planted fact above is asserted present; the sixteen re-derived answers are compared against a re-derivation that reads the generated JSONL directly, with no adapter and no access to the generator's bookkeeping; and every scored timestamp is asserted further than the 1 s tolerance from any other record time in its file. Three real disagreements were caught this way and fixed: the status-poll count was excluding the two failing labctl status calls, the child's inherited-history assertion was off, and op.runs/op.role were hand-kept with nothing checking them (see below).
  • bench/audit/score.test.ts — the gold, submitted as an arm, scores every question correct through the same path a real arm takes (including citation resolution through both adapters); plus the leaf rules, the absent-gold rule, time tolerance either side of 1 s, set and record matching, citation resolution and its resolved/verified/on-gold reporting, answers-file validation, the tallies and their denominators, and the runner end to end, including that a tampered fixture tree or a reworded prompts.jsonl is refused.

Full local run: pnpm check:source, pnpm typecheck, pnpm test (866 tests, 59 files), pnpm build, pnpm check:package all green.

The model arms stay manual on purpose — they cost money, they are not deterministic, and CI must not depend on a provider. bench/audit/README.md documents the two arms to run against each other and what to record.

Review repair (9cce570)

The blocking finding was right: op.runs and op.role were hand-kept bookkeeping that no test compared against the bytes, so the one guarantee this PR exists to give had a hole exactly where the counting was manual. Both are now re-derived — every started run <id> (spec <spec>[, variant <v>]) line across direct commands and exec scripts for launched, specs and beta_probe_variants; nonzero-exit labctl run calls for failed_launches, with the two required to account for every direct call; the merge confirmations, the same launches and the spawn_agent calls for op.role. Each hand-kept field was mutated to confirm the new assertions fail on it, including the reviewer's runs.launched += 3. The key itself was already correct, so no answer changed.

All nine non-blocking findings are fixed in the same commit: the absent-gold leaf is scorable, a span with no source record no longer resolves (176 of 1,438 keys mapped to an empty list), skipping a wording no longer shrinks the denominator, cost.basis reads the reported costs alone, assertUnmodified covers manifest.json and prompts.jsonl, the "2 s apart" claim is narrowed to the property the tolerance actually needs and asserted, the README says which answers are re-derived, the file order uses one comparator, and cli.ts writes its usage to stderr.

Follow-ups

  • Neither arm has been run yet. The paired subagent-fleet vs. traces run is the next step, and no claim that traces wins should be made before it is scored here.
  • Nothing in this PR needs agent-eval; the scorer is deliberately model-free and self-contained. If a later arm is driven through agent-eval, the answers-file shape here is what it would have to emit.
  • The root README.md does not link the benchmark yet.
  • tests/external.test.ts > terminates the detached descendant tree before rejecting cancellation flakes on a loaded machine (ps: timed out while enumerating analyzer descendants replaces the AbortError). It is untouched by this PR and reproduced once in five full local runs while three suites shared the machine; it belongs to src/external.ts and needs its own change.

🤖 Generated with Claude Code

Step 0 of the traces audit hill-climb: a benchmark that says whether an
audit arm beats a fleet of general subagents on a coding-agent session,
instead of asserting it.

- bench/audit/fixtures.ts generates three synthetic sessions (a Codex
  operator session over 600 spans, its forked child, and a Claude Code
  session with Task subagents) and records each planted fact as it writes
  the record that carries it, so the gold never passes through a traces
  adapter.
- bench/audit/questions.ts holds 19 questions, held-out paraphrases, and
  one JSON answer schema each.
- bench/audit/score.ts scores by exact match with no model judge: counts
  and paths by equality, times within 1 s, sets as sets, quotes by
  verbatim text plus a citation that resolves to the gold record.
- bench/audit/cli.ts writes fixtures, writes gold separately, and scores
  any arm's answers JSON.

The deterministic half runs in pnpm test; the model arms stay manual and
are documented in bench/audit/README.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@drewstone drewstone left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — feat/bench-audit @ b7901dc

Ran the full gate myself in a clean worktree off origin/feat/bench-audit: pnpm install --frozen-lockfile, check:source, typecheck, test (59 files / 861 tests passed, 34 s), build, check:package — all green. gh pr checks is green on Node 22.13.0 and 24.18.0.

Regression surface is genuinely nil. src/ is untouched; the only non-bench/ changes are one script and tsconfig.json's include. bench is absent from package.json#files, so the tarball is unchanged (check:package confirms). No adapter, analyzer, report format, evidence JSONL schema or exit code is in the diff.

Privacy: clean. Grepped bench/ for real-machine markers (tangle, webb, drew, hello@, /Users/, provider hosts, key shapes, the private map/bench paths) — the only hits are mkdtemp prefixes. Every fixture is invented (acme/orbit, @acme/runtime, labctl, /work/ledger), written in the inline-JSONL style of tests/codex-tool-status.test.ts. No duplication either: src/codetracebench*.ts imports an external trajectory corpus and shares nothing with this.

I re-derived the answer key from the generated bytes independently (raw Python over /tmp fixtures, no adapter, no generator code). Everything I could check matches: 523 labctl status; 16 spawn_agent / 1 failed; 6 nonzero direct exec_command exits over codes {1,2}; PRs 41/42/43 created and merged three distinct ways with the refused 41 merge excluded; the repeated token_count with an identical cumulative total; exactly one output >16 KiB (24,786 B); child = 18 inherited rows at the fork timestamp + 6 own tool calls + 1 own failure + 1 inherited spawn_agent distractor; Claude 9 Bash / 3 errors / 1 in a subagent, with the Read is_error correctly excluded. I also confirmed every gold <file>:<line> cite is reachable through at least one adapter span id, so the traces arm is not structurally penalised on citations.


Blocking

1. op.runs and op.role are hand-kept bookkeeping that no test re-derives, contrary to the PR body and README.

bench/audit/fixtures.ts:430-433 bumps runs.launched, runs.specs and runs.betaVariants by hand for the two launches that happen inside the exec script. fixtures.test.ts:190 (repeats and cancels run commands) re-derives only cancelled; launched, failed_launches, specs, beta_probe_variants and the whole of op.role are never compared against the bytes.

Proof it does not bite — change line 430 to runs.launched += 3 and the answer key now claims 9 launches where the file shows 8:

$ vitest run bench/audit
 Test Files  2 passed (2)
      Tests  59 passed (59)

Two of the nineteen questions would then mark every arm wrong with no signal anywhere. That is the one invariant this PR exists to guarantee ("the gold is compared against a re-derivation that reads the generated JSONL directly"), and it has a hole exactly where the bookkeeping is manual.

To be clear: today's key is correct — my independent count over the bytes is 7 direct labctl run (6 ok, 1 failed) plus 2 inside the script = launched: 8, failed_launches: 1, specs {alpha-sweep, beta-probe, gamma-grid}, beta_probe_variants {v1..v4}, and op.role = operator / 3 merged / 8 launched / 16 spawns. The fix is a few assertions, not a data change: count started run <id> (spec <spec>[, variant <v>]) across both exec_command and exec-script outputs plus the nonzero-exit labctl run calls, and assert op.runs and op.role against that.


Non-blocking

  1. A correct "not in trace" can never score correct. ANSWER_RULES tells arms to answer null for an absent value, and PullRequest.merged_at is declared optional, but scoreField compares against Date.parse(String(undefined)) / undefined for time, string and count, so a leaf whose gold is absent is unscoreable. No gold leaf hits this today; it will the first time a question has a legitimately empty scalar. Add a gold == null branch that accepts null.

  2. Span ids that map to no fixture record still report resolved: true. citations.ts:149 does spans.set(span.span_id, records) unconditionally; 176 of 1,438 span keys map to an empty array, and [] is truthy in resolve(). Such a cite comes out resolved: true, verified: false, onGold: false — identical in the report to citing the wrong record. Either skip empty mappings or count them separately, since resolved/verified/on-gold is documented as the diagnostic.

  3. Skipping a question is free. tally() uses attempts: rows.length (score.ts:275) as the denominator, so an arm that answers only the five easiest canonical wordings reports 5/5. notAttempted is printed, but not in the row a reader compares. Count unattempted canonical wordings as wrong, or render correct / of 19 wordings.

  4. "Neighboring records … are at least 2 s apart" is false in codex-child. Measured minimum gaps per file: operator 2.015 s, Claude 2.279 s, subagents 2.001–3.482 s — but the child has 18 records sharing the fork timestamp exactly (min gap 0.000 s). No scored time is affected (child.own-work.task_started_at is ≥2 s clear of the fork block), so this is a wording fix in README.md:86 / fixtures.ts:9 plus, ideally, an assertion that every gold-scored timestamp is >1 s from its neighbours — right now nothing enforces the property the 1 s tolerance rests on.

  5. Say which facts are re-derived and which are asserted by construction. op.corrections only checks that each cited line carries the quoted text (not that the set is complete — inherently a judgement call), and op.local-copy / child.spawned are literals. That is defensible; the README's blanket "every planted fact is re-derived" is not.

  6. assertUnmodified covers bench.files only — manifest.json and prompts.jsonl sit in the same tree and are not checked, and extra files are not detected. Harmless (the index is rebuilt in-process from regenerated bytes), but worth a comment so the guarantee is not read as broader than it is.

  7. cost.basis becomes mixed whenever any attempt omitted cost_usd, even when every reported cost shares one basis — missing already carries that. score.test.ts locks the behaviour in.

  8. generateBench() orders files with localeCompare while fixtures.test.ts:107 asserts the manifest equals a default code-unit sort(). They agree for these paths on full-ICU Node, but they are not the same comparator; pick one.

  9. cli.ts main() prints USAGE to stdout and returns 1 when called with no command, while every other failure goes to stderr.

Nothing here needs agent-eval. Fix 1 and I am happy to see this merge; 2–10 are cheap and can ride along or follow.

The audit gold for `op.runs` and `op.role` was hand-kept bookkeeping that no
test compared against the generated JSONL, so the guarantee the benchmark rests
on, that the gold equals a re-derivation of the bytes, had a hole exactly where
the counting was manual. Changing `runs.launched += 2` to `+= 3` in the
generator left all 59 tests green while two of the nineteen questions would have
marked every arm wrong with no signal. The key itself was already correct.

Both answers now come from the output lines. Every `started run <id> (spec
<spec>[, variant <v>])` line, across direct commands and exec scripts alike,
gives `launched`, `specs` and `beta_probe_variants`; nonzero-exit `labctl run`
calls give `failed_launches`, and the two together must account for every direct
`labctl run` call; the merge confirmations, the same launches and the
`spawn_agent` calls give `op.role`. Each hand-kept field was mutated to confirm
the new assertions fail on it.

The rest are the review's non-blocking findings:

- A gold leaf with no value is now scorable: `null`, and only `null`, answers
  it. Before, an optional field was compared against `Date.parse(String(
  undefined))` and no answer could be correct.
- A span carrying no source record no longer resolves. 176 of 1,438 span keys
  mapped to an empty list, which reported in the same shape as a citation of
  the wrong record.
- The two aggregate score rows count every wording the benchmark asks, so
  answering only the five easiest no longer reports 5/5.
- `cost.basis` reads the reported costs alone; an omitted cost is already
  counted as missing.
- `assertUnmodified` covers `manifest.json` and `prompts.jsonl` too, so an arm
  cannot reword its own prompts. Files the arm added stay ignored, and the
  comment says why that is safe.
- The "records are at least 2 s apart" claim is narrowed to what is true and now
  asserted: every scored timestamp is further than the 1 s tolerance from any
  other record time in its file. The child's inherited history shares the fork
  timestamp, and nothing scored reads a time from it.
- The README names the sixteen answers that are re-derived and the three that
  are asserted by construction.
- The generated file order and the manifest assertion use one comparator.
- `cli.ts` writes its usage to stderr when given no command, as every other
  failure path does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@drewstone

Copy link
Copy Markdown
Contributor Author

Repair pushed — 9cce570

Blocking finding accepted in full; nothing to refute. All nine non-blocking findings are fixed in the same commit.

1. op.runs and op.role are re-derived

You were right that this was the one hole that mattered: it sat exactly where the bookkeeping was manual, and it hid the guarantee the PR exists to give. Both answers now come from the bytes, as you specified.

fixtures.test.ts now derives, at module scope so both tests share one derivation:

  • startedRuns(calls) — every ^started run (\S+) \(spec ([^,)]+)(?:, variant ([^)]+))?\)$ line in a tool output, over exec_command and exec-script outputs alike. That gives launched, specs and beta_probe_variants.
  • failedLauncheslabctl run calls whose output matches Process exited with code (?!0). That gives failed_launches.
  • mergedIn(text) — the Squashed and merged pull request [^#]+#(\d+) matches, now shared with the merge test instead of local to it.

op.runs and op.role are each asserted with toEqual against the whole derived object. There is also a closure check that a direct launch either announced a run or exited nonzero: startedRuns(execCalls).length + failedLaunches.length === directLaunches.length, so an announcement pattern that stops matching cannot silently drop launches instead of failing.

role: 'operator' is still a literal in both the gold and the test — it is a classification, not a count — but the test asserts the evidence behind it: the session has no parent_thread_id and carries typed human turns, alongside the merge, launch and spawn counts.

Mutation-checked, one field at a time, restoring the generator between each:

Generator mutation Result
runs.launched += 2+= 3 (yours) 2 failed (op.runs, op.role)
runs.failed += 1+= 2 1 failed
runs.betaVariants.add('v3')'v9' 1 failed
runs.specs.add('beta-probe')'delta' 1 failed
merged_prs: … .length4 1 failed
launched_runs: runs.launched+ 1 1 failed
op.role spawn_calls: spawns.length15 1 failed

The key was correct, as you found, so no gold value changed. My derivation over the bytes agrees with yours: launched 8, failed_launches 1, cancelled 3, specs {alpha-sweep, beta-probe, gamma-grid}, variants {v1..v4}, role operator / 3 / 8 / 16.

2–10

  • 2. Absent gold is now scorable. scoreField takes a gold == null branch before the kind switch: null (or "not in trace") is correct, anything else is wrong, and it does not count as a false "not in trace". Tested by scoring against a gold whose optional merged_at was removed — null scores the question correct, the real time scores that leaf wrong.
  • 3. Record-less spans no longer resolve. spanRecordMap skips a span whose record set is empty rather than mapping it to []. score.test.ts asserts no mapping is empty; reverting the guard fails it with your exact number, 176 keys.
  • 4. Skipping is no longer free. row() takes the denominator: the two aggregate rows use every canonical wording and every paraphrase, so an arm that answers one question renders 1/0/0 of 19 (18 not attempted). The header column is now Correct/partial/wrong of wordings, and the test asserts that exact cell.
  • 5. The 2 s claim is narrowed and asserted. The comment and README now claim only what is true and what the tolerance needs: every timestamp the gold scores is more than 1 s from any other record time in its file, and the child's inherited block shares the fork timestamp by design. A new test walks every time leaf in every schema (including the records sub-fields) and asserts no other record in that file is within TIME_TOLERANCE_MS.
  • 6. Coverage is stated. The README names the sixteen re-derived answers and the three asserted by construction, with op.corrections explained as the judgement it is.
  • 7. assertUnmodified widened. One armInputs() map is now the single source for both writing and checking, so manifest.json and prompts.jsonl are covered too — an arm cannot reword its own prompts. Extra files are still ignored, and the doc comment says why that is safe: scoring reads session paths from the manifest regenerated in memory, never from the directory listing. The runner test tampers prompts.jsonl as well as a session file.
  • 8. cost.basis. Computed over the attempts that reported a cost. Two reported costs with different bases are still mixed; reported costs with no basis at all are unknown. score.test.ts covers all three.
  • 9. One comparator. generateBench() sorts by code unit, matching the manifest assertion.
  • 10. cli.ts. No command writes USAGE to stderr and returns 1; --help still prints to stdout and returns 0. Asserted on stdout: '' plus stderr containing the usage.

Gate

pnpm check:source, pnpm typecheck, pnpm test (866 tests / 59 files), pnpm build, pnpm check:package — green. Merges cleanly onto origin/main.

One flake to flag, not from this PR: tests/external.test.ts > terminates the detached descendant tree before rejecting cancellation failed once in five full local runs, with ps: timed out while enumerating analyzer descendants replacing the AbortError, while three suites shared the machine. src/ and tests/ are untouched here, so I left it alone rather than mixing an unrelated src/external.ts change into a bench-only diff — it is listed as a follow-up in the PR body.

🤖 Generated with Claude Code

@drewstone drewstone left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — feat/bench-audit @ 9cce570 (second pass)

Fresh worktree off origin/feat/bench-audit, full gate run myself: pnpm install --frozen-lockfile, check:source, typecheck, test (59 files / 866 tests passed, 75 s), build, check:package — all green. gh pr checks green on Node 22.13.0 and 24.18.0.

The nine non-blocking findings and the blocking one from the first pass are all genuinely fixed. I re-checked each: op.runs/op.role are now re-derived from started run <id> (spec …) across direct calls and exec scripts plus the nonzero-exit labctl run calls; the absent-gold leaf is scorable and tested; spanRecordMap skips record-less spans and score.test.ts asserts no key maps to []; the aggregate denominator is every wording (| all canonical | 1/0/0 of 19 (18 not attempted) |); assertUnmodified covers manifest.json and prompts.jsonl and the runner test tampers with both; the time claim is narrowed and asserted; the README states which three answers are asserted by construction; the file order uses one comparator; cli.ts writes usage to stderr.

Regression surface is still nil. src/ is untouched — the only non-bench/ changes are the bench:audit script and tsconfig.json's include. bench is not in package.json#files, so the tarball is unchanged. No adapter, analyzer, report format, evidence JSONL schema or exit code is in the diff.

Privacy: clean. Grepped both bench/ and the generated tree for real-machine markers (drew, tangle, webb, /Users/, hello@, key shapes, credential words). Zero hits. The only cwd values that exist anywhere in the fixtures are /work/orbit and /work/ledger; everything else is acme/orbit, @acme/runtime, labctl. Style matches the inline-JSONL tests, and the node_modules/tsx/dist/loader.mjs spawn pattern is the one tests/cli.test.ts:197 already uses. No duplication: src/codetracebench*.ts / src/replay-verify.ts score replayed shell trajectories against gold incorrect steps, which shares nothing with question-answering over sessions.

End to end from the README, not just through the tests: pnpm bench:audit fixtures|gold|score all work as documented; 9 session files, 520 KB, gold replayed as an arm scores 19/0/0 of 19, 7/7 verify, 7 on gold, exit 0.


Blocking

op.pull-requests is the hole 9cce570 did not close, and the README now claims it is closed.

README.md lists op.pull-requests among the sixteen answers that are "re-derived that way, counted or read straight back out of the bytes". It is not. fixtures.test.ts re-derives only the three PR numbers:

expect((bench.gold['op.pull-requests']!.prs as Array<{ number: number }>).map((pr) => pr.number)).toEqual([41, 42, 43])
...
expect(prs.filter((pr) => pr.merged_at)).toHaveLength(3)

The other nine leaves — created_at, merged_at and reviewed_before_merge for each of 41/42/43, set by hand at fixtures.ts:384, :386, :390, :403, :460, :512 — are never compared against the records. The only thing standing near them is keeps every scored time further apart than the scorer tolerates, and that asserts a scored time is a record timestamp in the file, not the right record's timestamp.

Proof that it does not bite. One edit at fixtures.ts:460:

-  prs.set(42, { ...prs.get(42)!, merged_at: merged42.at, reviewed_before_merge: true })
+  prs.set(42, { ...prs.get(42)!, merged_at: merged41.at, reviewed_before_merge: false })

The key now says PR 42 merged at PR 41's merge call, 71 minutes early, and that no review was visible when the bytes show {"reviews":[{"author":{"login":"review-bot"},"state":"APPROVED"}]} in the call immediately before:

$ ./node_modules/.bin/vitest run bench/audit
 Test Files  2 passed (2)
      Tests  64 passed (64)

Every arm would then be marked wrong on two of the ten op.pull-requests leaves with no signal anywhere. (A fabricated time is caught — shifting created_at by 60 s fails the tolerance test — so the gap is exactly "wrong record, right file".)

This matters more here than it did for op.runs: op.pull-requests is the question carrying F3 and F4, it is the only records question, and its times are the benchmark's only test of the 1 s tolerance against a real arm.

Today's key is correct. I re-derived all nine leaves from the generated bytes with no generator code and no adapter — linking gh pr create to the URL that appears only in a later write_stdin poll of session 73012, linking gh pr merge 42 to the confirmation that arrives only through the poll of session 66401, and reading reviewed_before_merge off the last gh pr view <n> --json reviews before each merge call:

41  created 09:03:14.626  merged 09:06:09.573  reviewed false
42  created 09:03:30.988  merged 10:17:16.180  reviewed true
43  created 09:04:09.547  merged 10:52:11.321  reviewed false
EQUAL to gold: true

So this is assertions, not a data change — roughly the 30 lines above, and the same shape as the op.runs re-derivation already in the file. Mutate each of the six hand-set values once to confirm the new assertions fail on them, as you did for runs.launched += 3.


Non-blocking

  1. The time test reads stronger than it is. keeps every scored time further apart than the scorer tolerates proves the tolerance is safe, not that any scored time is the right record's. That distinction is exactly what let the mutation above through, so the comment at fixtures.test.ts:162 is worth one sentence saying which property it does not give.

  2. child.spawned need not stay a literal. The child inherits exactly one spawn_agent call in its fork-timestamped block (18 rows after session_meta) and makes none of its own — I measured both. expect(rows.filter((r) => r.timestamp !== fork && r.payload.name === 'spawn_agent')).toHaveLength(0) alongside expect(inherited.filter(…)).toHaveLength(1) turns the literal into a re-derivation and asserts the distractor is present, which is the trap the question exists for. That would leave op.local-copy and op.corrections as the only construction-asserted answers.

  3. scoreArm trusts its input. questionById(row.question)! (score.ts:303) throws a bare TypeError on an unvalidated file; parseAnswersFile is what produces the good message, and only cli.ts calls it. score.test.ts itself builds AnswersFile objects directly, so nothing enforces the order. Either call parseAnswersFile inside scoreArm or narrow its parameter to a branded validated type.

  4. reviewed_before_merge for 41 and 43 rests on reading an empty list as "no review". The bytes show gh pr view 41 --json reviews{"reviews":[]}, and the gold says false. That is the right answer to "was the session shown a review", but an arm that scores it as "yes, it checked" loses a leaf for a reading of the wording rather than for retrieval. Consider naming the empty-list case in the question text — the F4 difficulty lives in linking the merge across the poll, not in this.

  5. Per-question rows still use attempts as their denominator, so three repeats of one wording render 3/0/0 of 3 while the aggregate rows render over all 19. Deliberate, I think, but the README paragraph that explains the aggregate denominator is the place to say so.

  6. Root README.md does not link bench/audit/ — already in your follow-ups; worth doing in this PR since the directory is the public artifact.

Nothing here needs agent-eval. Fix the blocking item and this is ready.

`op.pull-requests` re-derived only the three PR numbers. Its nine other
leaves - created_at, merged_at and reviewed_before_merge for 41/42/43 -
were hand-kept bookkeeping no test compared against the records, so a
wrong merge time or review flag would mark every arm wrong on the
question that carries F3 and F4 with no signal anywhere.

`fixtures.test.ts` now pairs each call with the outputs that answered it,
including the outputs of later polls of a process the call backgrounded,
and reads all ten leaves back out: the create call's own timestamp, the
timestamp of the call that ran the merge the bytes confirm (excluding the
merge the harness refused), and whether the last review listing shown
before that call named a review. `op.pull-requests` is asserted whole.
Mutating each of the six hand-set values, one at a time, now fails.

Also from the review, none of it changing a gold value:

- `child.spawned` is re-derived from the child's own spawn outputs, with
  the inherited spawn call asserted present, so the distractor the
  question exists for is pinned. Only `op.local-copy` and
  `op.corrections` remain asserted by construction.
- The time-spacing test says which property it does not give.
- `scoreArm` validates its input, so an unvalidated answers file reports
  every problem instead of throwing a bare TypeError.
- The question names the empty-review-list case, so a leaf turns on
  retrieval rather than on a reading of the wording.
- The bench README says the per-question rows use their own attempts as
  the denominator; the root README links `bench/audit/`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@drewstone

Copy link
Copy Markdown
Contributor Author

Repair pushed — 8e3822f

Blocking finding accepted in full; nothing to refute. All six non-blocking items are fixed in the same commit. No gold value changed — your independent derivation and mine agree with what the generator already wrote.

op.pull-requests is re-derived, every leaf

fixtures.test.ts now derives the whole answer from the records, in the shape you specified:

  • sourceOf(call) — the shell command for exec_command, the script source for exec, so a gh pr create or gh pr merge inside a script is found the same way as a direct one.
  • reported(calls, call) — the call's own output plus the output of every later write_stdin poll of the process its output backgrounded (Process running with session ID <n>). That is the only way PR 43's URL and PR 42's merge confirmation are reachable.
  • created_at — the timestamp of the call record whose source contains gh pr create and whose reported text carries exactly one /pull/<n> URL; the number comes from that URL.
  • merged_at — the timestamp of the call whose source contains gh pr merge <n> and whose reported text carries Squashed and merged pull request …#<n>. The refused gh pr merge 41 --squash never confirms, so it is not the merge; a second confirming call for one number throws.
  • reviewed_before_merge — the last gh pr view <n> --json reviews call before the merge call's line, parsed out of the shell body, true when reviews is non-empty.

expect(bench.gold['op.pull-requests']).toEqual({ prs: derived }) asserts the whole object. The two weaker gold assertions in the create/merge tests are gone; those tests now prove only the three discovery paths, which is what they were for.

Mutation-checked one field at a time, restoring the generator between each. All nine fail (vitest run bench/audit, 66 tests):

Generator mutation Result
:384 41 created_at := first record time 1 failed
:386 42 created_at := PR 41 create call 1 failed
:390 43 created_at := PR 42 create call 1 failed
:403 41 merged_at := PR 43 create call, reviewed true 1 failed
:403 41 reviewed_before_merge falsetrue 1 failed
:460 42 merged_at := merge-41 call, reviewed false (yours) 1 failed
:460 42 reviewed_before_merge truefalse 1 failed
:512 43 merged_at := merge-42 call, reviewed true 2 failed
:512 43 reviewed_before_merge falsetrue 1 failed

My derivation over the bytes reproduces your numbers exactly: 41 created 09:03:14.626 merged 09:06:09.573 reviewed false; 42 09:03:30.988 / 10:17:16.180 / true; 43 09:04:09.547 / 10:52:11.321 / false.

Non-blocking

  1. The time test says what it does not cover. Two added sentences: it proves the tolerance is safe, not that a scored time belongs to the right record; each question's own test proves the record.
  2. child.spawned is no longer a literal. The test asserts exactly one spawn_agent call in the fork-timestamped inherited block, none outside it, and derives spawned_session_ids from the agent_id of the child's own spawn outputs. Checked that it bites: forking the child from the first spawn instead of the second (so nothing is inherited) fails it. op.local-copy and op.corrections are now the only construction-asserted answers, and the README says so.
  3. scoreArm validates its input. It takes unknown and calls parseAnswersFile itself, so an unvalidated file reports every listed problem instead of a bare TypeError from questionById(...)!. cli.ts still parses first, to fail before it builds the fixture tree. New test in score.test.ts scores an unknown question id and expects the listed message.
  4. The empty-review-list case is named in the canonical text ("A review listing that came back empty is not a review.") and in the paraphrase, so that leaf turns on the F4 link rather than on a reading of the wording. Gold unchanged.
  5. The per-question denominator is documented in the same README paragraph that explains the aggregate one: per-question rows keep their own attempts, so three repeats of one wording read 3/0/0 of 3; compare arms on the two aggregate rows.
  6. The root README.md links bench/audit/ — a short section between Examples and Develop, with the three commands and a pointer to bench/audit/README.md, plus a Contents entry.

Gate

pnpm check:source, pnpm typecheck, pnpm test (59 files / 868 tests), pnpm build, pnpm check:package — green. Merges cleanly onto origin/main. End to end from the README: fixtures, gold, and the gold replayed as an arm still score 19/0/0 of 19, 7/7 verify, 7 on gold.

src/ is still untouched; the diff is bench/ plus the root README section.

🤖 Generated with Claude Code

@drewstone drewstone left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — bench/audit (PR #109)

Checked out origin/feat/bench-audit (8e3822f) in a clean worktree. pnpm install --frozen-lockfile, check:source, typecheck, test (868 tests / 59 files), build, check:package all green locally; gh pr checks green on Node 22.13.0 and 24.18.0.

No regression surface. git diff main..HEAD --name-only touches nothing under src/, tests/, scripts/, skills/, docs/, examples/. Adapters, analyzers, report formats, the evidence JSONL schema and CLI exit codes are untouched. files still excludes bench, and check:package confirms the tarball is unchanged. The only cross-cutting edits are tsconfig.json include (so tsc --noEmit covers bench) and one package.json script.

Privacy: clean. Grepped the whole diff for home paths, drew, tangle, webb, mail addresses, tokens, ~/.local/share, /tmp/tae-*. Nothing. Every fixture is invented (acme/orbit, /work/orbit, /work/ledger) and written inline in the style of tests/codex-tool-status.test.ts.

Not duplicated. src/codetracebench* and tests/replay-corpus-fixture.ts are the SWE-trajectory replay corpus, a different artifact; nothing in the repo already generates synthetic Codex/Claude sessions with a gold key.

Unknown shapes. Parsing all three fixtures through both adapters yields 720 spans, zero carrying an unknown attribute, and spanRecordMap explicitly drops spans with no source record rather than mapping them to an empty list, keeping "cited a record-less span" apart from "cited the wrong record". Good.

I verified the benchmark's own claims independently, not just its tests: generator byte-determinism across runs, the gold self-scoring 19/19 through the real citation path, and each planted fact from the brief present (675 spans, 523 labctl status, 16 spawns / 1 failure, 3 PRs and 3 merges each opened/merged three ways, cancels including a nonexistent id, injected blocks after a 3-character last human turn, apply_patch nested in exec, one >16 KiB output, the forked child, the Claude Task sidechains with is_error). Span-id citations resolve to the gold record for all four quote questions. This is careful work.

Two findings below are blocking because both let a wrong or evasive answers file score as correct, which is the one property the whole PR exists to provide.


Blocking 1 — the aggregate row hides skips as soon as attempts reach the wording count

bench/audit/score.ts, row() + scoreArm. The numerator is item.attempts (raw attempt count) while the denominator is a fixed wording count, and skipped = wordings - attempts. Once attempts >= wordings the "(N not attempted)" suffix disappears and the numerator overflows the denominator.

Reproduced against the branch:

answers = 19 rows, all {question: "op.status-polls", variant: 0, answer: <gold>}
| all canonical | 19/0/0 of 19 | ...      <- reads as a perfect canonical score
scored.notAttempted.length === 37

and with the repeat workflow the README itself describes (3 repeats of every wording):

| all canonical | 57/0/0 of 19 | ...

bench/audit/README.md says "the two aggregate rows are scored over every wording the benchmark asks, not over the attempts made, so an arm that answers only the easy questions reports its skips in the same cell a reader compares" and then "compare arms on the two aggregate rows, not on those". That claim is false in this direction, and it is false exactly on the cell a reader is told to compare. score.test.ts > renders one table row per attempted wording only covers the under-attempted direction (1 attempt of 19), so nothing catches it.

Fix: make the aggregate rows aggregate over wordings, not attempts — build them from the per-wording questions entries with a stated rule for repeats (e.g. a wording counts correct only when every repeat of it is correct), and compute skipped as wordings - distinct wordings attempted. Alternatively/additionally reject duplicate (question, variant, repeat) triples in parseAnswersFile. Please add a test with attempts >= wordings; today's assertion passes either way.

Blocking 2 — a records answer that lists the same key twice, with contradictory fields, scores correct

bench/audit/score.ts, scoreField case 'records'. keysCorrect compares sameSet(answerKeys, goldKeys), and sameSet puts both sides through a Set, so duplicates collapse; rows.length === answer.length does not catch them either. Sub-fields are then read with rows.find(...), which takes the first matching row and ignores the rest.

Reproduced against the branch, on op.pull-requests — the F4 question this benchmark leans hardest on:

answer.prs = [PR41(gold), PR41(copy with merged_at = 1999-01-01), PR42, PR43]
verdict: correct      prs[*].number leaf: correct

The answer asserts two mutually exclusive merge times for PR 41 and is graded as fully right. The JSON Schema does not stop it either: fieldJsonSchema for records emits an array with no uniqueItems and no key constraint, so hedging is schema-valid. That defeats the "exact match, counts and numbers by equality" contract for the only field kind that carries nested leaves.

Fix: reject duplicate keys in keysCorrect (compare the key list, not a set of it, after sorting), and add a matching case to score.test.ts next to matches records by key and fails the ones it cannot find.


Non-blocking

  1. The PR description is stale against the merged README. It says "Sixteen of the nineteen answers are re-derived ... Three are asserted by construction: op.corrections, op.local-copy and child.spawned"; bench/audit/README.md (after 8e3822f) says seventeen and two, and lists child.spawned as re-derived — which matches what fixtures.test.ts now asserts. Please update the body before merging so the merge record is right.

  2. claude.first-bash-error is listed among the re-derived answers, but fixtures.test.ts > quotes the first failing Bash output from the record that carries it only reads the line out of the gold's own cite and checks that record holds exactly one is_error block with that text. It never checks the record is the first Bash error, nor that it is a Bash result at all — the Read result in the same transcript also carries is_error. Either assert the ordering and the tool name, or move this one into the "asserted by construction" list.

  3. bench/audit/fixtures.ts is the answer key. A coding-agent arm run inside a checkout of this repo can reconstruct the gold without reading a fixture. The README warns only about gold.json. Add a line under "Running it": the arm's working directory must contain the fixture tree and not this repository.

  4. parseAnswersFile validates arm, question, variant, the presence of answer, and cost_basis, but not the types of repeat, wall_ms, model_calls, tool_calls. {"wall_ms": "fast"} is accepted and silently becomes missing in the distribution — indistinguishable from an arm that honestly omitted it, which is the distinction the missing column exists to preserve.

  5. fixtures.ts: the op.large-output gold is built as quote(file, { line: logOutputLine, at: '' }, lastLogLine) — a synthetic Placed with an empty timestamp that only works because quote() reads .line. Take a line number there, or keep the real Placed from the exec output record.

  6. fixtures.test.ts > keeps every scored time further apart than the scorer tolerates resolves a scored time to a file with codexTimes.find((list) => list.includes(at)), i.e. the first file that happens to contain that instant. It holds today, but if the operator and child ever share a timestamp value the child's scored time gets checked against the operator's record list. Match the time to its question's session instead.

  7. citations.ts > contains is a substring test over every decoded string in the record. That only feeds the reported verified column (correctness still needs onGold), but the interface comment says "a named record holds the text" — worth saying it is a containment test, so nobody later reads verified as verbatim equality.

  8. op.pull-requests is the one leaf where a careful arm could reasonably disagree with the key: for PR 42 the merge is only confirmed by a later write_stdin poll, and the gold takes the time of the call that issued the command. The test comment already spells this out ("the timestamps of the call records themselves ... not of the outputs that answered them"); consider putting that same clause in the question text, since arms are scored on the question, not on the test.

I did not touch agent-eval, and nothing here needs it — the scorer is deliberately model-free.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant