From a4e752cdc13413041d1c038e1ec8724260e609c6 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 9 May 2025 11:21:29 -0400 Subject: [PATCH 1/2] Fix handling of tool calls with some endpoints Most assistant messages containing tool calls don't contain text as well (though some can). In such a case, we were still creating the assistant with empty text. While OpenAI's service permits that, some other endpoints are more finicky about it. This avoids doing so. --- .../OpenAIChatClient.cs | 50 ++++++++++++------- .../ChatClientIntegrationTests.cs | 2 + 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIChatClient.cs b/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIChatClient.cs index c644bd77f21..f6d8981ec26 100644 --- a/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIChatClient.cs +++ b/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIChatClient.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; using System.Text; @@ -157,28 +158,38 @@ void IDisposable.Dispose() } else if (input.Role == ChatRole.Assistant) { - AssistantChatMessage message = new(ToOpenAIChatContent(input.Contents)) + var contentParts = ToOpenAIChatContent(input.Contents, ensureNotEmpty: false); + + var toolCalls = input.Contents.OfType().Select(c => + ChatToolCall.CreateFunctionToolCall(c.CallId, c.Name, new(JsonSerializer.SerializeToUtf8Bytes( + c.Arguments, options.GetTypeInfo(typeof(IDictionary)))))).ToArray(); + + AssistantChatMessage message; + if (contentParts.Count > 0) + { + message = new(contentParts); + foreach (var toolCall in toolCalls) + { + message.ToolCalls.Add(toolCall); + } + } + else if (toolCalls.Length > 0) { - ParticipantName = input.AuthorName - }; + message = new(toolCalls); + } + else + { + message = new(ChatMessageContentPart.CreateTextPart(string.Empty)); + } + + message.ParticipantName = input.AuthorName; foreach (var content in input.Contents) { - switch (content) + if (content is ErrorContent { ErrorCode: nameof(message.Refusal) } refusal) { - case ErrorContent errorContent when errorContent.ErrorCode is nameof(message.Refusal): - message.Refusal = errorContent.Message; - break; - - case FunctionCallContent callRequest: - message.ToolCalls.Add( - ChatToolCall.CreateFunctionToolCall( - callRequest.CallId, - callRequest.Name, - new(JsonSerializer.SerializeToUtf8Bytes( - callRequest.Arguments, - options.GetTypeInfo(typeof(IDictionary)))))); - break; + message.Refusal = refusal.Message; + break; } } @@ -188,7 +199,8 @@ void IDisposable.Dispose() } /// Converts a list of to a list of . - private static List ToOpenAIChatContent(IList contents) + private static List ToOpenAIChatContent( + IList contents, bool ensureNotEmpty = true) { List parts = []; foreach (var content in contents) @@ -226,7 +238,7 @@ private static List ToOpenAIChatContent(IList } } - if (parts.Count == 0) + if (ensureNotEmpty && parts.Count == 0) { parts.Add(ChatMessageContentPart.CreateTextPart(string.Empty)); } diff --git a/test/Libraries/Microsoft.Extensions.AI.Integration.Tests/ChatClientIntegrationTests.cs b/test/Libraries/Microsoft.Extensions.AI.Integration.Tests/ChatClientIntegrationTests.cs index edb6c5dd14c..d5686f6b861 100644 --- a/test/Libraries/Microsoft.Extensions.AI.Integration.Tests/ChatClientIntegrationTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.Integration.Tests/ChatClientIntegrationTests.cs @@ -77,7 +77,9 @@ public virtual async Task GetResponseAsync_WithEmptyMessage() var response = await _chatClient.GetResponseAsync( [ + new(ChatRole.System, []), new(ChatRole.User, []), + new(ChatRole.Assistant, []), new(ChatRole.User, "What is 1 + 2? Reply with a single number."), ]); From 192d76ad8941c9991dcfa5e416c585638c6d58c0 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 9 May 2025 16:27:05 -0400 Subject: [PATCH 2/2] Reduce to single iteration through assistant content --- .../OpenAIChatClient.cs | 127 ++++++++++-------- .../ChatClientIntegrationTests.cs | 4 +- .../OpenAIResponseClientIntegrationTests.cs | 2 + 3 files changed, 78 insertions(+), 55 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIChatClient.cs b/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIChatClient.cs index f6d8981ec26..98cf49fd696 100644 --- a/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIChatClient.cs +++ b/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIChatClient.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; using System.Text; @@ -158,40 +157,54 @@ void IDisposable.Dispose() } else if (input.Role == ChatRole.Assistant) { - var contentParts = ToOpenAIChatContent(input.Contents, ensureNotEmpty: false); + List? contentParts = null; + List? toolCalls = null; + string? refusal = null; + foreach (var content in input.Contents) + { + switch (content) + { + case ErrorContent ec when ec.ErrorCode == nameof(AssistantChatMessage.Refusal): + refusal = ec.Message; + break; + + case FunctionCallContent fc: + (toolCalls ??= []).Add( + ChatToolCall.CreateFunctionToolCall(fc.CallId, fc.Name, new(JsonSerializer.SerializeToUtf8Bytes( + fc.Arguments, options.GetTypeInfo(typeof(IDictionary)))))); + break; + + default: + if (ToChatMessageContentPart(content) is { } part) + { + (contentParts ??= []).Add(part); + } - var toolCalls = input.Contents.OfType().Select(c => - ChatToolCall.CreateFunctionToolCall(c.CallId, c.Name, new(JsonSerializer.SerializeToUtf8Bytes( - c.Arguments, options.GetTypeInfo(typeof(IDictionary)))))).ToArray(); + break; + } + } AssistantChatMessage message; - if (contentParts.Count > 0) + if (contentParts is not null) { message = new(contentParts); - foreach (var toolCall in toolCalls) + if (toolCalls is not null) { - message.ToolCalls.Add(toolCall); + foreach (var toolCall in toolCalls) + { + message.ToolCalls.Add(toolCall); + } } } - else if (toolCalls.Length > 0) - { - message = new(toolCalls); - } else { - message = new(ChatMessageContentPart.CreateTextPart(string.Empty)); + message = toolCalls is not null ? + new(toolCalls) : + new(ChatMessageContentPart.CreateTextPart(string.Empty)); } message.ParticipantName = input.AuthorName; - - foreach (var content in input.Contents) - { - if (content is ErrorContent { ErrorCode: nameof(message.Refusal) } refusal) - { - message.Refusal = refusal.Message; - break; - } - } + message.Refusal = refusal; yield return message; } @@ -199,46 +212,19 @@ void IDisposable.Dispose() } /// Converts a list of to a list of . - private static List ToOpenAIChatContent( - IList contents, bool ensureNotEmpty = true) + private static List ToOpenAIChatContent(IList contents) { List parts = []; + foreach (var content in contents) { - switch (content) + if (ToChatMessageContentPart(content) is { } part) { - case TextContent textContent: - parts.Add(ChatMessageContentPart.CreateTextPart(textContent.Text)); - break; - - case UriContent uriContent when uriContent.HasTopLevelMediaType("image"): - parts.Add(ChatMessageContentPart.CreateImagePart(uriContent.Uri, GetImageDetail(content))); - break; - - case DataContent dataContent when dataContent.HasTopLevelMediaType("image"): - parts.Add(ChatMessageContentPart.CreateImagePart(BinaryData.FromBytes(dataContent.Data), dataContent.MediaType, GetImageDetail(content))); - break; - - case DataContent dataContent when dataContent.HasTopLevelMediaType("audio"): - var audioData = BinaryData.FromBytes(dataContent.Data); - if (dataContent.MediaType.Equals("audio/mpeg", StringComparison.OrdinalIgnoreCase)) - { - parts.Add(ChatMessageContentPart.CreateInputAudioPart(audioData, ChatInputAudioFormat.Mp3)); - } - else if (dataContent.MediaType.Equals("audio/wav", StringComparison.OrdinalIgnoreCase)) - { - parts.Add(ChatMessageContentPart.CreateInputAudioPart(audioData, ChatInputAudioFormat.Wav)); - } - - break; - - case DataContent dataContent when dataContent.MediaType.StartsWith("application/pdf", StringComparison.OrdinalIgnoreCase): - parts.Add(ChatMessageContentPart.CreateFilePart(BinaryData.FromBytes(dataContent.Data), dataContent.MediaType, $"{Guid.NewGuid():N}.pdf")); - break; + parts.Add(part); } } - if (ensureNotEmpty && parts.Count == 0) + if (parts.Count == 0) { parts.Add(ChatMessageContentPart.CreateTextPart(string.Empty)); } @@ -246,6 +232,39 @@ private static List ToOpenAIChatContent( return parts; } + private static ChatMessageContentPart? ToChatMessageContentPart(AIContent content) + { + switch (content) + { + case TextContent textContent: + return ChatMessageContentPart.CreateTextPart(textContent.Text); + + case UriContent uriContent when uriContent.HasTopLevelMediaType("image"): + return ChatMessageContentPart.CreateImagePart(uriContent.Uri, GetImageDetail(content)); + + case DataContent dataContent when dataContent.HasTopLevelMediaType("image"): + return ChatMessageContentPart.CreateImagePart(BinaryData.FromBytes(dataContent.Data), dataContent.MediaType, GetImageDetail(content)); + + case DataContent dataContent when dataContent.HasTopLevelMediaType("audio"): + var audioData = BinaryData.FromBytes(dataContent.Data); + if (dataContent.MediaType.Equals("audio/mpeg", StringComparison.OrdinalIgnoreCase)) + { + return ChatMessageContentPart.CreateInputAudioPart(audioData, ChatInputAudioFormat.Mp3); + } + else if (dataContent.MediaType.Equals("audio/wav", StringComparison.OrdinalIgnoreCase)) + { + return ChatMessageContentPart.CreateInputAudioPart(audioData, ChatInputAudioFormat.Wav); + } + + break; + + case DataContent dataContent when dataContent.MediaType.StartsWith("application/pdf", StringComparison.OrdinalIgnoreCase): + return ChatMessageContentPart.CreateFilePart(BinaryData.FromBytes(dataContent.Data), dataContent.MediaType, $"{Guid.NewGuid():N}.pdf"); + } + + return null; + } + private static ChatImageDetailLevel? GetImageDetail(AIContent content) { if (content.AdditionalProperties?.TryGetValue("detail", out object? value) is true) diff --git a/test/Libraries/Microsoft.Extensions.AI.Integration.Tests/ChatClientIntegrationTests.cs b/test/Libraries/Microsoft.Extensions.AI.Integration.Tests/ChatClientIntegrationTests.cs index d5686f6b861..994fef47517 100644 --- a/test/Libraries/Microsoft.Extensions.AI.Integration.Tests/ChatClientIntegrationTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.Integration.Tests/ChatClientIntegrationTests.cs @@ -620,9 +620,11 @@ public virtual async Task Caching_AfterFunctionInvocation_FunctionOutputUnchange var secondResponse = await chatClient.GetResponseAsync([message]); Assert.Equal(response.Text, secondResponse.Text); Assert.Equal(2, functionCallCount); - Assert.Equal(2, llmCallCount!.CallCount); + Assert.Equal(FunctionInvokingChatClientSetsConversationId ? 3 : 2, llmCallCount!.CallCount); } + public virtual bool FunctionInvokingChatClientSetsConversationId => false; + [ConditionalFact] public virtual async Task Caching_AfterFunctionInvocation_FunctionOutputChangedAsync() { diff --git a/test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAIResponseClientIntegrationTests.cs b/test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAIResponseClientIntegrationTests.cs index bbfad1c571d..2c1d6cdc80e 100644 --- a/test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAIResponseClientIntegrationTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAIResponseClientIntegrationTests.cs @@ -9,4 +9,6 @@ public class OpenAIResponseClientIntegrationTests : ChatClientIntegrationTests IntegrationTestHelpers.GetOpenAIClient() ?.GetOpenAIResponseClient(TestRunnerConfiguration.Instance["OpenAI:ChatModel"] ?? "gpt-4o-mini") .AsIChatClient(); + + public override bool FunctionInvokingChatClientSetsConversationId => true; }