Repository navigation
Conversation
…rsor `BasePage.has_next_page()` returned False whenever the page held zero rows, so every cursor-based page (`PageCursor`, `TokenPage`, `BidirectionalPageCursor`, used by `files.list()`, `skills.list()`, the vault/environment/memory/session endpoints) ended auto-pagination on an empty page even though `next_page_info()` still returned a cursor, silently dropping all remaining items. The opaque cursor is independent of the rows that came back with it, so only `next_page_info()` decides whether there is a way forward.
473ffc8 to
aac10d6
Compare
|
Checking in on this one — CI has been green since The two open decisions are both about where the fix belongs, so a one-line steer is all I need:
Happy either way, in one commit. |
|
Hi @anthropic-sdk-python maintainers — #1943 has been open since 27 September with CI green, so this is the one nudge I planned. Recap: auto-pagination stops at an empty page even when the response still carries a cursor, so The question I would most like settled, because it is the one design decision I could not make from the code: with the fix, a server that returns an empty page plus a cursor is now followed to the next page, which means a caller iterating a cursor that never terminates can loop — |
|
Thanks. This is fixed on main in c027a26, so we're closing this PR. |
Problem
Auto-pagination stops at an empty page even when the response still carries a cursor. A caller doing
for f in client.files.list(): ...receives only the items from the first page.Reproduced against a mock transport that returns
{"data": [], "next_page": "tok-3"}forGET /v1/files?page=tok-2: the client issuestok-2, gets the empty page with a further cursor, and stops.['file_1']where['file_1', 'file_3']is expected. The sameBasePagebacksskills.list()and the vault, environment, memory-store and session endpoints.Root cause
src/anthropic/_base_client.py:211dropped the cursor check behind an emptiness check:For the cursor-based page types (
PageCursor,TokenPage,BidirectionalPageCursor) the cursor is independent of the rows returned with it, so those two statements disagree, anditer_pages()(:285-291) gates only onhas_next_page().Change
has_next_page()now defers to the cursor:Behaviour that is already cursor-driven is unchanged:
SyncPageand other id-cursor pages still stop on an empty page, because theirnext_page_info()returnsNonewhenlast_idis null.has_more: falseshort-circuit from feat(pagination): avoid fetching when has_more: false #860 is preserved.Tests and results
tests/test_pagination.pygains a case per cursor page type: an empty first-or-middle page that still returns a cursor must be followed.0af01906, test file present, source unpatched):DEFER_PYDANTIC_BUILD=false uv run --no-sync pytest tests/test_pagination.py -q -p no:randomly->3 failed, exit 1. The log shows thetok-2response carryingnext_page: tok-3with no following request.aac10d6): same command ->3 passed, exit 0 (re-run after a comment reword, 3 passed in 2.84 s).323 passed, 2 skipped, exit 0.tests/api_resources/:1 failed, 2101 passed, 11 skipped, 1 xfailed. The one failure,test_agent_toolset.py::test_text_io_is_utf8_under_ascii_locale, reproduces identically at base; it is a non-ASCII path artifact in this environment, not a change in behaviour.ruff checkpasses,ruff format --checkclean,pyright --project .reports 0 errors.mypyreports 42 errors, all in the untouchedtests/test_client.py, and the same 42 when that file is checked alone, so this change adds none.tests/api_resources/needs the repository's npm mock server and was not run here.Scope and open questions
Two points a maintainer may want to decide differently:
BasePage.has_next_page(). The alternative is to override it per class inpagination.py, which keeps the base class as-is but repeats the same three lines for each cursor page.iter_pages()compares successive cursors, so this change does not introduce it; it does widen when it can be reached.Note on authorship
Produced by an AI agent under this account owner's direction; the commands and counts above are its own local runs. No human has reviewed the diff yet.