diff --git a/python/packages/core/agent_framework/_serialization.py b/python/packages/core/agent_framework/_serialization.py index cb7f3a0db89..8bf93da8b22 100644 --- a/python/packages/core/agent_framework/_serialization.py +++ b/python/packages/core/agent_framework/_serialization.py @@ -286,21 +286,33 @@ def __init__(self, **kwargs): _SHALLOW_COPY_FIELDS: ClassVar[set[str]] = {"raw_representation"} def __deepcopy__(self, memo: dict[int, Any]) -> SerializationMixin: - """Create a deep copy, preserving ``_SHALLOW_COPY_FIELDS`` by reference. + """Create a deep copy of this instance. - Fields listed in ``_SHALLOW_COPY_FIELDS`` may contain LLM SDK objects - (e.g., proto/gRPC responses) that are not safe to deep-copy. They are - kept as shallow references in the copy; all other attributes are - deep-copied normally. + Fields listed in ``_SHALLOW_COPY_FIELDS`` (typically ``raw_representation``) + may hold LLM SDK objects that are not deep-copyable. When deepcopy of such a + field fails, the clone gets ``None`` for that field and a warning is logged, + rather than sharing a shallow reference that would obscure deepcopy semantics + (#7851). All other attributes are deep-copied normally. """ cls = type(self) result = cls.__new__(cls) memo[id(self)] = result + unsafe = cls._SHALLOW_COPY_FIELDS for k, v in self.__dict__.items(): - if k in cls._SHALLOW_COPY_FIELDS: - object.__setattr__(result, k, v) - else: + try: object.__setattr__(result, k, copy.deepcopy(v, memo)) + except Exception as exc: + if k not in unsafe: + raise + logger.warning( + "Discarding non-deep-copyable field %r on %s during deepcopy " + "(clone will use None). Original error: %s: %s", + k, + cls.__name__, + type(exc).__name__, + exc, + ) + object.__setattr__(result, k, None) return result def to_dict(self, *, exclude: set[str] | None = None, exclude_none: bool = True) -> dict[str, Any]: diff --git a/python/packages/core/agent_framework/_types.py b/python/packages/core/agent_framework/_types.py index 624e5aa8d79..435471c0221 100644 --- a/python/packages/core/agent_framework/_types.py +++ b/python/packages/core/agent_framework/_types.py @@ -589,20 +589,33 @@ def __init__( self.consent_link = consent_link def __deepcopy__(self, memo: dict[int, Any]) -> Content: - """Create a deep copy, preserving ``_SHALLOW_COPY_FIELDS`` by reference. + """Create a deep copy of this content item. - Fields listed in ``_SHALLOW_COPY_FIELDS`` may contain LLM SDK objects - (e.g., proto/gRPC responses) that are not safe to deep-copy. + Fields listed in ``_SHALLOW_COPY_FIELDS`` (currently ``raw_representation``) + may hold LLM SDK objects that are not deep-copyable. When deepcopy of such a + field fails, the clone gets ``None`` for that field and a warning is logged, + rather than sharing a shallow reference that would obscure deepcopy semantics + (#7851). """ cls = type(self) result = cls.__new__(cls) memo[id(self)] = result - shallow = cls._SHALLOW_COPY_FIELDS + unsafe = cls._SHALLOW_COPY_FIELDS for k, v in self.__dict__.items(): - if k in shallow: - object.__setattr__(result, k, v) - else: + try: object.__setattr__(result, k, deepcopy(v, memo)) + except Exception as exc: + if k not in unsafe: + raise + logger.warning( + "Discarding non-deep-copyable field %r on %s during deepcopy " + "(clone will use None). Original error: %s: %s", + k, + cls.__name__, + type(exc).__name__, + exc, + ) + object.__setattr__(result, k, None) return result @classmethod diff --git a/python/packages/core/tests/core/test_serializable_mixin.py b/python/packages/core/tests/core/test_serializable_mixin.py index 8cbebdd7927..1b0b9d1690d 100644 --- a/python/packages/core/tests/core/test_serializable_mixin.py +++ b/python/packages/core/tests/core/test_serializable_mixin.py @@ -471,8 +471,8 @@ def __init__(self, value: str, options: dict | None = None): assert obj.options["existing"] == "value" assert obj.options["injected"] == "option" - def test_deepcopy_preserves_shallow_copy_fields_by_reference(self): - """Test that deepcopy keeps _SHALLOW_COPY_FIELDS fields as shallow references.""" + def test_deepcopy_discards_non_copyable_shallow_copy_fields(self, caplog: pytest.LogCaptureFixture): + """Non-copyable _SHALLOW_COPY_FIELDS values are discarded with a warning (#7851).""" import copy class NonCopyable: @@ -491,14 +491,17 @@ def __init__(self, items: list, raw_representation: Any = None, other_opaque: An opaque = NonCopyable() original_items = ["a", "b"] obj = TestClass(items=original_items, raw_representation=raw, other_opaque=opaque) - cloned = copy.deepcopy(obj) - # _SHALLOW_COPY_FIELDS fields should be the same object (shallow copy) - assert cloned.raw_representation is raw - assert cloned.other_opaque is opaque - # Normal attributes should be independent copies + with caplog.at_level(logging.WARNING, logger="agent_framework"): + cloned = copy.deepcopy(obj) + + assert cloned.raw_representation is None + assert cloned.other_opaque is None + assert obj.raw_representation is raw + assert obj.other_opaque is opaque assert cloned.items is not original_items assert cloned.items == ["a", "b"] + assert len([r for r in caplog.records if "Discarding non-deep-copyable field" in r.getMessage()]) == 2 def test_deepcopy_deep_copies_non_shallow_copy_fields(self): """Test that deepcopy fully copies fields not in _SHALLOW_COPY_FIELDS.""" @@ -512,14 +515,15 @@ def __init__(self, items: list, raw_representation: Any = None): self.raw_representation = raw_representation original_list = ["a", "b"] - obj = TestClass(items=original_list, raw_representation="raw") + obj = TestClass(items=original_list, raw_representation={"raw": True}) cloned = copy.deepcopy(obj) # list should be a new object assert cloned.items is not original_list assert cloned.items == ["a", "b"] - # raw_representation should be the same object - assert cloned.raw_representation is obj.raw_representation + # copyable raw_representation is deep-copied + assert cloned.raw_representation == {"raw": True} + assert cloned.raw_representation is not obj.raw_representation def test_deepcopy_deep_copies_default_exclude_fields(self): """Test that DEFAULT_EXCLUDE fields are deep-copied unless also in _SHALLOW_COPY_FIELDS.""" @@ -540,8 +544,10 @@ def __init__(self, items: list, additional_properties: dict | None = None): assert cloned.additional_properties is not original_props assert cloned.additional_properties == {"key": "value"} - def test_deepcopy_shallow_copy_fields_override_default_exclude(self): - """Test that _SHALLOW_COPY_FIELDS controls deepcopy independently of DEFAULT_EXCLUDE.""" + def test_deepcopy_discards_non_copyable_fields_even_if_default_excluded( + self, caplog: pytest.LogCaptureFixture + ): + """_SHALLOW_COPY_FIELDS discard-on-failure applies independently of DEFAULT_EXCLUDE (#7851).""" import copy class NonCopyable: @@ -560,13 +566,13 @@ def __init__(self, items: list, opaque: Any = None, additional_properties: dict opaque = NonCopyable() original_props = {"key": "value"} obj = TestClass(items=["a"], opaque=opaque, additional_properties=original_props) - cloned = copy.deepcopy(obj) - # Field in both DEFAULT_EXCLUDE and _SHALLOW_COPY_FIELDS: shallow-copied - assert cloned.opaque is opaque - # Field in DEFAULT_EXCLUDE only: deep-copied + with caplog.at_level(logging.WARNING, logger="agent_framework"): + cloned = copy.deepcopy(obj) + + assert cloned.opaque is None assert cloned.additional_properties is not original_props assert cloned.additional_properties == {"key": "value"} - # Normal field: deep-copied assert cloned.items is not obj.items assert cloned.items == ["a"] + assert any("opaque" in record.getMessage() for record in caplog.records) diff --git a/python/packages/core/tests/core/test_types.py b/python/packages/core/tests/core/test_types.py index fc8118680f5..30d147f2434 100644 --- a/python/packages/core/tests/core/test_types.py +++ b/python/packages/core/tests/core/test_types.py @@ -2,6 +2,7 @@ import base64 import json +import logging from collections.abc import AsyncIterable, Sequence from dataclasses import dataclass from datetime import datetime, timezone @@ -2440,36 +2441,52 @@ def __deepcopy__(self, memo: dict) -> Any: raise TypeError("Cannot deepcopy this object") -def test_content_deepcopy_preserves_raw_representation(): - """Test that deepcopy of Content keeps raw_representation by reference.""" +def test_content_deepcopy_discards_non_copyable_raw_representation(caplog: pytest.LogCaptureFixture): + """Non-copyable raw_representation is dropped (None) with a warning, not shallow-shared (#7851).""" import copy raw = _NonCopyableRaw() content = Content.from_text("hello", raw_representation=raw) - cloned = copy.deepcopy(content) + with caplog.at_level(logging.WARNING, logger="agent_framework"): + cloned = copy.deepcopy(content) assert cloned.text == "hello" - assert cloned.raw_representation is raw + assert cloned.raw_representation is None + assert content.raw_representation is raw assert cloned.additional_properties is not content.additional_properties + assert any("raw_representation" in record.getMessage() for record in caplog.records) -def test_message_deepcopy_preserves_raw_representation(): - """Test that deepcopy of Message keeps raw_representation by reference.""" +def test_content_deepcopy_preserves_copyable_raw_representation(): + """Copyable raw_representation values are deep-copied normally.""" + import copy + + content = Content.from_text("hello", raw_representation={"provider": "test"}) + cloned = copy.deepcopy(content) + + assert cloned.raw_representation == {"provider": "test"} + assert cloned.raw_representation is not content.raw_representation + + +def test_message_deepcopy_discards_non_copyable_raw_representation(caplog: pytest.LogCaptureFixture): + """Non-copyable Message.raw_representation is discarded on deepcopy (#7851).""" import copy raw = _NonCopyableRaw() msg = Message("assistant", ["hello"], raw_representation=raw) - cloned = copy.deepcopy(msg) + with caplog.at_level(logging.WARNING, logger="agent_framework"): + cloned = copy.deepcopy(msg) assert cloned.text == "hello" - assert cloned.raw_representation is raw + assert cloned.raw_representation is None assert cloned.contents is not msg.contents + assert any("raw_representation" in record.getMessage() for record in caplog.records) -def test_agent_response_deepcopy_preserves_raw_representation(): - """Test that deepcopy of AgentResponse keeps raw_representation by reference.""" +def test_agent_response_deepcopy_discards_non_copyable_raw_representation(caplog: pytest.LogCaptureFixture): + """Non-copyable AgentResponse.raw_representation is discarded on deepcopy (#7851).""" import copy raw = _NonCopyableRaw() @@ -2478,15 +2495,16 @@ def test_agent_response_deepcopy_preserves_raw_representation(): raw_representation=raw, ) - cloned = copy.deepcopy(response) + with caplog.at_level(logging.WARNING, logger="agent_framework"): + cloned = copy.deepcopy(response) assert cloned.text == "test" - assert cloned.raw_representation is raw + assert cloned.raw_representation is None assert cloned.messages is not response.messages -def test_chat_response_deepcopy_preserves_raw_representation(): - """Test that deepcopy of ChatResponse keeps raw_representation by reference.""" +def test_chat_response_deepcopy_discards_non_copyable_raw_representation(caplog: pytest.LogCaptureFixture): + """Non-copyable ChatResponse.raw_representation is discarded on deepcopy (#7851).""" import copy raw = _NonCopyableRaw() @@ -2495,15 +2513,18 @@ def test_chat_response_deepcopy_preserves_raw_representation(): raw_representation=raw, ) - cloned = copy.deepcopy(response) + with caplog.at_level(logging.WARNING, logger="agent_framework"): + cloned = copy.deepcopy(response) assert cloned.text == "test" - assert cloned.raw_representation is raw + assert cloned.raw_representation is None assert cloned.messages is not response.messages -def test_chat_response_update_deepcopy_preserves_raw_representation(): - """Test that deepcopy of ChatResponseUpdate keeps raw_representation by reference.""" +def test_chat_response_update_deepcopy_discards_non_copyable_raw_representation( + caplog: pytest.LogCaptureFixture, +): + """Non-copyable ChatResponseUpdate.raw_representation is discarded on deepcopy (#7851).""" import copy raw = _NonCopyableRaw() @@ -2513,15 +2534,18 @@ def test_chat_response_update_deepcopy_preserves_raw_representation(): raw_representation=raw, ) - cloned = copy.deepcopy(update) + with caplog.at_level(logging.WARNING, logger="agent_framework"): + cloned = copy.deepcopy(update) assert cloned.text == "hello" - assert cloned.raw_representation is raw + assert cloned.raw_representation is None assert cloned.contents is not update.contents -def test_agent_response_update_deepcopy_preserves_raw_representation(): - """Test that deepcopy of AgentResponseUpdate keeps raw_representation by reference.""" +def test_agent_response_update_deepcopy_discards_non_copyable_raw_representation( + caplog: pytest.LogCaptureFixture, +): + """Non-copyable AgentResponseUpdate.raw_representation is discarded on deepcopy (#7851).""" import copy raw = _NonCopyableRaw() @@ -2531,15 +2555,16 @@ def test_agent_response_update_deepcopy_preserves_raw_representation(): raw_representation=raw, ) - cloned = copy.deepcopy(update) + with caplog.at_level(logging.WARNING, logger="agent_framework"): + cloned = copy.deepcopy(update) assert cloned.text == "hello" - assert cloned.raw_representation is raw + assert cloned.raw_representation is None assert cloned.contents is not update.contents -def test_nested_deepcopy_preserves_raw_representation(): - """Test that deepcopy of an AgentResponse with nested Message raw_representations works.""" +def test_nested_deepcopy_discards_non_copyable_raw_reps(caplog: pytest.LogCaptureFixture): + """Nested Message/AgentResponse non-copyable raw_reps are discarded (#7851).""" import copy raw_msg = _NonCopyableRaw() @@ -2549,27 +2574,28 @@ def test_nested_deepcopy_preserves_raw_representation(): raw_representation=raw_response, ) - cloned = copy.deepcopy(response) + with caplog.at_level(logging.WARNING, logger="agent_framework"): + cloned = copy.deepcopy(response) - assert cloned.raw_representation is raw_response - assert cloned.messages[0].raw_representation is raw_msg + assert cloned.raw_representation is None + assert cloned.messages[0].raw_representation is None assert cloned.messages is not response.messages assert cloned.text == "hello" + assert len([r for r in caplog.records if "raw_representation" in r.getMessage()]) >= 2 -def test_content_deepcopy_shallow_copy_fields_identity(): - """Test that Content._SHALLOW_COPY_FIELDS fields are identity-preserved while others are deep-copied.""" +def test_content_deepcopy_unsafe_fields_discarded_others_deep_copied(caplog: pytest.LogCaptureFixture): + """Unsafe fields become None; other fields remain independently deep-copied (#7851).""" import copy raw = _NonCopyableRaw() content = Content.from_text("hello", raw_representation=raw) content.additional_properties["key"] = "value" - cloned = copy.deepcopy(content) + with caplog.at_level(logging.WARNING, logger="agent_framework"): + cloned = copy.deepcopy(content) - # _SHALLOW_COPY_FIELDS (raw_representation) should be same object - assert cloned.raw_representation is raw - # Non-shallow fields should be independent deep copies + assert cloned.raw_representation is None assert cloned.additional_properties is not content.additional_properties assert cloned.additional_properties == {"key": "value"} diff --git a/python/packages/core/tests/workflow/test_agent_executor.py b/python/packages/core/tests/workflow/test_agent_executor.py index ccb1e9425bf..66f7edc530c 100644 --- a/python/packages/core/tests/workflow/test_agent_executor.py +++ b/python/packages/core/tests/workflow/test_agent_executor.py @@ -380,11 +380,12 @@ async def test_agent_executor_workflow_with_non_copyable_raw_representation() -> assert len(completed_a) == 1 assert completed_a[0].data is not None - # The yielded AgentResponse should preserve its raw_representation reference + # Workflow completes even when AgentResponse.raw_representation cannot be deep-copied; + # the yielded clone discards the unsafe field (#7851) instead of sharing a shallow reference. agent_responses = [d for d in completed_a[0].data if isinstance(d, AgentResponse)] assert len(agent_responses) > 0 assert agent_responses[0].text == "reply from AgentA" - assert agent_responses[0].raw_representation is raw + assert agent_responses[0].raw_representation is None # ---------------------------------------------------------------------------