Add arena rating snapshot job - #149
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a complete arena leaderboard rating snapshot pipeline: new ChangesArena Leaderboard Snapshot Workflow
Sequence Diagram(s)sequenceDiagram
participant CLI as arena snapshot CLI
participant run_snapshot
participant snapshot_lock as ArenaStore.snapshot_lock
participant _refit_and_persist
participant ArenaStore
participant compute_ratings
CLI->>run_snapshot: run_snapshot(store, metric, domain, rounds, seed, force)
alt force=False
run_snapshot->>snapshot_lock: async with store.snapshot_lock()
snapshot_lock-->>run_snapshot: acquired=True/False
alt lock not acquired
run_snapshot-->>CLI: None (skipped)
end
end
run_snapshot->>_refit_and_persist: _refit_and_persist(store, ...)
_refit_and_persist->>ArenaStore: list_battles(domain, limit=None)
ArenaStore-->>_refit_and_persist: battles[]
_refit_and_persist->>ArenaStore: list_votes()
ArenaStore-->>_refit_and_persist: votes[]
_refit_and_persist->>compute_ratings: BattleOutcome[], bootstrap_rounds, seed
compute_ratings-->>_refit_and_persist: RatingResult
_refit_and_persist->>ArenaStore: insert_snapshot_board(LeaderboardSnapshot rows)
ArenaStore-->>_refit_and_persist: inserted_count
_refit_and_persist-->>run_snapshot: RatingResult
run_snapshot-->>CLI: RatingResult (models count printed as JSON)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Refit Davidson-BT ratings from all votes and persist one leaderboard board via coval-bench arena snapshot. The board is written in a single transaction so readers never see a partial board, and a non-blocking advisory lock keeps two runs from computing at once (--force bypasses it).
91cbf36 to
7f66e99
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@runner/src/coval_bench/arena/snapshot.py`:
- Around line 30-31: The store.list_battles() and store.list_votes() calls on
lines 30-31 load all data regardless of the domain parameter provided to
run_snapshot(), but the caller-provided domain is persisted on line 51, causing
a mismatch between the loaded data and the domain label. Pass the domain
parameter to both store.list_battles() and store.list_votes() calls to filter
the loaded data by the specified domain, ensuring the persisted domain label
accurately reflects the data that was actually loaded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 96b337ba-a029-4ddc-8f77-07f4831d2381
📒 Files selected for processing (5)
runner/src/coval_bench/__main__.pyrunner/src/coval_bench/arena/snapshot.pyrunner/src/coval_bench/db/arena_store.pyrunner/src/coval_bench/db/models.pyrunner/tests/unit/test_arena_snapshot.py
Filter battles by domain so the persisted board label matches the data that was rated. Catch ConvergenceError in the CLI and emit an error JSON line plus a non-zero exit instead of a raw traceback. Insert the board with executemany.
Adds the rating snapshot job (layer 7): refit Davidson-BT ratings from all votes and persist one leaderboard board.
coval-bench arena snapshotreads every vote, runs the rating engine, and writes one board.The whole board is written in a single transaction with one
computed_at, so a reader never sees a partial board.A non-blocking advisory lock keeps two runs from computing at once.
--forcebypasses it; a crashed run self-heals when its connection drops.Status comes from the engine's CI-half-width classifier. Models with no votes are never written.
Writes the
alldomain board only. Per-domain boards, retention/pruning, and trigger cadence are follow-ups.Read-API files are untouched: math stays in
arena/rating.py, SQL indb/arena_store.py, the job is glue.Summary by CodeRabbit
arena snapshotcommand to compute and persist leaderboard snapshots with configurable parameters including rating metric, bootstrap resampling rounds, and random seed.--forceflag to bypass snapshot locks when needed.Greptile Summary
This PR adds the
arena snapshotCLI command (layer 7 of the rating pipeline) that reads all votes, fits the Davidson-BT leaderboard viacompute_ratings, and atomically persists one board toarena.leaderboard_snapshots. The previously-flagged gaps — uncaughtConvergenceErrorand individual-executeinserts in a loop — are both resolved in this diff.run_snapshotinsnapshot.pyorchestrates the job: battles and votes are fetched, converted toBattleOutcomeobjects, and fed tocompute_ratings; the resulting rows are bulk-inserted withexecutemanyinside a single transaction socomputed_atis identical for every row in a board.pg_try_advisory_lockprevents concurrent recomputes;--forcebypasses the lock;ConvergenceErroris caught and serialised as structured JSON before a non-zero exit.list_battlesgains an optionallimit=Nonepath needed by the refit job, andLeaderboardSnapshot/SnapshotStatusare added tomodels.py.Confidence Score: 5/5
Safe to merge — the orchestration logic, advisory lock lifecycle, and atomic board insert are all correct, and the previously-raised gaps are resolved in this diff.
The lock is correctly held on a dedicated connection for the job's full duration and always released in a finally block, executemany inside conn.transaction() guarantees all rows share a single computed_at, ConvergenceError is caught and serialised before exit, and the five integration tests exercise all branching paths including force-override and domain scoping.
No files require special attention.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant CLI as CLI (arena snapshot) participant S as run_snapshot participant Store as ArenaStore participant DB as PostgreSQL participant Eng as compute_ratings CLI->>S: run_snapshot(store, metric, domain, force, ...) alt "force=False" S->>Store: snapshot_lock() Store->>DB: pg_try_advisory_lock(arena_snapshot) DB-->>Store: acquired (true/false) alt not acquired S-->>CLI: None - JSON skipped true end end S->>S: _refit_and_persist(...) S->>Store: "list_battles(domain, limit=None)" Store->>DB: SELECT FROM arena.battles DB-->>Store: battles[] S->>Store: list_votes() Store->>DB: SELECT FROM arena.votes DB-->>Store: votes[] S->>Eng: compute_ratings(outcomes, bootstrap_rounds, seed) Eng-->>S: RatingResult or ConvergenceError S->>Store: insert_snapshot_board(rows) Store->>DB: BEGIN / executemany INSERT / COMMIT DB-->>Store: ok S-->>CLI: RatingResult CLI->>CLI: JSON event metric models%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant CLI as CLI (arena snapshot) participant S as run_snapshot participant Store as ArenaStore participant DB as PostgreSQL participant Eng as compute_ratings CLI->>S: run_snapshot(store, metric, domain, force, ...) alt "force=False" S->>Store: snapshot_lock() Store->>DB: pg_try_advisory_lock(arena_snapshot) DB-->>Store: acquired (true/false) alt not acquired S-->>CLI: None - JSON skipped true end end S->>S: _refit_and_persist(...) S->>Store: "list_battles(domain, limit=None)" Store->>DB: SELECT FROM arena.battles DB-->>Store: battles[] S->>Store: list_votes() Store->>DB: SELECT FROM arena.votes DB-->>Store: votes[] S->>Eng: compute_ratings(outcomes, bootstrap_rounds, seed) Eng-->>S: RatingResult or ConvergenceError S->>Store: insert_snapshot_board(rows) Store->>DB: BEGIN / executemany INSERT / COMMIT DB-->>Store: ok S-->>CLI: RatingResult CLI->>CLI: JSON event metric modelsReviews (3): Last reviewed commit: "Address snapshot job review feedback" | Re-trigger Greptile