Skip to content

fix: prevent record filters from prematurely stopping CursorPagination via last_page_size#1074

Open
Anatolii Yatsuk (tolik0) wants to merge 2 commits into
mainfrom
tolik0/fix-cursor-pagination-record-filter-last-page-size
Open

fix: prevent record filters from prematurely stopping CursorPagination via last_page_size#1074
Anatolii Yatsuk (tolik0) wants to merge 2 commits into
mainfrom
tolik0/fix-cursor-pagination-record-filter-last-page-size

Conversation

@tolik0

@tolik0 Anatolii Yatsuk (tolik0) commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Completes the record-filter pagination fix family started in #484 (OffsetIncrement) and #1073 (PageIncrement), this time for CursorPaginationStrategy.

SimpleRetriever._read_pages computes last_page_size from records after the RecordSelector runs (extract → filter), so a record filter (e.g. client-side incremental filtering) makes it smaller than the page the API actually returned. CursorPaginationStrategy exposes last_page_size in the interpolation context for both stop_condition and cursor_value, so a manifest with a stop condition like {{ last_page_size < 100 }} stops paginating prematurely when a full API page is partially or fully filtered out — silently dropping the remaining pages.

Changes

  • CursorPaginationStrategy gains an optional extractor: Optional[RecordExtractor] field; when set, next_page_token recomputes last_page_size = len(extractor.extract_records(response)) (the raw, pre-filter count) before evaluating stop_condition and cursor_value.
  • ModelToComponentFactory.create_cursor_pagination now accepts the extractor_model that create_default_paginator has been passing to every pagination strategy since fix(OffsetIncrement Pagination Strategy): Fix bug where streams that use record filtering do not paginate correctly #484 (previously silently swallowed by **kwargs) and builds the extractor from it — same pattern as create_offset_increment and create_page_increment.
  • last_record is intentionally left post-filter: raw extracted records don't have RecordSelector transformations applied, so overriding it could break cursor_value expressions referencing transformed fields.

Behavior is unchanged for direct CursorPaginationStrategy instantiations without an extractor.

Tests

  • Strategy-level: full page with all/some records filtered continues paginating; genuine partial/empty pages still stop; raw count also visible to cursor_value interpolation.
  • Factory-level: create_cursor_pagination wires the extractor from extractor_model — the wiring gap that let fix(OffsetIncrement Pagination Strategy): Fix bug where streams that use record filtering do not paginate correctly #484 miss this strategy (and PageIncrement) in the first place.
  • Fixed test_create_default_paginator, which passed a runtime DpathExtractor component instead of a DpathExtractorModel as extractor_model; the mistake was invisible while create_cursor_pagination discarded the kwarg, and now the test also asserts the extractor lands on the strategy.

Note: unit_tests/sources/declarative/test_concurrent_declarative_source.py has a pre-existing flaky failure (sqlite3.OperationalError: database table is locked: responses from requests-cache) that reproduces on clean main and is unrelated to this change.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved cursor-based pagination to evaluate page size using records extracted directly from API responses.
    • Fixed pagination behavior when downstream filtering reduces the number of records returned from a page.
    • Ensured cursor and stop-condition calculations remain accurate for full, partial, and empty pages.
  • Tests

    • Added coverage for extracted record counts and cursor pagination scenarios.

Anatolii Yatsuk (tolik0) and others added 2 commits July 15, 2026 16:00
…n via last_page_size

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 15, 2026 13:37
@github-actions

Copy link
Copy Markdown

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

💡 Show Tips and Tricks

Testing This CDK Version

You can test this version of the CDK using the following:

# Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@tolik0/fix-cursor-pagination-record-filter-last-page-size#egg=airbyte-python-cdk[dev]' --help

# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch tolik0/fix-cursor-pagination-record-filter-last-page-size

PR Slash Commands

Airbyte Maintainers can execute the following slash commands on your PR:

  • /autofix - Fixes most formatting and linting issues
  • /poetry-lock - Updates poetry.lock file
  • /test - Runs connector tests with the updated CDK
  • /prerelease - Triggers a prerelease publish with default arguments
  • /poe build - Regenerate git-committed build artifacts, such as the pydantic models which are generated from the manifest JSON schema in YAML.
  • /poe <command> - Runs any poe command in the CDK environment
📚 Show Repo Guidance

Helpful Resources

📝 Edit this welcome message.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 81d7e4f2-a148-42ca-a8cc-49d344631960

📥 Commits

Reviewing files that changed from the base of the PR and between 468d9e8 and ea74d97.

📒 Files selected for processing (4)
  • airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py
  • airbyte_cdk/sources/declarative/requesters/paginators/strategies/cursor_pagination_strategy.py
  • unit_tests/sources/declarative/parsers/test_model_to_component_factory.py
  • unit_tests/sources/declarative/requesters/paginators/test_cursor_pagination_strategy.py

📝 Walkthrough

Walkthrough

Cursor pagination now optionally extracts records directly from HTTP responses to recalculate raw page sizes. The model factory constructs and passes this extractor, while tests cover factory wiring, continuation, stopping, and cursor interpolation.

Changes

Cursor pagination extraction

Layer / File(s) Summary
Raw page-size evaluation
airbyte_cdk/sources/declarative/requesters/paginators/strategies/cursor_pagination_strategy.py, unit_tests/sources/declarative/requesters/paginators/test_cursor_pagination_strategy.py
CursorPaginationStrategy optionally extracts response records and uses their count for last_page_size, with tests covering continuation, stopping, and cursor interpolation.
Extractor construction and factory integration
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py, unit_tests/sources/declarative/parsers/test_model_to_component_factory.py
create_cursor_pagination accepts an extractor model, builds it with the pagination decoder, and passes it to the strategy; factory tests verify the resulting extractor configuration.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ModelToComponentFactory
  participant CursorPaginationStrategy
  participant RecordExtractor
  participant HTTPResponse
  ModelToComponentFactory->>RecordExtractor: create optional extractor from extractor_model
  ModelToComponentFactory->>CursorPaginationStrategy: pass extractor to strategy
  CursorPaginationStrategy->>RecordExtractor: extract records from HTTP response
  RecordExtractor-->>CursorPaginationStrategy: return extracted records
  CursorPaginationStrategy->>CursorPaginationStrategy: recalculate last_page_size
Loading

Possibly related PRs

Suggested reviewers: copilot, devin-ai-integration[bot], darynaishchenko

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main fix: record filters no longer cause CursorPagination to stop early via last_page_size.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tolik0/fix-cursor-pagination-record-filter-last-page-size

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

PyTest Results (Fast)

4 137 tests  +6   4 125 ✅ +6   7m 42s ⏱️ -5s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit ea74d97. ± Comparison against base commit 468d9e8.

@github-actions

Copy link
Copy Markdown

PyTest Results (Full)

4 140 tests  +6   4 128 ✅ +6   11m 55s ⏱️ -23s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit ea74d97. ± Comparison against base commit 468d9e8.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants