Clean up leaderboard code + speed up hot LB API

[skyfallwastaken]

Summary of the problem

• API calls always skipped the leaderboard cache, so the leaderboard API was slightly slow.
• Leaderboard code was slightly scattered and messy

Describe your changes

Centralises the leaderboard code!

Screenshots / Media

N/A

[Copilot]

Pull request overview

This PR centralizes leaderboard entry serialization/filtering into a dedicated LeaderboardEntries service and speeds up the public leaderboard API by introducing a short-lived cache that is invalidated via the existing LeaderboardPageCache versioning mechanism.

Changes:

• Add LeaderboardEntries service to unify viewer-aware filtering, ranking, and optional active-project enrichment.
• Update web + API controllers to use the new service (public API now uses cached public entries).
• Warm the new public entries cache during leaderboard generation and add service-level tests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test/services/leaderboard_entries_test.rb	Adds tests for LeaderboardEntries.fetch and fetch_public, including shadowban behavior and cache invalidation.
app/services/leaderboard_page_cache.rb	Exposes LeaderboardPageCache.version to let other caches key off the page-cache version.
app/services/leaderboard_entries.rb	Introduces centralized leaderboard entry fetching/serialization and a cached “public” fetch path.
app/jobs/leaderboard_update_job.rb	Warms the public leaderboard entries cache after rebuilding a leaderboard.
app/controllers/leaderboards_controller.rb	Replaces inline entry shaping/filtering with LeaderboardEntries.fetch.
app/controllers/api/v1/leaderboard_controller.rb	Switches API formatting to consume LeaderboardEntries.fetch_public (cached).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

Greptile Summary

This PR centralises all leaderboard entry-fetching logic into a new LeaderboardEntries service and adds a caching layer to the public API endpoint, which previously always hit the database directly.

• New LeaderboardEntries service provides two paths: fetch (web UI, viewer-aware, delegates to LeaderboardPageCache) and fetch_public (API, shadowban-filtered, cached with its own Rails cache key tied to LeaderboardPageCache.version).
• API controller now calls LeaderboardEntries.fetch_public instead of querying the database inline, gaining cache reuse; the public cache is pre-warmed by LeaderboardUpdateJob via the new warm_public call.
• Web UI controller entries_payload is simplified to a single LeaderboardEntries.fetch delegation; the old inline shadowban filtering, rank assignment, and active-project enrichment are now encapsulated in the service.

Confidence Score: 4/5

Safe to merge — the refactoring is correct, cache invalidation is properly wired through the existing User after_update_commit callback, and the new service is covered by tests.

The centralisation is clean and the two-path caching design is well thought out. The implicit coupling between fetch_public cache invalidation and LeaderboardPageCache.version is verified by the test and backed by the User model's after_update_commit callback. The only notable trade-off is the extra DB query added to LeaderboardUpdateJob, which is a background-job concern rather than a request-path issue.

app/jobs/leaderboard_update_job.rb — the new warm_public call adds a second DB query per job run; app/services/leaderboard_entries.rb — the public_cache_key's dependency on LeaderboardPageCache.version should remain in sync with any future changes to shadowban or admin cache-clearing logic.

Important Files Changed

Filename	Overview
app/services/leaderboard_entries.rb	New service centralising fetch/fetch_public/warm_public logic; cache-key design correctly ties public entry cache invalidation to LeaderboardPageCache.version; ranks are re-assigned after shadowban filtering on both paths.
app/controllers/api/v1/leaderboard_controller.rb	Replaced inline DB query with LeaderboardEntries.fetch_public; mapping uses entry[:user_id] for the user id field while entry.dig(:user, :id) also exists — both are identical but slightly inconsistent.
app/controllers/leaderboards_controller.rb	entries_payload simplified cleanly; nil-leaderboard guard moved into LeaderboardEntries#fetch, which handles it correctly with the safe-navigation operator.
app/jobs/leaderboard_update_job.rb	Adds warm_public call after warm; this introduces a second independent DB query on each job run since warm_public queries the DB separately from LeaderboardPageCache.warm.
app/services/leaderboard_page_cache.rb	Exposes the previously private cache_version as a public version class method; minimal, correct change.
test/services/leaderboard_entries_test.rb	New test file covers shadowban filtering, self-visibility, country scope, active-project enrichment, and cache invalidation on shadowban — good coverage of the new service's public interface.

Sequence Diagram

sequenceDiagram
    participant Job as LeaderboardUpdateJob
    participant LPC as LeaderboardPageCache
    participant LE as LeaderboardEntries
    participant Cache as Rails.cache
    participant DB as Database

    Job->>LPC: warm(leaderboard:)
    LPC->>Cache: fetch(page_cache_key)
    Cache-->>LPC: miss
    LPC->>DB: entries + preload(user: :email_addresses)
    DB-->>LPC: rows
    LPC->>Cache: write(page_cache_key, payload)

    Job->>LE: warm_public(leaderboard:)
    LE->>Cache: fetch(public_cache_key using LPC.version)
    Cache-->>LE: miss
    LE->>DB: "entries WHERE shadowbanned=false + preload(:user)"
    DB-->>LE: rows
    LE->>Cache: write(public_cache_key, entries)

    Note over Job,DB: Both caches now warm

    participant API as API Controller
    participant UI as Leaderboards Controller

    API->>LE: fetch_public(leaderboard:)
    LE->>Cache: fetch(public_cache_key)
    Cache-->>LE: hit
    LE-->>API: "{entries, total}"

    UI->>LE: fetch(leaderboard:, viewer:, ...)
    LE->>LPC: fetch(leaderboard:, scope:, country_code:)
    LPC->>Cache: fetch(page_cache_key)
    Cache-->>LPC: hit
    LPC-->>LE: "{entries including shadowbanned}"
    LE->>LE: filter shadowbanned, re-rank from 1
    LE-->>UI: "{entries, total}"

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
app/controllers/api/v1/leaderboard_controller.rb:17-21
The `id` field is read from the top-level `entry[:user_id]` but the same value is available at `entry.dig(:user, :id)` — both are set to the same integer in `public_entry_payload`. Using the nested path keeps the mapping consistent with how `display_name` and `avatar_url` are accessed on the same line.

```suggestion
    entries = LeaderboardEntries.fetch_public(leaderboard: leaderboard)[:entries].map do |entry|
      { rank: entry[:rank],
        user: { id: entry.dig(:user, :id), username: entry.dig(:user, :display_name), avatar_url: entry.dig(:user, :avatar_url) },
        total_seconds: entry[:total_seconds] }
    end
```

### Issue 2 of 2
app/jobs/leaderboard_update_job.rb:64-65
**Double DB round-trip on every job run**

`LeaderboardPageCache.warm` and `LeaderboardEntries.warm_public` each independently query `leaderboard.entries` with a user preload — the job now issues two separate DB queries where it previously issued one. For a background job this is unlikely to be a problem in practice, but worth knowing: `public_entries_from_database` in `warm_public` fetches a strict subset of what `LeaderboardPageCache.warm` already loaded (non-shadowbanned users, fewer user fields). If the extra query ever becomes a concern, `warm_public` could be seeded from the already-loaded page cache data rather than hitting the DB again.

_{Reviews (1): Last reviewed commit: "Clean up leaderboard code + speed up hot..." | Re-trigger Greptile}