Skip to content

feat: cap the leaderboard at the department's top 50 - #22

Merged
duuyguyiilmaz merged 1 commit into
mainfrom
feat/leaderboard-top-50
Aug 31, 2026
Merged

duuyguyiilmaz merged 1 commit into
mainfrom
feat/leaderboard-top-50

Conversation

@duuyguyiilmaz

Copy link
Copy Markdown
Owner

The leaderboard read every note in its department and sorted them in memory. Scoping it to one department already cut that by roughly the number of departments, but it is still one document read per note every time the tab opens — and the sort runs in the listener callback, on the main thread.

Firestore now does both: orderBy(ratingSum) descending, limit(50). The client-side sortedByDescending goes away.

Capped rather than paged

A leaderboard is a "top N" view by nature; paging one out to rank 300 is machinery for a question nobody asks.

It is also not an access restriction — every note stays reachable through the feed, which is the path to the content. This screen ranks it.

New index

department + ratingSum (descending), added to firestore.indexes.json alongside the feed's. Without it the query fails with FAILED_PRECONDITION and the leaderboard never opens.

firebase deploy --only firestore:indexes

One assumption made explicit

LeaderboardAdapter derives each rank from list position, which assumed a sorted list. That assumption is now held by the query rather than by a call in the repository, so the comment says where it comes from.

Verification

Android suite: 114 tests, unchanged, all passing. assembleDebug and lintDebug pass.

Not verified here: the index and the query behind it. The fakes stop at the repository interface and this machine has no credentials for a live project — the gap the README already records for every Firebase repository.

🤖 Generated with Claude Code

The leaderboard read every note in its department and sorted them in memory. Scoping it
to one department already cut that by roughly the number of departments, but it is still
one document read per note in the department every time the tab opens, and the sort runs
in the listener callback on the main thread.

Firestore now does both: orderBy(ratingSum) descending, limit 50. The client-side
sortedByDescending goes away, and the query needs a composite index on department +
ratingSum, added alongside the feed's.

Capped rather than paged, and worth being explicit about why. A leaderboard is a "top N"
view by nature; paging one out to rank 300 is machinery for a question nobody asks. It
is also not an access restriction — every note stays reachable through the feed, which
is the path to the content. This screen ranks it.

LeaderboardAdapter derives each rank from list position, which assumed a sorted list.
That assumption is now held by the query rather than by a call in the repository, so the
comment says where it comes from.

Not verified here: the index and the query behind it. The fakes stop at the repository
interface and this machine has no credentials for a live project — the gap the README
already records for every Firebase repository. Android suite: 114 tests, unchanged, all
passing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@duuyguyiilmaz
duuyguyiilmaz merged commit f635529 into main Aug 31, 2026
2 checks passed
@duuyguyiilmaz
duuyguyiilmaz deleted the feat/leaderboard-top-50 branch August 31, 2026 16:25
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.

1 participant