From b154868920c1b61967fb20995cc2bfbb6a66ca63 Mon Sep 17 00:00:00 2001 From: withrvr Date: Mon, 14 Sep 2026 13:28:44 +0000 Subject: [PATCH 1/3] feat(leaderboard): per-account and per-model leaderboard (Phase 12) RoomSeat is already each finished game's durable participant record (a room plays exactly one game for its whole lifetime), so app.leaderboard just joins game_record to room_seat on room_id -- no new snapshot table. Two aggregate views (leaderboard_by_account, leaderboard_by_model), an optional per-game filter, a new leaderboard:view permission granted to every default role, and a /leaderboard page with an Accounts/Models tab split. Claude-Session: https://claude.ai/code/session_01UbiPa1rKCyqhqufgwJWkeW --- PROGRESS.md | 99 +++++++- .../30c74160ca50_leaderboard_permission.py | 62 +++++ backend/app/api/main.py | 2 + backend/app/api/routes/leaderboard.py | 33 +++ backend/app/authz/permissions.py | 4 +- backend/app/leaderboard.py | 105 +++++++++ backend/app/models.py | 13 ++ backend/tests/authz/test_authz.py | 2 +- backend/tests/leaderboard/__init__.py | 0 backend/tests/leaderboard/test_leaderboard.py | 211 ++++++++++++++++++ docs/admin-and-observability.md | 10 +- docs/api/api-reference.md | 14 ++ docs/leaderboard.md | 85 +++++++ frontend/src/client/index.ts | 4 +- frontend/src/client/sdk.gen.ts | 28 ++- frontend/src/client/types.gen.ts | 95 ++++++++ .../src/components/Leaderboard/columns.tsx | 34 +++ .../src/components/Rooms/CreateRoomDialog.tsx | 18 +- .../src/components/Sidebar/AppSidebar.tsx | 7 +- frontend/src/lib/games.ts | 17 ++ frontend/src/routeTree.gen.ts | 21 ++ frontend/src/routes/_layout/leaderboard.tsx | 92 ++++++++ frontend/tests/leaderboard.spec.ts | 23 ++ 23 files changed, 950 insertions(+), 29 deletions(-) create mode 100644 backend/app/alembic/versions/30c74160ca50_leaderboard_permission.py create mode 100644 backend/app/api/routes/leaderboard.py create mode 100644 backend/app/leaderboard.py create mode 100644 backend/tests/leaderboard/__init__.py create mode 100644 backend/tests/leaderboard/test_leaderboard.py create mode 100644 docs/leaderboard.md create mode 100644 frontend/src/components/Leaderboard/columns.tsx create mode 100644 frontend/src/lib/games.ts create mode 100644 frontend/src/routes/_layout/leaderboard.tsx create mode 100644 frontend/tests/leaderboard.spec.ts diff --git a/PROGRESS.md b/PROGRESS.md index 7a9431b..e7cdea3 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -38,10 +38,10 @@ branch; only the phase branch opens a PR into `master`. | 9 | Persistence and replay | done 2026-09-13 (PR #14) | | 9.5 | Stabilize, document, rebrand | done 2026-09-13 (PR #15) | | 9.6 | Upgrade to latest stable versions | done 2026-09-13 (PR #17) | -| 9.7 | Docker, CI/CD, and server-start optimization | done 2026-09-14 (branch phase-9.7-infra-optimization, PR pending) | -| 10 | Frontend: play, spectate, admin, focus view | done (branch phase-10-frontend, PR pending) | -| 11 | Chat and personas | done (branch phase-11-chat-and-personas, PR pending) | -| 12 | Leaderboard | | +| 9.7 | Docker, CI/CD, and server-start optimization | done 2026-09-14 (PR #18) | +| 10 | Frontend: play, spectate, admin, focus view | done 2026-09-14 (PR #20) | +| 11 | Chat and personas | done 2026-09-14 (PR #21) | +| 12 | Leaderboard | done 2026-09-14 (branch phase-12-leaderboard, PR pending) | | 13 | Observability | | | 14 | CI/CD (local dry-run; no server yet) | | | 15 | Production deployment (deferred until local complete) | | @@ -944,6 +944,97 @@ correctly reading genuinely new, valid syntax and misclassifying it as an old error. No code change needed; this closes out the anomaly noted repeatedly in Phase 10's review-fix section above. +## Phase 12 — done 2026-09-14 (branch phase-12-leaderboard) + +Self-planned (no formal spec; a direct continuation of context already in +hand, per CLAUDE.md's "phases, not sprints" guidance). DECISIONS.md §11: +"leaderboard per human account and per model, with aggregate admin views." + +- **The core design question**: a finished game's `GameRecord.winner_seat` + names a seat, not a player -- something has to tie that seat back to the + account/model that actually played it. Checked rather than assumed + whether a new snapshot table was needed: it isn't. `RoomService.start()` + refuses any room that isn't `"waiting"`, and nothing anywhere ever puts a + room back into `"waiting"` -- confirmed by reading every status + transition in `app/rooms/service.py`, not by inference -- so a room + plays exactly one game for its entire lifetime. `RoomSeat` (immutable + from the moment `start()` runs) already *is* that one game's durable + participant record. `app.leaderboard` just joins `game_record` to + `room_seat` on `room_id`. Full reasoning: `docs/leaderboard.md`. +- **`app/leaderboard.py`**: `leaderboard_by_account`/`leaderboard_by_model`, + each an aggregate SQL query (join + group by + `func.sum(case(...))` for + wins), taking an optional `game` slug filter. Only `outcome == + "finished"` rows count -- a `"stopped"` game's `winner_seat` is `None`, + so it has no result to credit. `display_name` for the accounts view is + `coalesce(full_name, email)` computed in SQL, not Python: SQLModel's + typed `select()` only has overloads up to four positional columns, and + pulling `full_name`/`email` as two separate columns would have been a + fifth. +- **`GET /leaderboard/accounts?game=`, `GET /leaderboard/models?game=`** + (`app/api/routes/leaderboard.py`): gated by a new `leaderboard:view` + permission, deliberately granted to *every* default role + (`admin`/`player`/`spectator`) rather than admin-only -- a leaderboard is + meant to be seen by the players in it; "aggregate admin views" just means + the same data also surfaces from the admin side, not that it needs a + separate, more-restricted endpoint. New permission needed its own + Alembic migration (`30c74160ca50`) rather than editing the original + Phase 6 seed migration: that migration's own `ALL_PERMISSIONS`/ + `DEFAULT_ROLES` imports are live, growing constants, so re-running it + would re-seed permissions it already inserted and collide on the unique + `name` constraint. The new migration names its one new permission + literally instead. +- **Frontend**: `/leaderboard` (new sidebar nav item, shown to everyone, + matching the permission grant) -- two tabs (Accounts, Models) sharing one + column set (`components/Leaderboard/columns.tsx`) over the same + `LeaderboardEntry` shape, plus a game-filter `Select`. Extracted + `CreateRoomDialog.tsx`'s hardcoded four-game catalog into `lib/games.ts` + so the filter dropdown could reuse it instead of duplicating it. +- **A wrong turn, caught before it shipped**: first drafted a "fix" in + `RoomService.start()` to persist a resolved default model/persona back + onto an agent seat, reasoning that a seat created without an explicit + model would otherwise stay `model=None` forever -- a real-looking gap + for a per-model leaderboard. Wrote the fix, then verified it empirically + (a throwaway script creating a real agent seat with no explicit model) + before trusting the reasoning, and the premise was false: + `RoomService.create()` already resolves and persists the default at + seat-creation time (`model = seat_spec.model or + self.registry.default_model`, written into the row immediately), and + `force_start`'s backfill does the same for the seats it creates. The + `start()`-time fallback is genuinely unreachable for any room made + through the real API. Reverted the change rather than ship a fix (and a + comment asserting a premise) for a bug that doesn't exist -- confirmed + by re-running the same empirical check after reverting. +- **Type-checker friction, resolved properly rather than suppressed**: `ty` + (this project's strict checker) rejected `RoomSeat.model.is_not(None)` + (plain `str | None` at the class level, not a method SQLAlchemy's + declarative typing exposes to a static checker without help) and several + `.join()`/`.group_by()` calls comparing two model columns directly. Fixed + by wrapping column references in SQLModel's `col()` helper (the same + pattern already used in `app/authz/service.py`), not by suppressing the + checker -- and dropped the `is_not(None)` filter entirely once the + model-backfill investigation above proved it was defensive code guarding + against a state that can't occur. + +New tests: `backend/tests/leaderboard/test_leaderboard.py` (7 tests: wins +and games-played aggregate correctly across separate rooms, stopped games +excluded, model grouping ignores persona, the `game` filter scopes +correctly, both HTTP routes 200, the permission gate 403s when +`leaderboard:view` is revoked). One self-caught bug while writing them: an +early draft of the "stopped games are excluded" test asserted the *entire* +leaderboard was empty, but `db` is a session-scoped fixture shared across +every test in the file -- an earlier test's own rows were still present. +Fixed to scope the assertion to this test's own two accounts, matching +every other test in the file. `tests/authz/test_authz.py::test_migration_seeds` +also needed its hardcoded `len(ALL_PERMISSIONS) == 12` bumped to `13` -- +a real, expected consequence of adding a permission, not a regression. +`frontend/tests/leaderboard.spec.ts` (2 tests: tabs/filter render and +switch; the sidebar nav link reaches the page) (779 backend tests total, +59 Playwright tests total). + +Full gate: backend suite (779 tests) green, `ty`/`mypy`/`ruff` clean, +`bun run build`/`bun run lint` clean, full Playwright suite (59 tests) +green against the real docker compose stack, `prek run --all-files` clean. + ## Phase 9.7 — done 2026-09-14 (branch phase-9.7-infra-optimization) Infrastructure-only pass: Docker image build, Compose, CI/CD workflows, diff --git a/backend/app/alembic/versions/30c74160ca50_leaderboard_permission.py b/backend/app/alembic/versions/30c74160ca50_leaderboard_permission.py new file mode 100644 index 0000000..13e5200 --- /dev/null +++ b/backend/app/alembic/versions/30c74160ca50_leaderboard_permission.py @@ -0,0 +1,62 @@ +"""Leaderboard view permission (Phase 12) + +Revision ID: 30c74160ca50 +Revises: f54e8ebd4583 +Create Date: 2026-09-14 12:00:00.000000 + +""" +from alembic import op +import sqlalchemy as sa + +from app.authz.permissions import permission_id, role_id + +# revision identifiers, used by Alembic. +revision = '30c74160ca50' +down_revision = 'f54e8ebd4583' +branch_labels = None +depends_on = None + +# Deliberately literal, not `app.authz.permissions.P.LEADERBOARD_VIEW` / +# `ALL_PERMISSIONS` / `DEFAULT_ROLES` -- those constants keep growing as +# later phases add permissions, so re-deriving from them here would re-seed +# rows the *original* rbac migration (a1b2c3d4e5f6) already inserted and +# collide on the unique `name` constraint. A new permission gets a new, +# self-contained migration naming exactly what it adds. +_PERMISSION = "leaderboard:view" +_GRANTED_TO = ("admin", "player", "spectator") + + +def upgrade(): + permission = sa.table( + "permission", + sa.column("id"), + sa.column("name"), + sa.column("description"), + ) + role_permission = sa.table( + "role_permission", sa.column("role_id"), sa.column("permission_id") + ) + op.bulk_insert( + permission, + [{"id": permission_id(_PERMISSION), "name": _PERMISSION, "description": ""}], + ) + op.bulk_insert( + role_permission, + [ + {"role_id": role_id(name), "permission_id": permission_id(_PERMISSION)} + for name in _GRANTED_TO + ], + ) + + +def downgrade(): + op.execute( + sa.text("DELETE FROM role_permission WHERE permission_id = :pid").bindparams( + pid=permission_id(_PERMISSION) + ) + ) + op.execute( + sa.text("DELETE FROM permission WHERE id = :pid").bindparams( + pid=permission_id(_PERMISSION) + ) + ) diff --git a/backend/app/api/main.py b/backend/app/api/main.py index d59ef56..380a7fd 100644 --- a/backend/app/api/main.py +++ b/backend/app/api/main.py @@ -5,6 +5,7 @@ games, health, items, + leaderboard, login, private, roles, @@ -25,6 +26,7 @@ api_router.include_router(admin.me_router) api_router.include_router(rooms.router) api_router.include_router(games.router) +api_router.include_router(leaderboard.router) if settings.FASTAPI_ENV == "development": diff --git a/backend/app/api/routes/leaderboard.py b/backend/app/api/routes/leaderboard.py new file mode 100644 index 0000000..15e6782 --- /dev/null +++ b/backend/app/api/routes/leaderboard.py @@ -0,0 +1,33 @@ +"""Aggregate leaderboards (Phase 12, DECISIONS.md §11): "leaderboard per +human account and per model, with aggregate admin views." Granted to every +default role (``admin``/``player``/``spectator``, see +``app/authz/permissions.py``) -- a leaderboard is meant to be seen, not an +admin-only report; "aggregate admin views" (docs/admin-and-observability.md) +just means the same data also surfaces in the admin dashboard, not a +separate, more-restricted endpoint.""" + +from __future__ import annotations + +from typing import Annotated, Any + +from fastapi import APIRouter, Depends, Query + +from app.api.deps import SessionDep +from app.authz import P, require_permission +from app.leaderboard import leaderboard_by_account, leaderboard_by_model +from app.models import LeaderboardEntry + +router = APIRouter(prefix="/leaderboard", tags=["leaderboard"]) + +_view = Depends(require_permission(P.LEADERBOARD_VIEW)) +GameFilter = Annotated[str | None, Query(max_length=64)] + + +@router.get("/accounts", response_model=list[LeaderboardEntry], dependencies=[_view]) +def accounts_leaderboard(session: SessionDep, game: GameFilter = None) -> Any: + return leaderboard_by_account(session, game) + + +@router.get("/models", response_model=list[LeaderboardEntry], dependencies=[_view]) +def models_leaderboard(session: SessionDep, game: GameFilter = None) -> Any: + return leaderboard_by_model(session, game) diff --git a/backend/app/authz/permissions.py b/backend/app/authz/permissions.py index 6aae1d7..05fed31 100644 --- a/backend/app/authz/permissions.py +++ b/backend/app/authz/permissions.py @@ -27,6 +27,7 @@ class P: ADMIN_MANAGE_ROLES = "admin:manage_roles" ADMIN_MANAGE_SETTINGS = "admin:manage_settings" ADMIN_MANAGE_FLAGS = "admin:manage_flags" + LEADERBOARD_VIEW = "leaderboard:view" ALL_PERMISSIONS: frozenset[str] = frozenset( @@ -47,13 +48,14 @@ class P: P.ROOM_CHAT, P.ROOM_VIEW, P.ROOM_VIEW_PUBLIC, + P.LEADERBOARD_VIEW, } ), ), "spectator": ( "global", "Can watch rooms.", - frozenset({P.ROOM_VIEW, P.ROOM_VIEW_PUBLIC}), + frozenset({P.ROOM_VIEW, P.ROOM_VIEW_PUBLIC, P.LEADERBOARD_VIEW}), ), "moderator": ( "room", diff --git a/backend/app/leaderboard.py b/backend/app/leaderboard.py new file mode 100644 index 0000000..ab0424b --- /dev/null +++ b/backend/app/leaderboard.py @@ -0,0 +1,105 @@ +"""Leaderboard aggregation (Phase 12, DECISIONS.md §11): "leaderboard per +human account and per model, with aggregate admin views." + +A finished game's ``GameRecord.winner_seat`` names a seat, not a player -- +``RoomSeat`` is what ties that seat back to the account or model that +actually played it. No separate snapshot table is needed to do that +durably: ``RoomService.start()`` refuses a room that isn't ``"waiting"``, +and nothing ever puts a room back into ``"waiting"`` after that, so a room +plays exactly one game for its entire lifetime -- ``room_seat`` already *is* +that one game's durable participant record, immutable from the moment +``start()`` runs (``join``/``force_start`` are likewise gated on +``"waiting"``). + +Only ``outcome == "finished"`` games count: a ``"stopped"`` game's +``winner_seat`` is ``None`` (docs/games.md), so it has no result to credit +a win or loss for. It still happened, but "games played" here means "games +that produced a result," matching what a leaderboard is for. +""" + +from __future__ import annotations + +from typing import Any + +from sqlalchemy import case, func +from sqlmodel import Session, col, select + +from app.models import GameRecord, LeaderboardEntry, RoomSeat, User + + +def _wins() -> Any: + return func.sum(case((col(GameRecord.winner_seat) == col(RoomSeat.seat), 1), else_=0)) + + +def leaderboard_by_account( + session: Session, game: str | None = None +) -> list[LeaderboardEntry]: + """One row per human account with at least one finished game.""" + # `func.coalesce(...)` folds `full_name`/`email` into one selected column + # rather than two -- not just tidier, `select()`'s typed overloads only + # go up to four positional columns, and a fifth (`email` as its own + # column, alongside user_id/display_name/games_played/wins) has no + # matching overload. + display_name = func.coalesce(User.full_name, User.email) + stmt = ( + select( + RoomSeat.user_id, + display_name.label("display_name"), + func.count().label("games_played"), + _wins().label("wins"), + ) + .join(RoomSeat, col(RoomSeat.room_id) == col(GameRecord.room_id)) + .join(User, col(User.id) == col(RoomSeat.user_id)) + .where(GameRecord.outcome == "finished", RoomSeat.kind == "human") + .group_by(col(RoomSeat.user_id), col(User.full_name), col(User.email)) + .order_by(_wins().desc(), func.count().desc()) + ) + if game is not None: + stmt = stmt.where(GameRecord.game == game) + return [ + LeaderboardEntry( + subject_id=str(user_id), + display_name=name, + games_played=games_played, + wins=wins, + win_rate=wins / games_played, + ) + for user_id, name, games_played, wins in session.exec(stmt).all() + ] + + +def leaderboard_by_model( + session: Session, game: str | None = None +) -> list[LeaderboardEntry]: + """One row per model name with at least one finished game.""" + stmt = ( + select( + RoomSeat.model, + func.count().label("games_played"), + _wins().label("wins"), + ) + .join(RoomSeat, col(RoomSeat.room_id) == col(GameRecord.room_id)) + .where(GameRecord.outcome == "finished", RoomSeat.kind == "agent") + .group_by(RoomSeat.model) + .order_by(_wins().desc(), func.count().desc()) + ) + if game is not None: + stmt = stmt.where(GameRecord.game == game) + entries = [] + for model, games_played, wins in session.exec(stmt).all(): + # `RoomSeat.model` is nullable at the schema level, but a kind="agent" + # row is never written without one (`RoomService.create`/ + # `force_start` both always resolve a default before insert) -- + # asserted, not silently coerced, so a real future violation of that + # invariant fails loudly here rather than mislabeling a row "None". + assert model is not None + entries.append( + LeaderboardEntry( + subject_id=model, + display_name=model, + games_played=games_played, + wins=wins, + win_rate=wins / games_played, + ) + ) + return entries diff --git a/backend/app/models.py b/backend/app/models.py index 1491952..ec71cfa 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -447,6 +447,19 @@ class GameRecordPublic(SQLModel): finished_at: datetime +class LeaderboardEntry(SQLModel): + """One row of an aggregate leaderboard (Phase 12, DECISIONS.md §11): + either a human account (``subject_id`` a user id) or a model + (``subject_id`` the model name), never both -- ``app.leaderboard`` never + mixes the two in one query.""" + + subject_id: str + display_name: str + games_played: int + wins: int + win_rate: float + + class GameEvent(SQLModel, table=True): __tablename__ = "game_event" __table_args__ = ( diff --git a/backend/tests/authz/test_authz.py b/backend/tests/authz/test_authz.py index 604f577..34c1c3b 100644 --- a/backend/tests/authz/test_authz.py +++ b/backend/tests/authz/test_authz.py @@ -43,7 +43,7 @@ def test_migration_seeds(db: Session) -> None: assert set(roles) == {"admin", "player", "spectator", "moderator"} assert roles["moderator"].scope == "room" assert {p.name for p in db.exec(select(Permission)).all()} == ALL_PERMISSIONS - assert len(ALL_PERMISSIONS) == 12 + assert len(ALL_PERMISSIONS) == 13 admin_perms = db.exec( select(Permission.name) .join(RolePermission, RolePermission.permission_id == Permission.id) # type: ignore[arg-type] diff --git a/backend/tests/leaderboard/__init__.py b/backend/tests/leaderboard/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/backend/tests/leaderboard/test_leaderboard.py b/backend/tests/leaderboard/test_leaderboard.py new file mode 100644 index 0000000..6d0ff93 --- /dev/null +++ b/backend/tests/leaderboard/test_leaderboard.py @@ -0,0 +1,211 @@ +"""Leaderboard aggregation (Phase 12, DECISIONS.md §11): "leaderboard per +human account and per model, with aggregate admin views." + +``app.leaderboard`` joins ``game_record`` to ``room_seat`` on ``room_id`` -- +a room plays exactly one game for its whole lifetime (``RoomService.start`` +refuses anything but ``"waiting"``, and nothing ever reopens a room), so +``room_seat`` already is that game's durable participant record. These +tests build real ``Room``/``RoomSeat`` rows directly (no need for +``RoomService``/Redis/a real game run -- that path is already proven by +``tests/rooms/test_game_history.py``) and write ``GameRecord`` rows +directly too, to test the aggregation query in isolation. +""" + +from __future__ import annotations + +import uuid +from datetime import UTC, datetime + +from fastapi.testclient import TestClient +from sqlmodel import Session, select + +from app.authz import service as authz +from app.core.config import settings +from app.leaderboard import leaderboard_by_account, leaderboard_by_model +from app.models import GameRecord, Room, RoomSeat, User +from tests.utils.user import create_random_user, user_authentication_headers +from tests.utils.utils import random_email, random_lower_string + +API = settings.API_V1_STR + + +def _room(db: Session, creator_id: uuid.UUID, game: str = "uno_classic") -> Room: + room = Room( + code=uuid.uuid4().hex[:8], + game=game, + seat_count=2, + status="finished", + visibility="private", + creator_id=creator_id, + seed=1, + ) + db.add(room) + db.flush() + return room + + +def _human_seat(db: Session, room_id: uuid.UUID, seat: int, user_id: uuid.UUID) -> None: + db.add( + RoomSeat( + room_id=room_id, seat=seat, kind="human", user_id=user_id, status="filled" + ) + ) + + +def _agent_seat(db: Session, room_id: uuid.UUID, seat: int, model: str) -> None: + db.add( + RoomSeat( + room_id=room_id, + seat=seat, + kind="agent", + model=model, + persona="steady", + status="filled", + ) + ) + + +def _record( + room_id: uuid.UUID, + *, + game: str = "uno_classic", + outcome: str = "finished", + winner_seat: int | None = 0, +) -> GameRecord: + now = datetime.now(UTC) + finished = outcome == "finished" + return GameRecord( + id=uuid.uuid4().hex, + room_id=room_id, + game=game, + seat_count=2, + seed=1, + outcome=outcome, + winner_seat=winner_seat if finished else None, + ranking=[winner_seat, 1 - winner_seat] if finished else [], + scores=[0, 0] if finished else [], + move_count=1, + started_at=now, + finished_at=now, + ) + + +def test_leaderboard_by_account_counts_wins_and_games_played_across_rooms( + db: Session, +) -> None: + creator = create_random_user(db) + a, b = create_random_user(db), create_random_user(db) + + room1 = _room(db, creator.id) + _human_seat(db, room1.id, 0, a.id) + _human_seat(db, room1.id, 1, b.id) + db.add(_record(room1.id, winner_seat=0)) # a wins + + room2 = _room(db, creator.id) + _human_seat(db, room2.id, 0, b.id) + _human_seat(db, room2.id, 1, a.id) + db.add(_record(room2.id, winner_seat=0)) # b wins, a loses + db.commit() + + entries = {e.subject_id: e for e in leaderboard_by_account(db)} + assert entries[str(a.id)].games_played == 2 + assert entries[str(a.id)].wins == 1 + assert entries[str(a.id)].win_rate == 0.5 + assert entries[str(b.id)].games_played == 2 + assert entries[str(b.id)].wins == 1 + + +def test_stopped_games_have_no_winner_and_are_excluded(db: Session) -> None: + creator = create_random_user(db) + a, b = create_random_user(db), create_random_user(db) + + room = _room(db, creator.id) + _human_seat(db, room.id, 0, a.id) + _human_seat(db, room.id, 1, b.id) + db.add(_record(room.id, outcome="stopped", winner_seat=None)) + db.commit() + + subject_ids = {e.subject_id for e in leaderboard_by_account(db)} + assert str(a.id) not in subject_ids + assert str(b.id) not in subject_ids + + +def test_leaderboard_by_model_groups_by_model_not_persona(db: Session) -> None: + creator = create_random_user(db) + a = create_random_user(db) + + room1 = _room(db, creator.id) + _agent_seat(db, room1.id, 0, "model-x") + _agent_seat(db, room1.id, 1, "model-y") + db.add(_record(room1.id, winner_seat=0)) # model-x wins + + room2 = _room(db, creator.id) + _agent_seat(db, room2.id, 0, "model-x") + _human_seat(db, room2.id, 1, a.id) + db.add(_record(room2.id, winner_seat=1)) # a wins, model-x loses + db.commit() + + entries = {e.subject_id: e for e in leaderboard_by_model(db)} + assert entries["model-x"].games_played == 2 + assert entries["model-x"].wins == 1 + assert entries["model-y"].games_played == 1 + assert entries["model-y"].wins == 0 + + +def test_game_filter_scopes_to_one_game_type(db: Session) -> None: + creator = create_random_user(db) + a, b = create_random_user(db), create_random_user(db) + + uno_room = _room(db, creator.id, game="uno_classic") + _human_seat(db, uno_room.id, 0, a.id) + _human_seat(db, uno_room.id, 1, b.id) + db.add(_record(uno_room.id, game="uno_classic", winner_seat=0)) + + hearts_room = _room(db, creator.id, game="hearts") + _human_seat(db, hearts_room.id, 0, a.id) + _human_seat(db, hearts_room.id, 1, b.id) + db.add(_record(hearts_room.id, game="hearts", winner_seat=1)) + db.commit() + + hearts_entries = {e.subject_id: e for e in leaderboard_by_account(db, "hearts")} + assert hearts_entries[str(a.id)].games_played == 1 + assert hearts_entries[str(a.id)].wins == 0 + assert hearts_entries[str(b.id)].games_played == 1 + assert hearts_entries[str(b.id)].wins == 1 + + all_entries = {e.subject_id: e for e in leaderboard_by_account(db)} + assert all_entries[str(a.id)].games_played == 2 + assert all_entries[str(a.id)].wins == 1 + + +def _new_user_headers(client: TestClient) -> dict[str, str]: + email, password = random_email(), random_lower_string() + r = client.post(f"{API}/users/signup", json={"email": email, "password": password}) + assert r.status_code == 200, r.text + return user_authentication_headers(client=client, email=email, password=password) + + +def test_get_leaderboard_accounts_http_200(client: TestClient) -> None: + headers = _new_user_headers(client) + r = client.get(f"{API}/leaderboard/accounts", headers=headers) + assert r.status_code == 200, r.text + assert isinstance(r.json(), list) + + +def test_get_leaderboard_models_http_200(client: TestClient) -> None: + headers = _new_user_headers(client) + r = client.get(f"{API}/leaderboard/models", headers=headers) + assert r.status_code == 200, r.text + assert isinstance(r.json(), list) + + +def test_leaderboard_requires_leaderboard_view_permission( + client: TestClient, db: Session, normal_user_token_headers: dict[str, str] +) -> None: + user = db.exec(select(User).where(User.email == settings.EMAIL_TEST_USER)).one() + authz.revoke_role(db, user.id, "player") + try: + r = client.get(f"{API}/leaderboard/accounts", headers=normal_user_token_headers) + assert r.status_code == 403 + finally: + authz.grant_role(db, user.id, "player") diff --git a/docs/admin-and-observability.md b/docs/admin-and-observability.md index be9fd68..432eddf 100644 --- a/docs/admin-and-observability.md +++ b/docs/admin-and-observability.md @@ -41,4 +41,12 @@ moves per minute, model swaps. ## Admin dashboard (Phase 10/13) All rooms, per-room seats and status, force-stop, live log tail, runtime -settings (capacity caps), feature flags, aggregate leaderboards. +settings (capacity caps), feature flags. + +## Leaderboard (Phase 12, docs/leaderboard.md) + +Per human account and per model, DECISIONS.md §11. Ended up as its own +top-level `/leaderboard` page rather than folded into the admin dashboard +above: `leaderboard:view` is granted to every default role +(`admin`/`player`/`spectator`), since a leaderboard is meant to be seen by +players, not gated behind admin-only access. diff --git a/docs/api/api-reference.md b/docs/api/api-reference.md index f225fd2..1f2a229 100644 --- a/docs/api/api-reference.md +++ b/docs/api/api-reference.md @@ -150,6 +150,20 @@ some `game_id` could otherwise use). | `GET /games/{game_id}/events` | Bearer + room-admitted | query `since` (archived `seq` integer, not a live cursor), `limit` (default 200) | `{"events": [...], "next_since": }` | `404` unknown game or denied | | `GET /games/{game_id}/replay` | Bearer + room-admitted | query `seat?` (default: caller's own), `upto?` (default: every move) | that seat's `PlayerView` at that point, via `Referee.replay` | `400` no `seat` given and caller holds none, `403` requesting another seat without privilege, `422` `upto` exceeds the game's actual move count, `404` unknown game or denied, `500` archived history failed to replay (`ReplayMismatchError`, logged) | +## Leaderboard (`leaderboard.py`, prefix `/leaderboard`) — Phase 12, DECISIONS.md §11 + +Aggregate, not per-room: joins every `GameRecord` (`outcome == "finished"` +only -- a `"stopped"` game has no `winner_seat`) to `RoomSeat` on `room_id` +(docs/leaderboard.md explains why that join alone is durable and correct). +Gated by `leaderboard:view`, granted to every default role +(`admin`/`player`/`spectator`) -- a leaderboard is meant to be seen, not an +admin-only report. + +| Method & path | Auth | Request | Response | Status codes | +|---|---|---|---|---| +| `GET /leaderboard/accounts` | Bearer + `leaderboard:view` | query `game?` (a game slug, e.g. `hearts`; omitted = every game) | `list[LeaderboardEntry {subject_id, display_name, games_played, wins, win_rate}]`, one row per human account, ordered by wins desc | `403` missing the permission | +| `GET /leaderboard/models` | Bearer + `leaderboard:view` | query `game?` | same shape, one row per model name | `403` missing the permission | + ## Events and visibility Both the live polling route (`GET /rooms/{room_id}/events`) and the durable diff --git a/docs/leaderboard.md b/docs/leaderboard.md new file mode 100644 index 0000000..93b52ee --- /dev/null +++ b/docs/leaderboard.md @@ -0,0 +1,85 @@ +# Leaderboard (Phase 12) + +Status: done. DECISIONS.md §11: "leaderboard per human account and per +model, with aggregate admin views." + +## Why `room_seat` alone is enough + +A finished game's `GameRecord.winner_seat` (docs/games.md) names a seat, +not a player -- `RoomSeat` is what ties that seat back to the account or +model that actually played it. No separate per-game participant snapshot +table is needed to do that durably: `RoomService.start()` refuses a room +that isn't `"waiting"`, and nothing anywhere puts a room back into +`"waiting"` after that (`join`/`force_start` are likewise gated on +`"waiting"`) -- so a room plays exactly **one** game for its entire +lifetime. `room_seat` already *is* that one game's durable participant +record, immutable from the moment `start()` runs. + +`app.leaderboard` joins `game_record` to `room_seat` on `room_id` and +credits a win when `game_record.winner_seat == room_seat.seat`. + +## What counts + +Only `game_record.outcome == "finished"` rows: a `"stopped"` game's +`winner_seat` is `None` (it never produced a result), so it contributes +neither a game-played nor a win/loss to either leaderboard. It still +happened -- it's just not what a leaderboard is counting. + +## Two views, one query shape + +- `leaderboard_by_account`: one row per human account (`room_seat.kind == + "human"`), grouped by `user_id`. `display_name` is + `coalesce(full_name, email)`, computed in SQL rather than in Python so a + five-column `select()` doesn't need a fifth positional argument (SQLModel's + typed `select()` overloads only go up to four). +- `leaderboard_by_model`: one row per model name (`room_seat.kind == + "agent"`), grouped by `model`. Deliberately not grouped by persona too -- + DECISIONS.md §11 says "per model," and a model's leaderboard entry is + meant to answer "how does this model play," independent of which persona + prompt happened to be layered on top of it in any given game. + +Both take an optional `game` filter (a game slug, e.g. `"hearts"`) scoping +the aggregate to one game type -- without it, a strong Hearts record and a +strong Classic UNO record combine into one number, which is rarely what +"how good is this account/model" actually means once more than one game +type has real history. + +## Routes (`app/api/routes/leaderboard.py`, prefix `/leaderboard`) + +| Route | Notes | +|---|---| +| `GET /leaderboard/accounts?game=` | `list[LeaderboardEntry]`, ordered by wins desc then games played desc. | +| `GET /leaderboard/models?game=` | Same shape, grouped by model instead of account. | + +Both gated by `leaderboard:view` -- granted to every default role +(`admin`/`player`/`spectator`, `app/authz/permissions.py`), not admin-only: +a leaderboard is meant to be seen by the players in it, not filed away as +an internal report. "Aggregate admin views" (DECISIONS.md §11) just means +this same data is also visible from the admin side, not that it needs a +separate, more-restricted endpoint. + +## Frontend + +`/leaderboard` (sidebar nav item, shown to everyone): two tabs (Accounts, +Models) over the same `LeaderboardEntry` shape, sharing one column set +(`components/Leaderboard/columns.tsx`), plus a game-filter dropdown reusing +the same `GAMES` catalog `CreateRoomDialog` already hardcodes +(`lib/games.ts`, extracted from `CreateRoomDialog.tsx` during this phase so +the two didn't duplicate the same four-entry list). + +## Left for later (explicitly, not silently) + +- **No persona-level breakdown.** Deliberately out of scope per DECISIONS + §11's own wording ("per model," not "per model per persona") -- would be + a natural, cheap follow-on if wanted (persona is already on `room_seat`). +- **No pagination.** Every account/model that has ever finished a game + comes back in one response. Fine at this project's scale; would need + `limit`/`cursor` query params (matching `GET /games/{id}/events`'s own + pattern) if the account/model count ever grows large enough to matter. +- **A pre-existing games-archived-before-a-fix problem is generic to this + whole approach, not specific to leaderboard**: if a later change alters + what `room_seat.model`/`kind` mean for future games, older archived + `GameRecord`s still join against whatever `room_seat` rows existed when + *they* were played -- there is no re-derivation step, by design (the + point of `room_seat` being immutable-after-start is that old history + never silently reinterprets itself). diff --git a/frontend/src/client/index.ts b/frontend/src/client/index.ts index b8b2935..c7a6222 100644 --- a/frontend/src/client/index.ts +++ b/frontend/src/client/index.ts @@ -1,4 +1,4 @@ // This file is auto-generated by @hey-api/openapi-ts -export { AdminService, FlagsService, GamesService, HealthService, ItemsService, LoginService, type Options, PrivateService, RolesService, RoomsService, UsersService, UtilsService } from './sdk.gen'; -export type { adminCreateOverrideData, adminCreateOverrideError, adminCreateOverrideErrors, adminCreateOverrideResponse, adminCreateOverrideResponses, adminReadFlagsData, adminReadFlagsResponse, adminReadFlagsResponses, adminReadSettingsData, adminReadSettingsResponse, adminReadSettingsResponses, adminRemoveOverrideData, adminRemoveOverrideError, adminRemoveOverrideErrors, adminRemoveOverrideResponse, adminRemoveOverrideResponses, adminUpdateFlagData, adminUpdateFlagError, adminUpdateFlagErrors, adminUpdateFlagResponse, adminUpdateFlagResponses, adminUpdateSettingData, adminUpdateSettingError, adminUpdateSettingErrors, adminUpdateSettingResponse, adminUpdateSettingResponses, Body_login_login_access_token, ChatSend, ClientOptions, FeatureFlagOverrideCreate, FeatureFlagOverridePublic, FeatureFlagPublic, FeatureFlagUpdate, flagsReadMyFlagsData, flagsReadMyFlagsResponse, flagsReadMyFlagsResponses, GameRecordPublic, gamesGameEventsData, gamesGameEventsError, gamesGameEventsErrors, gamesGameEventsResponses, gamesGetGameRecordData, gamesGetGameRecordError, gamesGetGameRecordErrors, gamesGetGameRecordResponse, gamesGetGameRecordResponses, gamesReplayData, gamesReplayError, gamesReplayErrors, gamesReplayResponses, healthHealthData, healthHealthLlmData, healthHealthLlmResponse, healthHealthLlmResponses, healthHealthResponse, healthHealthResponses, HealthReport, HTTPValidationError, ItemCreate, ItemPublic, itemsCreateItemData, itemsCreateItemError, itemsCreateItemErrors, itemsCreateItemResponse, itemsCreateItemResponses, itemsDeleteItemData, itemsDeleteItemError, itemsDeleteItemErrors, itemsDeleteItemResponse, itemsDeleteItemResponses, ItemsPublic, itemsReadItemData, itemsReadItemError, itemsReadItemErrors, itemsReadItemResponse, itemsReadItemResponses, itemsReadItemsData, itemsReadItemsError, itemsReadItemsErrors, itemsReadItemsResponse, itemsReadItemsResponses, itemsUpdateItemData, itemsUpdateItemError, itemsUpdateItemErrors, itemsUpdateItemResponse, itemsUpdateItemResponses, ItemUpdate, LLMHealth, loginLoginAccessTokenData, loginLoginAccessTokenError, loginLoginAccessTokenErrors, loginLoginAccessTokenResponse, loginLoginAccessTokenResponses, loginRecoverPasswordData, loginRecoverPasswordError, loginRecoverPasswordErrors, loginRecoverPasswordHtmlContentData, loginRecoverPasswordHtmlContentError, loginRecoverPasswordHtmlContentErrors, loginRecoverPasswordHtmlContentResponse, loginRecoverPasswordHtmlContentResponses, loginRecoverPasswordResponse, loginRecoverPasswordResponses, loginResetPasswordData, loginResetPasswordError, loginResetPasswordErrors, loginResetPasswordResponse, loginResetPasswordResponses, loginTestTokenData, loginTestTokenResponse, loginTestTokenResponses, Message, MoveSubmission, NewPassword, PermissionsPublic, privateCreateUserData, privateCreateUserError, privateCreateUserErrors, privateCreateUserResponse, privateCreateUserResponses, PrivateUserCreate, RoleAssignment, RolePublic, rolesGrantUserRoleData, rolesGrantUserRoleError, rolesGrantUserRoleErrors, rolesGrantUserRoleResponse, rolesGrantUserRoleResponses, rolesReadPermissionsData, rolesReadPermissionsResponse, rolesReadPermissionsResponses, rolesReadRolesData, rolesReadRolesResponse, rolesReadRolesResponses, rolesReadUserRolesData, rolesReadUserRolesError, rolesReadUserRolesErrors, rolesReadUserRolesResponse, rolesReadUserRolesResponses, rolesRevokeUserRoleData, rolesRevokeUserRoleError, rolesRevokeUserRoleErrors, rolesRevokeUserRoleResponse, rolesRevokeUserRoleResponses, RoomCreate, RoomJoinRequest, RoomPublic, roomsCreateRoomData, roomsCreateRoomError, roomsCreateRoomErrors, roomsCreateRoomResponse, roomsCreateRoomResponses, RoomSeatJoinPublic, RoomSeatPublic, RoomSeatSpec, roomsForceStartRoomData, roomsForceStartRoomError, roomsForceStartRoomErrors, roomsForceStartRoomResponse, roomsForceStartRoomResponses, roomsGetRoomData, roomsGetRoomError, roomsGetRoomErrors, roomsGetRoomResponse, roomsGetRoomResponses, roomsJoinRoomData, roomsJoinRoomError, roomsJoinRoomErrors, roomsJoinRoomResponse, roomsJoinRoomResponses, roomsLeaveRoomData, roomsLeaveRoomError, roomsLeaveRoomErrors, roomsLeaveRoomResponse, roomsLeaveRoomResponses, roomsListRoomsData, roomsListRoomsError, roomsListRoomsErrors, roomsListRoomsResponse, roomsListRoomsResponses, roomsMovesSchemaData, roomsMovesSchemaError, roomsMovesSchemaErrors, roomsMovesSchemaResponses, RoomsPublic, roomsRoomEventsData, roomsRoomEventsError, roomsRoomEventsErrors, roomsRoomEventsResponses, roomsRoomQueueData, roomsRoomQueueError, roomsRoomQueueErrors, roomsRoomQueueResponses, roomsRoomViewData, roomsRoomViewError, roomsRoomViewErrors, roomsRoomViewResponses, roomsSendChatData, roomsSendChatError, roomsSendChatErrors, roomsSendChatResponse, roomsSendChatResponses, roomsStartRoomData, roomsStartRoomError, roomsStartRoomErrors, roomsStartRoomResponse, roomsStartRoomResponses, roomsStopRoomData, roomsStopRoomError, roomsStopRoomErrors, roomsStopRoomResponse, roomsStopRoomResponses, roomsSubmitMoveData, roomsSubmitMoveError, roomsSubmitMoveErrors, roomsSubmitMoveResponse, roomsSubmitMoveResponses, roomsWsTicketData, roomsWsTicketError, roomsWsTicketErrors, roomsWsTicketResponse, roomsWsTicketResponses, RuntimeSettingPublic, RuntimeSettingsPublic, RuntimeSettingUpdate, Token, UpdatePassword, UserCreate, UserPublic, UserRegister, UserRolesPublic, usersCreateUserData, usersCreateUserError, usersCreateUserErrors, usersCreateUserResponse, usersCreateUserResponses, usersDeleteUserData, usersDeleteUserError, usersDeleteUserErrors, usersDeleteUserMeData, usersDeleteUserMeResponse, usersDeleteUserMeResponses, usersDeleteUserResponse, usersDeleteUserResponses, UsersPublic, usersReadUserByIdData, usersReadUserByIdError, usersReadUserByIdErrors, usersReadUserByIdResponse, usersReadUserByIdResponses, usersReadUserMeData, usersReadUserMePermissionsData, usersReadUserMePermissionsResponse, usersReadUserMePermissionsResponses, usersReadUserMeResponse, usersReadUserMeResponses, usersReadUsersData, usersReadUsersError, usersReadUsersErrors, usersReadUsersResponse, usersReadUsersResponses, usersRegisterUserData, usersRegisterUserError, usersRegisterUserErrors, usersRegisterUserResponse, usersRegisterUserResponses, usersUpdatePasswordMeData, usersUpdatePasswordMeError, usersUpdatePasswordMeErrors, usersUpdatePasswordMeResponse, usersUpdatePasswordMeResponses, usersUpdateUserData, usersUpdateUserError, usersUpdateUserErrors, usersUpdateUserMeData, usersUpdateUserMeError, usersUpdateUserMeErrors, usersUpdateUserMeResponse, usersUpdateUserMeResponses, usersUpdateUserResponse, usersUpdateUserResponses, UserUpdate, UserUpdateMe, utilsHealthCheckData, utilsHealthCheckResponse, utilsHealthCheckResponses, utilsTestEmailData, utilsTestEmailError, utilsTestEmailErrors, utilsTestEmailResponse, utilsTestEmailResponses, ValidationError } from './types.gen'; +export { AdminService, FlagsService, GamesService, HealthService, ItemsService, LeaderboardService, LoginService, type Options, PrivateService, RolesService, RoomsService, UsersService, UtilsService } from './sdk.gen'; +export type { adminCreateOverrideData, adminCreateOverrideError, adminCreateOverrideErrors, adminCreateOverrideResponse, adminCreateOverrideResponses, adminReadFlagsData, adminReadFlagsResponse, adminReadFlagsResponses, adminReadSettingsData, adminReadSettingsResponse, adminReadSettingsResponses, adminRemoveOverrideData, adminRemoveOverrideError, adminRemoveOverrideErrors, adminRemoveOverrideResponse, adminRemoveOverrideResponses, adminUpdateFlagData, adminUpdateFlagError, adminUpdateFlagErrors, adminUpdateFlagResponse, adminUpdateFlagResponses, adminUpdateSettingData, adminUpdateSettingError, adminUpdateSettingErrors, adminUpdateSettingResponse, adminUpdateSettingResponses, Body_login_login_access_token, ChatSend, ClientOptions, FeatureFlagOverrideCreate, FeatureFlagOverridePublic, FeatureFlagPublic, FeatureFlagUpdate, flagsReadMyFlagsData, flagsReadMyFlagsResponse, flagsReadMyFlagsResponses, GameRecordPublic, gamesGameEventsData, gamesGameEventsError, gamesGameEventsErrors, gamesGameEventsResponses, gamesGetGameRecordData, gamesGetGameRecordError, gamesGetGameRecordErrors, gamesGetGameRecordResponse, gamesGetGameRecordResponses, gamesReplayData, gamesReplayError, gamesReplayErrors, gamesReplayResponses, healthHealthData, healthHealthLlmData, healthHealthLlmResponse, healthHealthLlmResponses, healthHealthResponse, healthHealthResponses, HealthReport, HTTPValidationError, ItemCreate, ItemPublic, itemsCreateItemData, itemsCreateItemError, itemsCreateItemErrors, itemsCreateItemResponse, itemsCreateItemResponses, itemsDeleteItemData, itemsDeleteItemError, itemsDeleteItemErrors, itemsDeleteItemResponse, itemsDeleteItemResponses, ItemsPublic, itemsReadItemData, itemsReadItemError, itemsReadItemErrors, itemsReadItemResponse, itemsReadItemResponses, itemsReadItemsData, itemsReadItemsError, itemsReadItemsErrors, itemsReadItemsResponse, itemsReadItemsResponses, itemsUpdateItemData, itemsUpdateItemError, itemsUpdateItemErrors, itemsUpdateItemResponse, itemsUpdateItemResponses, ItemUpdate, leaderboardAccountsLeaderboardData, leaderboardAccountsLeaderboardError, leaderboardAccountsLeaderboardErrors, leaderboardAccountsLeaderboardResponse, leaderboardAccountsLeaderboardResponses, LeaderboardEntry, leaderboardModelsLeaderboardData, leaderboardModelsLeaderboardError, leaderboardModelsLeaderboardErrors, leaderboardModelsLeaderboardResponse, leaderboardModelsLeaderboardResponses, LLMHealth, loginLoginAccessTokenData, loginLoginAccessTokenError, loginLoginAccessTokenErrors, loginLoginAccessTokenResponse, loginLoginAccessTokenResponses, loginRecoverPasswordData, loginRecoverPasswordError, loginRecoverPasswordErrors, loginRecoverPasswordHtmlContentData, loginRecoverPasswordHtmlContentError, loginRecoverPasswordHtmlContentErrors, loginRecoverPasswordHtmlContentResponse, loginRecoverPasswordHtmlContentResponses, loginRecoverPasswordResponse, loginRecoverPasswordResponses, loginResetPasswordData, loginResetPasswordError, loginResetPasswordErrors, loginResetPasswordResponse, loginResetPasswordResponses, loginTestTokenData, loginTestTokenResponse, loginTestTokenResponses, Message, MoveSubmission, NewPassword, PermissionsPublic, privateCreateUserData, privateCreateUserError, privateCreateUserErrors, privateCreateUserResponse, privateCreateUserResponses, PrivateUserCreate, RoleAssignment, RolePublic, rolesGrantUserRoleData, rolesGrantUserRoleError, rolesGrantUserRoleErrors, rolesGrantUserRoleResponse, rolesGrantUserRoleResponses, rolesReadPermissionsData, rolesReadPermissionsResponse, rolesReadPermissionsResponses, rolesReadRolesData, rolesReadRolesResponse, rolesReadRolesResponses, rolesReadUserRolesData, rolesReadUserRolesError, rolesReadUserRolesErrors, rolesReadUserRolesResponse, rolesReadUserRolesResponses, rolesRevokeUserRoleData, rolesRevokeUserRoleError, rolesRevokeUserRoleErrors, rolesRevokeUserRoleResponse, rolesRevokeUserRoleResponses, RoomCreate, RoomJoinRequest, RoomPublic, roomsCreateRoomData, roomsCreateRoomError, roomsCreateRoomErrors, roomsCreateRoomResponse, roomsCreateRoomResponses, RoomSeatJoinPublic, RoomSeatPublic, RoomSeatSpec, roomsForceStartRoomData, roomsForceStartRoomError, roomsForceStartRoomErrors, roomsForceStartRoomResponse, roomsForceStartRoomResponses, roomsGetRoomData, roomsGetRoomError, roomsGetRoomErrors, roomsGetRoomResponse, roomsGetRoomResponses, roomsJoinRoomData, roomsJoinRoomError, roomsJoinRoomErrors, roomsJoinRoomResponse, roomsJoinRoomResponses, roomsLeaveRoomData, roomsLeaveRoomError, roomsLeaveRoomErrors, roomsLeaveRoomResponse, roomsLeaveRoomResponses, roomsListRoomsData, roomsListRoomsError, roomsListRoomsErrors, roomsListRoomsResponse, roomsListRoomsResponses, roomsMovesSchemaData, roomsMovesSchemaError, roomsMovesSchemaErrors, roomsMovesSchemaResponses, RoomsPublic, roomsRoomEventsData, roomsRoomEventsError, roomsRoomEventsErrors, roomsRoomEventsResponses, roomsRoomQueueData, roomsRoomQueueError, roomsRoomQueueErrors, roomsRoomQueueResponses, roomsRoomViewData, roomsRoomViewError, roomsRoomViewErrors, roomsRoomViewResponses, roomsSendChatData, roomsSendChatError, roomsSendChatErrors, roomsSendChatResponse, roomsSendChatResponses, roomsStartRoomData, roomsStartRoomError, roomsStartRoomErrors, roomsStartRoomResponse, roomsStartRoomResponses, roomsStopRoomData, roomsStopRoomError, roomsStopRoomErrors, roomsStopRoomResponse, roomsStopRoomResponses, roomsSubmitMoveData, roomsSubmitMoveError, roomsSubmitMoveErrors, roomsSubmitMoveResponse, roomsSubmitMoveResponses, roomsWsTicketData, roomsWsTicketError, roomsWsTicketErrors, roomsWsTicketResponse, roomsWsTicketResponses, RuntimeSettingPublic, RuntimeSettingsPublic, RuntimeSettingUpdate, Token, UpdatePassword, UserCreate, UserPublic, UserRegister, UserRolesPublic, usersCreateUserData, usersCreateUserError, usersCreateUserErrors, usersCreateUserResponse, usersCreateUserResponses, usersDeleteUserData, usersDeleteUserError, usersDeleteUserErrors, usersDeleteUserMeData, usersDeleteUserMeResponse, usersDeleteUserMeResponses, usersDeleteUserResponse, usersDeleteUserResponses, UsersPublic, usersReadUserByIdData, usersReadUserByIdError, usersReadUserByIdErrors, usersReadUserByIdResponse, usersReadUserByIdResponses, usersReadUserMeData, usersReadUserMePermissionsData, usersReadUserMePermissionsResponse, usersReadUserMePermissionsResponses, usersReadUserMeResponse, usersReadUserMeResponses, usersReadUsersData, usersReadUsersError, usersReadUsersErrors, usersReadUsersResponse, usersReadUsersResponses, usersRegisterUserData, usersRegisterUserError, usersRegisterUserErrors, usersRegisterUserResponse, usersRegisterUserResponses, usersUpdatePasswordMeData, usersUpdatePasswordMeError, usersUpdatePasswordMeErrors, usersUpdatePasswordMeResponse, usersUpdatePasswordMeResponses, usersUpdateUserData, usersUpdateUserError, usersUpdateUserErrors, usersUpdateUserMeData, usersUpdateUserMeError, usersUpdateUserMeErrors, usersUpdateUserMeResponse, usersUpdateUserMeResponses, usersUpdateUserResponse, usersUpdateUserResponses, UserUpdate, UserUpdateMe, utilsHealthCheckData, utilsHealthCheckResponse, utilsHealthCheckResponses, utilsTestEmailData, utilsTestEmailError, utilsTestEmailErrors, utilsTestEmailResponse, utilsTestEmailResponses, ValidationError } from './types.gen'; diff --git a/frontend/src/client/sdk.gen.ts b/frontend/src/client/sdk.gen.ts index faa6e18..745a455 100644 --- a/frontend/src/client/sdk.gen.ts +++ b/frontend/src/client/sdk.gen.ts @@ -2,7 +2,7 @@ import { type Client, type ClientMeta, type Options as Options2, type RequestResult, type TDataShape, urlSearchParamsBodySerializer } from './client'; import { client } from './client.gen'; -import type { adminCreateOverrideData, adminCreateOverrideErrors, adminCreateOverrideResponses, adminReadFlagsData, adminReadFlagsResponses, adminReadSettingsData, adminReadSettingsResponses, adminRemoveOverrideData, adminRemoveOverrideErrors, adminRemoveOverrideResponses, adminUpdateFlagData, adminUpdateFlagErrors, adminUpdateFlagResponses, adminUpdateSettingData, adminUpdateSettingErrors, adminUpdateSettingResponses, flagsReadMyFlagsData, flagsReadMyFlagsResponses, gamesGameEventsData, gamesGameEventsErrors, gamesGameEventsResponses, gamesGetGameRecordData, gamesGetGameRecordErrors, gamesGetGameRecordResponses, gamesReplayData, gamesReplayErrors, gamesReplayResponses, healthHealthData, healthHealthLlmData, healthHealthLlmResponses, healthHealthResponses, itemsCreateItemData, itemsCreateItemErrors, itemsCreateItemResponses, itemsDeleteItemData, itemsDeleteItemErrors, itemsDeleteItemResponses, itemsReadItemData, itemsReadItemErrors, itemsReadItemResponses, itemsReadItemsData, itemsReadItemsErrors, itemsReadItemsResponses, itemsUpdateItemData, itemsUpdateItemErrors, itemsUpdateItemResponses, loginLoginAccessTokenData, loginLoginAccessTokenErrors, loginLoginAccessTokenResponses, loginRecoverPasswordData, loginRecoverPasswordErrors, loginRecoverPasswordHtmlContentData, loginRecoverPasswordHtmlContentErrors, loginRecoverPasswordHtmlContentResponses, loginRecoverPasswordResponses, loginResetPasswordData, loginResetPasswordErrors, loginResetPasswordResponses, loginTestTokenData, loginTestTokenResponses, privateCreateUserData, privateCreateUserErrors, privateCreateUserResponses, rolesGrantUserRoleData, rolesGrantUserRoleErrors, rolesGrantUserRoleResponses, rolesReadPermissionsData, rolesReadPermissionsResponses, rolesReadRolesData, rolesReadRolesResponses, rolesReadUserRolesData, rolesReadUserRolesErrors, rolesReadUserRolesResponses, rolesRevokeUserRoleData, rolesRevokeUserRoleErrors, rolesRevokeUserRoleResponses, roomsCreateRoomData, roomsCreateRoomErrors, roomsCreateRoomResponses, roomsForceStartRoomData, roomsForceStartRoomErrors, roomsForceStartRoomResponses, roomsGetRoomData, roomsGetRoomErrors, roomsGetRoomResponses, roomsJoinRoomData, roomsJoinRoomErrors, roomsJoinRoomResponses, roomsLeaveRoomData, roomsLeaveRoomErrors, roomsLeaveRoomResponses, roomsListRoomsData, roomsListRoomsErrors, roomsListRoomsResponses, roomsMovesSchemaData, roomsMovesSchemaErrors, roomsMovesSchemaResponses, roomsRoomEventsData, roomsRoomEventsErrors, roomsRoomEventsResponses, roomsRoomQueueData, roomsRoomQueueErrors, roomsRoomQueueResponses, roomsRoomViewData, roomsRoomViewErrors, roomsRoomViewResponses, roomsSendChatData, roomsSendChatErrors, roomsSendChatResponses, roomsStartRoomData, roomsStartRoomErrors, roomsStartRoomResponses, roomsStopRoomData, roomsStopRoomErrors, roomsStopRoomResponses, roomsSubmitMoveData, roomsSubmitMoveErrors, roomsSubmitMoveResponses, roomsWsTicketData, roomsWsTicketErrors, roomsWsTicketResponses, usersCreateUserData, usersCreateUserErrors, usersCreateUserResponses, usersDeleteUserData, usersDeleteUserErrors, usersDeleteUserMeData, usersDeleteUserMeResponses, usersDeleteUserResponses, usersReadUserByIdData, usersReadUserByIdErrors, usersReadUserByIdResponses, usersReadUserMeData, usersReadUserMePermissionsData, usersReadUserMePermissionsResponses, usersReadUserMeResponses, usersReadUsersData, usersReadUsersErrors, usersReadUsersResponses, usersRegisterUserData, usersRegisterUserErrors, usersRegisterUserResponses, usersUpdatePasswordMeData, usersUpdatePasswordMeErrors, usersUpdatePasswordMeResponses, usersUpdateUserData, usersUpdateUserErrors, usersUpdateUserMeData, usersUpdateUserMeErrors, usersUpdateUserMeResponses, usersUpdateUserResponses, utilsHealthCheckData, utilsHealthCheckResponses, utilsTestEmailData, utilsTestEmailErrors, utilsTestEmailResponses } from './types.gen'; +import type { adminCreateOverrideData, adminCreateOverrideErrors, adminCreateOverrideResponses, adminReadFlagsData, adminReadFlagsResponses, adminReadSettingsData, adminReadSettingsResponses, adminRemoveOverrideData, adminRemoveOverrideErrors, adminRemoveOverrideResponses, adminUpdateFlagData, adminUpdateFlagErrors, adminUpdateFlagResponses, adminUpdateSettingData, adminUpdateSettingErrors, adminUpdateSettingResponses, flagsReadMyFlagsData, flagsReadMyFlagsResponses, gamesGameEventsData, gamesGameEventsErrors, gamesGameEventsResponses, gamesGetGameRecordData, gamesGetGameRecordErrors, gamesGetGameRecordResponses, gamesReplayData, gamesReplayErrors, gamesReplayResponses, healthHealthData, healthHealthLlmData, healthHealthLlmResponses, healthHealthResponses, itemsCreateItemData, itemsCreateItemErrors, itemsCreateItemResponses, itemsDeleteItemData, itemsDeleteItemErrors, itemsDeleteItemResponses, itemsReadItemData, itemsReadItemErrors, itemsReadItemResponses, itemsReadItemsData, itemsReadItemsErrors, itemsReadItemsResponses, itemsUpdateItemData, itemsUpdateItemErrors, itemsUpdateItemResponses, leaderboardAccountsLeaderboardData, leaderboardAccountsLeaderboardErrors, leaderboardAccountsLeaderboardResponses, leaderboardModelsLeaderboardData, leaderboardModelsLeaderboardErrors, leaderboardModelsLeaderboardResponses, loginLoginAccessTokenData, loginLoginAccessTokenErrors, loginLoginAccessTokenResponses, loginRecoverPasswordData, loginRecoverPasswordErrors, loginRecoverPasswordHtmlContentData, loginRecoverPasswordHtmlContentErrors, loginRecoverPasswordHtmlContentResponses, loginRecoverPasswordResponses, loginResetPasswordData, loginResetPasswordErrors, loginResetPasswordResponses, loginTestTokenData, loginTestTokenResponses, privateCreateUserData, privateCreateUserErrors, privateCreateUserResponses, rolesGrantUserRoleData, rolesGrantUserRoleErrors, rolesGrantUserRoleResponses, rolesReadPermissionsData, rolesReadPermissionsResponses, rolesReadRolesData, rolesReadRolesResponses, rolesReadUserRolesData, rolesReadUserRolesErrors, rolesReadUserRolesResponses, rolesRevokeUserRoleData, rolesRevokeUserRoleErrors, rolesRevokeUserRoleResponses, roomsCreateRoomData, roomsCreateRoomErrors, roomsCreateRoomResponses, roomsForceStartRoomData, roomsForceStartRoomErrors, roomsForceStartRoomResponses, roomsGetRoomData, roomsGetRoomErrors, roomsGetRoomResponses, roomsJoinRoomData, roomsJoinRoomErrors, roomsJoinRoomResponses, roomsLeaveRoomData, roomsLeaveRoomErrors, roomsLeaveRoomResponses, roomsListRoomsData, roomsListRoomsErrors, roomsListRoomsResponses, roomsMovesSchemaData, roomsMovesSchemaErrors, roomsMovesSchemaResponses, roomsRoomEventsData, roomsRoomEventsErrors, roomsRoomEventsResponses, roomsRoomQueueData, roomsRoomQueueErrors, roomsRoomQueueResponses, roomsRoomViewData, roomsRoomViewErrors, roomsRoomViewResponses, roomsSendChatData, roomsSendChatErrors, roomsSendChatResponses, roomsStartRoomData, roomsStartRoomErrors, roomsStartRoomResponses, roomsStopRoomData, roomsStopRoomErrors, roomsStopRoomResponses, roomsSubmitMoveData, roomsSubmitMoveErrors, roomsSubmitMoveResponses, roomsWsTicketData, roomsWsTicketErrors, roomsWsTicketResponses, usersCreateUserData, usersCreateUserErrors, usersCreateUserResponses, usersDeleteUserData, usersDeleteUserErrors, usersDeleteUserMeData, usersDeleteUserMeResponses, usersDeleteUserResponses, usersReadUserByIdData, usersReadUserByIdErrors, usersReadUserByIdResponses, usersReadUserMeData, usersReadUserMePermissionsData, usersReadUserMePermissionsResponses, usersReadUserMeResponses, usersReadUsersData, usersReadUsersErrors, usersReadUsersResponses, usersRegisterUserData, usersRegisterUserErrors, usersRegisterUserResponses, usersUpdatePasswordMeData, usersUpdatePasswordMeErrors, usersUpdatePasswordMeResponses, usersUpdateUserData, usersUpdateUserErrors, usersUpdateUserMeData, usersUpdateUserMeErrors, usersUpdateUserMeResponses, usersUpdateUserResponses, utilsHealthCheckData, utilsHealthCheckResponses, utilsTestEmailData, utilsTestEmailErrors, utilsTestEmailResponses } from './types.gen'; export type Options = Options2 & { /** @@ -857,6 +857,32 @@ export class GamesService { } } +export class LeaderboardService { + /** + * Accounts Leaderboard + */ + public static accountsLeaderboard(options?: Options): RequestResult { + return (options?.client ?? client).get({ + responseType: 'json', + security: [{ scheme: 'bearer', type: 'http' }], + url: '/api/v1/leaderboard/accounts', + ...options + }); + } + + /** + * Models Leaderboard + */ + public static modelsLeaderboard(options?: Options): RequestResult { + return (options?.client ?? client).get({ + responseType: 'json', + security: [{ scheme: 'bearer', type: 'http' }], + url: '/api/v1/leaderboard/models', + ...options + }); + } +} + export class PrivateService { /** * Create User diff --git a/frontend/src/client/types.gen.ts b/frontend/src/client/types.gen.ts index b4ec48b..db45ed4 100644 --- a/frontend/src/client/types.gen.ts +++ b/frontend/src/client/types.gen.ts @@ -320,6 +320,37 @@ export type LLMHealth = { checked_at: string; }; +/** + * LeaderboardEntry + * + * One row of an aggregate leaderboard (Phase 12, DECISIONS.md §11): + * either a human account (``subject_id`` a user id) or a model + * (``subject_id`` the model name), never both -- ``app.leaderboard`` never + * mixes the two in one query. + */ +export type LeaderboardEntry = { + /** + * Subject Id + */ + subject_id: string; + /** + * Display Name + */ + display_name: string; + /** + * Games Played + */ + games_played: number; + /** + * Wins + */ + wins: number; + /** + * Win Rate + */ + win_rate: number; +}; + /** * Message */ @@ -2378,6 +2409,70 @@ export type gamesReplayResponses = { 200: unknown; }; +export type leaderboardAccountsLeaderboardData = { + body?: never; + path?: never; + query?: { + /** + * Game + */ + game?: string | null; + }; + url: '/api/v1/leaderboard/accounts'; +}; + +export type leaderboardAccountsLeaderboardErrors = { + /** + * Validation Error + */ + 422: HTTPValidationError; +}; + +export type leaderboardAccountsLeaderboardError = leaderboardAccountsLeaderboardErrors[keyof leaderboardAccountsLeaderboardErrors]; + +export type leaderboardAccountsLeaderboardResponses = { + /** + * Response Leaderboard-Accounts Leaderboard + * + * Successful Response + */ + 200: Array; +}; + +export type leaderboardAccountsLeaderboardResponse = leaderboardAccountsLeaderboardResponses[keyof leaderboardAccountsLeaderboardResponses]; + +export type leaderboardModelsLeaderboardData = { + body?: never; + path?: never; + query?: { + /** + * Game + */ + game?: string | null; + }; + url: '/api/v1/leaderboard/models'; +}; + +export type leaderboardModelsLeaderboardErrors = { + /** + * Validation Error + */ + 422: HTTPValidationError; +}; + +export type leaderboardModelsLeaderboardError = leaderboardModelsLeaderboardErrors[keyof leaderboardModelsLeaderboardErrors]; + +export type leaderboardModelsLeaderboardResponses = { + /** + * Response Leaderboard-Models Leaderboard + * + * Successful Response + */ + 200: Array; +}; + +export type leaderboardModelsLeaderboardResponse = leaderboardModelsLeaderboardResponses[keyof leaderboardModelsLeaderboardResponses]; + export type privateCreateUserData = { body: PrivateUserCreate; path?: never; diff --git a/frontend/src/components/Leaderboard/columns.tsx b/frontend/src/components/Leaderboard/columns.tsx new file mode 100644 index 0000000..cc6c3dd --- /dev/null +++ b/frontend/src/components/Leaderboard/columns.tsx @@ -0,0 +1,34 @@ +import type { ColumnDef } from "@tanstack/react-table" + +import type { LeaderboardEntry } from "@/client" +import type { features } from "@/components/Common/DataTable" + +export const columns: ColumnDef[] = [ + { + id: "rank", + header: "#", + cell: ({ row }) => ( + {row.index + 1} + ), + }, + { + accessorKey: "display_name", + header: "Name", + cell: ({ row }) => ( + {row.original.display_name} + ), + }, + { + accessorKey: "games_played", + header: "Games", + }, + { + accessorKey: "wins", + header: "Wins", + }, + { + accessorKey: "win_rate", + header: "Win rate", + cell: ({ row }) => `${Math.round(row.original.win_rate * 100)}%`, + }, +] diff --git a/frontend/src/components/Rooms/CreateRoomDialog.tsx b/frontend/src/components/Rooms/CreateRoomDialog.tsx index 0575ff5..93e7c17 100644 --- a/frontend/src/components/Rooms/CreateRoomDialog.tsx +++ b/frontend/src/components/Rooms/CreateRoomDialog.tsx @@ -25,25 +25,9 @@ import { } from "@/components/ui/select" import useAuth from "@/hooks/useAuth" import useCustomToast from "@/hooks/useCustomToast" +import { GAMES } from "@/lib/games" import { handleError } from "@/utils" -/** Not exposed by any API -- these four slugs are this project's whole - * game catalog (backend/app/games/__init__.py's GAME_CATALOG), stable - * enough to hardcode rather than add a catalog endpoint nothing else - * needs yet. Seat bounds come straight from each game's own - * min_seats/max_seats. */ -const GAMES = [ - { slug: "uno_classic", title: "Classic UNO", minSeats: 2, maxSeats: 10 }, - { slug: "uno_flip", title: "UNO Flip!", minSeats: 2, maxSeats: 10 }, - { - slug: "uno_no_mercy", - title: "UNO Show 'em No Mercy", - minSeats: 2, - maxSeats: 10, - }, - { slug: "hearts", title: "Hearts", minSeats: 3, maxSeats: 6 }, -] as const - /** The persona registry (backend/app/agents/persona.py) is application * code, not environment config -- stable enough to hardcode, unlike * available LLM models (LLM_MODELS is per-deployment, so an AI seat's diff --git a/frontend/src/components/Sidebar/AppSidebar.tsx b/frontend/src/components/Sidebar/AppSidebar.tsx index 302db7e..87f0f4a 100644 --- a/frontend/src/components/Sidebar/AppSidebar.tsx +++ b/frontend/src/components/Sidebar/AppSidebar.tsx @@ -1,4 +1,4 @@ -import { Home, Users } from "lucide-react" +import { Home, Trophy, Users } from "lucide-react" import { SidebarAppearance } from "@/components/Common/Appearance" import { Logo } from "@/components/Common/Logo" @@ -12,7 +12,10 @@ import useAuth from "@/hooks/useAuth" import { type Item, Main } from "./Main" import { User } from "./User" -const baseItems: Item[] = [{ icon: Home, title: "Lobby", path: "/" }] +const baseItems: Item[] = [ + { icon: Home, title: "Lobby", path: "/" }, + { icon: Trophy, title: "Leaderboard", path: "/leaderboard" }, +] export function AppSidebar() { const { user: currentUser } = useAuth() diff --git a/frontend/src/lib/games.ts b/frontend/src/lib/games.ts new file mode 100644 index 0000000..3cde3b1 --- /dev/null +++ b/frontend/src/lib/games.ts @@ -0,0 +1,17 @@ +/** Not exposed by any API -- these four slugs are this project's whole + * game catalog (backend/app/games/__init__.py's GAME_CATALOG), stable + * enough to hardcode rather than add a catalog endpoint nothing else + * needs yet. Seat bounds come straight from each game's own + * min_seats/max_seats. Shared by `CreateRoomDialog` (room creation) and + * the leaderboard's game filter. */ +export const GAMES = [ + { slug: "uno_classic", title: "Classic UNO", minSeats: 2, maxSeats: 10 }, + { slug: "uno_flip", title: "UNO Flip!", minSeats: 2, maxSeats: 10 }, + { + slug: "uno_no_mercy", + title: "UNO Show 'em No Mercy", + minSeats: 2, + maxSeats: 10, + }, + { slug: "hearts", title: "Hearts", minSeats: 3, maxSeats: 6 }, +] as const diff --git a/frontend/src/routeTree.gen.ts b/frontend/src/routeTree.gen.ts index 054c0f8..7e9f5c6 100644 --- a/frontend/src/routeTree.gen.ts +++ b/frontend/src/routeTree.gen.ts @@ -16,6 +16,7 @@ import { Route as ResetPasswordRouteImport } from './routes/reset-password' import { Route as SignupRouteImport } from './routes/signup' import { Route as LayoutIndexRouteImport } from './routes/_layout/index' import { Route as LayoutAdminRouteImport } from './routes/_layout/admin' +import { Route as LayoutLeaderboardRouteImport } from './routes/_layout/leaderboard' import { Route as LayoutSettingsRouteImport } from './routes/_layout/settings' import { Route as LayoutRoomsRoomIdRouteImport } from './routes/_layout/rooms.$roomId' @@ -53,6 +54,11 @@ const LayoutAdminRoute = LayoutAdminRouteImport.update({ path: '/admin', getParentRoute: () => LayoutRoute, } as any) +const LayoutLeaderboardRoute = LayoutLeaderboardRouteImport.update({ + id: '/leaderboard', + path: '/leaderboard', + getParentRoute: () => LayoutRoute, +} as any) const LayoutSettingsRoute = LayoutSettingsRouteImport.update({ id: '/settings', path: '/settings', @@ -71,6 +77,7 @@ export interface FileRoutesByFullPath { '/reset-password': typeof ResetPasswordRoute '/signup': typeof SignupRoute '/admin': typeof LayoutAdminRoute + '/leaderboard': typeof LayoutLeaderboardRoute '/settings': typeof LayoutSettingsRoute '/rooms/$roomId': typeof LayoutRoomsRoomIdRoute } @@ -80,6 +87,7 @@ export interface FileRoutesByTo { '/reset-password': typeof ResetPasswordRoute '/signup': typeof SignupRoute '/admin': typeof LayoutAdminRoute + '/leaderboard': typeof LayoutLeaderboardRoute '/settings': typeof LayoutSettingsRoute '/': typeof LayoutIndexRoute '/rooms/$roomId': typeof LayoutRoomsRoomIdRoute @@ -92,6 +100,7 @@ export interface FileRoutesById { '/reset-password': typeof ResetPasswordRoute '/signup': typeof SignupRoute '/_layout/admin': typeof LayoutAdminRoute + '/_layout/leaderboard': typeof LayoutLeaderboardRoute '/_layout/settings': typeof LayoutSettingsRoute '/_layout/': typeof LayoutIndexRoute '/_layout/rooms/$roomId': typeof LayoutRoomsRoomIdRoute @@ -105,6 +114,7 @@ export interface FileRouteTypes { | '/reset-password' | '/signup' | '/admin' + | '/leaderboard' | '/settings' | '/rooms/$roomId' fileRoutesByTo: FileRoutesByTo @@ -114,6 +124,7 @@ export interface FileRouteTypes { | '/reset-password' | '/signup' | '/admin' + | '/leaderboard' | '/settings' | '/' | '/rooms/$roomId' @@ -125,6 +136,7 @@ export interface FileRouteTypes { | '/reset-password' | '/signup' | '/_layout/admin' + | '/_layout/leaderboard' | '/_layout/settings' | '/_layout/' | '/_layout/rooms/$roomId' @@ -189,6 +201,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof LayoutAdminRouteImport parentRoute: typeof LayoutRoute } + '/_layout/leaderboard': { + id: '/_layout/leaderboard' + path: '/leaderboard' + fullPath: '/leaderboard' + preLoaderRoute: typeof LayoutLeaderboardRouteImport + parentRoute: typeof LayoutRoute + } '/_layout/settings': { id: '/_layout/settings' path: '/settings' @@ -208,6 +227,7 @@ declare module '@tanstack/react-router' { interface LayoutRouteChildren { LayoutAdminRoute: typeof LayoutAdminRoute + LayoutLeaderboardRoute: typeof LayoutLeaderboardRoute LayoutSettingsRoute: typeof LayoutSettingsRoute LayoutIndexRoute: typeof LayoutIndexRoute LayoutRoomsRoomIdRoute: typeof LayoutRoomsRoomIdRoute @@ -215,6 +235,7 @@ interface LayoutRouteChildren { const LayoutRouteChildren: LayoutRouteChildren = { LayoutAdminRoute: LayoutAdminRoute, + LayoutLeaderboardRoute: LayoutLeaderboardRoute, LayoutSettingsRoute: LayoutSettingsRoute, LayoutIndexRoute: LayoutIndexRoute, LayoutRoomsRoomIdRoute: LayoutRoomsRoomIdRoute, diff --git a/frontend/src/routes/_layout/leaderboard.tsx b/frontend/src/routes/_layout/leaderboard.tsx new file mode 100644 index 0000000..8f26084 --- /dev/null +++ b/frontend/src/routes/_layout/leaderboard.tsx @@ -0,0 +1,92 @@ +import { useQuery } from "@tanstack/react-query" +import { createFileRoute } from "@tanstack/react-router" +import { useState } from "react" + +import { LeaderboardService } from "@/client" +import { DataTable } from "@/components/Common/DataTable" +import { columns } from "@/components/Leaderboard/columns" +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select" +import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs" +import { GAMES } from "@/lib/games" + +export const Route = createFileRoute("/_layout/leaderboard")({ + component: Leaderboard, +}) + +const ALL_GAMES = "all" + +function Leaderboard() { + const [game, setGame] = useState(ALL_GAMES) + const gameFilter = game === ALL_GAMES ? undefined : game + + const accounts = useQuery({ + queryKey: ["leaderboard", "accounts", gameFilter], + queryFn: async () => + ( + await LeaderboardService.accountsLeaderboard({ + query: { game: gameFilter }, + }) + ).data, + }) + const models = useQuery({ + queryKey: ["leaderboard", "models", gameFilter], + queryFn: async () => + ( + await LeaderboardService.modelsLeaderboard({ + query: { game: gameFilter }, + }) + ).data, + }) + + return ( +
+
+

