.NET: Propagate FunctionResult.Metadata to AgentRole.Tool ChatMessageContent.Metadata#13855
.NET: Propagate FunctionResult.Metadata to AgentRole.Tool ChatMessageContent.Metadata#13855gmtorres wants to merge 3 commits intomicrosoft:mainfrom
Conversation
|
@gmtorres please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
Pull request overview
This PR enables propagation of user defined FunctionResult.Metadata created or modified in auto function invocation filters into the ChatHistory tool message metadata, making it accessible to downstream consumers like OrchestrationResponseCallback.
Changes:
- Populate
ChatMessageContent.Metadatafor tool messages withAutoFunctionInvocationContext.Result.Metadatawhen adding function results to chat history. - Add a unit test validating metadata flow from a filter set
FunctionResult.Metadatato the resulting toolChatMessageContent.Metadata.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| dotnet/src/InternalUtilities/connectors/AI/FunctionCalling/FunctionCallsProcessor.cs | Propagates FunctionResult.Metadata into the tool ChatMessageContent added to ChatHistory. |
| dotnet/src/SemanticKernel.UnitTests/Utilities/AIConnectors/FunctionCallsProcessorTests.cs | Adds coverage to verify function result metadata is preserved on the tool message in chat history. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 91%
✓ Correctness
This PR propagates FunctionResult.Metadata to the ChatMessageContent added to chat history after function call execution. The implementation accesses resultContext.Context.Result.Metadata to pass as the metadata parameter to the ChatMessageContent constructor. The code is correct: Result is a non-nullable property on AutoFunctionInvocationContext (always initialized), Metadata is nullable IReadOnlyDictionary which the ChatMessageContent constructor accepts as nullable, and the test properly validates the round-trip through a filter that sets metadata on the FunctionResult. The change is minimal and well-tested.
✓ Security Reliability
This PR propagates FunctionResult.Metadata from function invocation results to the ChatMessageContent added to chat history. The change is minimal and safe. The metadata parameter on the ChatMessageContent constructor is nullable (IReadOnlyDictionary<string, object?>?), so null metadata in the error path (when ExecuteFunctionCallAsync catches an exception and the FunctionResult retains its initial null metadata) is handled gracefully. No injection risks, resource leaks, or unsafe deserialization are introduced. The metadata is user-controlled (set by function implementations or auto-invocation filters), so propagating it to chat history is an intentional and appropriate design choice. The test adequately verifies the new behavior by exercising a filter that sets custom metadata and asserting it appears on the tool message.
✓ Test Coverage
The PR adds metadata propagation from FunctionResult to ChatMessageContent in the tool message added to chat history. The test covers the key scenario where a filter sets metadata on the FunctionResult and verifies it appears on the resulting ChatMessageContent. The test is well-structured with meaningful assertions. However, there are two missing edge-case tests: (1) the default case where no filter modifies metadata (metadata should be null), and (2) the error/exception path where AddFunctionCallResultToChatHistory is also called but the FunctionResult has no metadata. These are not blocking since the null-metadata case is implicitly covered by all existing tests that don't set metadata, but an explicit null-metadata assertion would strengthen confidence.
✗ Design Approach
The single-line change propagates
FunctionResult.Metadatato theChatMessageContentthat wraps the tool response, which is the intended behavior. However, the implementation has a subtle design flaw: by passingmetadatato theChatMessageContentconstructor together withcontent, the metadata is inadvertently copied into the auto-createdTextContentitem insideItems. This happens because theContentproperty setter (called from the constructor afterthis.Metadatais already set) creates anew TextContent(metadata: this.Metadata). As a result, the function result metadata ends up on bothmessage.Metadataandmessage.Items[0](the TextContent), but not onmessage.Items[1](theFunctionResultContent), which is the semantically correct carrier for function-invocation metadata. The fix is simple: construct the message without metadata (so the TextContent is created with null metadata), then assignmessage.Metadataseparately. The test is well-structured and validates the right observable behavior.
Flagged Issues
- Passing
metadatain theChatMessageContentconstructor causes the same metadata dictionary to be copied into the auto-createdTextContentitem (via theContentsetter'snew TextContent(metadata: this.Metadata)path). Function-result metadata is semantically tied to the function invocation, not to a text representation item. The correct approach is to setmessage.Metadataafter construction, so the TextContent stays metadata-free while the message-level metadata still propagates correctly.
Suggestions
- Consider adding a test for the error/exception path (function throws) to verify that
AddFunctionCallResultToChatHistorydoes not surface unexpected metadata on the tool message—currently the error path still flows through this method and will propagate whatever metadata exists on theFunctionResultat the time of failure. - Consider adding a test that verifies
ChatMessageContent.Metadatais null when no filter modifies theFunctionResultmetadata (the default path), ensuring the change doesn't accidentally introduce non-null metadata where none existed before.
Automated review by gmtorres's agents
| private void AddFunctionCallResultToChatHistory(ChatHistory chatHistory, FunctionResultContext resultContext) | ||
| { | ||
| var message = new ChatMessageContent(role: AuthorRole.Tool, content: resultContext.Result); | ||
| var message = new ChatMessageContent(role: AuthorRole.Tool, content: resultContext.Result, metadata: resultContext.Context.Result.Metadata); |
There was a problem hiding this comment.
Passing metadata to the constructor causes the Content setter (called inside the constructor after this.Metadata is already set) to forward the same metadata to the auto-created TextContent item via new TextContent(metadata: this.Metadata). Function-result metadata doesn't belong on the text item. Assign Metadata after construction to avoid this leakage.
| var message = new ChatMessageContent(role: AuthorRole.Tool, content: resultContext.Result, metadata: resultContext.Context.Result.Metadata); | |
| var message = new ChatMessageContent(role: AuthorRole.Tool, content: resultContext.Result); | |
| message.Metadata = resultContext.Context.Result.Metadata; |
| Assert.Equal(42, toolMessage.Metadata["key2"]); | ||
| } | ||
|
|
||
| [Fact] |
There was a problem hiding this comment.
The test only covers the case where metadata is explicitly set by a filter. Consider adding analogous test without the filter (or with a no-op filter) to assert that toolMessage.Metadata is null when no metadata is provided, validating the change doesn't regress default behavior.
| [Fact] |
Motivation and Context
There is currently no way to propagate custom metadata from function invocation filters to
OrchestrationResponseCallback. Right nowAddFunctionCallResultToChatHistorydiscardsFunctionResult.Metadataand it is not exposed toOrchestrationResponseCallback.This enables scenarios where we want to consume user-defined metadata in invocation filters, such as function result token count ReductionFilter, exception handling, etc
Description
Pass resultContext.Context.Result.Metadata to the ChatMessageContent constructor in
AddFunctionCallResultToChatHistory, so that metadata set on FunctionResult during the filter pipeline flows through to
the chat history message and ultimately to OrchestrationResponseCallback.
Contribution Checklist