Skip to content

fix: do not advertise incremental sync mode for streams without cursor field#876

Merged
Aaron ("AJ") Steers (aaronsteers) merged 2 commits intomainfrom
devin/1767906101-fix-incremental-sync-mode-for-streams-without-cursor
Jan 8, 2026
Merged

fix: do not advertise incremental sync mode for streams without cursor field#876
Aaron ("AJ") Steers (aaronsteers) merged 2 commits intomainfrom
devin/1767906101-fix-incremental-sync-mode-for-streams-without-cursor

Conversation

@aaronsteers
Copy link
Copy Markdown
Member

@aaronsteers Aaron ("AJ") Steers (aaronsteers) commented Jan 8, 2026

Summary

Fixes a bug where declarative streams without incremental_sync configuration were incorrectly advertising support for incremental sync mode with an empty cursor field (defaultCursorField: [""]).

Root cause: When a stream doesn't have incremental_sync configured, _build_concurrent_cursor() returns a FinalStateCursor which lacks a cursor_field attribute. The code was falling back to CursorField(cursor_field_key="") instead of None. Since DefaultStream.as_airbyte_stream() checks if self._cursor_field:, the CursorField object (despite having an empty key) caused incremental sync to be advertised.

The fix: Change the fallback from CursorField(cursor_field_key="") to None, so streams without incremental sync configuration correctly advertise only full_refresh mode.

Updates since last revision

  • Wrapped the ternary expression in parentheses for better readability (per reviewer feedback)

Review & Testing Checklist for Human

  • Verify streams WITH incremental_sync still work correctly - Test a connector with incremental sync configured and confirm it still advertises incremental mode with the correct cursor field
  • Verify streams WITHOUT incremental_sync now only show full_refresh - Test source-pokeapi or similar connector and confirm the catalog no longer shows incremental as a supported sync mode
  • Check for test failures - Ensure existing tests pass and consider if new tests should be added to prevent regression

Recommended test plan: Run discover on source-pokeapi and verify the pokemon stream only shows supported_sync_modes: ["full_refresh"] (not ["full_refresh", "incremental"]).

Notes

  • This removes a FIXME comment that acknowledged the issue
  • Existing connections configured with incremental sync on affected streams may need to be reconfigured to use full_refresh

Link to Devin run: https://app.devin.ai/sessions/81b4056e835d448fa41bf4ae376f8b7f
Requested by: Aaron ("AJ") Steers (@aaronsteers)

Summary by CodeRabbit

Release Notes

  • Refactor
    • Improved cursor field handling in stream construction to support optional cursor fields, allowing more flexible processing of streams without explicit cursor field definitions.

✏️ Tip: You can customize this high-level summary in your review settings.

…r field

Co-Authored-By: AJ Steers <aj@airbyte.io>
Copilot AI review requested due to automatic review settings January 8, 2026 21:03
@devin-ai-integration
Copy link
Copy Markdown
Contributor

Original prompt from AJ Steers
Received message in Slack channel #ask-devin-ai:

@Devin - this custom source connector is an example where The stream should not be advertising that it can support incremental because incremental is not supported. Specifically, there's no cursor key for the stream, so incremental is impossible. It needs you to identify if the problem is in the CDK, aka the connector itself, or in the UI where we're offering perhaps replication modes that are not actually possible for the source in question. And specifically for the source stream in question. <https://cloud.airbyte.com/workspaces/bf7d3df8-8a91-4fd4-bd4c-89c293ba1d6b/connections/3622f032-b288-4cf5-8202-9b0d468160f8/timeline|https://cloud.airbyte.com/workspaces/bf7d3df8-8a91-4fd4-bd4c-89c293ba1d6b/connections/3622f032-b288-4cf5-8202-9b0d468160f8/timeline>
Thread URL: https://airbytehq-team.slack.com/archives/C08BHPUMEPJ/p1767905062740199

@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@github-actions github-actions bot added bug Something isn't working security labels Jan 8, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where declarative streams without incremental sync configuration were incorrectly advertising support for incremental sync mode. The root cause was that streams without incremental_sync configured were receiving an empty CursorField object instead of None, causing them to incorrectly report incremental sync capability.

Key changes:

  • Changed the fallback value for cursor_field from CursorField(cursor_field_key="") to None when a cursor doesn't have a cursor_field attribute
  • Removed a FIXME comment acknowledging this issue

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 8, 2026

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

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@devin/1767906101-fix-incremental-sync-mode-for-streams-without-cursor#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 devin/1767906101-fix-incremental-sync-mode-for-streams-without-cursor

Helpful Resources

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

📝 Edit this welcome message.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 8, 2026

📝 Walkthrough

Walkthrough

The create_default_stream method in the model-to-component factory was updated to set cursor_field to None instead of creating an empty CursorField when the concurrent cursor lacks a cursor field definition, making the field optionally nullable.

Changes

Cohort / File(s) Summary
Cursor field initialization logic
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py
Changed cursor_field construction in create_default_stream from instantiating CursorField(cursor_field_key="") to setting cursor_field=None when the concurrent cursor does not expose a cursor field, altering the type contract to Optional[CursorField]

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: preventing streams without cursor fields from advertising incremental sync mode.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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 and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (1)

2113-2117: Nice fix! This correctly prevents advertising incremental sync for streams without a cursor.

The change properly handles the case where FinalStateCursor (returned when there's no incremental_sync configuration) doesn't have a cursor_field attribute. By returning None instead of an empty CursorField, the DefaultStream.as_airbyte_stream() method's if self._cursor_field: check ensures streams without incremental sync only advertise full_refresh mode and skip the incremental sync mode addition.

Would it be worth adding a test to verify both scenarios (streams with and without incremental_sync advertising correctly)? This could help prevent regressions if the cursor field handling logic changes in the future, wdyt?

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 53c696f and 3c36c8e.

📒 Files selected for processing (1)
  • airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ChristoGrab
Repo: airbytehq/airbyte-python-cdk PR: 58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the `YamlDeclarativeSource` class in `airbyte_cdk/sources/declarative/yaml_declarative_source.py`, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
🧬 Code graph analysis (1)
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (5)
airbyte_cdk/sources/file_based/stream/concurrent/adapters.py (1)
  • cursor_field (133-137)
airbyte_cdk/sources/streams/concurrent/adapters.py (1)
  • cursor_field (189-193)
airbyte_cdk/sources/streams/concurrent/default_stream.py (1)
  • cursor_field (52-53)
airbyte_cdk/sources/streams/concurrent/cursor.py (1)
  • cursor_field (213-214)
airbyte_cdk/sources/streams/core.py (1)
  • cursor_field (363-368)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: Check: destination-motherduck
  • GitHub Check: Check: source-shopify
  • GitHub Check: Check: source-hardcoded-records
  • GitHub Check: Check: source-intercom
  • GitHub Check: Pytest (Fast)
  • GitHub Check: Manifest Server Docker Image Build
  • GitHub Check: SDM Docker Image Build
  • GitHub Check: Pytest (All, Python 3.12, Ubuntu)
  • GitHub Check: Pytest (All, Python 3.10, Ubuntu)
  • GitHub Check: Pytest (All, Python 3.11, Ubuntu)
  • GitHub Check: Pytest (All, Python 3.13, Ubuntu)

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 8, 2026

PyTest Results (Fast)

3 824 tests  ±0   3 812 ✅ ±0   6m 37s ⏱️ +10s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 3c36c8e. ± Comparison against base commit 53c696f.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 8, 2026

PyTest Results (Full)

3 827 tests  ±0   3 815 ✅ ±0   10m 52s ⏱️ -2s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 3c36c8e. ± Comparison against base commit 53c696f.

@aaronsteers Aaron ("AJ") Steers (aaronsteers) merged commit 80bcfc5 into main Jan 8, 2026
30 of 31 checks passed
@aaronsteers Aaron ("AJ") Steers (aaronsteers) deleted the devin/1767906101-fix-incremental-sync-mode-for-streams-without-cursor branch January 8, 2026 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants