fix(cli): Route all limit-bounded pagination through paginate() by sentry-junior[bot] · Pull Request #1625 · getsentry/toolkit

sentry-junior · 2026-10-23T12:24:10Z

Follow-up to #1608. That PR fixed one hand-rolled loop. This one makes the shared helper the only way to do limit-bounded pagination, and has lint and a property test enforce it.

The "remaining budget" approach skips rows on page-number paginators. OffsetPaginator (used for org projects, teams, members, repositories, and dashboard revisions) encodes the page size in the cursor as <per_page>:<page>:0. If you send that cursor with a smaller per_page, the server reads the end of the old-sized window (results[-(limit + 1):] in src/sentry/api/paginator.py). So --limit 150 came back as rows 0–99 plus 150–199, with 100–149 silently missing. Since #1615, paginate() and the loop from #1608 both did this. It wasn't caught because every test modeled cursors as absolute offsets.

paginate() now returns opaque CLI cursors, cli1~<perPage>~<skip>~<server cursor>, which record the walk's page size and how many rows of the current page were already returned:

Every request of a walk uses the per_page of its first request (min(limit, 100)), including later -c next calls with a different --limit.
When a call stops inside a page (a smaller --limit, or a server that ignored per_page), the next call refetches that page and skips the rows already returned. No rows are skipped or repeated.
Raw Sentry cursors are still accepted as a starting point. A malformed cli1~ cursor is rejected with a ValidationError.

To keep this class of bug from coming back:

Migrated the six remaining hand-rolled limit loops to paginate(): alert issue and metric lists, dashboard revisions, listIssuesAllPages, listIssueEvents, and queryEvents.
New lint rule sentry-cli/no-manual-pagination: rejects any loop in src/ (outside infrastructure.ts) that follows cursors and also counts toward a limit. It flags all six original sites, including the fix(cli): Stop dashboard revisions pagination from skipping rows #1608 version of revisions. dashboard/list.ts keeps a disable with a reason, because it filters client-side and resumes mid-page with its own bookmarks. prefer-paginate-helper now covers all of src/.
New property test test/lib/paginate.property.test.ts: ports Sentry's OffsetPaginator and GenericOffsetPaginator cursor math, plus a server that ignores per_page. It walks -c next with a different --limit on each call and asserts every row comes back exactly once, in order.
Docs: packages/cli/AGENTS.md used to tell people to write exactly this loop by hand. It now points to paginate() and explains the cursor format.

One behavior change: listIssueEvents used to keep the server cursor after an overshoot, on the theory that offset cursors would re-include the trimmed rows. GenericOffsetPaginator computes the next offset from the server's own page size, so keeping it would skip rows. It now follows the shared contract.

Follow-up: #1642 splits large limits evenly across pages (so --limit 201 doesn't fetch a full third page for one row) and makes cursors safe across CLI upgrades and downgrades.

via BYK.

--

View Junior Session in Sentry

Sentry's page-number paginators (OffsetPaginator, used by projects, teams, members, repositories, and dashboard revisions) put the page size in the cursor. Asking for "the remaining budget" on the last page reads the wrong window and silently skips rows, so paginate() now keeps one page size per walk, picks one that divides the limit, and stops a page early rather than trimming away a usable cursor. Hand-written limit-bounded cursor loops are migrated to paginate() and rejected by a new sentry-cli/no-manual-pagination lint rule. A property test ports Sentry's paginator cursor math so this class of bug fails CI. Co-Authored-By: BYK <[email protected]>

vercel · 2026-10-23T12:24:17Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
sentry-local	Skipped		Oct 23, 2026 2:54pm UTC

BYK

Known gap, not addressed here and not new: resuming a page-number cursor with a different --limit still misaligns, because the cursor's page size no longer matches. Fixing that needs CLI-side cursors that carry the page size.

Let's do this here too

BYK

We should also clean up any redundant tests now.

Resuming a page-number cursor with a different --limit misaligned, because Sentry's OffsetPaginator encodes the page size in its cursor and a different per_page reads the wrong window. paginate() now returns opaque CLI cursors (cli1~<perPage>~<skip>~<server cursor>) that record the walk's page size and how many rows of the current page were already returned. A later -c next reuses that page size whatever --limit is, and resumes mid-page by refetching the page and skipping returned rows, so it no longer stops a page early or drops the cursor when a server ignores per_page. Raw server cursors are still accepted as a starting point. issue list resumes through listIssuesAllPages so wrapped cursors are decoded, and dashboard list always requests full pages, since its afterId bookmarks already handle mid-page resumes.

Now that paginate() cursors carry the page size and resume mid-page, a page size that divides the limit no longer buys correctness. Use min(limit, API_MAX_PER_PAGE) so large limits need fewer requests, and remove pageSizeForLimit(). Drop per-endpoint tests that re-checked paginate()'s trimming, overshoot, and page-size rules; the property test and paginate's unit tests cover them. Each caller keeps its per_page and cursor wiring tests.

paginate() now wraps server cursors with the walk's page size, so the e2e test checks that the server cursor is carried instead of matching it exactly.

