[devin-ai-integration]

TL;DR

• What: manifest propagation (propagate_types_and_parameters) used to deep-copy the component and its parameters at every recursion level. It now copies once and then walks the tree in place.
• Why: to speed up building every declarative source (check, discover, read, and Connector Builder requests). Over the 518 manifest-only manifests on airbytehq/airbyte master, propagation takes about 1.0 s in total instead of 14.8 s. p90 per manifest goes from 57 ms to 1.7 ms.
• What stays the same: the propagated output is identical to main on all 518 manifests, both sorted and unsorted (so key order is unchanged). The public method still returns a copy and never mutates its input.
• Also changed: the resolver now splits YAML aliases inside config_overrides, so they are no longer one shared object that the in-place walk could change in two places.
• Risk: low. The public signature is unchanged and the dynamic-stream paths still use the copying entry point. The only output difference is in a shape no fleet template uses (details below).

Summary

ManifestComponentTransformer.propagate_types_and_parameters deep-copied the component and the parent parameters at every recursion level, so a node at depth d was copied d times. This PR copies once and walks in place. The output of the first pass is the same as on main, key order included. Dynamic-stream re-propagation is also the same, except in one shape that no template in the fleet uses: a stream template with a list-valued $parameters entry that gets written onto a field. On main the second pass also rewrote $parameters[k]; on this PR it keeps its first-pass content. Component fields are the same either way.

• propagate_types_and_parameters (public) keeps its contract: it copies the input once with _copy_tree, which rebuilds every dict and list with no memo so any shared containers in the input are split. Then it calls the new private _propagate_in_place. All recursion now goes to _propagate_in_place, which does no per-level copying.
• Inside _propagate_in_place:
  • current_parameters = dict(parent_parameters) is now a shallow copy, and parameter values are never mutated in place.
  • A parameter value is copy.deepcopy'd when it is written onto a missing or falsy component field.
  • The stored $parameters is copy.deepcopy(current_parameters), so no two components share a container.
• ConcurrentDeclarativeSource._pre_process_manifest and legacy ManifestDeclarativeSource._pre_process_manifest call _propagate_in_place directly on the ManifestReferenceResolver output. That output is built fresh, and the script below checked that it has no shared containers. The dynamic-stream paths still use the copying public entry point.
• ManifestReferenceResolver now copies _FIELDS_HOLDING_CONFIG_VALUES (config_overrides) with _copy_without_sharing instead of deepcopy. Before, a YAML alias stayed one shared object after the copy, so the in-place walk could change it in both places.

List-valued parameter edge case (kept as today). Take $parameters: {transformations: [{type: AddFields, fields: []}]} on a parent. On main, the field written from the parameter and the entry in current_parameters are the same list. The in-place field_value[i] = ... therefore also leaves propagated elements in the restored parameter, which then shows up in that component's $parameters and in the parameters of later sibling fields. Deep-copying alone would change this output, so a list field that was written from parameters at this level now puts the propagated list back into current_parameters when the excluded parameter is restored:

current_parameters[field_name] = field_value if field_name in written_from_parameters else excluded_parameter

Dict-valued parameters keep restoring the original value, because today the child replaces the field with a copy.

Resolves https://github.com/airbytehq/airbyte-internal-issues/issues/18066:

• https://github.com/airbytehq/airbyte-internal-issues/issues/18066

Close on merge: yes — internally filed, no customer to confirm with.

Part of https://github.com/airbytehq/airbyte-internal-issues/issues/17920.

Verification

Byte-identical output across the fleet. The script ran on every manifest-only source on airbytehq/airbyte origin/master (518 manifests). For each one it compared three outputs, both as the sha256 of json.dumps(sort_keys=True) and as unsorted JSON (to catch key-order changes): the old transformer, the new public method, and the new _propagate_in_place run on fresh resolver output. It also checked that the input was not mutated and that the resolver output has no shared dict or list objects.

total=518 identical=518 mismatches=0 errors=0
key-order identical=518 mismatches=0
mutated inputs: []
shared resolver outputs: []
old:          total=14.83s median=7.6ms  p90=48.1ms
new-in-place: total=1.13s  median=0.33ms p90=1.7ms

Timings are local, propagation only, one run each. Slowest sources before: github-g2 1.18 s, gitlab 0.85 s, github 0.72 s, stripe 0.53 s. After: gitlab 0.12 s, github-g2 0.08 s.

New tests in test_manifest_component_transformer.py:

• Equivalence cases with literal expected output generated from main: $parameters at several levels, use_parent_parameters, JSON-schema objects (including a list type and a properties map with a property named type), untyped objects with nested components, and the list-valued parameter case for a missing field and a falsy field.
• The input is not mutated.
• No container in the output is shared with another component or with the input.
• _propagate_in_place gives the same result as the public method.
• _propagate_in_place keeps the input tree's identities. This test fails if the recursion goes back through the public method.
• A component's own non-empty list field named like a $parameters key (the restore arm with the excluded parameter). This test fails if that arm is changed to restore the propagated field instead.
• Resolver: aliases in config_overrides are not shared after resolution.

With the list-restore rule removed, the list case fails as expected:

{'$parameters': {'name': 'users', 'transformations': [{'fields': [], 'type': 'AddFields'}]}} != {'$parameters': {'name': 'users', 'transformations': [{'$parameters': {'name': 'users'}, 'fields': [], 'name': 'users', 'type': 'AddFields'}]}}

Local checks:

• unit_tests/sources/declarative, unit_tests/connector_builder and the entrypoint tests: 2535 passed, 1 skipped.
• ruff check and ruff format: clean.
• mypy on the changed files: clean.

Test Coverage

Unit tests are added as described above. They were checked to fail without the list-parameter handling.

Declarative-First Evaluation

Not applicable. This is a CDK-internal performance refactor, not a connector change.

Link to Devin session: https://app.devin.ai/sessions/a8bcdc45129844f0809397e236bb2391
Open in Devin Desktop: https://app.devin.ai/desktop/session/a8bcdc45129844f0809397e236bb2391?variant=devin

[devin-ai-integration]

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

[github-actions]

👋 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@devin/1792686381-propagate-copy-once#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/1792686381-propagate-copy-once

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

• CDK API Reference

📝 Edit this welcome message.

[github-actions]

PyTest Results (Fast)

5 877 tests  +13   5 846 ✅ +13   5m 41s ⏱️ + 1m 2s
    1 suites ± 0      31 💤 ± 0 
    1 files   ± 0       0 ❌ ± 0 

Results for commit d375a7e. ± Comparison against base commit c60b837.

♻️ This comment has been updated with latest results.

[github-actions]

PyTest Results (Full)

5 880 tests  +13   5 849 ✅ +13   10m 21s ⏱️ -15s
    1 suites ± 0      31 💤 ± 0 
    1 files   ± 0       0 ❌ ± 0 

Results for commit d375a7e. ± Comparison against base commit c60b837.

♻️ This comment has been updated with latest results.

[Patrick Nilan (pnilan)]

🤖 This review was generated by an AI Agent.

Review: fix before merge

This change is correct. Propagation across the 539 monorepo manifests drops from 9.21 s to 0.53 s with identical output. Please address the three items under Before merge, then the low-effort items after them. None of them changes any connector's behaviour today.

Breaking change: none. I ran main (c60b837d) and this PR (6860044c) side by side on every manifest.yaml in airbytehq/airbyte at 2720fd1a:

Check	Result
Transformer first pass, value and key order	539 / 539 identical
Dynamic-stream re-propagation (14 templates in 9 manifests)	14 / 14 identical
Full ConcurrentDeclarativeSource build, migrate and normalize off	530 identical, 9 fail identically on both
Same build, migrate and normalize on (migrated_at removed before comparing)	528 identical, 11 fail identically on both
Caller's manifest changed by the build	0 on main, 0 on this PR

The PR's own fleet check hashed json.dumps(sort_keys=True), which cannot detect key-order changes. The rows above compare unsorted output.

---

Before merge

1. Add a test for the else arm of the list-restore rule (P1)

manifest_component_transformer.py#L213-L216

The tests cover only the field_value arm (PROPAGATION_CASES[5]). The else excluded_parameter arm runs when a component's own non-empty list field has the same name as a $parameters key. That happens 758 times across the fleet, and no test reaches it.

