fix: handle mixed-type lists in _flatten across instrumentors; isolate dspy extraction errors - #3728
Open
Harsh23Kashyap wants to merge 3 commits into
Open
Harsh23Kashyap wants to merge 3 commits into
Harsh23Kashyap wants to merge 3 commits into
Conversation
Contributor
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
Apply the same mixed-list handling added for dspy to the identical _flatten copies in autogen-agentchat, google-genai, guardrails, instructor, and llama-index: recurse only into Mapping items and pass non-mapping items through with an indexed key. - instructor: fixes an AttributeError raised into user calls when a message content list mixes dict parts and plain strings - llama-index: fixes silent loss of every attribute on an event whose payload carries the same mixed-list shape - autogen-agentchat, guardrails, google-genai: consistency fixes; the mixed-list path is not reachable from their current call sites (the google-genai copy is unused today), so these prevent the bug rather than fix a live one Each package gains a regression test covering the mixed-type list.
Extend the mixed-list handling to the remaining three defective _flatten copies in the repo. With this commit every _flatten copy in the tree recurses only into Mapping items and passes non-mapping items through with an indexed key. None of the three copies is reachable from current call sites: langchain's message-content path is pre-flattened by _extract_message_kwargs before it reaches _flatten, and the portkey and groq copies have no call sites at all. These are consistency fixes so future call sites inherit the safe behavior. Each package gains a regression test covering the mixed-type list.
Author
|
I have read the CLA Document and I hereby sign the CLA |
caroger
self-requested a review
September 17, 2026 16:15
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.
Description
Resolves #3727
_flattenhas the same defect in nine instrumentors: for a list that mixes mappings with other types it takes the mapping branch and then recurses into every item, so a mixed-typecontentlist (a normal shape for vision and tool messages) breaks attribute extraction. The visible impact differs by package:AttributeErrorraised into user code from the patched call, before the wrapped function runs.AttributeErrorinto user code from LM spans._extract_message_kwargsbefore it reaches_flatten, and other structured payloads (document metadata, prompts) are JSON-serialized first. Fixed for consistency._flattenare unused today; fixed for consistency.The dspy instrumentor had a second problem: no extractor had error isolation, so a circular reference in the inputs surfaced as
ValueError: Circular reference detectedfromsafe_json_dumpsin user code. litellm got the same class of fix in #3578.Changes:
_flattenrecurses only into mapping items of a list; non-mapping parts pass through indexed. Output for all-mapping and all-scalar lists is unchanged. Same edit in all nine copies; every_flattenin the tree is now a safe copy._suppress_extractor_errors, the same shape as litellm's: a failing extractor drops its attributes with a logged warning while the span and the wrapped call proceed._get_input_valueand_module_prediction_output_attributesfall back to a safe value on failure.Verification:
_PatchWrapperwith a fake create function. On main the mixed content list crashes withAttributeError: 'str' object has no attribute 'items'; with the fix the call completes and both content parts land on the span. Ran twice identical both ways._finish_tracingwith a fabricated event carrying the mixed list. On main the event keeps none of its attributes; with the fix the model name and both content parts survive. Ran twice identical both ways._update_spanwith a realHumanMessagecarrying a mixed content list - no crash on either side, because langchain-core and_extract_message_kwargsnormalize and pre-flatten content before_flattenruns. That run is what established the not-reachable status above.Checklist: