Repository navigation
fix!: keep pagination iterators advancing past fully filtered pages - #1044
Merged
Merged
Conversation
vdusek
added
the
t-tooling
Contributor
|
See more at https://github.com/apify/apify-client-js/actions/runs/34572429119#summary-103177244239 |
vdusek
marked this pull request as ready for review
September 9, 2026 08:52
barjin
reviewed
Sep 10, 2026
vdusek
changed the title
vdusek
added a commit
to apify/apify-client-python
that referenced
this pull request
Sep 11, 2026
…1059) ## What was wrong `iterate_items` advanced the dataset offset by `max(scanned_rows, len(items))`. With `unwind`, a page returns more items than the rows it scanned, so the offset jumped past rows the next request never read. At the default `chunk_size`, a 5000-row dataset with a 3x unwind yielded about a third of it. ## The fix The offset now advances by at most the number of rows the call asked for. The items endpoint applies `offset` and `limit` to the rows first and shapes the result afterwards, and it passes no `maxLimit` (unlike the collection list routes), so a page never covers more rows than the `limit` it was sent. Capping the advance there keeps an unwound page from running past the window it actually read. The `max()` stays underneath the cap on purpose. `x-apify-pagination-count` comes from the dataset's `itemCount`, which the API increments through a throttled write while the items themselves stream fresh. Following the header alone, which is what [apify-client-js#1044](apify/apify-client-js#1044) does and what #1058 suggested, would truncate `iterate_items` right after `push_items` for anything over one page. The JS iterator bounds itself by `total` and so is exposed to that lag either way; this one consults neither, which is why the two clients end up with different fixes. `DatasetItemsPage.count` keeps its current value of `max(header, len(items))`. That is not the no-op it looks like: it is what makes `count` usable right after a push, and two integration assertions depend on it. Its docstring changes, along with the `limit` docs on `iterate_items` and `chunk_size` on both twins, since all of them promised items where the field and the parameters have always counted scanned rows. One case stays imperfect. On a final short page with `unwind`, the advance is still the full `limit` rather than the rows the page covered, so rows appended by a concurrent push after that page can be missed. Reaching them needs the raw header, which `list_items` folds into `count`. It is strictly better than before, and the docstring says so rather than claiming the overshoot is free. ## Behavior change worth a changelog line `iterate_items(limit=N)` with `unwind` and a `chunk_size` below `N` now walks all N rows instead of stopping after the first page, so it can yield several times more items for the same `limit`. That is the fix working, but it is user-visible. ## Tests - `unwind` reached the pagination tests for the first time. The fake API now splits each row into three items after the row window is picked, exactly as the transform stream does, and three cases iterate through it. - The fake API used to cap every page at 1000 items "mirroring the real API", which is only true of the collection endpoints. It now applies the requested limit verbatim on the items endpoint, which is what let a `chunk_size` above that cap be covered at all. - A page whose `count` lags behind its items has a test of its own. Nothing pinned that before, and it is the invariant the `max()` rests on. - `test_dataset_iterate_items_unwound` covers the whole thing against the live API. It has not run locally, so CI is its first real execution. ## One thing found while verifying `get_cursor_iterator` justified its termination rule with filters that "can drop every item on a page while a live cursor still points at more data". The API rules that out: the key-value store returns the last key of the page as the next cursor, and the request queue returns a cursor only once a page came back full, so neither can hand back a cursor that outlives its page. The behavior is fine and unchanged; the docstring now says why it holds. Closes #1058 *✍️ Drafted by Claude Code*
B4nan
pushed a commit
that referenced
this pull request
Sep 30, 2026
…1044) ### TLDR - Mirrors apify/apify-client-python#964, which fixed the same bug in the Python client. - The API-side root cause is described in apify/apify-core#27324. - Closes: #979 ### Description The offset-based iterator stopped as soon as a page came back with no items, and advanced the offset by the number of items returned. Dataset filters (`clean`, `skipEmpty`, `skipHidden`) apply after `offset` and `limit`, so a page can scan up to `limit` rows while returning fewer, or none at all. `listItems()` used as an async iterable therefore ended early on a fully filtered page, and re-yielded rows an earlier page had already returned. The iterator now paginates by the scanned count the API reports in `x-apify-pagination-count`, and by that number alone. `items.length` cannot stand in for it in either direction: filters leave it below the rows scanned and `unwind` can leave it above, so mixing the two would skip rows. `PaginatedList.count` stays `items.length`, since it is public and intentionally set that way; the scanned number travels with the page as a non-enumerable symbol property outside the public shape. Collection endpoints send no such header and fall back to `items.length`, so nothing about them changes. The cursor iterators (`listKeys()`, `listRequests()`, `paginateRequests()`) are unchanged. The API never returns an empty page together with a cursor: the request queue listing hands out `nextCursor` only when it filled the whole page, and the key-value store listing derives `nextExclusiveStartKey` from the last key it returned. ### Test plan Regression tests for the fully filtered page, the duplicated offset, and `unwind` returning more items than rows scanned. ### Breaking change When `DatasetClient.listItems()` is consumed as an async iterable, `limit` caps the dataset rows the API scans, as it does on a single call, where v2 counted the items yielded. With `clean`, `skipEmpty` or `skipHidden` set, the loop can yield fewer than `limit` items, each of them once. v2 made up the difference with items it had already yielded, or stopped early on an empty page. With `unwind` the loop can yield more. The v3 upgrading guide covers it. *✍️ Drafted by Claude Code*
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
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.
TLDR
Description
The offset-based iterator stopped as soon as a page came back with no items, and advanced the offset by the number of items returned. Dataset filters (
clean,skipEmpty,skipHidden) apply afteroffsetandlimit, so a page can scan up tolimitrows while returning fewer, or none at all.listItems()used as an async iterable therefore ended early on a fully filtered page, and re-yielded rows an earlier page had already returned.The iterator now paginates by the scanned count the API reports in
x-apify-pagination-count, and by that number alone.items.lengthcannot stand in for it in either direction: filters leave it below the rows scanned andunwindcan leave it above, so mixing the two would skip rows.PaginatedList.countstaysitems.length, since it is public and intentionally set that way; the scanned number travels with the page as a non-enumerable symbol property outside the public shape. Collection endpoints send no such header and fall back toitems.length, so nothing about them changes.The cursor iterators (
listKeys(),listRequests(),paginateRequests()) are unchanged. The API never returns an empty page together with a cursor: the request queue listing hands outnextCursoronly when it filled the whole page, and the key-value store listing derivesnextExclusiveStartKeyfrom the last key it returned.Test plan
Regression tests for the fully filtered page, the duplicated offset, and
unwindreturning more items than rows scanned.Breaking change
When
DatasetClient.listItems()is consumed as an async iterable,limitcaps the dataset rows the API scans, as it does on a single call, where v2 counted the items yielded. Withclean,skipEmptyorskipHiddenset, the loop can yield fewer thanlimititems, each of them once. v2 made up the difference with items it had already yielded, or stopped early on an empty page. Withunwindthe loop can yield more. The v3 upgrading guide covers it.✍️ Drafted by Claude Code