Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
Loading