fix(leaderboard): match period rank tiebreakers across profile and embed surfaces - #1192
Merged
Conversation
…bed surfaces The leaderboard ranks a finite period with a sequential ROW_NUMBER over `total_tokens DESC, total_cost DESC, LOWER(username) ASC, user_id ASC` (cost sort swaps the two metrics), and ranks all-time with a shared `RANK() OVER (ORDER BY <metric> DESC)`. The public profile's period rank and the embed card's period rank each used a bare `RANK() OVER (ORDER BY total_tokens DESC)` instead, so two users on the same period total read #1 and #2 on the leaderboard's week or month tab but #1 and #1 on their own profile and embed card. Move both finite-period queries onto the leaderboard's expression and tiebreak columns. The CTEs now carry `u.username` and, on the profile side, `SUM(CAST(d.cost AS DECIMAL(18,4)))` so the tiebreak has its columns in scope; the `leaderboard_hidden = false` filter, the date bounds, and the embed's `of N` denominator are unchanged, so the same rows are ranked and only the order within a tie moves. Both all-time queries stay on shared RANK, which is what the leaderboard's all-time tab does — switching them would be the same divergence mirrored. Constraint: profile period rank must keep its unstable_cache key and the empty-window skip Rejected: sequential ranks everywhere | all-time surfaces would then disagree with the leaderboard's all-time tab Rejected: shared RANK everywhere | loses the leaderboard's deterministic pagination order Confidence: high Scope-risk: narrow Directive: the finite and lifetime windows rank differently on purpose; each mirrors the leaderboard tab it sits beside Not-tested: no database integration test — the regression tests assert the emitted SQL, not Postgres' ordering of a real tie
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Three surfaces show a user's rank for a finite period — the leaderboard's week/month/custom tab, the public profile's 7d/30d view, and the embed card's week/month view — and until now they did not agree on what a tie means.
getLeaderboard.tsbuilds its rank expression from asequentialRanksflag. The period path passestrueand emitsROW_NUMBER() OVER (ORDER BY total_tokens DESC, total_cost DESC, LOWER(username) ASC, user_id ASC)(a cost sort swaps the two metrics), which gives every user a distinct position and gives pagination a stable order. The all-time path passesfalseand emitsRANK() OVER (ORDER BY total_tokens DESC), so tied users share a position.publicProfileData.tsandgetUserEmbedStats.tseach rolled their own period rank with a bareRANK() OVER (ORDER BY total_tokens DESC). So two users on the same period total read #1 and #2 on the leaderboard's week tab, but #1 and #1 — followed by a gap — on their own profile pages and on their embed cards, from the same data over the same window at the same moment.Change
Both finite-period rank queries now use the leaderboard's period expression and its tiebreak columns, matched to each surface's own ranking metric (the embed's cost sort orders
total_cost DESC, total_tokens DESC, the tokens sort the other way around, and the profile ranks by tokens only).The tiebreak needs
usernameanduser_idin scope, so each period CTE now also selectsu.username(and, on the profile side,SUM(CAST(d.cost AS DECIMAL(18,4))) AS total_costfor the secondary term) and groups bys.user_id, u.username.users.idis the primary key the join runs through, so the added grouping column produces the same groups and the embed'sSELECT COUNT(*)::int FROM rankabledenominator is unchanged. Theu.leaderboard_hidden = falsefilter and the date-range bounds are untouched — the same rows are ranked, and only the order within a tie moves. TheNumber(...)coercion of the returned bigint, theunstable_cachekeys and tags, and the profile's empty-window skip all stay as they were.The all-time paths deliberately stay on shared
RANK(). They already agree with the leaderboard's all-time tab, and moving them toROW_NUMBERwould introduce exactly the same divergence in the other direction. Both files now carry a comment saying so, and the tests below pin it.Tests
__tests__/api/usersProfile.test.tsand__tests__/lib/getUserEmbedStats.test.tsmocknext/cacheanddrizzle-orm'ssqltag, so they can assert on the SQL each path emits. Added there:ranks the profile period window the way the leaderboard's period tab does— asserts the period rank emits the fullROW_NUMBER() OVER (ORDER BY total_tokens DESC, total_cost DESC, LOWER(username) ASC, user_id ASC)clause, that noRANK() OVERis emitted on that path, that the hidden-user filter and both date bounds are unchanged, and that the cache key still carries the anchored window.ranks the finite embed window the way the leaderboard's period tab ranks $sortBy— the same check for the embed card, once per sort, pinning the metric order for each and theof Ndenominator.keeps the lifetime profile rank on shared RANKand the renamedkeeps the lifetime embed rank on shared RANK with no tie-breakers— assert the all-time paths still emitRANK()and emit noROW_NUMBERat all, so the mirror-image bug fails the suite too.Four existing assertions that matched the old period SQL shape were updated from
RANK() OVERtoROW_NUMBER() OVER. The embed test's SQL serializer now recurses into nestedsqlfragments — a conditionalORDER BYis interpolated as a value, and the old serializer flattened it to[object Object], which hid the clause these tests are about.Verified by falsification in both directions: reverting the source change fails 7 tests (the 3 new period tests plus the 4 updated ones) and leaves the two all-time tests passing; flipping the all-time queries to
ROW_NUMBERinstead fails exactly those two and nothing else.Verification
From
packages/frontend, on the branch head:npx tsc --noEmit— exit 0bun run test— exit 0, 925 passed | 6 skipped (931) across 87 passing / 1 skipped test filesbun run lint— exit 0, 17 pre-existing warnings, none in the changed filesNo database integration test: the repo has no harness for one on this path, so these tests pin the emitted SQL rather than Postgres' ordering of a real tie.
Summary by cubic
Aligns finite-period ranks on profile and embed with the leaderboard’s period tab. Previously, profile/embed used shared RANK on period totals (ties showed the same position); now they use sequential ROW_NUMBER with the same tie-breakers as the leaderboard, so tied users read distinct positions consistently. All-time ranks remain on shared RANK.
u.username(and profile includes summedtotal_cost) and group bys.user_id, u.username. Filters, date bounds, cache keys/tags, and the embed “of N” denominator are unchanged; only tie ordering moves. Tests assert emitted SQL for each path and update the SQL serializer to recurse nested fragments.Written for commit ddf66e6. Summary will update on new commits.