Skip to content

Commit 7381306

Browse files
stephentoubjeffhandley
authored andcommitted
Fix a couple of issues in M.E.AI.OpenAI clients (#6827)
1. Setting AllowMultipleToolCalls if there aren't any tools results in failure. Fix it to only set the property if tools have been added. 2. The chat completion service fails if a participant name containing anything other than a constrained set of characters are included. Fix it with a sanitized value.
1 parent 48ac362 commit 7381306

File tree

5 files changed

+100
-19
lines changed

5 files changed

+100
-19
lines changed

src/Libraries/Microsoft.Extensions.AI.OpenAI/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
- Updated to depend on OpenAI 2.5.0.
66
- Added M.E.AI to OpenAI conversions for response format types.
77
- Added `ResponseTool` to `AITool` conversions.
8+
- Fixed the handling of `HostedCodeInterpreterTool` with Responses when no file IDs were provided.
9+
- Fixed an issue where requests would fail when AllowMultipleToolCalls was set with no tools provided.
10+
- Fixed an issue where requests would fail when an AuthorName was provided containing invalid characters.
811

912
## 9.9.0-preview.1.25458.4
1013

src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIChatClient.cs

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using System.Runtime.CompilerServices;
1111
using System.Text;
1212
using System.Text.Json;
13+
using System.Text.RegularExpressions;
1314
using System.Threading;
1415
using System.Threading.Tasks;
1516
using Microsoft.Shared.Diagnostics;
@@ -19,14 +20,16 @@
1920
#pragma warning disable CA1308 // Normalize strings to uppercase
2021
#pragma warning disable EA0011 // Consider removing unnecessary conditional access operator (?)
2122
#pragma warning disable S1067 // Expressions should not be too complex
23+
#pragma warning disable S2333 // Unnecessary partial
2224
#pragma warning disable S3011 // Reflection should not be used to increase accessibility of classes, methods, or fields
2325
#pragma warning disable SA1202 // Elements should be ordered by access
26+
#pragma warning disable SA1203 // Constants should appear before fields
2427
#pragma warning disable SA1204 // Static elements should appear before instance elements
2528

2629
namespace Microsoft.Extensions.AI;
2730

2831
/// <summary>Represents an <see cref="IChatClient"/> for an OpenAI <see cref="OpenAIClient"/> or <see cref="ChatClient"/>.</summary>
29-
internal sealed class OpenAIChatClient : IChatClient
32+
internal sealed partial class OpenAIChatClient : IChatClient
3033
{
3134
// These delegate instances are used to call the internal overloads of CompleteChatAsync and CompleteChatStreamingAsync that accept
3235
// a RequestOptions. These should be replaced once a better way to pass RequestOptions is available.
@@ -157,10 +160,11 @@ internal static ChatTool ToOpenAIChatTool(AIFunctionDeclaration aiFunction, Chat
157160
input.Role == OpenAIClientExtensions.ChatRoleDeveloper)
158161
{
159162
var parts = ToOpenAIChatContent(input.Contents);
163+
string? name = SanitizeAuthorName(input.AuthorName);
160164
yield return
161-
input.Role == ChatRole.System ? new SystemChatMessage(parts) { ParticipantName = input.AuthorName } :
162-
input.Role == OpenAIClientExtensions.ChatRoleDeveloper ? new DeveloperChatMessage(parts) { ParticipantName = input.AuthorName } :
163-
new UserChatMessage(parts) { ParticipantName = input.AuthorName };
165+
input.Role == ChatRole.System ? new SystemChatMessage(parts) { ParticipantName = name } :
166+
input.Role == OpenAIClientExtensions.ChatRoleDeveloper ? new DeveloperChatMessage(parts) { ParticipantName = name } :
167+
new UserChatMessage(parts) { ParticipantName = name };
164168
}
165169
else if (input.Role == ChatRole.Tool)
166170
{
@@ -233,7 +237,7 @@ internal static ChatTool ToOpenAIChatTool(AIFunctionDeclaration aiFunction, Chat
233237
new(ChatMessageContentPart.CreateTextPart(string.Empty));
234238
}
235239

236-
message.ParticipantName = input.AuthorName;
240+
message.ParticipantName = SanitizeAuthorName(input.AuthorName);
237241
message.Refusal = refusal;
238242

239243
yield return message;
@@ -568,7 +572,6 @@ private ChatCompletionOptions ToOpenAIOptions(ChatOptions? options)
568572
result.TopP ??= options.TopP;
569573
result.PresencePenalty ??= options.PresencePenalty;
570574
result.Temperature ??= options.Temperature;
571-
result.AllowParallelToolCalls ??= options.AllowMultipleToolCalls;
572575
result.Seed ??= options.Seed;
573576

574577
if (options.StopSequences is { Count: > 0 } stopSequences)
@@ -589,6 +592,11 @@ private ChatCompletionOptions ToOpenAIOptions(ChatOptions? options)
589592
}
590593
}
591594

595+
if (result.Tools.Count > 0)
596+
{
597+
result.AllowParallelToolCalls ??= options.AllowMultipleToolCalls;
598+
}
599+
592600
if (result.ToolChoice is null && result.Tools.Count > 0)
593601
{
594602
switch (options.ToolMode)
@@ -749,11 +757,41 @@ internal static void ConvertContentParts(ChatMessageContent content, IList<AICon
749757
_ => new ChatFinishReason(s),
750758
};
751759

760+
/// <summary>Sanitizes the author name to be appropriate for including as an OpenAI participant name.</summary>
761+
private static string? SanitizeAuthorName(string? name)
762+
{
763+
if (name is not null)
764+
{
765+
const int MaxLength = 64;
766+
767+
name = InvalidAuthorNameRegex().Replace(name, string.Empty);
768+
if (name.Length == 0)
769+
{
770+
name = null;
771+
}
772+
else if (name.Length > MaxLength)
773+
{
774+
name = name.Substring(0, MaxLength);
775+
}
776+
}
777+
778+
return name;
779+
}
780+
752781
/// <summary>POCO representing function calling info. Used to concatenation information for a single function call from across multiple streaming updates.</summary>
753782
private sealed class FunctionCallInfo
754783
{
755784
public string? CallId;
756785
public string? Name;
757786
public StringBuilder? Arguments;
758787
}
788+
789+
private const string InvalidAuthorNamePattern = @"[^a-zA-Z0-9_]+";
790+
#if NET
791+
[GeneratedRegex(InvalidAuthorNamePattern)]
792+
private static partial Regex InvalidAuthorNameRegex();
793+
#else
794+
private static Regex InvalidAuthorNameRegex() => _invalidAuthorNameRegex;
795+
private static readonly Regex _invalidAuthorNameRegex = new(InvalidAuthorNamePattern, RegexOptions.Compiled);
796+
#endif
759797
}

src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,6 @@ private ResponseCreationOptions ToOpenAIResponseCreationOptions(ChatOptions? opt
413413

414414
// Handle strongly-typed properties.
415415
result.MaxOutputTokenCount ??= options.MaxOutputTokens;
416-
result.ParallelToolCallsEnabled ??= options.AllowMultipleToolCalls;
417416
result.PreviousResponseId ??= options.ConversationId;
418417
result.Temperature ??= options.Temperature;
419418
result.TopP ??= options.TopP;
@@ -517,6 +516,11 @@ private ResponseCreationOptions ToOpenAIResponseCreationOptions(ChatOptions? opt
517516
}
518517
}
519518

519+
if (result.Tools.Count > 0)
520+
{
521+
result.ParallelToolCallsEnabled ??= options.AllowMultipleToolCalls;
522+
}
523+
520524
if (result.ToolChoice is null && result.Tools.Count > 0)
521525
{
522526
switch (options.ToolMode)

test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAIChatClientTests.cs

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ public async Task BasicRequestResponse_NonStreaming()
152152

153153
var response = await client.GetResponseAsync("hello", new()
154154
{
155+
AllowMultipleToolCalls = false,
155156
MaxOutputTokens = 10,
156157
Temperature = 0.5f,
157158
});
@@ -658,15 +659,46 @@ public async Task StronglyTypedOptions_AllSent()
658659
{
659660
const string Input = """
660661
{
661-
"messages":[{"role":"user","content":"hello"}],
662-
"model":"gpt-4o-mini",
663-
"logprobs":true,
664-
"top_logprobs":42,
665-
"logit_bias":{"12":34},
666-
"parallel_tool_calls":false,
667-
"user":"12345",
668-
"metadata":{"something":"else"},
669-
"store":true
662+
"metadata": {
663+
"something": "else"
664+
},
665+
"user": "12345",
666+
"messages": [
667+
{
668+
"role": "user",
669+
"content": "hello"
670+
}
671+
],
672+
"model": "gpt-4o-mini",
673+
"top_logprobs": 42,
674+
"store": true,
675+
"logit_bias": {
676+
"12": 34
677+
},
678+
"logprobs": true,
679+
"tools": [
680+
{
681+
"type": "function",
682+
"function": {
683+
"description": "",
684+
"name": "GetPersonAge",
685+
"parameters": {
686+
"type": "object",
687+
"required": [
688+
"name"
689+
],
690+
"properties": {
691+
"name": {
692+
"type": "string"
693+
}
694+
},
695+
"additionalProperties": false
696+
}
697+
}
698+
}
699+
],
700+
"tool_choice": "auto",
701+
"parallel_tool_calls": false
670702
}
671703
""";
672704

@@ -694,6 +726,7 @@ public async Task StronglyTypedOptions_AllSent()
694726
Assert.NotNull(await client.GetResponseAsync("hello", new()
695727
{
696728
AllowMultipleToolCalls = false,
729+
Tools = [AIFunctionFactory.Create((string name) => 42, "GetPersonAge")],
697730
RawRepresentationFactory = (c) =>
698731
{
699732
var openAIOptions = new ChatCompletionOptions

test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAIConversionTests.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ public void AsOpenAIChatMessages_ProducesExpectedOutput(bool withOptions)
159159
List<ChatMessage> messages =
160160
[
161161
new(ChatRole.System, "You are a helpful assistant."),
162-
new(ChatRole.User, "Hello"),
162+
new(ChatRole.User, "Hello") { AuthorName = "Jane" },
163163
new(ChatRole.Assistant,
164164
[
165165
new TextContent("Hi there!"),
@@ -168,9 +168,9 @@ public void AsOpenAIChatMessages_ProducesExpectedOutput(bool withOptions)
168168
["param1"] = "value1",
169169
["param2"] = 42
170170
}),
171-
]),
171+
]) { AuthorName = "!@#$%John Smith^*)" },
172172
new(ChatRole.Tool, [new FunctionResultContent("callid123", "theresult")]),
173-
new(ChatRole.Assistant, "The answer is 42."),
173+
new(ChatRole.Assistant, "The answer is 42.") { AuthorName = "@#$#$@$" },
174174
];
175175

176176
ChatOptions? options = withOptions ? new ChatOptions { Instructions = "You talk like a parrot." } : null;
@@ -196,6 +196,7 @@ public void AsOpenAIChatMessages_ProducesExpectedOutput(bool withOptions)
196196

197197
UserChatMessage m1 = Assert.IsType<UserChatMessage>(convertedMessages[index + 1], exactMatch: false);
198198
Assert.Equal("Hello", Assert.Single(m1.Content).Text);
199+
Assert.Equal("Jane", m1.ParticipantName);
199200

200201
AssistantChatMessage m2 = Assert.IsType<AssistantChatMessage>(convertedMessages[index + 2], exactMatch: false);
201202
Assert.Single(m2.Content);
@@ -208,13 +209,15 @@ public void AsOpenAIChatMessages_ProducesExpectedOutput(bool withOptions)
208209
["param1"] = "value1",
209210
["param2"] = 42
210211
}), JsonSerializer.Deserialize<JsonElement>(tc.FunctionArguments.ToMemory().Span)));
212+
Assert.Equal("JohnSmith", m2.ParticipantName);
211213

212214
ToolChatMessage m3 = Assert.IsType<ToolChatMessage>(convertedMessages[index + 3], exactMatch: false);
213215
Assert.Equal("callid123", m3.ToolCallId);
214216
Assert.Equal("theresult", Assert.Single(m3.Content).Text);
215217

216218
AssistantChatMessage m4 = Assert.IsType<AssistantChatMessage>(convertedMessages[index + 4], exactMatch: false);
217219
Assert.Equal("The answer is 42.", Assert.Single(m4.Content).Text);
220+
Assert.Null(m4.ParticipantName);
218221
}
219222

220223
[Fact]

0 commit comments

Comments
 (0)