Skip to content

fix(api): add id tiebreaker to paginated queries#6127

Merged
gustavosbarreto merged 1 commit intomasterfrom
fix/pagination-ordering
Apr 7, 2026
Merged

fix(api): add id tiebreaker to paginated queries#6127
gustavosbarreto merged 1 commit intomasterfrom
fix/pagination-ordering

Conversation

@gustavosbarreto
Copy link
Copy Markdown
Member

Problem

Paginated queries return rows in non-deterministic order when the primary sort column has duplicate values (e.g. multiple devices with the same last_seen), causing items to shift between pages.

Fix

Add id DESC as a secondary sort key to all paginated queries in Sort().

Fixes #6124

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

Claude finished @gustavosbarreto's task in 3m 45s —— View job


Code Review Complete

Reviewed 1 file. 1 inline issue posted (1 critical).

The fix for deterministic pagination ordering is the right idea, but the hardcoded id DESC tiebreaker will break at runtime for tables that don't have an id column (api_keys, announcements). See the inline comment for details and suggested approaches.


To request another review round, comment /review.

@gustavosbarreto gustavosbarreto force-pushed the fix/pagination-ordering branch from 13d5e95 to 5cfc499 Compare April 7, 2026 14:53
@gustavosbarreto
Copy link
Copy Markdown
Member Author

/review

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

Claude finished @gustavosbarreto's task in 4m 58s —— View job


Code Review Complete

Reviewed 7 files. 2 inline issues posted (1 critical, 1 high).

The configurable tiebreaker approach is sound and addresses the original review feedback well. The Tiebreak field is correctly scoped (no query tag, service-layer only), backwards compatible with cloud callers, and the bun.Ident() usage is safe. Two issues need attention before merge:

  1. api-key.go — tiebreak is dead code when sort_by isn't provided (missing default By)
  2. Service tests — mock expectations across 5 test files need Tiebreak field added

To request another review round, comment /review.

@gustavosbarreto gustavosbarreto force-pushed the fix/pagination-ordering branch from 5cfc499 to 2a70dda Compare April 7, 2026 15:01
Without a secondary sort key, PostgreSQL returns rows in
non-deterministic order when the primary sort column has
duplicate values. This causes items to shift between pages,
making pagination unreliable.

Add `id DESC` as a tiebreaker to every sorted query so that
ordering is stable across pages.

Fixes: #6124
@gustavosbarreto gustavosbarreto force-pushed the fix/pagination-ordering branch from 2a70dda to 70ccd47 Compare April 7, 2026 15:02
@gustavosbarreto
Copy link
Copy Markdown
Member Author

/review

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

Claude finished @gustavosbarreto's task in 46s —— View job


Code Review Complete

Reviewed 12 files across code quality, security, testing, language patterns, and architecture — no issues found. The code looks good as-is.

The configurable Tiebreak approach is well-designed: backward-compatible with cloud callers (empty Tiebreak preserves existing behavior), safe from injection (bun.Ident() + no query struct tag), and correctly applied across all paginated call sites with appropriate tiebreak columns per table.


To request another review round, comment /review.

View job

@gustavosbarreto gustavosbarreto merged commit 209ab45 into master Apr 7, 2026
12 checks passed
@gustavosbarreto gustavosbarreto deleted the fix/pagination-ordering branch April 7, 2026 18:02
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.

Pagination is hard to use due to inconsistent ordering

1 participant