Repository navigation
fix(models,tools): release dynamic output models from structured-output caches - #5585
Open
Wilson0511 wants to merge 1 commit into
Open
Wilson0511 wants to merge 1 commit into
Wilson0511 wants to merge 1 commit into
Conversation
…ut caches Two process-lifetime caches key on pydantic model classes with strong references: - StructuredOutputTool._TOOL_SPEC_CACHE caches the converted ToolSpec per output-model class; StructuredOutputContext builds a tool on every structured-output pass, so each distinct class caches forever. - _native_output._REJECTED_NATIVE_SCHEMAS records models whose schema a provider rejected from native structured output; each rejection pins the class. Dynamically created models (pydantic.create_model) are distinct classes on every call - the new DecisionModel/LLMDecisionModel builds one per ask() - so applications and the SDK's own features accumulate classes, schemas, and specs for the life of the process. The rejected-schema record also can never hit again for a dynamic class (fresh key next call), so it is pure retention there. Key both caches weakly (weakref.WeakKeyDictionary), mirroring the ToolWatcher registry fix (strands-agents#5536): module-level classes stay cached as before because their module holds the reference; dynamic classes are released with their last strong reference. Change: - _TOOL_SPEC_CACHE -> WeakKeyDictionary (drop-in; same read path) - _REJECTED_NATIVE_SCHEMAS: set[tuple] -> WeakKeyDictionary[cls, set of (provider, model_id)]; consult and record updated accordingly - tests: lifecycle tests pin both caches with create_model classes and assert release after gc.collect; conftest isolation fixture updated Why: Unbounded class retention in long-running services; weak-keying removes the retention without changing behavior for stable classes. Proof: - red on main: both release assertions fail (classes stay alive after gc.collect) - green with fix: 3 lifecycle tests pass - regression: strands models+tools suites 2387 passed, 31 skipped - ruff check + format clean Risk: Low. WeakKeyDictionary preserves the membership/iteration semantics used here; entries for imported classes live exactly as long as before. 🤖 Generated with ZCode (AI agent) on behalf of Wilson0511 Co-Authored-By: ZCode <agent@zcode.dev>
github-actions
Bot
added
python
This branch has not been deployed
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.
Human Overview
I read the diff myself, agree with the fix, and ran the tests locally.
Description
Fixes #5584.
Two process-lifetime caches key on pydantic model classes with strong references:
StructuredOutputTool._TOOL_SPEC_CACHE— aToolSpecis cached per output-model class;StructuredOutputContextbuilds a tool on every structured-output pass, so each distinct class caches forever._native_output._REJECTED_NATIVE_SCHEMAS— a rejected-from-native-output schema pins its class in a module-level set.Dynamically created models (
pydantic.create_model) are distinct classes on every call — the SDK's ownLLMDecisionModelbuilds one perask()— so both caches accumulate classes (plus schemas/specs) for the life of the process, and the rejected-schema record can never hit again for a dynamic class anyway (fresh key next call): pure retention.Key both caches weakly with
weakref.WeakKeyDictionary, mirroring the ToolWatcher registry fix in #5536. Module-level classes keep their entries exactly as before (their module holds the reference); dynamic classes are released with their last strong reference.Changes
_TOOL_SPEC_CACHE: dict → WeakKeyDictionary (drop-in; read path unchanged)_REJECTED_NATIVE_SCHEMAS:set[tuple[provider, model_id, cls]]→WeakKeyDictionary[cls, set[(provider, model_id)]]; consult (unsupported_mechanisms) and record (handle_rejected_mechanism) updated to the new shapegc.collect); the models conftest isolation fixture resets the new shapeTesting
gc.collect)tests/strands/models/+tests/strands/tools/— 2387 passed, 31 skipped🤖 Generated with ZCode (AI agent) on behalf of Wilson0511
Co-Authored-By: ZCode agent@zcode.dev