Arena DB access layer and seed script - #138
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds arena battle and vote persistence to the ChangesArena Battle/Vote Persistence
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ArenaStore
participant PostgreSQL
rect rgba(100, 149, 237, 0.5)
Note over Caller,PostgreSQL: Write path
Caller->>ArenaStore: insert_battle(battle)
ArenaStore->>PostgreSQL: INSERT INTO arena.battles RETURNING *
PostgreSQL-->>ArenaStore: DictRow
ArenaStore->>PostgreSQL: COMMIT
ArenaStore-->>Caller: Battle
Caller->>ArenaStore: upsert_vote(battle_id, outcome, voter_type, voter_id)
ArenaStore->>PostgreSQL: INSERT INTO arena.votes ON CONFLICT DO UPDATE RETURNING *
PostgreSQL-->>ArenaStore: DictRow
ArenaStore->>PostgreSQL: COMMIT
ArenaStore-->>Caller: Vote
end
rect rgba(144, 238, 144, 0.5)
Note over Caller,PostgreSQL: Read path
Caller->>ArenaStore: list_battles(domain, limit)
ArenaStore->>PostgreSQL: SELECT FROM arena.battles WHERE domain=? ORDER BY created_at LIMIT ?
PostgreSQL-->>ArenaStore: DictRow[]
ArenaStore-->>Caller: list[Battle]
Caller->>ArenaStore: list_votes(battle_id, limit)
ArenaStore->>PostgreSQL: SELECT FROM arena.votes WHERE battle_id=? LIMIT ?
PostgreSQL-->>ArenaStore: DictRow[]
ArenaStore-->>Caller: list[Vote]
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
b929ccc to
bfbe25d
Compare
coval-cale
left a comment
There was a problem hiding this comment.
Approved but give some thought to the structure before merging. We should either match the existing structure or commit to the modular one.
I default to staying with the current structure.
Typed Battle/Vote models and an async ArenaStore (insert battle, upsert vote, fetch battles/votes) over the shared pool, plus a local seed of 4 models / 6 battles / 42 votes. Unit tests cover the vote dedup-as-update and the schema checks.
Default None keeps the full result for the rating refit; an int bounds it for paginated callers.
Snapshots are derived rows with no FK to battles/votes, so a reset left stale rankings behind.
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/tests/unit/test_arena_store.py`:
- Around line 63-75: The _make_pool function currently returns
AsyncConnectionPool with type-ignore comments, but it should return the
properly-typed ArenaPool to fix type-checking failures at all ArenaStore
instantiation calls. Import ArenaPool and cast from typing, update the return
type annotation of _make_pool from AsyncConnectionPool to ArenaPool, remove the
type-ignore comments from the function signature and AsyncConnectionPool
instantiation, and use cast(ArenaPool, pool) in the return statement to properly
cast the pool instance before returning it.
🪄 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: 2da17f2e-67b4-4389-a65b-d3e4f56f3483
📒 Files selected for processing (5)
runner/src/coval_bench/db/__init__.pyrunner/src/coval_bench/db/arena_store.pyrunner/src/coval_bench/db/models.pyrunner/src/coval_bench/db/seed_arena.pyrunner/tests/unit/test_arena_store.py
Fold the arena models and store into the existing db/ package to match the current persistence pattern, and refuse to seed/reset unless the DB host is local. Also URL-encode credentials in the test DSN helper.
10f619f to
aebf47d
Compare
Layer 2 (+1) of the Voice Arena plan: the typed data-access layer for the arena schema, plus a local seed fixture. Stacks on feat/voice-arena.
arena/models.py— PydanticBattle/Vote(mirrorsdb/models.py); outcome/voter_type asStrEnummatching the DB checks.arena/store.py— asyncArenaStoreover the shared pool:insert_battle,upsert_vote(ON CONFLICT → updates the existing vote; the trigger bumpsupdated_at),get_battle,list_battles,list_votes.arena/seed.py—python -m coval_bench.arena.seed: 4 models, 6 battles (all pairs = connected graph), 7 deterministic votes each. Skips if already seeded;--resetto reseed. Local dev only.Validated: ruff, ruff format, mypy --strict, 5/5 tests; seed verified against the local DB (6 battles, 42 votes).
No API/web/rating/audio changes.
Summary by CodeRabbit
Release Notes
New Features
Tests
Greptile Summary
Adds the typed data-access layer (
ArenaStore) and Pydantic models (Battle,Vote,VoteOutcome,VoterType) for the voice arena schema, along with a local seed script and embedded-Postgres tests. All SQL is parameterised, transaction management is correct (explicitconn.commit()for writes, no-op for reads), and the seed script includes a conservative_assert_localguard that blocks execution against any non-loopback host.arena_store.pyexposesinsert_battle,upsert_vote(ON CONFLICT → update),get_battle,list_battles, andlist_votes; thelist_votesunbounded-by-default behaviour is intentional and documented (rating refit needs all votes).seed_arena.pyseeds 4 models × 6 pairings × 7 deterministic votes and is idempotent (skip-if-seeded /--reset); the_clearhelper deletes in FK-safe order (leaderboard_snapshots → votes → battles).Confidence Score: 5/5
Safe to merge — the change is purely additive (new store, models, seed script, tests) with no modifications to existing production paths.
All SQL is parameterised with no string interpolation of caller data. Transaction management follows the existing writer.py convention and correctly commits writes. The models mirror the DB schema with optional DB-generated fields. Tests exercise every store method against a real embedded Postgres. The only observations are non-deterministic ORDER BY when timestamps collide, which is a minor hardening note and does not affect correctness today.
No files require special attention. arena_store.py is the core of the change and looks solid end-to-end.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Caller participant ArenaStore participant Pool participant Postgres Note over Caller,Postgres: insert_battle Caller->>ArenaStore: insert_battle(battle) ArenaStore->>Pool: connection() Pool->>Postgres: acquire conn ArenaStore->>Postgres: INSERT INTO arena.battles ... RETURNING ... Postgres-->>ArenaStore: row ArenaStore->>Postgres: COMMIT Pool->>Postgres: return conn ArenaStore-->>Caller: Battle (with id, created_at) Note over Caller,Postgres: upsert_vote Caller->>ArenaStore: upsert_vote(battle_id, outcome, voter_type, voter_id) ArenaStore->>Pool: connection() Pool->>Postgres: acquire conn ArenaStore->>Postgres: INSERT ... ON CONFLICT DO UPDATE ... RETURNING ... Note right of Postgres: BEFORE UPDATE trigger sets updated_at Postgres-->>ArenaStore: row ArenaStore->>Postgres: COMMIT Pool->>Postgres: return conn ArenaStore-->>Caller: Vote (id, outcome, updated_at) Note over Caller,Postgres: list_votes (rating refit path) Caller->>ArenaStore: list_votes(battle_id?) ArenaStore->>Pool: connection() Pool->>Postgres: acquire conn ArenaStore->>Postgres: SELECT ... FROM arena.votes [WHERE] ORDER BY created_at [LIMIT] Postgres-->>ArenaStore: rows[] Pool->>Postgres: return conn ArenaStore-->>Caller: list[Vote]%%{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 Caller participant ArenaStore participant Pool participant Postgres Note over Caller,Postgres: insert_battle Caller->>ArenaStore: insert_battle(battle) ArenaStore->>Pool: connection() Pool->>Postgres: acquire conn ArenaStore->>Postgres: INSERT INTO arena.battles ... RETURNING ... Postgres-->>ArenaStore: row ArenaStore->>Postgres: COMMIT Pool->>Postgres: return conn ArenaStore-->>Caller: Battle (with id, created_at) Note over Caller,Postgres: upsert_vote Caller->>ArenaStore: upsert_vote(battle_id, outcome, voter_type, voter_id) ArenaStore->>Pool: connection() Pool->>Postgres: acquire conn ArenaStore->>Postgres: INSERT ... ON CONFLICT DO UPDATE ... RETURNING ... Note right of Postgres: BEFORE UPDATE trigger sets updated_at Postgres-->>ArenaStore: row ArenaStore->>Postgres: COMMIT Pool->>Postgres: return conn ArenaStore-->>Caller: Vote (id, outcome, updated_at) Note over Caller,Postgres: list_votes (rating refit path) Caller->>ArenaStore: list_votes(battle_id?) ArenaStore->>Pool: connection() Pool->>Postgres: acquire conn ArenaStore->>Postgres: SELECT ... FROM arena.votes [WHERE] ORDER BY created_at [LIMIT] Postgres-->>ArenaStore: rows[] Pool->>Postgres: return conn ArenaStore-->>Caller: list[Vote]Reviews (5): Last reviewed commit: "Move arena DB layer into db/ and guard s..." | Re-trigger Greptile