Add Voice Arena read endpoints - #147
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds three read-only Voice Arena API endpoints ( ChangesVoice Arena Read Endpoints
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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/tests/api/test_arena.py (1)
153-156: ⚡ Quick winStrengthen blind-response assertions to cover B-side identity fields.
This test currently guards only
provider_a/model_a. Add checks forprovider_bandmodel_bso an accidental leak on side B is caught.Suggested test delta
assert "provider_a" not in data assert "model_a" not in data + assert "provider_b" not in data + assert "model_b" not in data assert data["domain"] == "support"🤖 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/tests/api/test_arena.py` around lines 153 - 156, The blind-response test assertions currently only verify that provider_a and model_a fields are not present in the response data, but they should also check for B-side identity fields to ensure no accidental leaks on that side. Add two additional assertions after the existing provider_a and model_a checks to verify that "provider_b" and "model_b" are also not present in the data dictionary, using the same assertion pattern as the existing checks.
🤖 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/tests/api/test_arena.py`:
- Around line 153-156: The blind-response test assertions currently only verify
that provider_a and model_a fields are not present in the response data, but
they should also check for B-side identity fields to ensure no accidental leaks
on that side. Add two additional assertions after the existing provider_a and
model_a checks to verify that "provider_b" and "model_b" are also not present in
the data dictionary, using the same assertion pattern as the existing checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a13a2358-907f-4b14-b3bd-1b257561b478
📒 Files selected for processing (4)
runner/src/coval_bench/api/app.pyrunner/src/coval_bench/api/routers/arena.pyrunner/src/coval_bench/api/schemas.pyrunner/tests/api/test_arena.py
Adds the read-only API for the Voice Arena (layer 4 of the build plan).
Endpoints
GET /v1/arena/battlereturns one battle to vote onGET /v1/arena/battle/{id}returns a specific battleGET /v1/arena/leaderboard?metric=&domain=returns the latest computed boardBattles are served blind: the response omits provider/model identities so voting stays unbiased. The A/B to model mapping stays server-side.
The leaderboard returns the rows sharing the most recent
computed_atfor the requested metric and domain, sorted by Elo. It is empty until the snapshot job (a later layer) has run.Battle selection is a placeholder (uniform random). Adaptive pairing replaces it in a later layer; the endpoint contract does not change.
Reads the pool directly with raw SQL like the other routers, so this lands independently of the DB access layer PR.
Summary by CodeRabbit
Release Notes
Greptile Summary
This PR adds three read-only Voice Arena endpoints (
GET /v1/arena/battle,GET /v1/arena/battle/{id}, andGET /v1/arena/leaderboard) along with their Pydantic schemas and a thorough test suite. Battles are served blind (provider/model identities stripped), and the leaderboard returns the most recent computed snapshot for a given metric and domain.ORDER BY random()(acknowledged in comments) with all model identity columns withheld from the response schema.(computed_at, methodology_version)pair per metric/domain before joining back for the full row set, correctly preventing mixed-version boards.Confidence Score: 5/5
Safe to merge — all three endpoints use parameterized queries, the blind invariant is enforced at the SQL column-selection layer, and the leaderboard CTE correctly isolates single-version boards.
The core logic is correct: battles are served blind, leaderboard filtering by metric/domain is applied both in the CTE and outer WHERE, and all DB access is parameterized. Two minor observations (lexicographic version tiebreaker and unconstrained metric string) do not affect current correctness.
runner/src/coval_bench/api/routers/arena.py — the methodology_version tiebreaker and metric param type are worth a second look before the leaderboard snapshot job ships.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Client participant ArenaRouter participant PgPool as Postgres Pool participant PostHog Client->>ArenaRouter: GET /v1/arena/battle ArenaRouter->>PgPool: SELECT id, prompt_text, domain, audio_a_url, audio_b_url FROM arena.battles ORDER BY random() LIMIT 1 PgPool-->>ArenaRouter: row (or None) alt No rows ArenaRouter-->>Client: 404 no battles available else Row found ArenaRouter->>PostHog: capture arena_battle_served ArenaRouter-->>Client: 200 BattleOut (blind) end Client->>ArenaRouter: "GET /v1/arena/battle/{id}" ArenaRouter->>PgPool: "SELECT ... FROM arena.battles WHERE id = %(id)s" PgPool-->>ArenaRouter: row (or None) alt Not found ArenaRouter-->>Client: 404 else Found ArenaRouter-->>Client: 200 BattleOut (blind) end Client->>ArenaRouter: "GET /v1/arena/leaderboard?metric=&domain=" ArenaRouter->>PgPool: CTE picks latest (computed_at, methodology_version), JOIN snapshots WHERE metric/domain match, ORDER BY rating_elo DESC PgPool-->>ArenaRouter: board rows ArenaRouter->>PostHog: capture arena_leaderboard_queried ArenaRouter-->>Client: 200 ArenaLeaderboardResponse%%{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 Client participant ArenaRouter participant PgPool as Postgres Pool participant PostHog Client->>ArenaRouter: GET /v1/arena/battle ArenaRouter->>PgPool: SELECT id, prompt_text, domain, audio_a_url, audio_b_url FROM arena.battles ORDER BY random() LIMIT 1 PgPool-->>ArenaRouter: row (or None) alt No rows ArenaRouter-->>Client: 404 no battles available else Row found ArenaRouter->>PostHog: capture arena_battle_served ArenaRouter-->>Client: 200 BattleOut (blind) end Client->>ArenaRouter: "GET /v1/arena/battle/{id}" ArenaRouter->>PgPool: "SELECT ... FROM arena.battles WHERE id = %(id)s" PgPool-->>ArenaRouter: row (or None) alt Not found ArenaRouter-->>Client: 404 else Found ArenaRouter-->>Client: 200 BattleOut (blind) end Client->>ArenaRouter: "GET /v1/arena/leaderboard?metric=&domain=" ArenaRouter->>PgPool: CTE picks latest (computed_at, methodology_version), JOIN snapshots WHERE metric/domain match, ORDER BY rating_elo DESC PgPool-->>ArenaRouter: board rows ArenaRouter->>PostHog: capture arena_leaderboard_queried ArenaRouter-->>Client: 200 ArenaLeaderboardResponseReviews (2): Last reviewed commit: "Select a single leaderboard board so met..." | Re-trigger Greptile