fix: prevent record filters from prematurely stopping CursorPagination via last_page_size#1074
Conversation
…n via last_page_size Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This CDK VersionYou 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-sizePR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughCursor 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. ChangesCursor pagination extraction
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
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Summary
Completes the record-filter pagination fix family started in #484 (
OffsetIncrement) and #1073 (PageIncrement), this time forCursorPaginationStrategy.SimpleRetriever._read_pagescomputeslast_page_sizefrom records after theRecordSelectorruns (extract → filter), so a record filter (e.g. client-side incremental filtering) makes it smaller than the page the API actually returned.CursorPaginationStrategyexposeslast_page_sizein the interpolation context for bothstop_conditionandcursor_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
CursorPaginationStrategygains an optionalextractor: Optional[RecordExtractor]field; when set,next_page_tokenrecomputeslast_page_size = len(extractor.extract_records(response))(the raw, pre-filter count) before evaluatingstop_conditionandcursor_value.ModelToComponentFactory.create_cursor_paginationnow accepts theextractor_modelthatcreate_default_paginatorhas 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 ascreate_offset_incrementandcreate_page_increment.last_recordis intentionally left post-filter: raw extracted records don't have RecordSelector transformations applied, so overriding it could breakcursor_valueexpressions referencing transformed fields.Behavior is unchanged for direct
CursorPaginationStrategyinstantiations without an extractor.Tests
cursor_valueinterpolation.create_cursor_paginationwires the extractor fromextractor_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 (andPageIncrement) in the first place.test_create_default_paginator, which passed a runtimeDpathExtractorcomponent instead of aDpathExtractorModelasextractor_model; the mistake was invisible whilecreate_cursor_paginationdiscarded the kwarg, and now the test also asserts the extractor lands on the strategy.Note:
unit_tests/sources/declarative/test_concurrent_declarative_source.pyhas a pre-existing flaky failure (sqlite3.OperationalError: database table is locked: responsesfrom requests-cache) that reproduces on cleanmainand is unrelated to this change.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests