From c0a3e14b51e39dfdba37bacf023b2f98d4a3be69 Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Fri, 12 Jun 2026 15:32:57 -0400 Subject: [PATCH 1/2] fix(pagination): stop skipping a page per auto-pagination iteration 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 --- src/kernel/pagination.py | 14 ++++++-------- tests/test_pagination.py | 42 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 8 deletions(-) create mode 100644 tests/test_pagination.py diff --git a/src/kernel/pagination.py b/src/kernel/pagination.py index cdf83c2f..2a7dde8e 100644 --- a/src/kernel/pagination.py +++ b/src/kernel/pagination.py @@ -42,10 +42,9 @@ def next_page_info(self) -> Optional[PageInfo]: if next_offset is None: return None # type: ignore[unreachable] - length = len(self._get_page_items()) - current_count = next_offset + length - - return PageInfo(params={"offset": current_count}) + # X-Next-Offset already holds the offset where the next page starts; + # adding the current page length on top skips a full page per iteration. + return PageInfo(params={"offset": next_offset}) @classmethod def build(cls: Type[_BaseModelT], *, response: Response, data: object) -> _BaseModelT: # noqa: ARG003 @@ -85,10 +84,9 @@ def next_page_info(self) -> Optional[PageInfo]: if next_offset is None: return None # type: ignore[unreachable] - length = len(self._get_page_items()) - current_count = next_offset + length - - return PageInfo(params={"offset": current_count}) + # X-Next-Offset already holds the offset where the next page starts; + # adding the current page length on top skips a full page per iteration. + return PageInfo(params={"offset": next_offset}) @classmethod def build(cls: Type[_BaseModelT], *, response: Response, data: object) -> _BaseModelT: # noqa: ARG003 diff --git a/tests/test_pagination.py b/tests/test_pagination.py new file mode 100644 index 00000000..0c49517f --- /dev/null +++ b/tests/test_pagination.py @@ -0,0 +1,42 @@ +from typing import Any, List, Optional + +import httpx + +from kernel.pagination import SyncOffsetPagination + + +def _page( + *, items: List[Any], next_offset: Optional[int], has_more: Optional[bool] +) -> SyncOffsetPagination[Any]: + headers: dict[str, str] = {} + if next_offset is not None: + headers["X-Next-Offset"] = str(next_offset) + if has_more is not None: + headers["X-Has-More"] = "true" if has_more else "false" + response = httpx.Response(200, headers=headers) + return SyncOffsetPagination.build(response=response, data=items) + + +def test_next_page_starts_at_exactly_x_next_offset() -> None: + # X-Next-Offset already holds the next page's start. Adding the current + # page length on top (the old behavior) skipped a full page per iteration. + page = _page(items=[{}] * 100, next_offset=100, has_more=True) + info = page.next_page_info() + assert info is not None + assert info.params == {"offset": 100} + + +def test_stops_when_x_next_offset_absent() -> None: + page = _page(items=[{}] * 100, next_offset=None, has_more=True) + assert page.next_page_info() is None + assert page.has_next_page() is False + + +def test_stops_when_x_has_more_false() -> None: + page = _page(items=[{}] * 50, next_offset=200, has_more=False) + assert page.has_next_page() is False + + +def test_stops_on_empty_page() -> None: + page = _page(items=[], next_offset=300, has_more=True) + assert page.has_next_page() is False From 4554697a9d850848b602a2422199fb2ce72d8efc Mon Sep 17 00:00:00 2001 From: Ilyaas Kapadia <86218345+IlyaasK@users.noreply.github.com> Date: Fri, 12 Jun 2026 16:07:37 -0400 Subject: [PATCH 2/2] fix(pagination): fail loudly on contradictory headers; test both classes 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 --- README.md | 4 ++-- src/kernel/pagination.py | 18 ++++++++++++---- tests/test_pagination.py | 46 ++++++++++++++++++++++++++++------------ 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index d7639da5..2e8ed560 100644 --- a/README.md +++ b/README.md @@ -185,8 +185,8 @@ first_page = await client.deployments.list( ) print( - f"the current start offset for this page: {first_page.next_offset}" -) # => "the current start offset for this page: 1" + f"the offset where the next page starts: {first_page.next_offset}" +) # => "the offset where the next page starts: 2" for deployment in first_page.items: print(deployment.id) diff --git a/src/kernel/pagination.py b/src/kernel/pagination.py index 2a7dde8e..6e3ebb54 100644 --- a/src/kernel/pagination.py +++ b/src/kernel/pagination.py @@ -39,8 +39,13 @@ def has_next_page(self) -> bool: @override def next_page_info(self) -> Optional[PageInfo]: next_offset = self.next_offset - if next_offset is None: - return None # type: ignore[unreachable] + if next_offset is None: # type: ignore[unreachable] + if self.has_more: + raise RuntimeError( + "Server reported X-Has-More: true without an X-Next-Offset header; " + "refusing to silently truncate pagination" + ) + return None # X-Next-Offset already holds the offset where the next page starts; # adding the current page length on top skips a full page per iteration. @@ -81,8 +86,13 @@ def has_next_page(self) -> bool: @override def next_page_info(self) -> Optional[PageInfo]: next_offset = self.next_offset - if next_offset is None: - return None # type: ignore[unreachable] + if next_offset is None: # type: ignore[unreachable] + if self.has_more: + raise RuntimeError( + "Server reported X-Has-More: true without an X-Next-Offset header; " + "refusing to silently truncate pagination" + ) + return None # X-Next-Offset already holds the offset where the next page starts; # adding the current page length on top skips a full page per iteration. diff --git a/tests/test_pagination.py b/tests/test_pagination.py index 0c49517f..46723c97 100644 --- a/tests/test_pagination.py +++ b/tests/test_pagination.py @@ -1,42 +1,62 @@ -from typing import Any, List, Optional +from typing import Any, List, Type, Union, Optional import httpx +import pytest -from kernel.pagination import SyncOffsetPagination +from kernel.pagination import AsyncOffsetPagination, SyncOffsetPagination + +PageClass = Union[Type[SyncOffsetPagination[Any]], Type[AsyncOffsetPagination[Any]]] + +# build() and next_page_info() are plain synchronous methods on both classes, +# so both generated variants are pinned against drifting apart on regeneration. +both_classes = pytest.mark.parametrize("cls", [SyncOffsetPagination, AsyncOffsetPagination]) def _page( - *, items: List[Any], next_offset: Optional[int], has_more: Optional[bool] -) -> SyncOffsetPagination[Any]: + cls: PageClass, *, items: List[Any], next_offset: Optional[int], has_more: Optional[bool] +) -> Any: headers: dict[str, str] = {} if next_offset is not None: headers["X-Next-Offset"] = str(next_offset) if has_more is not None: headers["X-Has-More"] = "true" if has_more else "false" response = httpx.Response(200, headers=headers) - return SyncOffsetPagination.build(response=response, data=items) + return cls.build(response=response, data=items) -def test_next_page_starts_at_exactly_x_next_offset() -> None: +@both_classes +def test_next_page_starts_at_exactly_x_next_offset(cls: PageClass) -> None: # X-Next-Offset already holds the next page's start. Adding the current # page length on top (the old behavior) skipped a full page per iteration. - page = _page(items=[{}] * 100, next_offset=100, has_more=True) + page = _page(cls, items=[{}] * 100, next_offset=100, has_more=True) info = page.next_page_info() assert info is not None assert info.params == {"offset": 100} -def test_stops_when_x_next_offset_absent() -> None: - page = _page(items=[{}] * 100, next_offset=None, has_more=True) +@both_classes +def test_stops_cleanly_when_last_page_omits_x_next_offset(cls: PageClass) -> None: + page = _page(cls, items=[{}] * 50, next_offset=None, has_more=False) assert page.next_page_info() is None assert page.has_next_page() is False -def test_stops_when_x_has_more_false() -> None: - page = _page(items=[{}] * 50, next_offset=200, has_more=False) +@both_classes +def test_stops_when_x_has_more_false(cls: PageClass) -> None: + # Covers the 0 sentinel the API emits on last pages: has_more is false + # whenever next_offset is 0, and has_more gates first. + page = _page(cls, items=[{}] * 50, next_offset=0, has_more=False) assert page.has_next_page() is False -def test_stops_on_empty_page() -> None: - page = _page(items=[], next_offset=300, has_more=True) +@both_classes +def test_stops_on_empty_page(cls: PageClass) -> None: + page = _page(cls, items=[], next_offset=300, has_more=True) assert page.has_next_page() is False + + +@both_classes +def test_refuses_to_silently_truncate_on_contradictory_headers(cls: PageClass) -> None: + page = _page(cls, items=[{}] * 100, next_offset=None, has_more=True) + with pytest.raises(RuntimeError, match="refusing to silently truncate"): + page.has_next_page()