Skip to content

Commit 4554697

Browse files
IlyaasKclaude
andcommitted
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 <noreply@anthropic.com>
1 parent c0a3e14 commit 4554697

3 files changed

Lines changed: 49 additions & 19 deletions

File tree

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,8 @@ first_page = await client.deployments.list(
185185
)
186186

187187
print(
188-
f"the current start offset for this page: {first_page.next_offset}"
189-
) # => "the current start offset for this page: 1"
188+
f"the offset where the next page starts: {first_page.next_offset}"
189+
) # => "the offset where the next page starts: 2"
190190
for deployment in first_page.items:
191191
print(deployment.id)
192192

src/kernel/pagination.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,13 @@ def has_next_page(self) -> bool:
3939
@override
4040
def next_page_info(self) -> Optional[PageInfo]:
4141
next_offset = self.next_offset
42-
if next_offset is None:
43-
return None # type: ignore[unreachable]
42+
if next_offset is None: # type: ignore[unreachable]
43+
if self.has_more:
44+
raise RuntimeError(
45+
"Server reported X-Has-More: true without an X-Next-Offset header; "
46+
"refusing to silently truncate pagination"
47+
)
48+
return None
4449

4550
# X-Next-Offset already holds the offset where the next page starts;
4651
# adding the current page length on top skips a full page per iteration.
@@ -81,8 +86,13 @@ def has_next_page(self) -> bool:
8186
@override
8287
def next_page_info(self) -> Optional[PageInfo]:
8388
next_offset = self.next_offset
84-
if next_offset is None:
85-
return None # type: ignore[unreachable]
89+
if next_offset is None: # type: ignore[unreachable]
90+
if self.has_more:
91+
raise RuntimeError(
92+
"Server reported X-Has-More: true without an X-Next-Offset header; "
93+
"refusing to silently truncate pagination"
94+
)
95+
return None
8696

8797
# X-Next-Offset already holds the offset where the next page starts;
8898
# adding the current page length on top skips a full page per iteration.

tests/test_pagination.py

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,62 @@
1-
from typing import Any, List, Optional
1+
from typing import Any, List, Type, Union, Optional
22

33
import httpx
4+
import pytest
45

5-
from kernel.pagination import SyncOffsetPagination
6+
from kernel.pagination import AsyncOffsetPagination, SyncOffsetPagination
7+
8+
PageClass = Union[Type[SyncOffsetPagination[Any]], Type[AsyncOffsetPagination[Any]]]
9+
10+
# build() and next_page_info() are plain synchronous methods on both classes,
11+
# so both generated variants are pinned against drifting apart on regeneration.
12+
both_classes = pytest.mark.parametrize("cls", [SyncOffsetPagination, AsyncOffsetPagination])
613

714

815
def _page(
9-
*, items: List[Any], next_offset: Optional[int], has_more: Optional[bool]
10-
) -> SyncOffsetPagination[Any]:
16+
cls: PageClass, *, items: List[Any], next_offset: Optional[int], has_more: Optional[bool]
17+
) -> Any:
1118
headers: dict[str, str] = {}
1219
if next_offset is not None:
1320
headers["X-Next-Offset"] = str(next_offset)
1421
if has_more is not None:
1522
headers["X-Has-More"] = "true" if has_more else "false"
1623
response = httpx.Response(200, headers=headers)
17-
return SyncOffsetPagination.build(response=response, data=items)
24+
return cls.build(response=response, data=items)
1825

1926

20-
def test_next_page_starts_at_exactly_x_next_offset() -> None:
27+
@both_classes
28+
def test_next_page_starts_at_exactly_x_next_offset(cls: PageClass) -> None:
2129
# X-Next-Offset already holds the next page's start. Adding the current
2230
# page length on top (the old behavior) skipped a full page per iteration.
23-
page = _page(items=[{}] * 100, next_offset=100, has_more=True)
31+
page = _page(cls, items=[{}] * 100, next_offset=100, has_more=True)
2432
info = page.next_page_info()
2533
assert info is not None
2634
assert info.params == {"offset": 100}
2735

2836

29-
def test_stops_when_x_next_offset_absent() -> None:
30-
page = _page(items=[{}] * 100, next_offset=None, has_more=True)
37+
@both_classes
38+
def test_stops_cleanly_when_last_page_omits_x_next_offset(cls: PageClass) -> None:
39+
page = _page(cls, items=[{}] * 50, next_offset=None, has_more=False)
3140
assert page.next_page_info() is None
3241
assert page.has_next_page() is False
3342

3443

35-
def test_stops_when_x_has_more_false() -> None:
36-
page = _page(items=[{}] * 50, next_offset=200, has_more=False)
44+
@both_classes
45+
def test_stops_when_x_has_more_false(cls: PageClass) -> None:
46+
# Covers the 0 sentinel the API emits on last pages: has_more is false
47+
# whenever next_offset is 0, and has_more gates first.
48+
page = _page(cls, items=[{}] * 50, next_offset=0, has_more=False)
3749
assert page.has_next_page() is False
3850

3951

40-
def test_stops_on_empty_page() -> None:
41-
page = _page(items=[], next_offset=300, has_more=True)
52+
@both_classes
53+
def test_stops_on_empty_page(cls: PageClass) -> None:
54+
page = _page(cls, items=[], next_offset=300, has_more=True)
4255
assert page.has_next_page() is False
56+
57+
58+
@both_classes
59+
def test_refuses_to_silently_truncate_on_contradictory_headers(cls: PageClass) -> None:
60+
page = _page(cls, items=[{}] * 100, next_offset=None, has_more=True)
61+
with pytest.raises(RuntimeError, match="refusing to silently truncate"):
62+
page.has_next_page()

0 commit comments

Comments
 (0)