From 6cd056cc901fa3984402a5607c20c02f0a24d40f Mon Sep 17 00:00:00 2001 From: Peder Date: Sat, 13 Dec 2025 16:32:00 +0100 Subject: [PATCH 1/3] Fix message id mapping --- .../FunctionInvokingChatClient.cs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs b/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs index 74f9bf554fa..b3885542de4 100644 --- a/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs +++ b/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs @@ -1448,10 +1448,23 @@ private static (List? approvals, List? approvals, List Date: Sat, 13 Dec 2025 17:00:20 +0100 Subject: [PATCH 2/3] Add test --- ...unctionInvokingChatClientApprovalsTests.cs | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs index 7c42c0edaf9..0c618f5e9c7 100644 --- a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs @@ -186,6 +186,69 @@ public async Task ApprovedApprovalResponsesAreExecutedAsync() await InvokeAndAssertStreamingAsync(options, input, downstreamClientOutput, output, expectedDownstreamClientInput); } + /// + /// Regression test: When messages containing FunctionApprovalRequestContent and FunctionApprovalResponseContent + /// have null MessageId, all approvals from the same original message should still be grouped into a single + /// FunctionCallContent message when sent to the downstream client. + /// + /// Previously, a bug in ConvertToFunctionCallContentMessages caused a key mismatch when MessageId was null: + /// - Messages were stored in a dictionary under the fallbackMessageId key + /// - But lookups used (RequestMessage?.MessageId ?? "") which resolved to "" + /// - This caused each FCC to be placed in a separate message instead of being grouped + /// + [Fact] + public async Task ApprovedApprovalResponsesAreGroupedWhenMessageIdIsNullAsync() + { + var options = new ChatOptions + { + Tools = + [ + new ApprovalRequiredAIFunction(AIFunctionFactory.Create(() => "Result 1", "Func1")), + new ApprovalRequiredAIFunction(AIFunctionFactory.Create((int i) => $"Result 2: {i}", "Func2")), + ] + }; + + // Key difference from other tests: MessageId is NOT set on the assistant message + List input = + [ + new ChatMessage(ChatRole.User, "hello"), + new ChatMessage(ChatRole.Assistant, + [ + new FunctionApprovalRequestContent("callId1", new FunctionCallContent("callId1", "Func1")), + new FunctionApprovalRequestContent("callId2", new FunctionCallContent("callId2", "Func2", arguments: new Dictionary { { "i", 42 } })) + ]), // Note: No MessageId set - this is the bug trigger + new ChatMessage(ChatRole.User, + [ + new FunctionApprovalResponseContent("callId1", true, new FunctionCallContent("callId1", "Func1")), + new FunctionApprovalResponseContent("callId2", true, new FunctionCallContent("callId2", "Func2", arguments: new Dictionary { { "i", 42 } })) + ]), + ]; + + // Both FCCs should be in a SINGLE assistant message, not split across multiple messages + List expectedDownstreamClientInput = + [ + new ChatMessage(ChatRole.User, "hello"), + new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "Func1"), new FunctionCallContent("callId2", "Func2", arguments: new Dictionary { { "i", 42 } })]), + new ChatMessage(ChatRole.Tool, [new FunctionResultContent("callId1", result: "Result 1"), new FunctionResultContent("callId2", result: "Result 2: 42")]), + ]; + + List downstreamClientOutput = + [ + new ChatMessage(ChatRole.Assistant, "world"), + ]; + + List output = + [ + new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "Func1"), new FunctionCallContent("callId2", "Func2", arguments: new Dictionary { { "i", 42 } })]), + new ChatMessage(ChatRole.Tool, [new FunctionResultContent("callId1", result: "Result 1"), new FunctionResultContent("callId2", result: "Result 2: 42")]), + new ChatMessage(ChatRole.Assistant, "world"), + ]; + + await InvokeAndAssertAsync(options, input, downstreamClientOutput, output, expectedDownstreamClientInput); + + await InvokeAndAssertStreamingAsync(options, input, downstreamClientOutput, output, expectedDownstreamClientInput); + } + [Fact] public async Task ApprovedApprovalResponsesFromSeparateFCCMessagesAreExecutedAsync() { From 14eb134b7f85e8dc6043e4e08ca384854d963501 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Sun, 14 Dec 2025 23:34:03 -0500 Subject: [PATCH 3/3] Update test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs --- .../FunctionInvokingChatClientApprovalsTests.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs index 0c618f5e9c7..93298ba2ac1 100644 --- a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs @@ -186,16 +186,6 @@ public async Task ApprovedApprovalResponsesAreExecutedAsync() await InvokeAndAssertStreamingAsync(options, input, downstreamClientOutput, output, expectedDownstreamClientInput); } - /// - /// Regression test: When messages containing FunctionApprovalRequestContent and FunctionApprovalResponseContent - /// have null MessageId, all approvals from the same original message should still be grouped into a single - /// FunctionCallContent message when sent to the downstream client. - /// - /// Previously, a bug in ConvertToFunctionCallContentMessages caused a key mismatch when MessageId was null: - /// - Messages were stored in a dictionary under the fallbackMessageId key - /// - But lookups used (RequestMessage?.MessageId ?? "") which resolved to "" - /// - This caused each FCC to be placed in a separate message instead of being grouped - /// [Fact] public async Task ApprovedApprovalResponsesAreGroupedWhenMessageIdIsNullAsync() {