fix(explore): send a stable sort on errors/discover so cursor pagination is deterministic - #1520
Merged
Merged
Conversation
…ion is deterministic Multi-page grouped aggregate queries on the errors dataset returned duplicate and missing dimension tuples with a nondeterministic total, while reporting hasMore: false. The events cursor is offset-based, so correct pagination requires a stable total order across the separate page requests — but the CLI dropped the sort param for every non-spans dataset, leaving grouped results in an unstable order that overlapped and skipped rows between pages. The events endpoint accepts sort on errors/discover (confirmed against a live SaaS org: -count() and count() each produce a strictly monotone, repeatable order). Thread the already-computed `-<firstAggregate>` sort through for those datasets so pagination is deterministic and hasMore is accurate. metrics/logs still reject sort with 400, so they stay unsorted. Fixes #1519
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
BYK
approved these changes
Sep 2, 2026
| * grouped aggregate queries overlap and skip rows (#1519). `metrics` and `logs` | ||
| * reject `sort` with a 400, so they stay unsorted. | ||
| */ | ||
| const SORTABLE_DATASETS = new Set(["spans", "errors", "discover"]); |
Member
There was a problem hiding this comment.
I there a way to get this information directly from Sentry codebase, @sentry/api package or Snuba or something?
Contributor
Author
There was a problem hiding this comment.
Not in this change — the empirical confirmation from the live-org probe (and the existing spans-only guard) was the minimal, verified fix. A canonical source in @sentry/api or Snuba would be a nice follow-up, but it would require coordination across repos and is out of scope for closing #1519.
Member
There was a problem hiding this comment.
Can you file a follow up for this and merge your PR then?
Contributor
Author
There was a problem hiding this comment.
Done — opened #1523. Will merge once CI is green on the current commit.
BYK
pushed a commit
that referenced
this pull request
Sep 2, 2026
…#1524) Follow-up to #1520. Investigates whether a canonical, machine-readable source exists for "which datasets accept a sort on grouped aggregates" and records the finding. **Result: no such source exists today.** The OpenAPI spec (`@sentry/api` `listOrganizationEvents`) models `sort` as a flat query param whose only documented constraint is "must be in the `field` list" — it does not encode per-dataset sort capability, and there is no dataset-capability introspection endpoint. The spec's `dataset` enum (`errors | logs | profile_functions | spans | tracemetrics | uptime_results`) does not even include `discover`. Given that, the hand-curated `SORTABLE_DATASETS` set stays, but: - The `discover` entry is removed — it was unreachable, since `discover` is not a valid `--dataset` value (see `DATASET_ALIASES`). - The doc comment now records the research findings as an explicit contract, with a note to revisit if Sentry ships a per-dataset capability contract. ## Testing `vitest run test/commands/explore.test.ts` (40 passed) and `test/lib/api/explore.test.ts` (18 passed); biome clean on the changed file. Refs #1523 <!-- ## Plan 1. Investigate candidate canonical sources for sortable-dataset capability: - @sentry/api OpenAPI spec (getsentry/sentry-api-schema @ 0.256.0): fetched openapi-derefed.json. - listOrganizationEvents `sort` is a flat query param, doc constraint only "must be in field list". - dataset enum: errors, logs, profile_functions, spans, tracemetrics, uptime_results (no discover). - no capabilities/dataset-meta endpoint anywhere in spec. - Conclusion: no machine-readable source; best achievable deliverable is a documented contract. 2. explore.ts: remove dead `discover` from SORTABLE_DATASETS (unreachable via DATASET_ALIASES); rewrite the doc comment to record the findings and the contract. 3. Verify no test referenced `discover`; run explore unit tests + lint. --> Co-authored-by: jared-outpost[bot] <jared-outpost[bot]@users.noreply.github.com>
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
1 participant
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.
Multi-page grouped aggregate queries on the
errorsdataset returned duplicate and missing dimension tuples with a nondeterministic total, while reportinghasMore: false. The events cursor is offset-based, so correct pagination needs a stable total order across the separate page requests — but the CLI dropped thesortparam for every non-spans dataset, leaving grouped results in an unstable order that overlapped and skipped rows between pages.The events endpoint accepts
sortonerrors/discover(confirmed against a live SaaS org by the reporter:-count()andcount()each produce a strictly monotone, repeatable order). This threads the already-computed-<firstAggregate>sort through for those datasets so pagination is deterministic andhasMoreis accurate.metrics/logsstill rejectsortwith 400, so they stay unsorted.Testing
vitest run explore— 58 passing, including new cases assertingerrorsauto-sorts by the first aggregate andmetricsstays unsorted.Closes #1519