To check, replace the line with current_parameters[field_name] = field_value. All 32 transformer tests still pass, and so do the declarative, legacy, migration and Connector Builder suites. Yet the propagated manifest changes for 12 connectors, including nested primary_key field values in source-hubspot and source-google-ads.

Values that change per connector under that mutation

Connector	Component field values	$parameters values
source-hubspot	842	878
source-google-ads	528	576
source-gitlab	0	172
source-jira	0	162
source-tiktok-marketing	0	156
source-railz	0	96
source-mixpanel	0	34
source-stripe	0	21
source-marketo	0	12
source-mailchimp	0	8
source-slack	0	7
source-google-search-console	0	6

The field-value changes are nested primary_key fields. Elsewhere the change lands in the {{ parameters.* }} interpolation context, mostly AddedFieldDefinition.$parameters.path.

Add this PROPAGATION_CASES entry. The expected output is main's. It passes on this PR and fails under the mutation.

    (
        {
            "type": "DeclarativeStream",
            "$parameters": {"primary_key": ["param_pk"]},
            "primary_key": ["own_pk"],
            "retriever": {},
        },
        {},
        None,
        {
            "$parameters": {"primary_key": ["param_pk"]},
            "primary_key": ["own_pk"],
            "retriever": {
                "$parameters": {"primary_key": ["param_pk"]},
                "primary_key": ["param_pk"],
                "type": "SimpleRetriever",
            },
            "type": "DeclarativeStream",
        },
    ),

2. Copy config_overrides values in the resolver without sharing objects (P2)

concurrent_declarative_source.py#L314-L317 · legacy manifest_declarative_source.py#L189-L192 · cause in manifest_reference_resolver.py#L118

The new comment says the resolver output "contains no shared containers". That holds everywhere except under config_overrides. The resolver copies those values with deepcopy, which keeps a YAML alias as one shared object. _propagate_in_place then edits the shared object at its first location, and the edit shows up at the second. This happens even where propagation never visits the second location.

Repro through ConcurrentDeclarativeSource:

check:
  type: CheckStream
  stream_names: [s]
  config_overrides:
    a: &x {type: oauth, $parameters: {region: eu}}
    b: {nested: *x}

Build	config_overrides.b.nested after the build
main	{"$parameters": {"region": "eu"}, "type": "oauth"}
this PR	{"$parameters": {"region": "eu"}, "region": "eu", "type": "oauth"}
this PR with the fix below	{"$parameters": {"region": "eu"}, "type": "oauth"}

Impact today: none.

• Only source-github and source-github-g2 set config_overrides, each to one scalar.
• Builder and Cloud manifests arrive as JSON, which has no aliases.
• The precondition is guarded only indirectly: a resolver that memoizes $ref results fails test_read_with_concurrent_and_synchronous_streams with assert 21 == 9. Nothing guards the legacy call site.

Fix in manifest_reference_resolver.py:

def _copy_without_sharing(value: Any) -> Any:
    if isinstance(value, dict):
        return {k: _copy_without_sharing(v) for k, v in value.items()}
    if isinstance(value, list):
        return [_copy_without_sharing(v) for v in value]
    return deepcopy(value)

-                k: deepcopy(v)
+                k: _copy_without_sharing(v)
                 if k in _FIELDS_HOLDING_CONFIG_VALUES

Then add a resolver test. It fails on the current resolver and passes with the fix:

import yaml

from airbyte_cdk.sources.declarative.parsers.manifest_reference_resolver import (
    ManifestReferenceResolver,
)


def test_config_overrides_aliases_are_not_shared():
    manifest = yaml.safe_load(
        """
check:
  type: CheckStream
  stream_names: [s]
  config_overrides:
    a: &x {type: oauth, $parameters: {region: eu}}
    b: {nested: *x}
"""
    )
    overrides = ManifestReferenceResolver().preprocess_manifest(manifest)["check"]["config_overrides"]
    assert overrides["a"] == overrides["b"]["nested"]
    assert overrides["a"] is not overrides["b"]["nested"]

3. Add a test that guards the speed-up (P2)

