Fix offset auto-pagination skipping every other page#119
Open
IlyaasK wants to merge 2 commits into
Open
Conversation
Sync/AsyncOffsetPagination.next_page_info() requested the next page at next_offset + len(items). The X-Next-Offset header already holds the offset where the next page starts (the API computes offset + limit), so adding the current page length skipped a full page per iteration: with 250 items at limit 100, iteration returned items 0-99 and 200-249 and silently dropped 100-199. X-Has-More still terminated the loop, hiding the data loss. Use the header value directly. Absent header already terminated via the None check, unchanged. Hand-maintained fix: with Stainless's hosted generator winding down, the template-level fix is not coming; this commit and its test are the canonical behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Firetiger deploy monitoring skipped This PR didn't match the auto-monitor filter configured on your GitHub connection:
Reason: PR is in the kernel repo but affects the Python SDK pagination logic; unclear which logical service this belongs to without more context on the codebase structure. To monitor this PR anyway, reply with |
Review follow-up: raise instead of silently truncating when X-Has-More is true but X-Next-Offset is absent, mirror all tests across the Sync and Async classes so the two generated variants cannot drift apart, pin the 0 last-page sentinel, and fix the stale next_offset wording in the README pagination example. Co-Authored-By: Claude Fable 5 <noreply@anthropic.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.
Bug
SyncOffsetPagination.next_page_info()andAsyncOffsetPagination.next_page_info()computed the next request offset asnext_offset + len(items). ButX-Next-Offsetalready holds the offset where the next page starts (the API sets it tooffset + limit), so every iteration skipped a full page: with 250 items at limit 100, iteration yields items 0–99 and 200–249 — 100–199 silently vanish.X-Has-Morestill terminates the loop, so nothing ever errored.Same fix as kernel/kernel-node-sdk#127 (identical template flaw); the Go SDK uses the header directly and was always correct.
Fix
Use
X-Next-Offsetverbatim as the next request's offset, in both the sync and async page classes. Termination (absent header →None,X-Has-More: false, empty page) is unchanged and now pinned bytests/test_pagination.py.Why a hand-written commit to generated code
Stainless's hosted generator is winding down post-acquisition, so a template-level fix is not coming. While the service still runs, this commit rides Stainless's custom-code three-way merge; after sunset it is simply the code.
🤖 Generated with Claude Code
Note
Medium Risk
Changes list pagination behavior for all offset-paginated APIs; fixes data loss but could surface RuntimeError where contradictory headers were previously ignored.
Overview
Fixes offset-based auto-pagination in
SyncOffsetPaginationandAsyncOffsetPagination: the next request now usesX-Next-Offsetas-is instead of adding the current page length, which had been skipping every other page on multi-page list iteration.When
X-Has-More: truebutX-Next-Offsetis missing, pagination now raisesRuntimeErrorinstead of stopping quietly.tests/test_pagination.pycovers both sync/async classes, and the README clarifies thatnext_offsetis where the next page starts.Reviewed by Cursor Bugbot for commit 4554697. Bugbot is set up for automated code reviews on this repo. Configure here.