Add Voice Arena rating engine (Davidson-BT) - #139
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughA new ChangesDavidson-BT Arena Rating Engine
Sequence DiagramsequenceDiagram
participant Caller
participant compute_ratings
participant _aggregate
participant _fit
participant _bootstrap
participant classify_status
Caller->>compute_ratings: outcomes, bootstrap_rounds, seed, reg
compute_ratings->>compute_ratings: validate bootstrap_rounds ≥ 0, reg > 0 and finite
compute_ratings->>_aggregate: Sequence[BattleOutcome]
_aggregate-->>compute_ratings: win_counts, tie_counts, pair_indices, model_list
compute_ratings->>_fit: win_counts, tie_counts, pair_indices, reg
_fit-->>compute_ratings: theta (centered), nu → elo[]
compute_ratings->>_bootstrap: battles, model_list, bootstrap_rounds, seed, reg
loop each resample
_bootstrap->>_fit: resampled win/tie counts, reg
alt converged
_fit-->>_bootstrap: theta_r, nu_r
else ConvergenceError
_bootstrap->>_bootstrap: skip replicate
end
end
_bootstrap-->>compute_ratings: ci_low[], ci_high[] (or NaN)
compute_ratings->>classify_status: ci_half_width per model
classify_status-->>compute_ratings: "preliminary" | "usable" | "established"
compute_ratings-->>Caller: RatingResult sorted by Elo desc
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
runner/src/coval_bench/arena/__init__.py (1)
5-21: ⚡ Quick winConsider exporting
ConvergenceErrorfor callers who need to handle fit failures.
compute_ratingsraisesConvergenceErrorwhen the MLE optimizer fails to converge. Callers who want to handle this gracefully would need to import from the submodule (from coval_bench.arena.rating import ConvergenceError) rather than the package root. If this exception is part of the intended public contract, adding it to the re-exports would provide a cleaner API surface.🔧 Suggested change
from coval_bench.arena.rating import ( + ConvergenceError, METHODOLOGY_VERSION, BattleOutcome, ModelRating, RatingResult, classify_status, compute_ratings, ) __all__ = [ + "ConvergenceError", "METHODOLOGY_VERSION", "BattleOutcome", "ModelRating", "RatingResult", "classify_status", "compute_ratings", ]🤖 Prompt for 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. In `@runner/src/coval_bench/arena/__init__.py` around lines 5 - 21, The ConvergenceError exception is raised by the compute_ratings function but is not currently exported in the __all__ list of the __init__.py file, forcing callers to import it directly from the submodule. Add ConvergenceError to the import statement from coval_bench.arena.rating and include it in the __all__ list to provide a consistent public API surface for exception handling.
🤖 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.
Nitpick comments:
In `@runner/src/coval_bench/arena/__init__.py`:
- Around line 5-21: The ConvergenceError exception is raised by the
compute_ratings function but is not currently exported in the __all__ list of
the __init__.py file, forcing callers to import it directly from the submodule.
Add ConvergenceError to the import statement from coval_bench.arena.rating and
include it in the __all__ list to provide a consistent public API surface for
exception handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3fb98057-e7bd-496f-8600-6c1028b4470e
📒 Files selected for processing (3)
runner/src/coval_bench/arena/__init__.pyrunner/src/coval_bench/arena/rating.pyrunner/tests/unit/test_arena_rating.py
* Add Voice Arena rating engine (Davidson-BT) Pure-function rating engine for arena layer #3: Davidson Bradley-Terry MLE fit (theta + tie parameter nu), Elo conversion, percentile-bootstrap CIs, and a status classifier. Refuses a non-converged fit and skips non-converged bootstrap resamples. CI fields are null (not NaN) when unavailable. Unit tests cover ranking recovery, CI shrinkage with votes, and tie sensitivity. * Harden arena rating engine inputs and CI Skip self-battle outcomes in aggregation, treat fewer than two bootstrap samples as no CI (avoids a false zero-width 'established' interval), and validate bootstrap_rounds/reg. Document the disconnected-graph limitation and that adaptive pairing is the intended upstream remedy. * Add regression tests for rating engine hardening Cover the self-battle skip, single-sample CI falling back to preliminary, and the bootstrap_rounds/reg input validation.
A fully-separated dataset refits identically every resample, giving a zero-width interval that wrongly read as 'established'; skip it so it reads preliminary. Also require reg > 0, since the ridge the engine relies on for convergence is absent at reg = 0.
abc5e27 to
aa4cfdd
Compare
Rating engine for Voice Arena, now targeting
maindirectly.It was originally stacked on the schema PR (#119) and merged into
feat/voice-arenajust after that branch shipped to main — so the math never reachedmain. This re-lands it on its own.Pure functions, no DB or I/O: a sequence of A_WIN/B_WIN/TIE battle outcomes in, one rating row per model out (matching
arena.leaderboard_snapshots).Hardening from review: self-battles are skipped in aggregation, fewer than two bootstrap samples fall back to no-CI/preliminary, and
bootstrap_rounds/regare validated. The disconnected-comparison-graph limitation is documented (adaptive pairing is the intended upstream remedy). Tests cover ranking recovery, CI shrinkage, tie sensitivity, and each hardening fix.Summary by CodeRabbit
New Features
Greptile Summary
This PR introduces the Davidson-BT rating engine for Voice Arena, re-landing it on
mainafter the underlying schema PR (#119) had already shipped. The implementation is pure-function: a sequence ofBattleOutcomerecords in, oneModelRatingper model out, with no I/O or DB dependencies._fit): L-BFGS-B with an analytic gradient over Davidson's extended Bradley-Terry model; an L2 ridge on theta tames complete separation and ensures strict convexity. Bootstrap CIs are computed via percentile resampling over battles, with degenerate resamples (fewer than two samples, or zero-variance pool from fully-separated data) correctly left asNone/"preliminary"rather than a spurious narrow interval.reg > 0is enforced; self-battles are filtered in_aggregatebefore fitting and tally accumulation; non-converged resamples are skipped with a warning;bootstrap_rounds=0produces cleanNoneCI fields. The test suite covers ranking recovery, CI shrinkage, tie sensitivity, and every hardening path.Confidence Score: 5/5
Safe to merge — pure functions with no I/O, all prior review concerns addressed, comprehensive test coverage.
The rating math is correct (analytic gradient verified against the Davidson log-likelihood, recenter is consistent between full fit and bootstrap resamples), input validation is tight (reg > 0, bootstrap_rounds ≥ 0), and every hardening case called out in earlier review rounds is now covered: zero-variance bootstrap produces None CI and "preliminary" status, single-sample bootstrap likewise, reg=0 raises ValueError, non-converged resamples are skipped gracefully. The test suite exercises all of these paths deterministically.
No files require special attention; the only file with meaningful logic is rating.py and it is well-covered by the accompanying test file.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["compute_ratings(outcomes, bootstrap_rounds, seed, reg)"] --> V{"Validate\nbootstrap_rounds ≥ 0\nreg > 0 finite"} V -->|fail| E1["raise ValueError"] V -->|ok| AGG["_aggregate(outcomes)\nSkip self-battles\nBuild per-pair win/tie counts"] AGG --> N0{"n models == 0?"} N0 -->|yes| R0["Return RatingResult(models=[])"] N0 -->|no| FIT["_fit(agg, reg)\nL-BFGS-B MLE\nDavidson NLL + L2 ridge\nRecenter theta → mean 0"] FIT -->|not converged| E2["raise ConvergenceError"] FIT -->|theta, nu| ELO["_elo(theta)\n1500 + 400/ln10 × theta"] ELO --> BOOT["_bootstrap(outcomes, models, rounds, reg, rng)\nResample battles → refit → align Elo\nSkip non-converged resamples\nSkip < 2 samples or zero-variance pool"] BOOT --> CI["ci_low / ci_high arrays\n(NaN where unavailable)"] CI --> TALLY["Per-model win/loss/tie tallies\nvia np.add.at on pair arrays"] TALLY --> BUILD["Build ModelRating entries\nNaN CI → None\nhalf_width = (high-low)/2\nclassify_status(half_width)"] BUILD --> SORT["Sort by Elo descending"] SORT --> OUT["RatingResult(tie_param=nu, 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"}}}%% flowchart TD A["compute_ratings(outcomes, bootstrap_rounds, seed, reg)"] --> V{"Validate\nbootstrap_rounds ≥ 0\nreg > 0 finite"} V -->|fail| E1["raise ValueError"] V -->|ok| AGG["_aggregate(outcomes)\nSkip self-battles\nBuild per-pair win/tie counts"] AGG --> N0{"n models == 0?"} N0 -->|yes| R0["Return RatingResult(models=[])"] N0 -->|no| FIT["_fit(agg, reg)\nL-BFGS-B MLE\nDavidson NLL + L2 ridge\nRecenter theta → mean 0"] FIT -->|not converged| E2["raise ConvergenceError"] FIT -->|theta, nu| ELO["_elo(theta)\n1500 + 400/ln10 × theta"] ELO --> BOOT["_bootstrap(outcomes, models, rounds, reg, rng)\nResample battles → refit → align Elo\nSkip non-converged resamples\nSkip < 2 samples or zero-variance pool"] BOOT --> CI["ci_low / ci_high arrays\n(NaN where unavailable)"] CI --> TALLY["Per-model win/loss/tie tallies\nvia np.add.at on pair arrays"] TALLY --> BUILD["Build ModelRating entries\nNaN CI → None\nhalf_width = (high-low)/2\nclassify_status(half_width)"] BUILD --> SORT["Sort by Elo descending"] SORT --> OUT["RatingResult(tie_param=nu, models=[...])"]Reviews (3): Last reviewed commit: "Treat zero-variance bootstrap as no CI; ..." | Re-trigger Greptile