manifest_component_transformer.py#L191, #L207, #L245

Suppose the recursion went back through propagate_types_and_parameters, or _pre_process_manifest went back to the public method. The per-level copying would return, every test would stay green, and the output would not change. The sibling perf PRs in this epic ship this kind of guard: #1224 asserts a call never happens, and #1223 spies on CSafeLoader.

This test passes on this PR and fails when the three recursion sites call the public method:

def test_propagate_in_place_reuses_the_input_tree():
    stream = {"type": "DeclarativeStream", "retriever": {"requester": {}}}
    tree = {"type": "DeclarativeSource", "streams": [stream]}
    retriever, requester = stream["retriever"], stream["retriever"]["requester"]
    out = ManifestComponentTransformer()._propagate_in_place("", tree, {})
    assert out is tree and out["streams"][0] is stream
    assert out["streams"][0]["retriever"] is retriever
    assert out["streams"][0]["retriever"]["requester"] is requester

---

Also in this PR (low effort)

4. Narrow the "output stays the same" claim in the PR body (P3).

• The claim holds for every source build.
• On dynamic-stream re-propagation it holds except for $parameters contents:
  • On main, a list-valued parameter written onto a field stayed the same object as $parameters[k].
  • The resolvers' deepcopy kept that sharing, so main's second pass rewrote both.
  • This PR shares nothing, so $parameters[k] keeps its first-pass content. Component fields are identical either way.
• No fleet template has this shape (0 of 14).
• Suggested wording: "First pass identical. Dynamic re-propagation identical unless a stream template's list-valued $parameters entry is written onto a field."

5. Pass dict(resolved_manifest) at both call sites (P3). When a manifest's root is a $ref to a list or a string, the resolver returns a non-dict.

• Main coerced it with dict(...) and raised a schema ValidationError (empty list) or a ValueError (string).
• This PR raises TypeError: pop expected at most 1 argument, got 2 or AttributeError, which is harder to act on in the Connector Builder.

 propagated_manifest = ManifestComponentTransformer()._propagate_in_place(
-    "", resolved_manifest, {}
+    "", dict(resolved_manifest), {}
 )

6. Test hygiene (P3).

• test_list_parameter_fills_missing_stream_field picks its case with PROPAGATION_CASES[5], and the parametrize has no ids, so a failure reads component5-parameters5-None-expected5. Bind the case to a named constant and use pytest.param(..., id="list-param-falsy-field").
• test_propagation_invalid_parameters_preserves_input still passes when _copy_tree copies nothing. The TypeError fires at the root, which the shallow dict(...) already protects. Put the bad $parameters on a nested stream instead.

Not needed

• A prerelease or pinned connector run. The dict the component factory builds from is byte-identical for every monorepo connector.

CI notes

• Check: destination-motherduck fails while fetching a CI secret (No enabled version found). It fails the same way on #1240, #1241, #1242, #1247 and #1248, so this PR didn't cause it.
• Validate PR title doesn't run on drafts. The fix(declarative): title is the right type and releases as a patch.

[devin-ai-integration]

Patrick Nilan (@pnilan), thanks for the thorough review. All six items are addressed in d375a7e.

1. Restore arm: added a list-param-existing-truthy-field case using your expected output, which matches the old transformer. It fails when the restore line is changed to always use field_value.
2. Resolver: config_overrides values are now copied with _copy_without_sharing. Added test_config_overrides_break_yaml_aliases, which fails on the old resolver.
3. Speed-up guard: added test_propagate_in_place_reuses_the_input_tree. It fails when the recursion goes back through the public method.
4. PR body: I narrowed the "output stays the same" claim to cover the dynamic re-propagation case. The fleet check now also compares unsorted JSON: 518/518 identical, key order included.
5. Call sites: both _pre_process_manifest call sites now pass dict(resolved_manifest).
6. Test hygiene:
   • The propagation cases have pytest.param ids, and the list case is bound to LIST_PARAMETER_CASE.
   • The invalid-parameters test now puts the bad $parameters on a nested stream. It fails if _copy_tree copies nothing.

Locally, the declarative and connector builder suites pass (2486 passed), and ruff and mypy are clean.

Written by Devin