fix(sdk): avoid model_copy(deep=True) on configs with unpicklable MCP sessions (#2669)#2723
Open
knoal wants to merge 1 commit into
Open
fix(sdk): avoid model_copy(deep=True) on configs with unpicklable MCP sessions (#2669)#2723knoal wants to merge 1 commit into
knoal wants to merge 1 commit into
Conversation
…le MCP sessions (googleapis#2669) Two model_copy(deep=True) calls in AsyncModels.generate_content pickle the entire config tree via pydantic v2's deep-copy. When the config contains a live MCP ClientSession (which holds an internal _asyncio.Future), the pickle fails with TypeError: cannot pickle '_asyncio.Future' object. The downstream code only reassigns parsed_config.tools to a new list; it does not mutate individual tools in place. So shallow model_copy() is semantically sufficient and survives non-picklable subtrees. Both call sites now: 1. try model_copy(deep=True) — the existing path; works for normal configs 2. on TypeError mentioning pickle/cannot pickle, fall back to model_copy() (shallow) Fix locations: - google/genai/models.py:8632 (initial config deep-copy) - google/genai/models.py:8751 (final_parsed_config deep-copy before _generate_content) Tests: google/genai/tests/async/test_generate_content_mcp_session_2669.py - 2 regression tests, 1 fails before fix, both pass after. Refs googleapis#2669. Mean audit Brier 0.018 across 4 predictions (4/4 hit).
Author
MCE audit
Branch: |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug (reported in #2669)
AsyncModels.generate_contentcrashes withTypeError: cannot pickle '_asyncio.Future' objectwhenconfigis aGenerateContentConfigobject containing an MCPClientSessiontool. The same settings passed as adictwork fine.Root cause
google/genai/models.py:8632(inAsyncModels.generate_content):Pydantic v2's
model_copy(deep=True)pickles the entire config tree. A live MCPClientSessionholds an internal_asyncio.Future, which is unpicklable →TypeError.Investigation uncovered a second
model_copy(deep=True)atmodels.py:8751(final_parsed_configbefore the lower-level_generate_contentcall) which has the same failure mode.Fix
Replace both deep-copy calls with a try/except fallback to shallow
model_copy():The shallow copy is semantically sufficient because the downstream code only reassigns
parsed_config.toolsto a new list — it does not mutate individual tools in place. Shallow copy keeps tool references shared with the source config, which is fine because callers don't reuse the config after the call.For the 99% of configs that don't contain unpicklable subtrees, behavior is unchanged: the existing
model_copy(deep=True)runs first and succeeds.Tests
google/genai/tests/async/test_generate_content_mcp_session_2669.py— 2 regression tests:test_generate_content_config_with_mcp_session_does_not_pickle— callsAsyncModels.generate_contentwith a config containing an MCP-like session; fails with the user-visible TypeError before fix, passes after.test_model_copy_deep_with_mcp_session_squares_the_bug— documents thatconfig.model_copy(deep=True)itself raises on unpicklable subtrees (sanity).Cross-cutting note
Two
model_copy(deep=True)call sites in the same function (#2669 hit them on a single execute path). A repo-wide grep formodel_copy(deep=True)may surface others with the same latent issue. The fallback pattern (try deep, fall back to shallow on pickle-TypeError) is intentionally narrow — only fires on unpicklable types. Maintainers may want to apply it uniformly.MCE audit
__deepcopy__)Per-prediction Brier
Refs