Skip to content

fix: Stop dataset iterators from skipping items when unwind is used - #1059

Merged
vdusek merged 6 commits into
masterfrom
fix/1058-unwind-offset-advance
Sep 11, 2026
Merged

vdusek merged 6 commits into
masterfrom
fix/1058-unwind-offset-advance

Conversation

@vdusek

@vdusek vdusek commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

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 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

@vdusek vdusek self-assigned this Sep 10, 2026
@github-actions github-actions Bot added this to the 149th sprint - Tooling team milestone Sep 10, 2026
@github-actions github-actions Bot added t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics. labels Sep 10, 2026
@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.24%. Comparing base (3e6f1ea) to head (4cd1424).
⚠️ Report is 8 commits behind head on master.

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           
Flag Coverage Δ
integration 91.96% <100.00%> (-0.09%) ⬇️
unit 87.35% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vdusek
vdusek requested a review from szaganek as a code owner September 10, 2026 15:09
@vdusek
vdusek requested a review from Pijukatel September 10, 2026 15:11
Comment thread src/apify_client/_pagination.py Outdated
@vdusek
vdusek merged commit 8e30197 into master Sep 11, 2026
30 checks passed
@vdusek
vdusek deleted the fix/1058-unwind-offset-advance branch September 11, 2026 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dataset iterators skip items when unwind returns more items than the rows scanned

3 participants