fix: Stop dataset iterators from skipping items when unwind is used - #1059
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1059 +/- ##
=======================================
Coverage 95.24% 95.24%
=======================================
Files 59 59
Lines 5509 5514 +5
=======================================
+ Hits 5247 5252 +5
Misses 262 262
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Pijukatel
reviewed
Sep 11, 2026
Pijukatel
approved these changes
Sep 11, 2026
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.
What was wrong
iterate_itemsadvanced the dataset offset bymax(scanned_rows, len(items)). Withunwind, a page returns moreitems 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
offsetandlimitto the rows first and shapes the result afterwards, and it passes nomaxLimit(unlike the collection listroutes), so a page never covers more rows than the
limitit was sent. Capping the advance there keeps an unwoundpage from running past the window it actually read.
The
max()stays underneath the cap on purpose.x-apify-pagination-countcomes from the dataset'sitemCount,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 does and what #1058
suggested, would truncate
iterate_itemsright afterpush_itemsfor anything over one page. The JS iteratorbounds itself by
totaland so is exposed to that lag either way; this one consults neither, which is why the twoclients end up with different fixes.
DatasetItemsPage.countkeeps its current value ofmax(header, len(items)). That is not the no-op it looks like:it is what makes
countusable right after a push, and two integration assertions depend on it. Its docstringchanges, along with the
limitdocs oniterate_itemsandchunk_sizeon both twins, since all of them promiseditems 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 fulllimitrather than therows the page covered, so rows appended by a concurrent push after that page can be missed. Reaching them needs the
raw header, which
list_itemsfolds intocount. It is strictly better than before, and the docstring says sorather than claiming the overshoot is free.
Behavior change worth a changelog line
iterate_items(limit=N)withunwindand achunk_sizebelowNnow walks all N rows instead of stopping afterthe first page, so it can yield several times more items for the same
limit. That is the fix working, but it isuser-visible.
Tests
unwindreached the pagination tests for the first time. The fake API now splits each row into three items afterthe row window is picked, exactly as the transform stream does, and three cases iterate through it.
endpoints. It now applies the requested limit verbatim on the items endpoint, which is what let a
chunk_sizeabove that cap be covered at all.
countlags behind its items has a test of its own. Nothing pinned that before, and it is theinvariant the
max()rests on.test_dataset_iterate_items_unwoundcovers the whole thing against the live API. It has not run locally, so CIis its first real execution.
One thing found while verifying
get_cursor_iteratorjustified its termination rule with filters that "can drop every item on a page while a livecursor 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