… cursors (#1642) Follow-up to #1625. This fixes two things before `paginate()`'s cursors ship in a release. **Overfetching on the last page.** `paginate()` used 100-row pages for any `--limit` over 100. So `--limit 201` made three requests of 100 and threw away 99 rows. A fresh walk now uses the same, minimal number of requests and splits the limit evenly across them: 201 → 3 × 67, 250 → 3 × 84, 101 → 2 × 51. A property test checks the request count (`ceil(limit / 100)`) and that a walk overfetches by less than one row per request. **Cursors that work across CLI versions.** #1625 returned CLI-only `cli1~<perPage>~<skip>~<server cursor>` cursors. Older CLIs send any cursor straight to the API, so they couldn't use those, and the new CLI couldn't trust the raw cursors older CLIs stored. Now every cursor `paginate()` returns is a Sentry cursor: - **Page boundary:** the server's cursor, unchanged. - **Mid-page:** the cursor of the partly returned page, with the number of rows already returned written in binary into `is_prev`. After a leading `0`, a 1 bit is `_0` and a 0 bit is `0`, most significant first. So `84:2:0_00_000_00` means page `84:2:0`, skip 82 (`0b1010010`), and the worst case is about 15 characters. - Sentry parses `is_prev` with Python's `int()` (`Cursor.from_string` in `src/sentry/utils/cursors.py`). `int()` allows single underscores between digits (PEP 515), so every such string is `0`, and every endpoint accepts it as a normal next-page cursor. - `paginate()` refetches the page and skips those rows. An older CLI, or anything else that sends the cursor as is, sees those rows again but never misses any. - `encodeSkip()` documents the format. - **Cursors that can't carry a skip:** a server cursor that isn't a Sentry next-page cursor (e.g. a pasted `is_prev=1` cursor) is returned as is. Resuming refetches that page. - **Page size:** no longer stored in the cursor. `OffsetPaginator` cursors (`<per_page>:<page>:<is_prev>`) already encode it, so `paginate()` reads it back, and pasted or stored raw cursors resume with the right window. Other paginators (`BasePaginator`, `GenericOffsetPaginator`, `SequencePaginator`) treat `offset` as a row offset and take any page size. When resuming mid-page, the first request also fetches the rows to skip, so it uses as few requests as possible. Because both directions work, stored `-c next` history is shared across versions again: no namespacing, and nothing is discarded on upgrade or downgrade. Cursors stored by older CLIs are raw server cursors and resume correctly. `cli1~` cursors saved by nightly builds since #1625 are still read but never written. A damaged one is ignored with a warning, and the walk starts from the first page. Known limit: an older CLI that sends a page-number cursor with a *different* `--limit` still misreads it. That's the pre-#1625 bug, and this PR can't fix it on the old side. Tests: - Properties over ports of `OffsetPaginator`, `GenericOffsetPaginator`, and a server that ignores `per_page`. They check that every row comes back exactly once across `-c next` with changing limits, that every returned cursor is a Sentry cursor (with `is_prev` a Python int literal equal to 0) that the API resumes from without skipping a row, and that a pasted page-number cursor resumes without skipping. - Unit tests for the request sequences, encoded skips, cursors that can't carry a skip, and nightly `cli1~` cursors. - I checked the encoding against real Python `int()` for skips from 1 to 1000. Local checks: `tsc` and lint pass. CLI unit suite: 11,300 pass. The only failures are 6 chmod/permission tests (completions, code-scanner, file-changes, telemetry, upgrade) that also fail on clean `main` in this sandbox.  via **BYK**.    -- [View Junior Session in Sentry](https://sentry.sentry.io/explore/conversations/local%3Aweb%3A4269baf0761dc42b4e7fc238/?project=4510944073809921)  --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: BYK <[email protected]>

github-actions Bot added the risk: medium PR risk score: medium label Oct 23, 2026

vercel Bot deployed to Preview October 23, 2026 12:25

View deployment

BYK requested changes Oct 23, 2026

View reviewed changes

Comment thread packages/cli/test/lib/org-list.test.ts Outdated

vercel Bot temporarily deployed to Preview October 23, 2026 14:23 Inactive

vercel Bot temporarily deployed to Preview October 23, 2026 14:44 Inactive

test(cli): Expect CLI cursors in the feedback limit e2e test

113cef9

paginate() now wraps server cursors with the walk's page size, so the e2e test checks that the server cursor is carried instead of matching it exactly.

vercel Bot temporarily deployed to Preview October 23, 2026 14:54 Inactive

BYK approved these changes Oct 23, 2026

View reviewed changes

BYK merged commit e7f4a86 into main Oct 23, 2026
33 of 35 checks passed

BYK deleted the fix/cli-paginate-fixed-page-size branch October 23, 2026 21:25

sentry-junior Bot mentioned this pull request Oct 23, 2026

fix(cli): Split paginate() limits evenly and return Sentry-compatible cursors #1642

Merged

Navigation Menu

Repository navigation

fix(cli): Route all limit-bounded pagination through paginate() - #1625

fix(cli): Route all limit-bounded pagination through paginate()#1625
BYK merged 4 commits into
mainfrom
fix/cli-paginate-fixed-page-size

sentry-junior Bot commented Oct 23, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Oct 23, 2026 •

edited

Loading

Uh oh!

BYK left a comment

Uh oh!

BYK left a comment

Uh oh!

Uh oh!

Uh oh!

This branch was previously deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

sentry-junior Bot commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel Bot commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BYK left a comment

Choose a reason for hiding this comment

Uh oh!

BYK left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This branch was previously deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

sentry-junior Bot commented Oct 23, 2026 •

edited

Loading

vercel Bot commented Oct 23, 2026 •

edited

Loading