Skip to content

judge: fix the display order of each comparison per page and pair - #91

Merged
davanstrien merged 2 commits into
mainfrom
fix/display-order-per-comparison
Nov 10, 2026
Merged

davanstrien merged 2 commits into
mainfrom
fix/display-order-per-comparison

Conversation

@davanstrien

Copy link
Copy Markdown
Owner

What was wrong

build_comparisons created random.Random(seed) on every call. Adaptive judging (the default) calls it once per batch of five pages, so every batch replayed the same sequence of A/B display-order choices. A pair of models could be shown the same way round on most of its pages.

A judge can favour one position. When it does, the imbalance shifts ratings, and the page bootstrap does not show it.

A --no-adaptive run that finished in one go was not affected. A run that was resumed (after a --max-comparisons stop, or a Job continued from a checkpoint) restarted the sequence for the comparisons still to judge.

What this changes

  • Display order per comparison. display_swapped(seed, sample_idx, model_a, model_b) takes one bit of a SHA-256 over the seed, the page and the two model names (sorted). The order is the same in adaptive batches, in one pass and after a resume. It does not depend on column order or on which pairs are skipped. It is an independent even choice per comparison, so a pair is balanced in expectation, not by construction.
  • shown_first in the comparisons config. A when the judge saw model_a's output first, B when it saw model_b's, empty for an auto-tie or a row from an earlier version. The letters match winner, so winner = shown_first selects the comparisons the first output shown won. The stored winner stays un-swapped and a reloaded row keeps swapped=False: the field is a record and never drives an un-swap.
  • adaptive and ocr_bench_version in the metadata config. adaptive_strategy is written for both modes, so it could not tell an adaptive run from a single pass.
  • A note on resume. judge says how many stored comparisons have no recorded display order and points to --full-rejudge.
  • Duplicate model names are refused in build_comparisons. A pair with two equal names was never swapped; the loaders already rename such columns.

For a given seed the display order differs from earlier versions. Stored comparisons are not changed.

Evidence

On synthetic data (9 models, 340 pages, batches of five, seed 42):

Share of a pair's pages shown swapped
Before 0.00 to 1.00 across the 36 pairs; every share a multiple of 0.2
After 0.4 to 0.6 for all 36 pairs

A reviewer's simulation (4 equal models, 200 pages, a judge that picks the first position with p = 0.6, 20 seeds): before the fix, 14 of 80 model intervals excluded the true rating; after it, 3 of 80.

Tests

  • 27 new tests. With the old per-call generator put back, 8 of them fail.
  • Covered: per-pair and per-model balance across batches; batches equal one pass; resume and skipped pairs do not shift the order; column order does not matter; fixed expected orders for given inputs; shown_first through _convert_results, compute_elo and load_existing_comparisons with no second un-swap; the metadata fields in both run modes and on the refit-only path; the resume note.
  • Full suite: 1053 passed. ruff check, ruff format --check, ty check and rumdl check pass.

Review before this PR

Two reviews ran on the first commit; the second commit holds the follow-ups.

  • Codex: no correctness finding. Three wording and hardening points, all applied (the changelog claims about unaffected runs and about balance; duplicate model names).
  • Independent agent review: reproduced the bug on main, checked the hash over 225,000 keys (share swapped 0.495 to 0.501, no correlation between adjacent pages or between pairs that share a model), ran a parquet round trip with old and new rows, and ran 15 mutants against the tests. Applied from it: A/B casing for shown_first, the resume note, numpy integers in display_swapped, the refit-only metadata test, and changelog corrections.

Not in this PR

  • A first-position term in the Bradley-Terry fit. It needs judged rows with a stored order.
  • The share card still links the judge prompts at the sharing version; a follow-up PR uses the recorded judge-time version.

🤖 Generated with Claude Code

davanstrien and others added 2 commits November 10, 2026 14:20
Adaptive judging builds the comparison grid five pages at a time, and
build_comparisons seeded a new generator on every call. Each batch
replayed the same sequence of A/B choices, so a pair of models could be
shown the same way round on most of its pages. A judge favours one
position, so the imbalance can shift ratings in a way the page bootstrap
does not show.

The display order of a comparison now depends only on the seed, the page
and the two (sorted) model names. It is the same in adaptive batches, in
one pass and after a resume, and it does not depend on column order or
on which pairs are skipped.

Also record what was not recoverable from a results repo:
- comparisons: `shown_first` ("a", "b", or "" for an auto-tie or an
  older row). The stored winner stays un-swapped; a reloaded row keeps
  swapped=False.
- metadata: `adaptive` (batches or one pass) and `ocr_bench_version`.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
- shown_first uses "A"/"B", the letters of `winner`, so
  `winner == shown_first` selects the comparisons the first output won.
  A stored value that is not a display order reads back as "".
- On resume, judge says how many stored comparisons have no recorded
  display order and points to --full-rejudge.
- build_comparisons refuses two columns with the same model name: a
  pair of equal names was never swapped.
- display_swapped accepts numpy integers for the seed and the page.
- Tests: fixed expected orders for given inputs, the refit-only metadata
  path, the resume notice.
- CHANGELOG: resumed --no-adaptive runs were also affected; the order is
  an even choice per comparison, not a forced even split; what a stored
  reason refers to.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@davanstrien
davanstrien merged commit 799c557 into main Nov 10, 2026
1 check passed
@davanstrien
davanstrien deleted the fix/display-order-per-comparison branch November 10, 2026 15:05
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