Leaderboard

+ +
+ + + + Accounts + Models + + + {accounts.data && accounts.data.length > 0 ? ( + + ) : ( +

+ No finished games yet. +

+ )} + + + {models.data && models.data.length > 0 ? ( + + ) : ( +

+ No finished games yet. +

+ )} + + +
+ ) +} diff --git a/frontend/tests/leaderboard.spec.ts b/frontend/tests/leaderboard.spec.ts new file mode 100644 index 0000000..c9f67ee --- /dev/null +++ b/frontend/tests/leaderboard.spec.ts @@ -0,0 +1,23 @@ +import { expect, test } from "@playwright/test" + +test("Leaderboard page shows both tabs, a game filter, and switches tabs", async ({ + page, +}) => { + await page.goto("/leaderboard") + await expect(page.getByRole("heading", { name: "Leaderboard" })).toBeVisible() + await expect(page.getByRole("tab", { name: "Accounts" })).toBeVisible() + await expect(page.getByRole("tab", { name: "Models" })).toBeVisible() + + await page.getByRole("tab", { name: "Models" }).click() + await expect(page.getByRole("tab", { name: "Models" })).toHaveAttribute( + "aria-selected", + "true", + ) +}) + +test("Leaderboard is reachable from the sidebar", async ({ page }) => { + await page.goto("/") + await page.getByRole("link", { name: "Leaderboard" }).click() + await expect(page).toHaveURL(/\/leaderboard$/) + await expect(page.getByRole("heading", { name: "Leaderboard" })).toBeVisible() +}) From 01e57e9f84c2982216451a5a769c99f911853696 Mon Sep 17 00:00:00 2001 From: withrvr Date: Mon, 14 Sep 2026 13:35:53 +0000 Subject: [PATCH 2/3] fix: leaderboard-permission migration collides with the live rbac seed on a fresh DB a1b2c3d4e5f6 (Phase 6's seed migration) imports ALL_PERMISSIONS/ DEFAULT_ROLES live from permissions.py, so on any fresh database it already seeds leaderboard:view itself before 30c74160ca50 runs -- a plain bulk_insert then collided on the unique constraint (real CI failure on PR #22, never reproduced locally since the dev DB predates this permission). Switched both inserts to ON CONFLICT DO NOTHING; verified against two throwaway Postgres containers covering both a fresh DB and one migrated before this permission existed. Claude-Session: https://claude.ai/code/session_01UbiPa1rKCyqhqufgwJWkeW --- PROGRESS.md | 24 +++++++++++ .../30c74160ca50_leaderboard_permission.py | 43 +++++++++++++------ backend/app/leaderboard.py | 4 +- 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index e7cdea3..4ca13c9 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -1035,6 +1035,30 @@ Full gate: backend suite (779 tests) green, `ty`/`mypy`/`ruff` clean, `bun run build`/`bun run lint` clean, full Playwright suite (59 tests) green against the real docker compose stack, `prek run --all-files` clean. +### CI caught a real migration bug before merge (PR #22) + +`test-backend`/`pre-commit` failed on a fresh CI database: +`UniqueViolation` on `permission_pkey` for `leaderboard:view`. Root cause: +`a1b2c3d4e5f6_rbac.py` (the original Phase 6 seed migration) imports +`ALL_PERMISSIONS`/`DEFAULT_ROLES` *live* from `app/authz/permissions.py` +and re-evaluates them every time it runs -- so on any database that has +never run a migration before (a fresh CI run, a new deployment), that +older migration runs first and already sees `leaderboard:view` in the +*current* module state, seeding it itself. The new migration +(`30c74160ca50`) then tried to insert the identical row again with a plain +`op.bulk_insert` and collided on the unique constraint. This never +reproduced locally because the local dev DB had already applied +`a1b2c3d4e5f6` long before `leaderboard:view` existed -- the exact +scenario the new migration was written for, not the fresh-database one CI +actually hit. Fixed by switching both inserts to +`postgresql.insert(...).on_conflict_do_nothing(...)`, correct either way: +a no-op if the old migration already seeded it (fresh DB), a real insert +if it didn't (an existing DB migrated before this permission existed). +Verified both paths directly against two throwaway Postgres containers +(one migrated clean from scratch; one migrated to the pre-Phase-12 head +with `permissions.py` temporarily reverted, then to the new head with it +restored) rather than trusting the reasoning alone. + ## Phase 9.7 — done 2026-09-14 (branch phase-9.7-infra-optimization) Infrastructure-only pass: Docker image build, Compose, CI/CD workflows, diff --git a/backend/app/alembic/versions/30c74160ca50_leaderboard_permission.py b/backend/app/alembic/versions/30c74160ca50_leaderboard_permission.py index 13e5200..52bf581 100644 --- a/backend/app/alembic/versions/30c74160ca50_leaderboard_permission.py +++ b/backend/app/alembic/versions/30c74160ca50_leaderboard_permission.py @@ -7,6 +7,7 @@ """ from alembic import op import sqlalchemy as sa +from sqlalchemy.dialects import postgresql from app.authz.permissions import permission_id, role_id @@ -18,10 +19,24 @@ # Deliberately literal, not `app.authz.permissions.P.LEADERBOARD_VIEW` / # `ALL_PERMISSIONS` / `DEFAULT_ROLES` -- those constants keep growing as -# later phases add permissions, so re-deriving from them here would re-seed -# rows the *original* rbac migration (a1b2c3d4e5f6) already inserted and -# collide on the unique `name` constraint. A new permission gets a new, -# self-contained migration naming exactly what it adds. +# later phases add permissions, and a new permission gets a new, +# self-contained migration naming exactly what it adds, rather than +# re-deriving from module state that will look different by the time some +# future phase's migration runs. +# +# `ON CONFLICT DO NOTHING`, not a plain insert: the *original* rbac +# migration (a1b2c3d4e5f6) imports `ALL_PERMISSIONS`/`DEFAULT_ROLES` +# *live* from `app.authz.permissions` and re-evaluates them every time it +# runs -- so on any database that has never run any migration yet (a fresh +# CI run, a new deployment), that older migration runs first and already +# sees `leaderboard:view` in the current `ALL_PERMISSIONS`/`DEFAULT_ROLES`, +# seeding it itself. This migration exists for the *other* case: a +# database that already applied a1b2c3d4e5f6 before this permission +# existed, where nothing will ever seed it otherwise. A plain +# `op.bulk_insert` broke exactly the first case (a real CI failure caught +# on this PR: `UniqueViolation` on `permission_pkey`, since a1b2c3d4e5f6 +# had already inserted the identical row moments earlier in the same +# fresh-database migration run). _PERMISSION = "leaderboard:view" _GRANTED_TO = ("admin", "player", "spectator") @@ -36,17 +51,17 @@ def upgrade(): role_permission = sa.table( "role_permission", sa.column("role_id"), sa.column("permission_id") ) - op.bulk_insert( - permission, - [{"id": permission_id(_PERMISSION), "name": _PERMISSION, "description": ""}], - ) - op.bulk_insert( - role_permission, - [ - {"role_id": role_id(name), "permission_id": permission_id(_PERMISSION)} - for name in _GRANTED_TO - ], + op.execute( + postgresql.insert(permission) + .values(id=permission_id(_PERMISSION), name=_PERMISSION, description="") + .on_conflict_do_nothing(index_elements=["id"]) ) + for name in _GRANTED_TO: + op.execute( + postgresql.insert(role_permission) + .values(role_id=role_id(name), permission_id=permission_id(_PERMISSION)) + .on_conflict_do_nothing(index_elements=["role_id", "permission_id"]) + ) def downgrade(): diff --git a/backend/app/leaderboard.py b/backend/app/leaderboard.py index ab0424b..f6cf694 100644 --- a/backend/app/leaderboard.py +++ b/backend/app/leaderboard.py @@ -28,7 +28,9 @@ def _wins() -> Any: - return func.sum(case((col(GameRecord.winner_seat) == col(RoomSeat.seat), 1), else_=0)) + return func.sum( + case((col(GameRecord.winner_seat) == col(RoomSeat.seat), 1), else_=0) + ) def leaderboard_by_account( From 45b9b218fca79da7ceeab632dd70048aa16f49eb Mon Sep 17 00:00:00 2001 From: withrvr Date: Mon, 14 Sep 2026 13:44:41 +0000 Subject: [PATCH 3/3] fix: address python-reviewer/react-reviewer findings on PR #22 - leaderboard.tsx: add aria-label to the game filter, distinguish loading/error from a genuine empty result (shared LeaderboardTable) - leaderboard.py: correct docstring overclaiming finished games always have a winner_seat - persistence.md: document the RBAC-seed-migration trap this PR hit, so future phases don't rediscover it Claude-Session: https://claude.ai/code/session_01UbiPa1rKCyqhqufgwJWkeW --- PROGRESS.md | 42 ++++++++++++++++ backend/app/leaderboard.py | 11 ++-- docs/persistence.md | 20 ++++++++ frontend/src/routes/_layout/leaderboard.tsx | 56 +++++++++++++++------ 4 files changed, 110 insertions(+), 19 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index 4ca13c9..0e135db 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -1059,6 +1059,48 @@ Verified both paths directly against two throwaway Postgres containers with `permissions.py` temporarily reverted, then to the new head with it restored) rather than trusting the reasoning alone. +### Review fixes (PR #22) + +`python-reviewer` and `react-reviewer` each reviewed the PR (plus an +extra general-purpose review dispatched by mistake on the first attempt -- +kept its findings anyway since it independently re-verified the migration +fix). Real, fixed: + +- **Game-filter `Select` had no accessible name** (react-reviewer, HIGH): + every other `Select` in this codebase pairs a `Label htmlFor`/`id` or an + explicit `aria-label` -- this was the only one with neither. Added + `aria-label="Filter by game"`. +- **Loading/error state indistinguishable from "no games yet"** + (react-reviewer, HIGH): neither query checked `isLoading`/`isError`, so + every page load flashed a false empty state, and a real failure (a + permission regression, a 500) would render identically to "no games" + forever, hiding it. Factored a small shared `LeaderboardTable` + component (both tabs have the identical `LeaderboardEntry` shape) that + branches on loading/error/empty/data. +- **Docstring overstated the finished/winner_seat relationship** + (general-purpose reviewer, doc nit): claimed only a `"stopped"` game has + `winner_seat == None`, but `Referee.run()` + (`winner = ranking[0] if hands[ranking[0]] == () else None`) can produce + that for a `"finished"` game too. Not a code bug -- `leaderboard.py` + already handles it correctly (counts toward `games_played`, credits no + one a win) -- just a comment that overclaimed. Corrected the module + docstring. +- **Cheap insurance for future phases** (general-purpose reviewer's + suggestion): added a "Adding a permission after `a1b2c3d4e5f6`" section + to `docs/persistence.md` naming the exact trap this PR's own migration + hit, plus the missing `30c74160ca50` row in that doc's migration-chain + table. + +**Not fixed, deliberately**: react-reviewer's MEDIUM suggestion to gate +the inactive tab's query behind `enabled` so only one of +accounts/models fetches at a time. Left both fetching unconditionally -- +the aggregate query is cheap, and prefetching both means switching tabs is +instant rather than showing a spinner; left a one-line comment stating +this is deliberate rather than an oversight. Also not fixed: pagination on +the two list routes (python-reviewer, MEDIUM) -- already documented as a +found-but-deferred gap in `docs/leaderboard.md`'s "Left for later" section +before either review even ran. + ## Phase 9.7 — done 2026-09-14 (branch phase-9.7-infra-optimization) Infrastructure-only pass: Docker image build, Compose, CI/CD workflows, diff --git a/backend/app/leaderboard.py b/backend/app/leaderboard.py index f6cf694..4ba78bc 100644 --- a/backend/app/leaderboard.py +++ b/backend/app/leaderboard.py @@ -12,9 +12,14 @@ ``"waiting"``). Only ``outcome == "finished"`` games count: a ``"stopped"`` game's -``winner_seat`` is ``None`` (docs/games.md), so it has no result to credit -a win or loss for. It still happened, but "games played" here means "games -that produced a result," matching what a leaderboard is for. +``winner_seat`` is always ``None`` (docs/games.md), so it has no result to +credit a win or loss for. A *finished* game's ``winner_seat`` can be +``None`` too (``Referee.run()`` only credits a winner when the top-ranked +seat's hand is empty, so a game whose ranking is score-based rather than +hand-emptying can finish with no seat meeting that check) -- such a game +still counts toward ``games_played`` for every seat in it, just credits no +one a win, which is the right leaderboard behavior for "played, no +determinable winner" rather than a bug to work around here. """ from __future__ import annotations diff --git a/docs/persistence.md b/docs/persistence.md index 5c2f150..17f4f44 100644 --- a/docs/persistence.md +++ b/docs/persistence.md @@ -67,6 +67,26 @@ Chronological, oldest first (template migrations, then botpit's own): | `a1b2c3d4e5f6` | RBAC: `role`, `permission`, `role_permission`, `user_role`, `room_role` (Phase 6, `docs/roles-and-permissions.md`) | | `ce9cf9ad654e` | rooms: `room`, `room_seat`, `runtime_setting`, `feature_flag`, `feature_flag_override`, plus the deferred `room_role.room_id` FK (Phase 7, `docs/rooms.md`) | | `f54e8ebd4583` | game history: `game_record`, `game_event` (Phase 9, `docs/games.md`) | +| `30c74160ca50` | `leaderboard:view` permission + role grants (Phase 12, `docs/leaderboard.md`) | + +### Adding a permission after `a1b2c3d4e5f6` + +`a1b2c3d4e5f6` (the RBAC seed migration above) imports +`ALL_PERMISSIONS`/`DEFAULT_ROLES` **live** from `app/authz/permissions.py` +and re-evaluates them every time it runs -- not a frozen snapshot of what +existed when it was written. That means on any database that has never +run a migration before (a fresh CI run, a new deployment), *that* older +migration seeds whatever permission set is current at the time it runs -- +including a permission added by a much later phase. A later migration +that plain-inserts the same permission then collides with a row +`a1b2c3d4e5f6` already seeded. `30c74160ca50` hit exactly this (a real CI +failure on PR #22, never reproduced against the local dev DB because that +DB had already applied `a1b2c3d4e5f6` long before the new permission +existed). Any future migration that adds a permission needs +`postgresql.insert(...).on_conflict_do_nothing(...)` instead of +`op.bulk_insert`, correct on both a fresh database (a no-op, since +`a1b2c3d4e5f6` already seeded it) and an existing one that predates the +new permission (a real insert). ## How tests migrate diff --git a/frontend/src/routes/_layout/leaderboard.tsx b/frontend/src/routes/_layout/leaderboard.tsx index 8f26084..b623efc 100644 --- a/frontend/src/routes/_layout/leaderboard.tsx +++ b/frontend/src/routes/_layout/leaderboard.tsx @@ -1,7 +1,8 @@ -import { useQuery } from "@tanstack/react-query" +import { type UseQueryResult, useQuery } from "@tanstack/react-query" import { createFileRoute } from "@tanstack/react-router" import { useState } from "react" +import type { LeaderboardEntry } from "@/client" import { LeaderboardService } from "@/client" import { DataTable } from "@/components/Common/DataTable" import { columns } from "@/components/Leaderboard/columns" @@ -21,10 +22,45 @@ export const Route = createFileRoute("/_layout/leaderboard")({ const ALL_GAMES = "all" +/** Shared by both tabs (same `LeaderboardEntry` shape) -- distinguishes + * loading/error from a genuine empty result, so a real backend failure + * doesn't silently render as "no games yet" forever. */ +function LeaderboardTable({ + query, +}: { + query: UseQueryResult +}) { + if (query.isLoading) { + return ( +

+ Loading leaderboard... +

+ ) + } + if (query.isError) { + return ( +

+ Failed to load leaderboard. +

+ ) + } + if (!query.data || query.data.length === 0) { + return ( +

+ No finished games yet. +

+ ) + } + return +} + function Leaderboard() { const [game, setGame] = useState(ALL_GAMES) const gameFilter = game === ALL_GAMES ? undefined : game + // Both fetch unconditionally rather than gating the inactive tab's query + // on `enabled` -- deliberate: the aggregate query is cheap (PROGRESS.md), + // and prefetching both means switching tabs is instant, never a spinner. const accounts = useQuery({ queryKey: ["leaderboard", "accounts", gameFilter], queryFn: async () => @@ -49,7 +85,7 @@ function Leaderboard() {

Leaderboard