Skip to content

Fix tournament haystack previous cursor - #2540

Open
yaosongding wants to merge 2 commits into
heroiclabs:masterfrom
yaosongding:fix/tournament-haystack-prev-cursor
Open

Fix tournament haystack previous cursor#2540
yaosongding wants to merge 2 commits into
heroiclabs:masterfrom
yaosongding:fix/tournament-haystack-prev-cursor

Conversation

@yaosongding

@yaosongding yaosongding commented Aug 13, 2026

Copy link
Copy Markdown

Summary

Fixes a copy-paste bug in TournamentRecordsHaystack where the previous cursor was assigned the value of the next cursor:

// before
tournamentRecordList := &api.TournamentRecordList{Records: results.Records, NextCursor: results.NextCursor, PrevCursor: results.NextCursor, RankCount: results.RankCount}
// after
tournamentRecordList := &api.TournamentRecordList{Records: results.Records, NextCursor: results.NextCursor, PrevCursor: results.PrevCursor, RankCount: results.RankCount}

Impact

Clients paginating backward through ListTournamentRecordsAroundOwner receive a next-page cursor where a previous-page cursor belongs. Following the returned prev_cursor lands on the next page (duplicated forward navigation), and when the leaderboard end leaves no next cursor, no previous cursor is returned at all — backward pagination is broken in both cases.

Evidence

  • Introduced in 5666361a ("Add leaderboard record list around owner cursors", Add leaderboard record list around owner cursors #877, 2022) and present since. The sibling path in the same fileTournamentRecordsList (core_tournament.go) — already maps PrevCursor: records.PrevCursor correctly, as does LeaderboardRecordsHaystack (core_leaderboard.go). Only the haystack wrapper dropped it.
  • The source value results.PrevCursor has been correct and populated the entire time — only the tournament wrapper dropped it.
  • Verified with git history: the line was introduced once and never touched since.

Test

Adds TestApiTournamentHaystack (server/api_tournament_test.go), an API-level test that:

  1. Creates a non-authoritative tournament with enable_ranks
  2. Writes 5 records via WriteTournamentRecord (scores 10–50, so the owner at score 30 sits at rank 3)
  3. Queries ListTournamentRecordsAroundOwner from the middle with a small limit (3)
  4. Asserts the middle page contains exactly the records around the owner — scores 40, 30, 20 with ranks 2, 3, 4 — and that PrevCursor != NextCursor
  5. Asserts following prev_cursor returns the single record ranked above the middle page (score 50, rank 1), and following next_cursor returns the single record ranked below (score 10, rank 5)
  6. Asserts the three pages together cover all five records exactly once

Verified: test FAILS on the buggy code (the cursor-inequality assertion and the previous-page content assertion both fail — the buggy prev_cursor returns the score-10 record instead of the score-50 record), PASSES with the fix.

Verification

  • go build ./server/ — passes
  • golangci-lint run --new-from-rev=master ./server/ — 0 issues
  • New test passes with the fix, fails without it (red-green verified)

TournamentRecordsHaystack incorrectly assigned the next cursor value to
the previous cursor field, breaking backward pagination on tournament
records around an owner. Fixes a copy-paste error present since the
haystack cursors feature was introduced.

Adds an API-level test asserting the previous and next cursors are
distinct and that the previous cursor is usable.
@yaosongding
yaosongding marked this pull request as ready for review August 13, 2026 13:00
Assert the exact middle-page contents (scores 40/30/20, ranks 2/3/4), verify the previous cursor returns the record ranked above the page (score 50, rank 1) and the next cursor the record below (score 10, rank 5), and that all three pages cover every record exactly once. Also close the API server database connection instead of an unused one.
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