Skip to content

Python: Discard non-copyable fields on Content deepcopy - #7898

Closed
Shivani . (Shivani767) wants to merge 1 commit into
microsoft:mainfrom
Shivani767:fix/7851-content-deepcopy-discard-unsafe
Closed

Python: Discard non-copyable fields on Content deepcopy#7898
Shivani . (Shivani767) wants to merge 1 commit into
microsoft:mainfrom
Shivani767:fix/7851-content-deepcopy-discard-unsafe

Conversation

@Shivani767

@Shivani767 Shivani . (Shivani767) commented Aug 27, 2026

Copy link
Copy Markdown

Motivation & Context

Content.__deepcopy__ (and SerializationMixin.__deepcopy__) previously kept _SHALLOW_COPY_FIELDS such as raw_representation by reference. That created partially deep-copied objects and obscured deepcopy semantics when the field held non-copyable LLM SDK objects.

Fixes #7851

Description & Review Guide

  • What are the major changes?

    • Try a real deepcopy for every field.
    • If a field listed in _SHALLOW_COPY_FIELDS fails to deepcopy, set it to None on the clone and log a warning.
    • Copyable raw_representation values are still deep-copied normally.
    • Update unit and workflow regression tests for the new discard-on-failure behavior.
  • What is the impact of these changes?

    • Deepcopy no longer shares mutable/opaque provider objects with the original.
    • Callers that relied on identity preservation of non-copyable raw_representation after deepcopy will now see None (with a warning).
  • What do you want reviewers to focus on?

    • Whether discard-to-None + warning is preferable to raising, for workflow/checkpoint paths that deepcopy responses.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Stop shallow-sharing unsafe raw_representation during deepcopy. When a
marked field cannot be deep-copied, set it to None and log a warning so
clones are true deep copies.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates deepcopy behavior to discard non-copyable provider objects instead of retaining shared references.

Changes:

  • Deep-copies copyable fields and replaces failing designated fields with None.
  • Logs warnings when fields are discarded.
  • Updates unit and workflow regression tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
python/packages/core/agent_framework/_types.py Updates Content deepcopy behavior.
python/packages/core/agent_framework/_serialization.py Updates mixin deepcopy behavior.
python/packages/core/tests/core/test_types.py Tests core response and content copying.
python/packages/core/tests/core/test_serializable_mixin.py Tests generic mixin behavior.
python/packages/core/tests/workflow/test_agent_executor.py Tests workflow handling of non-copyable responses.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +605 to +609
try:
object.__setattr__(result, k, deepcopy(v, memo))
except Exception as exc:
if k not in unsafe:
raise
Comment on lines +302 to +306
try:
object.__setattr__(result, k, copy.deepcopy(v, memo))
except Exception as exc:
if k not in unsafe:
raise
@eavanvalkenburg

Copy link
Copy Markdown
Member

Closing this as a alternative PR already merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: Should Content.__deepcopy__ preserve unsafe fields

3 participants