Fix FunctionApprovalResponseContent to message mapping (#7152)

Co-authored-by: Stephen Toub <stoub@microsoft.com>

Author: PederHP

src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs

@@ -1448,10 +1448,23 @@ private static (List<ApprovalResultWithRequestMessage>? approvals, List<Approval
                     // The majority of the time, all FCC would be part of a single message, so no need to create a dictionary for this case.
                     // If we are dealing with multiple messages though, we need to keep track of them by their message ID.
                     messagesById = [];
-                    messagesById[currentMessage.MessageId ?? string.Empty] = currentMessage;
+
+                    // Use the effective key for the previous message, accounting for fallbackMessageId substitution.
+                    // If the message's MessageId was set to fallbackMessageId (because the original RequestMessage.MessageId was null),
+                    // we should use empty string as the key to match the lookup key used elsewhere.
+                    var previousMessageKey = currentMessage.MessageId == fallbackMessageId
+                        ? string.Empty
+                        : (currentMessage.MessageId ?? string.Empty);
+                    messagesById[previousMessageKey] = currentMessage;
                 }
 
-                _ = messagesById?.TryGetValue(resultWithRequestMessage.RequestMessage?.MessageId ?? string.Empty, out currentMessage);
+                // Use RequestMessage.MessageId for the lookup key, since that's the original message ID from the provider.
+                // We must use the same key for both lookup and storage to ensure proper grouping.
+                // Note: currentMessage.MessageId may differ from RequestMessage.MessageId because
+                // ConvertToFunctionCallContentMessage sets a fallbackMessageId when RequestMessage.MessageId is null.
+                var messageKey = resultWithRequestMessage.RequestMessage?.MessageId ?? string.Empty;
+
+                _ = messagesById?.TryGetValue(messageKey, out currentMessage);
 
                 if (currentMessage is null)
                 {
@@ -1463,7 +1476,7 @@ private static (List<ApprovalResultWithRequestMessage>? approvals, List<Approval
                 }
 
 #pragma warning disable IDE0058 // Temporary workaround for Roslyn analyzer issue (see https://github.com/dotnet/roslyn/issues/80499)
-                messagesById?[currentMessage.MessageId ?? string.Empty] = currentMessage;
+                messagesById?[messageKey] = currentMessage;
 #pragma warning restore IDE0058
             }
 

test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs

@@ -186,6 +186,59 @@ public async Task ApprovedApprovalResponsesAreExecutedAsync()
         await InvokeAndAssertStreamingAsync(options, input, downstreamClientOutput, output, expectedDownstreamClientInput);
     }
 
+    [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<ChatMessage> 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<string, object?> { { "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<string, object?> { { "i", 42 } }))
+            ]),
+        ];
+
+        // Both FCCs should be in a SINGLE assistant message, not split across multiple messages
+        List<ChatMessage> expectedDownstreamClientInput =
+        [
+            new ChatMessage(ChatRole.User, "hello"),
+            new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "Func1"), new FunctionCallContent("callId2", "Func2", arguments: new Dictionary<string, object?> { { "i", 42 } })]),
+            new ChatMessage(ChatRole.Tool, [new FunctionResultContent("callId1", result: "Result 1"), new FunctionResultContent("callId2", result: "Result 2: 42")]),
+        ];
+
+        List<ChatMessage> downstreamClientOutput =
+        [
+            new ChatMessage(ChatRole.Assistant, "world"),
+        ];
+
+        List<ChatMessage> output =
+        [
+            new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "Func1"), new FunctionCallContent("callId2", "Func2", arguments: new Dictionary<string, object?> { { "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()
     {