-
Notifications
You must be signed in to change notification settings - Fork 4.6k
.NET: Propagate FunctionResult.Metadata to AgentRole.Tool ChatMessageContent.Metadata #13855
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |||
|
|
||||
| using System; | ||||
| using System.Collections.Concurrent; | ||||
| using System.Collections.Generic; | ||||
| using System.Linq; | ||||
| using System.Text.Json; | ||||
| using System.Threading; | ||||
|
|
@@ -855,6 +856,54 @@ public void ItShouldSerializeFunctionResultsWithStringProperties() | |||
| Assert.Equal("{\"Text\":\"テスト\"}", result); | ||||
| } | ||||
|
|
||||
| [Fact] | ||||
| public async Task ItShouldPropagateFunctionResultMetadataToChatHistoryAsync() | ||||
| { | ||||
| // Arrange | ||||
| var expectedMetadata = new Dictionary<string, object?> | ||||
| { | ||||
| ["key1"] = "value1", | ||||
| ["key2"] = 42 | ||||
| }; | ||||
|
|
||||
| var function1 = KernelFunctionFactory.CreateFromMethod((string parameter) => parameter, "Function1"); | ||||
| var plugin = KernelPluginFactory.CreateFromFunctions("MyPlugin", [function1]); | ||||
|
|
||||
| var kernel = CreateKernel(plugin, async (context, next) => | ||||
| { | ||||
| await next(context); | ||||
|
|
||||
| // Filter sets metadata on the FunctionResult | ||||
| context.Result = new FunctionResult(context.Function, context.Result.GetValue<object>(), context.Result.Culture, expectedMetadata); | ||||
| }); | ||||
|
|
||||
| var chatMessageContent = new ChatMessageContent(); | ||||
| chatMessageContent.Items.Add(new FunctionCallContent("Function1", "MyPlugin", arguments: new KernelArguments() { ["parameter"] = "function1-result" })); | ||||
|
|
||||
| var chatHistory = new ChatHistory(); | ||||
|
|
||||
| // Act | ||||
| await this._sut.ProcessFunctionCallsAsync( | ||||
| chatMessageContent: chatMessageContent, | ||||
| executionSettings: this._promptExecutionSettings, | ||||
| chatHistory: chatHistory, | ||||
| requestIndex: 0, | ||||
| checkIfFunctionAdvertised: (_) => true, | ||||
| options: this._functionChoiceBehaviorOptions, | ||||
| kernel: kernel!, | ||||
| isStreaming: false, | ||||
| cancellationToken: CancellationToken.None); | ||||
|
|
||||
| // Assert | ||||
| Assert.Equal(2, chatHistory.Count); | ||||
|
|
||||
| var toolMessage = chatHistory[1]; // First is the assistant message, second is the tool result | ||||
| Assert.Equal(AuthorRole.Tool, toolMessage.Role); | ||||
| Assert.NotNull(toolMessage.Metadata); | ||||
| Assert.Equal("value1", toolMessage.Metadata["key1"]); | ||||
| Assert.Equal(42, toolMessage.Metadata["key2"]); | ||||
| } | ||||
|
|
||||
| [Fact] | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Suggested change
|
||||
| public async Task ItShouldPassPromptExecutionSettingsToAutoFunctionInvocationFilterAsync() | ||||
| { | ||||
|
|
||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing
metadatato the constructor causes theContentsetter (called inside the constructor afterthis.Metadatais already set) to forward the same metadata to the auto-createdTextContentitem vianew TextContent(metadata: this.Metadata). Function-result metadata doesn't belong on the text item. AssignMetadataafter construction to avoid this leakage.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, addressed