Python: Preserve metadata when merging streaming text content - #14287
Python: Preserve metadata when merging streaming text content#14287Roli Bosch (roli-lpci) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes StreamingTextContent.__add__ to actually combine metadata from both operands (with the right operand winning on key conflicts), aligning behavior with other streaming content types and preventing metadata loss during chunk accumulation.
Changes:
- Update
StreamingTextContent.__add__to merge metadata via dict union so later chunks’ metadata is preserved. - Add regression tests covering disjoint metadata merges, conflict precedence, and ensuring original operands’ metadata is unchanged.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| python/semantic_kernel/contents/streaming_text_content.py | Merge left/right metadata in __add__ using dict union (right side wins). |
| python/tests/unit/contents/test_streaming_text_content.py | Add unit tests validating metadata merge behavior and non-mutation of inputs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: No findings
Scope: full PR (1 commit(s)): b84a2c6b3cec
Model: claude-opus-4.8
Overview
This PR changes a single production line in StreamingTextContent.__add__ from metadata=self.metadata to metadata=self.metadata | other.metadata, fulfilling the class docstring's rule that metadata is combined and bringing the type into line with its siblings StreamingChatMessageContent and FunctionCallContent. The change is safe: KernelContent.metadata is a non-optional dict with default_factory=dict, so dict | dict can never raise on a None operand, and the | operator returns a new dict without mutating either input—behavior the added regression tests pin explicitly. No public-API, serialization, provider-parity, or security concern survived reconciliation; right-operand-wins precedence is the documented, tested, and consistent contract.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
Motivation and Context
StreamingTextContent.__add__documents that metadata is combined, but it currently retains only the left operand's metadata. Streaming connectors can attach response metadata to different chunks—for example, an identifier on an early chunk and usage or log-probability data on a later one—so accumulating chunks can silently lose the later metadata.No matching open issue or pull request was found for this narrow bug. The contribution guide permits skipping an issue for a trivial change.
Description
StreamingTextContent, with the right operand winning on conflicting keys. This matches the existing behavior of sibling streaming content types.Validation
Contribution Checklist