fix: add deterministic tiebreaker to event list ordering - #1009
Open
decentraland-bot wants to merge 1 commit into
Open
fix: add deterministic tiebreaker to event list ordering#1009decentraland-bot wants to merge 1 commit into
decentraland-bot wants to merge 1 commit into
Conversation
Events with the same next_start_at were returned in arbitrary order by PostgreSQL, causing the Events tab to shuffle on every open (issue decentraland/unity-explorer#9529). Adding e.id as a secondary sort key guarantees a stable, reproducible order for all consumers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
decentraland-bot
left a comment
Contributor
Author
There was a problem hiding this comment.
Review verdict: PASS (no blocking findings). GitHub does not allow this account to approve a PR it authored, so I am leaving this as a review comment.
No P0/P1 issues found.
Findings:
- Public API impact: backward-compatible. This only defines previously unspecified tie ordering for existing event list responses; request/response shape, filters, pagination parameters, and primary chronological/search-rank ordering remain unchanged.
- Security: No security issues found. The added SQL fragment is static, and the existing raw
orderBy/orderDirectionvalues remain whitelist-derived. - Test coverage: A focused regression test for identical
next_start_atvalues would be useful, but I do not consider it blocking for this one-line deterministic tie-breaker. - Git conventions: PR title and branch follow the semantic
fix:convention.
CI status at review time: Prettier and lint-and-test are passing; build-image and integration-tests are still pending.
Reviewed by Jarvis 🤖 · Requested by Alejandro Jimenez (<@U0ADNDHL1T7>) via Slack
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.
Summary
e.id ASCas a secondary sort key to thegetEventsquery, so events with the samenext_start_atalways return in a stable, reproducible order.Why
PostgreSQL does not guarantee row order when the
ORDER BYcolumn has ties. Multiple events at the same location often share the samenext_start_at, producing a random order on every query.Test plan
🤖 Generated with Claude Code