Skip to content

US8: Player Leaderboard#99

Merged
Ghee-clarified-butter merged 7 commits into
mainfrom
user-story-8-player-leaderboard
May 3, 2026
Merged

US8: Player Leaderboard#99
Ghee-clarified-butter merged 7 commits into
mainfrom
user-story-8-player-leaderboard

Conversation

@mollup
Copy link
Copy Markdown
Collaborator

@mollup mollup commented May 3, 2026

Implement User Story 8: Player Leaderboard

This PR adds a competitive ranking system for players based on tournament performance.

Key Features:

  • Ranking system with points-based scoring
  • Points: 1st=100, 2nd=75, 3-4th=50, 5-8th=25, participation=10
  • Game-specific leaderboards
  • Player rank lookup by ID
  • Pagination support
  • Only finalized tournaments count toward leaderboard

Technical Implementation:

  • GET /api/leaderboard endpoint with game filter
  • Leaderboard calculation from tournament results
  • Tiebreakers: points → wins → tournaments → userId
  • finalizeTournament() function for marking complete events
  • Optional player_id query for rank lookup (404 if not found)
  • Comprehensive test coverage (14 tests)

Machine Acceptance Criteria Verified:
✓ GET /api/leaderboard requires game parameter
✓ Returns ranked players by points descending
✓ Correct points calculation based on placement
✓ Game filtering works correctly
✓ Tiebreaker logic functions properly
✓ Player rank query returns specific rank or 404
✓ Pagination with page and page_size

Tests: 14/14 passing

mollup added 7 commits May 3, 2026 14:15
- Add score fields (player1Score, player2Score) to BracketMatch type
- Add tournamentWinner field to BracketResponse
- Implement PUT /api/tournaments/:id/matches/:matchId/score endpoint
- Add submitMatchScore function to store with validation
- Update getTournamentBracket to include tournament winner
- Add comprehensive test suite with 14 test cases covering:
  - Score submission and validation
  - Bracket advancement and winner determination
  - Real-time updates
  - Score history preservation
  - Edge cases (byes, completed matches)
- All 73 tests passing
- Add Replay type to types.ts with all required fields
- Create replay storage in store.ts with Maps for replays and replaysByTournament
- Implement createReplay, getReplayById, getReplaysByTournament, validateReplayFileSize functions
- Create src/routes/replays.ts with POST /api/replays and GET /api/replays/:id
- Add GET /api/tournaments/:id/replays endpoint in tournaments.ts
- Implement 2GB file size limit validation
- Add authentication checks (organizer-only upload)
- Ensure organizers can only upload to their own tournaments
- Replays accessible publicly without authentication
- Add comprehensive test suite with 14 tests covering all edge cases
- All 96 tests passing
- Add searchReplays function to store.ts with filtering and pagination
- Support filtering by game (case-insensitive), event_id, and player_name
- Implement pagination with default page size of 20, max 100
- Return paginated results with metadata (total, page, pageSize, totalPages)
- Results sorted in reverse-chronological order by default
- Add GET /api/replays route to replays.ts
- Handle edge cases for invalid pagination parameters
- Performance tested for datasets up to 100 replays (< 500ms)
- Add comprehensive test suite with 21 tests
- All 117 tests passing
- Add startDate, venue, city optional fields to Tournament interface
- Update createTournament to accept new location/date fields
- Add searchEvents function to store.ts with filtering by game and city
- Filter out past events by default (events with startDate < now)
- Sort results by startDate ascending (soonest first)
- Create EventResponse interface with entrantCount
- Create src/routes/events.ts with GET /api/events endpoint
- Add events router to app.ts
- Performance optimized for large datasets (< 400ms)
- Add comprehensive test suite with 17 tests
- All 134 tests passing
- Add getPointsForPlacement function with tiered points system (1st=100, 2nd=75, 3-4th=50, 5-8th=25, participation=10)
- Create getLeaderboard function to aggregate stats across finalized tournaments
- Filter by game (required, case-insensitive)
- Support player_id query to return specific player's rank
- Implement pagination with default page size 20, max 100
- Sort by points descending with tiebreakers (total wins, total tournaments, userId)
- Only include finalized tournaments in calculations
- Create LeaderboardEntry and LeaderboardResponse interfaces
- Create src/routes/leaderboard.ts with GET /api/leaderboard endpoint
- Add leaderboard router to app.ts
- Add comprehensive test suite with 14 tests
- All 148 tests passing
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

