Skip to content

fix(trace-item-stats): Resolve issue with pagination#118039

Merged
nsdeschenes merged 1 commit into
masterfrom
nd/EXP-1017/fix-trace-item-stats-pagination-has-more
Jun 19, 2026
Merged

fix(trace-item-stats): Resolve issue with pagination#118039
nsdeschenes merged 1 commit into
masterfrom
nd/EXP-1017/fix-trace-item-stats-pagination-has-more

Conversation

@nsdeschenes

Copy link
Copy Markdown
Contributor

Problem
The /organizations/{org}/trace-items/stats/ endpoint never reported a next page, so clients could only ever see the first page of attribute breakdowns.

TraceItemStatsPaginator decides whether there's another page with has_more = data[1] >= offset + limit + 1, where data[1] is the count returned by data_fn. But data_fn returned len(request_attrs_list), the page length after slicing sanitized_keys[offset : offset + limit], which is always ≤ limit. That makes has_more always False, so the Link header always emitted next; results="false".

Fix

  • Capture total_attributes = len(sanitized_keys) before slicing and return that as the paginator count, so has_more reflects whether attributes exist beyond the current page.
  • Updated the misleading "Request 1 more than limit" comment to document the real (payload, total_count) contract data_fn must satisfy.

Closes EXP-1017

The trace item stats endpoint's paginator computes has_more as
data[1] >= offset + limit + 1, where data[1] is the second element returned by
data_fn. data_fn returned len(request_attrs_list) — the page length after
slicing sanitized_keys to [offset : offset + limit] — which is always <= limit.
That made has_more always False, so the Link header reported
next; results="false" on every page and clients could never paginate past the
first page.

Return the total number of matching attributes (captured before slicing) as the
count so has_more reflects whether more attributes exist beyond the current
page. Document the (payload, total_count) contract on the paginator.

Rewrite test_pagination_with_limit (which gated its only next-page assertion
behind an if that was never true) into test_pagination_traverses_all_pages: it
walks both pages and asserts the previous/next results flags and full,
non-overlapping attribute coverage. It fails without this fix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jun 18, 2026

Copy link
Copy Markdown

EXP-1017

@nsdeschenes

Copy link
Copy Markdown
Contributor Author

@sentry review

@nsdeschenes

Copy link
Copy Markdown
Contributor Author

@cursor review

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jun 18, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit a06d935. Configure here.

@nsdeschenes nsdeschenes marked this pull request as ready for review June 18, 2026 18:05
@nsdeschenes nsdeschenes requested review from a team as code owners June 18, 2026 18:05
@nsdeschenes nsdeschenes merged commit f9c84cd into master Jun 19, 2026
65 checks passed
@nsdeschenes nsdeschenes deleted the nd/EXP-1017/fix-trace-item-stats-pagination-has-more branch June 19, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants