Skip to content

Python: Discard unsafe Content fields during deepcopy - #7903

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 3 commits into
microsoft:mainfrom
Sweetteabittersugar:codex/issue-7851-content-deepcopy
Aug 27, 2026
Merged

Python: Discard unsafe Content fields during deepcopy#7903
Eduard van Valkenburg (eavanvalkenburg) merged 3 commits into
microsoft:mainfrom
Sweetteabittersugar:codex/issue-7851-content-deepcopy

Conversation

@Sweetteabittersugar

Copy link
Copy Markdown
Contributor

Motivation & Context

Content.__deepcopy__ currently keeps fields listed in _SHALLOW_COPY_FIELDS by reference. This creates a partially deep-copied object that can still share mutable provider SDK state with the original. The issue proposes discarding designated unsafe data and logging the loss so deepcopy retains isolation without failing on opaque provider objects.

Description & Review Guide

  • What are the major changes?
    • Set every non-None Content._SHALLOW_COPY_FIELDS value to None in the clone and emit a warning.
    • Continue deep-copying all other Content fields normally.
    • Update direct deepcopy, response aggregation, and text-coalescing tests for the new contract.
  • What is the impact of these changes?
    • Deep-copied Content objects no longer share provider-specific raw state with their source.
    • Callers that inspect raw_representation after a Content deepcopy will receive None; the original object remains unchanged.
    • SerializationMixin behavior is intentionally unchanged because Python: [Bug]: Should Content.__deepcopy__ preserve unsafe fields #7851 is scoped to Content.
  • What do you want reviewers to focus on?
    • Whether fields explicitly classified in _SHALLOW_COPY_FIELDS should always be discarded, rather than copied on a best-effort basis.

Related Issue

Fixes #7851

This is an alternative to #7898. That PR first attempts to deepcopy marked values and discards them only on failure, and it also changes SerializationMixin. This PR applies the issue's narrower Content contract directly: every non-None designated unsafe value is discarded. Besides avoiding copies of values already classified as unsafe, this approach cannot leave partial entries in the shared deepcopy memo after a failed best-effort copy.

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.

Copilot AI balanced review requested due to automatic review settings August 27, 2026 08:47
@agent-framework-automation agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Aug 27, 2026

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 Content.__deepcopy__ to discard unsafe shallow-copy fields while preserving isolation.

Changes:

  • Replace unsafe fields with None and log warnings.
  • Update deepcopy and response-coalescing tests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
python/packages/core/agent_framework/_types.py Implements safe field discarding during deepcopy.
python/packages/core/tests/core/test_types.py Updates tests for the new behavior.

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

Comment thread python/packages/core/agent_framework/_types.py Outdated
@Sweetteabittersugar

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

Comment thread python/packages/core/agent_framework/_types.py Outdated
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/core/agent_framework
   _types.py13029892%63, 73, 127, 136, 155, 157, 161, 165, 167, 169, 171, 189, 193, 219, 241, 246, 251, 255, 285, 713–714, 895–896, 1354, 1429, 1467, 1487, 1497, 1691–1693, 1978–1983, 2008, 2063, 2068, 2078, 2086, 2093–2097, 2115, 2188, 2201, 2206, 2319, 2603, 2627, 2726, 2907–2908, 3010, 3293, 3346, 3365, 3404, 3415, 3417–3421, 3423, 3426–3434, 3444, 3609, 3616, 3620–3622, 3767, 3772, 3777, 3782, 3786, 3872–3874, 3903, 4019–4023
TOTAL48229448390% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
9764 36 💤 0 ❌ 0 🔥 2m 37s ⏱️

Merged via the queue into microsoft:main with commit 52a3764 Aug 27, 2026
35 checks passed
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