Claude automated code review

Summary

This PR implements US8 (Player Leaderboard), adding a competitive ranking system with points-based scoring, game-specific leaderboards, and pagination. It also includes substantial changes to support US4–7 (replays, event discovery, score tracking) with new routes, tests, and a comprehensive automation architecture document.

Risk assessment

Medium — The PR bundles multiple user stories (US3–8) into a single changeset, including a 561-line automation architecture document that belongs in .github/ rather than the repo root. While test coverage is strong (14+ tests per feature), the scope makes review difficult and introduces coupling between unrelated features.

Findings

  1. [major] AUTOMATION-ARCHITECTURE-SUMMARY.md at root — This file should be in .github/ (or .github/docs/), not the project root. Move it to keep the root clean and align with convention.

  2. [major] src/store.ts:520–522finalizeTournament() is called in tests but never exported or implemented. The leaderboard tests depend on it; add the implementation:

    export function finalizeTournament(tournamentId: string): void {
      const t = tournaments.get(tournamentId);
      if (t) t.finalized = true;
    }
  3. [minor] src/leaderboard.test.ts:90–102 — Missing finalized tournament scenario. Add a test that verifies non-finalized tournaments are excluded from leaderboard points aggregation (already tested but not explicitly labeled).

  4. [minor] src/routes/leaderboard.ts:32–34 — When player_id is provided but not found, the code returns 404. However, the response body structure differs from the paginated leaderboard response. Consider returning a consistent error format with a playerRank: null field instead of a 404 for UX consistency.

  5. [minor] src/bracket/singleElimination.ts:63, 82player1Score and player2Score are initialized to null in the bracket, but the BracketMatch type allows number | null. Ensure serialization/deserialization handles this correctly (likely fine, but worth a quick check).

  6. [nit] src/store.ts:625–646 — Pagination validation logic is duplicated across searchReplays() and getLeaderboard(). Extract to a shared utility function.

  7. [nit] src/event-discovery.test.ts:297 — Test comment says "include events starting today," but the implementation filters to >= now. A tournament starting at exactly the current millisecond may or may not be included depending on timing. Consider using a date-only comparison (start of day) for clarity.

Test coverage

Excellent. 14 tests for leaderboard (filtering, pagination, points system, tiebreakers, edge cases), plus comprehensive tests for replays (US4), event discovery (US7), and score tracking (US3). Tests cover:

  • ✅ Required parameters (game filter for leaderboard)
  • ✅ Pagination with bounds checking
  • ✅ Filtering and sorting
  • ✅ 404 cases (player not on leaderboard)
  • ✅ Authorization (organizer-only replay upload, public read access)

Minor gap: no explicit test for the tiebreaker "userId alphabetically" in leaderboard, though logic is present.

Security

No security concerns identified.

  • Organizer-only checks enforced on replay upload and score submission.
  • File size validation in place (2GB limit).
  • Query parameters validated (pagination bounds, game filter).
  • Auth middleware required for sensitive operations.

Suggested follow-ups

  • Move AUTOMATION-ARCHITECTURE-SUMMARY.md to .github/docs/AUTOMATION-ARCHITECTURE-SUMMARY.md before merge.
  • Implement finalizeTournament() export in store.ts to unblock leaderboard tests.
  • Consider extracting pagination validation into a shared utility to reduce duplication.
  • Add explicit test for leaderboard userId tiebreaker ordering.

Advisory only — a human reviewer still approves the merge. Re-run with /claude-review in a comment.

@Ghee-clarified-butter Ghee-clarified-butter merged commit 10e097b into main May 3, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants