airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (3)

2093-2098: Scope the zero-out to concurrent mode and add a debug trace for observability, wdyt?

The intent is clear and aligns with restoring the Concurrent CDK behavior. To make this safer and self-documenting, could we:

• Only zero out when a concurrent cursor is actually being used; and
• Emit a debug log so operators understand why SimpleRetriever.cursor is being nulled?

-            # FIXME to be removed once we migrate everything to DefaultStream
-            if isinstance(retriever, SimpleRetriever):
-                # We zero it out here, but since this is a cursor reference, the state is still properly
-                # instantiated for the other components that reference it
-                retriever.cursor = None
+            # FIXME to be removed once we migrate everything to DefaultStream
+            # When running in concurrent mode, the concurrent cursor owns state progression.
+            # Ensure the SimpleRetriever does not attempt to process a declarative cursor.
+            if isinstance(retriever, SimpleRetriever) and isinstance(concurrent_cursor, ConcurrentCursor):
+                logging.getLogger(f"airbyte.{stream_name}").debug(
+                    "Zeroing SimpleRetriever.cursor; ConcurrentCursor manages state for stream '%s'.",
+                    stream_name,
+                )
+                # We zero it out here, but since this is a cursor reference, the state instance remains
+                # available to other components that have their own references.
+                retriever.cursor = None

---

2093-2098: Add a regression test to lock in the behavior, wdyt?

A small test would catch future regressions where SimpleRetriever starts observing/closing slices in concurrent flows. For example:

• Build a declarative stream with a DatetimeBasedCursor that resolves to a ConcurrentCursor.
• Assert that in the produced DefaultStream, the underlying SimpleRetriever has cursor is None while DefaultStream.cursor is a ConcurrentCursor.

If helpful, I can draft a test skeleton targeting create_declarative_stream and asserting the invariants above. Want me to push that?

---

1956-1961: Double-check: partition_field_start set from partition_field_end looks unintended, wdyt?

This block configures DatetimeBasedRequestOptionsProvider for DatetimeBasedCursorModel, but both partition fields are sourced from cursor_model.partition_field_end. Should partition_field_start come from cursor_model.partition_field_start instead?

             request_options_provider = DatetimeBasedRequestOptionsProvider(
                 start_time_option=start_time_option,
                 end_time_option=end_time_option,
-                partition_field_start=cursor_model.partition_field_end,
-                partition_field_end=cursor_model.partition_field_end,
+                partition_field_start=cursor_model.partition_field_start,
+                partition_field_end=cursor_model.partition_field_end,
                 config=config,
                 parameters=model.parameters or {},
             )

If this is intentional (e.g., a specific API quirk), could we add a short comment to prevent future “fixes”?

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.