Skip to content

Commit 5d2afd7

Browse files
IlyaasKclaude
andcommitted
fix(pagination): correct type:ignore placement; stop on 0 sentinel
Two review follow-ups, both classes: - Move '# type: ignore[unreachable]' from the 'if next_offset is None:' line onto the first body statement ('if self.has_more:'). Under warn_unreachable mypy reports the unreachable diagnostic at the first body statement, so the ignore must sit there or the lint fails. Verified with mypy==1.17.0. - Treat next_offset == 0 as a stop, not an advancing offset (matching the Go and Node SDKs). A real next-page offset is always offset+limit >= 1, so 0 is unambiguously the last-page sentinel. New test asserts via next_page_info directly under has_more=True so the has_more gate can't mask it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 843fade commit 5d2afd7

2 files changed

Lines changed: 25 additions & 11 deletions

File tree

src/kernel/pagination.py

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,20 @@ 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: # type: ignore[unreachable]
43-
if self.has_more:
42+
if next_offset is None:
43+
if self.has_more: # type: ignore[unreachable]
4444
raise RuntimeError(
4545
"Server reported X-Has-More: true without an X-Next-Offset header; "
4646
"refusing to silently truncate pagination"
4747
)
4848
return None
4949

50-
# X-Next-Offset already holds the offset where the next page starts;
51-
# adding the current page length on top skips a full page per iteration.
50+
# X-Next-Offset is the next page's absolute start, or 0 on the last page
51+
# (the API's stop sentinel). Only a positive offset advances; the old code
52+
# added the current page length on top, skipping a page per iteration.
53+
if next_offset == 0:
54+
return None
55+
5256
return PageInfo(params={"offset": next_offset})
5357

5458
@classmethod
@@ -86,16 +90,20 @@ def has_next_page(self) -> bool:
8690
@override
8791
def next_page_info(self) -> Optional[PageInfo]:
8892
next_offset = self.next_offset
89-
if next_offset is None: # type: ignore[unreachable]
90-
if self.has_more:
93+
if next_offset is None:
94+
if self.has_more: # type: ignore[unreachable]
9195
raise RuntimeError(
9296
"Server reported X-Has-More: true without an X-Next-Offset header; "
9397
"refusing to silently truncate pagination"
9498
)
9599
return None
96100

97-
# X-Next-Offset already holds the offset where the next page starts;
98-
# adding the current page length on top skips a full page per iteration.
101+
# X-Next-Offset is the next page's absolute start, or 0 on the last page
102+
# (the API's stop sentinel). Only a positive offset advances; the old code
103+
# added the current page length on top, skipping a page per iteration.
104+
if next_offset == 0:
105+
return None
106+
99107
return PageInfo(params={"offset": next_offset})
100108

101109
@classmethod

tests/test_pagination.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,18 @@ def test_stops_cleanly_when_last_page_omits_x_next_offset(cls: PageClass) -> Non
4141

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

4947

48+
@both_classes
49+
def test_stops_on_next_offset_zero_sentinel(cls: PageClass) -> None:
50+
# 0 is the API's last-page sentinel. Assert via next_page_info directly,
51+
# with has_more=True, so the has_more gate cannot mask the 0 handling.
52+
page = _page(cls, items=[{}] * 50, next_offset=0, has_more=True)
53+
assert page.next_page_info() is None
54+
55+
5056
@both_classes
5157
def test_stops_on_empty_page(cls: PageClass) -> None:
5258
page = _page(cls, items=[], next_offset=300, has_more=True)

0 commit comments

Comments
 (0)