feat(declarative): support RequestPath tokens and first-request checks in SyncTokenCursor - #1238

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

• Disable automatic comment, CI, and merge conflict monitoring

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

# Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1792471568-sync-token-request-path#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/1792471568-sync-token-request-path

PyTest Results (Full)

6 110 tests  +97   6 079 ✅ +97   11m 42s ⏱️ + 4m 1s
    1 suites ± 0      31 💤 ± 0 
    1 files   ± 0       0 ❌ ± 0 

Results for commit 2890216. ± Comparison against base commit 7789d58.

♻️ This comment has been updated with latest results.

PyTest Results (Fast)

6 107 tests  +97   6 076 ✅ +97   6m 40s ⏱️ +38s
    1 suites ± 0      31 💤 ± 0 
    1 files   ± 0       0 ❌ ± 0 

Results for commit 2890216. ± Comparison against base commit 7789d58.

♻️ This comment has been updated with latest results.

/prerelease

Prerelease Job Info

This job triggers the publish workflow with default arguments to create a prerelease.

Prerelease job started... Check job output.

✅ Prerelease workflow triggered successfully.

View the publish workflow run: https://github.com/airbytehq/airbyte-python-cdk/actions/runs/39463931382

🤖 This comment was generated by an AI Agent.

Review of e09fd862d5c556f47d0f21fcba446693aca9ee89 against the stacked base 4ced6454cba98c2f18ffc271fe31594c6bd2663d: two P2 findings in the new collision validator. Please address both and add the regression coverage below.

1. [P2] Compare resolved option destinations, not raw template strings

Location: model_to_component_factory.py:2354–2361.

The equality checks can reject a working manifest. Give both options this field expression:

{{ parameters.get('target', 'nextCursor') }}

Then set incremental_sync.$parameters.target: syncToken. The cursor's option resolves to syncToken, while the paginator's option resolves to nextCursor. These are distinct destinations, and the base produces the expected request body:

{"syncToken": "STORED", "nextCursor": "NEXT"}

This PR rejects the manifest before the request because the two expression strings are identical. The first regression test below passes on the base and fails on the PR head.

The reverse gap exists too: a token option with field_name: syncToken and a paginator option with field_path: [syncToken] pass the new validator, then collide at runtime. This second case is incomplete coverage in the new validation; the runtime collision itself also existed before this PR.

Code recommendation:

• Keep structural checks in _validate_sync_token_stream, but move destination comparison to a construction stage where the actual RequestOption components and config are available. create_simple_retriever already has the unwrapped token provider and constructs the paginator.
• Reuse those constructed options so validation uses exactly the same interpolation context as request generation. Do not render both with a shared parameter dictionary or assume the option model's own $parameters is the context: create_request_option uses the parameters argument passed by its caller, and the cursor and paginator construction paths differ.
• Normalize the effective destination to (inject_into, tuple_of_path_segments). A scalar field name syncToken and a one-element field path [syncToken] must produce the same target; preserve nested path segments instead of comparing a dot-joined string.
• A way to avoid duplicating request-option rendering rules is to call the constructed option's inject_into_request on a scratch mapping with a unique sentinel value and the real config, then recover the sentinel's key path. Alternatively, extract/reuse a field-resolution helper from RequestOption. Neither approach needs an HTTP request.
• If a destination cannot be resolved reliably at construction time, do not infer a collision from identical template text; retain the runtime collision check for that case.

2. [P2] Validate active page-size collisions independently of the ignore flag

Location: model_to_component_factory.py:2345–2348 and the remediation at lines 2364–2368.

The new check treats page_token_option and page_size_option alike and recommends ignore_stream_slicer_parameters_on_paginated_requests: true for either conflict. That flag cannot fix a page-size conflict: the paginator sends its page size on the first request, when the stored token is also sent.

Reproduced with CursorPagination.page_size: 100, both page_size_option and token_option targeting body_json.syncToken, and a stored token of STORED. With the recommended flag enabled, construction succeeds, but request one attempts to merge {"syncToken": 100} with {"syncToken": "STORED"} and raises the same collision error. The new remediation therefore sends the author to a configuration that still fails.

Code recommendation: separate the two checks. The following is control-flow pseudocode; resolved_target represents the helper described above:

token_target = resolved_target(token_option, config)

# Only check an active page-size option that actually injects a page size.
if page_size_is_injected and same_known_target(
    token_target, resolved_target(page_size_option, config)
):
    raise ValueError(
        f"Stream {stream_name}: token_option and page_size_option target the "
        "same request field on the first request. Configure distinct fields; "
        "ignore_stream_slicer_parameters_on_paginated_requests cannot fix this."
    )

if same_known_target(token_target, resolved_target(page_token_option, config)):
    if page_token_is_injected_on_first_request:
        # The ignore flag cannot separate these values on request one either.
        raise ValueError(...)
    if not ignore_stream_slicer_parameters:
        # Here the existing first-request-only recommendation is applicable.
        raise ValueError(...)

Keep the existing RequestPath/first-request paginator checks. Preserve the valid Box case: a page-token collision with the ignore flag enabled and no paginator token on request one must still build and sync successfully. Also avoid rejecting an unused page_size_option when no page size is injected.

Reproduction and regression tests

These tests use the existing helpers in test_sync_token_cursor_integration.py; add them there. All three expected-behavior assertions fail on the reviewed head. The distinct-rendered-fields test passes on the stacked base.

def _review_retriever(manifest):
    source, _ = _source_and_catalog(manifest)
    stream = source.streams(config={})[0]
    return stream._stream_partition_generator._partition_factory._retriever


def test_distinct_rendered_fields_are_allowed():
    from airbyte_cdk.sources.types import StreamSlice

    manifest = _manifest()
    stream = manifest["streams"][0]
    options = (
        stream["incremental_sync"]["token_option"],
        stream["retriever"]["paginator"]["page_token_option"],
    )
    for option in options:
        option["field_name"] = "{{ parameters.get('target', 'nextCursor') }}"
    stream["incremental_sync"]["$parameters"] = {"target": "syncToken"}

    retriever = _review_retriever(manifest)
    assert retriever._request_body_json(
        stream_slice=StreamSlice(partition={}, cursor_slice={"token": "STORED"}),
        next_page_token={"next_page_token": "NEXT"},
    ) == {"syncToken": "STORED", "nextCursor": "NEXT"}


def test_page_size_collision_is_rejected_even_with_ignore_flag():
    manifest = _manifest()
    retriever_model = manifest["streams"][0]["retriever"]
    retriever_model["ignore_stream_slicer_parameters_on_paginated_requests"] = True
    paginator = retriever_model["paginator"]
    paginator["page_size_option"] = {
        "type": "RequestOption",
        "inject_into": "body_json",
        "field_name": "syncToken",
    }
    paginator["pagination_strategy"]["page_size"] = 100

    with pytest.raises(ValueError):
        _review_retriever(manifest)


def test_equivalent_field_name_and_path_are_rejected():
    manifest = _manifest()
    paginator = manifest["streams"][0]["retriever"]["paginator"]
    paginator["page_token_option"] = {
        "type": "RequestOption",
        "inject_into": "body_json",
        "field_path": ["syncToken"],
    }
    with pytest.raises(ValueError):
        _review_retriever(manifest)

For the final tests, assert the relevant error text as well as ValueError. Extend coverage to both directions of the field_name/field_path comparison, distinct expressions that resolve to the same field, nested JSON paths, page-size conflicts with the flag both off and on, and nonconflicting options. Keep the existing Box success test green.

Suggested change scope:

airbyte_cdk/sources/declarative/
  parsers/model_to_component_factory.py       — resolve destinations; separate validation rules
  requesters/request_option.py               — only if extracting a shared resolution helper
unit_tests/sources/declarative/incremental/
  test_sync_token_cursor_integration.py       — add the regression cases above

No state-format, cursor commit/reset, URL replay, or schema changes are needed for these findings.

Validation already performed: 85 focused tests passed across the sync-token cursor, integration, and request-options-provider suites. The three additional reproduction cases fail on the PR head; the valid-template case passes on the base. The MotherDuck CI failure is unrelated: its job cannot retrieve an enabled credential-secret version.

👍 On it. Both findings are valid. Here's the plan:

1. Remove the raw-template comparison from _validate_sync_token_stream. Collisions will instead be checked in create_simple_retriever, using the constructed RequestOption components and the real config. A new RequestOption.resolve_target will find each option's actual destination by injecting a sentinel value, then compare (inject_into, path segments). That way syncToken and [syncToken] count as the same target, and nested paths keep their segments. If a destination can't be resolved at build time, the check is skipped and the runtime collision check still applies.
2. Split the rules. A page-size collision (when a page size is actually injected) is rejected even when ignore_stream_slicer_parameters_on_paginated_requests is on. A page-token collision is rejected when the paginator sends a token on the first request. Otherwise, a page-token collision without the flag gets the existing first-request-only message. The Box case (flag on, no first-request page token) still builds.

I'll add your three tests, plus the extra cases you listed, with error-text assertions.

Posted by Devin, as requested by Patrick Nilan (@pnilan).

☑️ Resolved in 6f91913. Collisions are now checked against the resolved destinations of the constructed options, and the page-size and page-token rules are separate.

• RequestOption.resolve_target(config) returns the key path that inject_into_request actually writes, so field_name: syncToken and field_path: [syncToken] count as the same target and nested segments are kept. The factory compares (inject_into, path) after it builds the retriever. If a destination can't be resolved at build time, the check is skipped and the runtime check still applies.
• An injected page_size_option on the token's field is rejected even when the ignore flag is on. A page_token_option on the token's field is rejected when the paginator sends a token on the first request. Otherwise it's rejected only when the ignore flag is off. The Box configuration still builds, and the T-FIRSTREQ check is unchanged.
• Tests: your three cases, both field-name/path directions, different expressions that resolve to the same field, nested paths, page size with the flag on and off, an unused page-size option, and nonconflicting options. Error text is asserted with match=. Your three tests fail on e09fd862 and pass now.

Posted by Devin, as requested by Patrick Nilan (@pnilan).