From adb2ca51034831af4486e31f0c1624bd27e058d6 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 16 Jan 2025 19:44:46 +0000 Subject: [PATCH 1/3] Make a number of improvements to the OpenAI serialization helpers. --- .../JsonModelHelpers.cs | 7 ++++ .../OpenAIChatCompletionRequest.cs | 37 +++++++++++++++++ .../OpenAIModelMapper.ChatCompletion.cs | 4 +- ...nAIModelMappers.StreamingChatCompletion.cs | 6 +-- .../OpenAISerializationTests.cs | 40 +++++++++++++++++-- 5 files changed, 85 insertions(+), 9 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.AI.OpenAI/JsonModelHelpers.cs b/src/Libraries/Microsoft.Extensions.AI.OpenAI/JsonModelHelpers.cs index 5f6b92d2f01..53188b333ee 100644 --- a/src/Libraries/Microsoft.Extensions.AI.OpenAI/JsonModelHelpers.cs +++ b/src/Libraries/Microsoft.Extensions.AI.OpenAI/JsonModelHelpers.cs @@ -3,6 +3,7 @@ using System; using System.ClientModel.Primitives; +using System.Text.Json; namespace Microsoft.Extensions.AI; @@ -23,6 +24,12 @@ public static TModel Deserialize(BinaryData data) return JsonModelDeserializationWitness.Value.Create(data, ModelReaderWriterOptions.Json); } + public static TModel Deserialize(ref Utf8JsonReader reader) + where TModel : IJsonModel, new() + { + return JsonModelDeserializationWitness.Value.Create(ref reader, ModelReaderWriterOptions.Json); + } + private sealed class JsonModelDeserializationWitness where TModel : IJsonModel, new() { diff --git a/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIChatCompletionRequest.cs b/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIChatCompletionRequest.cs index dba0e5ecbf8..5b7d7fe5a34 100644 --- a/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIChatCompletionRequest.cs +++ b/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIChatCompletionRequest.cs @@ -1,13 +1,21 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.Collections.Generic; +using System.ComponentModel; +using System.Text.Json; +using System.Text.Json.Serialization; +using OpenAI.Chat; + +#pragma warning disable CA1034 // Nested types should not be visible namespace Microsoft.Extensions.AI; /// /// Represents an OpenAI chat completion request deserialized as Microsoft.Extension.AI models. /// +[JsonConverter(typeof(Converter))] public sealed class OpenAIChatCompletionRequest { /// @@ -29,4 +37,33 @@ public sealed class OpenAIChatCompletionRequest /// Gets the model id requested by the chat completion. /// public string? ModelId { get; init; } + + /// + /// Converts an OpenAIChatCompletionRequest object to and from JSON. + /// + [EditorBrowsable(EditorBrowsableState.Never)] + public sealed class Converter : JsonConverter + { + /// + /// Reads and converts the JSON to type OpenAIChatCompletionRequest. + /// + /// The reader. + /// The type to convert. + /// The serializer options. + /// The converted OpenAIChatCompletionRequest object. + public override OpenAIChatCompletionRequest? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + ChatCompletionOptions chatCompletionOptions = JsonModelHelpers.Deserialize(ref reader); + return OpenAIModelMappers.FromOpenAIChatCompletionRequest(chatCompletionOptions); + } + + /// + /// Writes the specified value as JSON. + /// + /// The writer. + /// The value to write. + /// The serializer options. + public override void Write(Utf8JsonWriter writer, OpenAIChatCompletionRequest value, JsonSerializerOptions options) => + throw new NotSupportedException("Request body serialization is not supported."); + } } diff --git a/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMapper.ChatCompletion.cs b/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMapper.ChatCompletion.cs index 6ef871fcbbe..d0ec84c947a 100644 --- a/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMapper.ChatCompletion.cs +++ b/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMapper.ChatCompletion.cs @@ -56,8 +56,8 @@ public static OpenAI.Chat.ChatCompletion ToOpenAIChatCompletion(ChatCompletion c return OpenAIChatModelFactory.ChatCompletion( id: chatCompletion.CompletionId ?? CreateCompletionId(), - model: chatCompletion.ModelId, - createdAt: chatCompletion.CreatedAt ?? default, + model: chatCompletion.ModelId ?? string.Empty, + createdAt: chatCompletion.CreatedAt ?? DateTimeOffset.UtcNow, role: ToOpenAIChatRole(chatCompletion.Message.Role).Value, finishReason: ToOpenAIFinishReason(chatCompletion.FinishReason), content: new(ToOpenAIChatContent(chatCompletion.Message.Contents)), diff --git a/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMappers.StreamingChatCompletion.cs b/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMappers.StreamingChatCompletion.cs index a4d8551461f..952db95f48f 100644 --- a/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMappers.StreamingChatCompletion.cs +++ b/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMappers.StreamingChatCompletion.cs @@ -47,10 +47,10 @@ internal static partial class OpenAIModelMappers yield return OpenAIChatModelFactory.StreamingChatCompletionUpdate( completionId: chatCompletionUpdate.CompletionId ?? CreateCompletionId(), - model: chatCompletionUpdate.ModelId, - createdAt: chatCompletionUpdate.CreatedAt ?? default, + model: chatCompletionUpdate.ModelId ?? string.Empty, + createdAt: chatCompletionUpdate.CreatedAt ?? DateTimeOffset.UtcNow, role: ToOpenAIChatRole(chatCompletionUpdate.Role), - finishReason: ToOpenAIFinishReason(chatCompletionUpdate.FinishReason), + finishReason: chatCompletionUpdate.FinishReason is null ? null : ToOpenAIFinishReason(chatCompletionUpdate.FinishReason), contentUpdate: [.. ToOpenAIChatContent(chatCompletionUpdate.Contents)], toolCallUpdates: toolCallUpdates, refusalUpdate: chatCompletionUpdate.AdditionalProperties.GetValueOrDefault(nameof(OpenAI.Chat.StreamingChatCompletionUpdate.RefusalUpdate)), diff --git a/test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAISerializationTests.cs b/test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAISerializationTests.cs index 87df45b5bf3..f636063804e 100644 --- a/test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAISerializationTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAISerializationTests.cs @@ -85,6 +85,38 @@ public static async Task RequestDeserialization_SimpleMessage_Stream() Assert.Null(textContent.AdditionalProperties); } + [Fact] + public static void RequestDeserialization_SimpleMessage_JsonSerializer() + { + const string RequestJson = """ + {"messages":[{"role":"user","content":"hello"}],"model":"gpt-4o-mini","max_completion_tokens":20,"stream":true,"stream_options":{"include_usage":true},"temperature":0.5} + """; + + OpenAIChatCompletionRequest? request = JsonSerializer.Deserialize(RequestJson); + + Assert.NotNull(request); + Assert.True(request.Stream); + Assert.Equal("gpt-4o-mini", request.ModelId); + + Assert.NotNull(request.Options); + Assert.Equal("gpt-4o-mini", request.Options.ModelId); + Assert.Equal(0.5f, request.Options.Temperature); + Assert.Equal(20, request.Options.MaxOutputTokens); + Assert.Null(request.Options.TopK); + Assert.Null(request.Options.TopP); + Assert.Null(request.Options.StopSequences); + Assert.Null(request.Options.AdditionalProperties); + Assert.Null(request.Options.Tools); + + ChatMessage message = Assert.Single(request.Messages); + Assert.Equal(ChatRole.User, message.Role); + AIContent content = Assert.Single(message.Contents); + TextContent textContent = Assert.IsType(content); + Assert.Equal("hello", textContent.Text); + Assert.Null(textContent.RawRepresentation); + Assert.Null(textContent.AdditionalProperties); + } + [Fact] public static async Task RequestDeserialization_MultipleMessages() { @@ -614,13 +646,13 @@ static async IAsyncEnumerable CreateStreamingComp string result = Encoding.UTF8.GetString(stream.ToArray()); AssertSseEqual(""" - data: {"id":"chatcmpl-ADymNiWWeqCJqHNFXiI1QtRcLuXcl","choices":[{"delta":{"content":"Streaming update 0","tool_calls":[],"role":"assistant"},"logprobs":{"content":[],"refusal":[]},"finish_reason":"stop","index":0}],"created":1727888631,"model":"gpt-4o-mini-2024-07-18","system_fingerprint":"fp_f85bea6784","object":"chat.completion.chunk"} + data: {"id":"chatcmpl-ADymNiWWeqCJqHNFXiI1QtRcLuXcl","choices":[{"delta":{"content":"Streaming update 0","tool_calls":[],"role":"assistant"},"logprobs":{"content":[],"refusal":[]},"index":0}],"created":1727888631,"model":"gpt-4o-mini-2024-07-18","system_fingerprint":"fp_f85bea6784","object":"chat.completion.chunk"} - data: {"id":"chatcmpl-ADymNiWWeqCJqHNFXiI1QtRcLuXcl","choices":[{"delta":{"content":"Streaming update 1","tool_calls":[],"role":"assistant"},"logprobs":{"content":[],"refusal":[]},"finish_reason":"stop","index":0}],"created":1727888631,"model":"gpt-4o-mini-2024-07-18","system_fingerprint":"fp_f85bea6784","object":"chat.completion.chunk"} + data: {"id":"chatcmpl-ADymNiWWeqCJqHNFXiI1QtRcLuXcl","choices":[{"delta":{"content":"Streaming update 1","tool_calls":[],"role":"assistant"},"logprobs":{"content":[],"refusal":[]},"index":0}],"created":1727888631,"model":"gpt-4o-mini-2024-07-18","system_fingerprint":"fp_f85bea6784","object":"chat.completion.chunk"} - data: {"id":"chatcmpl-ADymNiWWeqCJqHNFXiI1QtRcLuXcl","choices":[{"delta":{"content":"Streaming update 2","tool_calls":[{"index":0,"id":"callId","type":"function","function":{"name":"MyCoolFunc","arguments":"{\r\n \u0022arg1\u0022: 42,\r\n \u0022arg2\u0022: \u0022str\u0022\r\n}"}}],"role":"assistant"},"logprobs":{"content":[],"refusal":[]},"finish_reason":"stop","index":0}],"created":1727888631,"model":"gpt-4o-mini-2024-07-18","system_fingerprint":"fp_f85bea6784","object":"chat.completion.chunk"} + data: {"id":"chatcmpl-ADymNiWWeqCJqHNFXiI1QtRcLuXcl","choices":[{"delta":{"content":"Streaming update 2","tool_calls":[{"index":0,"id":"callId","type":"function","function":{"name":"MyCoolFunc","arguments":"{\r\n \u0022arg1\u0022: 42,\r\n \u0022arg2\u0022: \u0022str\u0022\r\n}"}}],"role":"assistant"},"logprobs":{"content":[],"refusal":[]},"index":0}],"created":1727888631,"model":"gpt-4o-mini-2024-07-18","system_fingerprint":"fp_f85bea6784","object":"chat.completion.chunk"} - data: {"id":"chatcmpl-ADymNiWWeqCJqHNFXiI1QtRcLuXcl","choices":[{"delta":{"content":"Streaming update 3","tool_calls":[],"role":"assistant"},"logprobs":{"content":[],"refusal":[]},"finish_reason":"stop","index":0}],"created":1727888631,"model":"gpt-4o-mini-2024-07-18","system_fingerprint":"fp_f85bea6784","object":"chat.completion.chunk"} + data: {"id":"chatcmpl-ADymNiWWeqCJqHNFXiI1QtRcLuXcl","choices":[{"delta":{"content":"Streaming update 3","tool_calls":[],"role":"assistant"},"logprobs":{"content":[],"refusal":[]},"index":0}],"created":1727888631,"model":"gpt-4o-mini-2024-07-18","system_fingerprint":"fp_f85bea6784","object":"chat.completion.chunk"} data: {"id":"chatcmpl-ADymNiWWeqCJqHNFXiI1QtRcLuXcl","choices":[{"delta":{"content":"Streaming update 4","tool_calls":[],"role":"assistant"},"logprobs":{"content":[],"refusal":[]},"finish_reason":"stop","index":0}],"created":1727888631,"model":"gpt-4o-mini-2024-07-18","system_fingerprint":"fp_f85bea6784","object":"chat.completion.chunk","usage":{"completion_tokens":9,"prompt_tokens":8,"total_tokens":17,"completion_tokens_details":{"audio_tokens":2,"reasoning_tokens":90},"prompt_tokens_details":{"audio_tokens":1,"cached_tokens":13}}} From 83f39a77cb00fab94ffe25de3ef140aa8945bdbc Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Fri, 17 Jan 2025 15:36:33 +0000 Subject: [PATCH 2/3] Normalize ModelId to null if the empty string. --- .../OpenAIModelMapper.ChatCompletion.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMapper.ChatCompletion.cs b/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMapper.ChatCompletion.cs index d0ec84c947a..239e2add074 100644 --- a/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMapper.ChatCompletion.cs +++ b/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMapper.ChatCompletion.cs @@ -17,6 +17,7 @@ #pragma warning disable S103 // Lines should not be too long #pragma warning disable CA1859 // Use concrete types when possible for improved performance #pragma warning disable S1067 // Expressions should not be too complex +#pragma warning disable S3440 // Variables should not be checked against the values they're about to be assigned namespace Microsoft.Extensions.AI; @@ -56,7 +57,7 @@ public static OpenAI.Chat.ChatCompletion ToOpenAIChatCompletion(ChatCompletion c return OpenAIChatModelFactory.ChatCompletion( id: chatCompletion.CompletionId ?? CreateCompletionId(), - model: chatCompletion.ModelId ?? string.Empty, + model: chatCompletion.ModelId, createdAt: chatCompletion.CreatedAt ?? DateTimeOffset.UtcNow, role: ToOpenAIChatRole(chatCompletion.Message.Role).Value, finishReason: ToOpenAIFinishReason(chatCompletion.FinishReason), @@ -148,7 +149,12 @@ public static ChatOptions FromOpenAIOptions(OpenAI.Chat.ChatCompletionOptions? o if (options is not null) { - result.ModelId = _getModelIdAccessor.Invoke(options, null)?.ToString(); + result.ModelId = _getModelIdAccessor.Invoke(options, null)?.ToString() switch + { + null or "" => null, + var modelId => modelId, + }; + result.FrequencyPenalty = options.FrequencyPenalty; result.MaxOutputTokens = options.MaxOutputTokenCount; result.TopP = options.TopP; From 7f0d0e3ff90dd41d5d70da196509309f14ffb7ed Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Fri, 17 Jan 2025 15:42:57 +0000 Subject: [PATCH 3/3] Undo accidental change. --- .../OpenAIModelMappers.StreamingChatCompletion.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMappers.StreamingChatCompletion.cs b/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMappers.StreamingChatCompletion.cs index 952db95f48f..5d743a1baea 100644 --- a/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMappers.StreamingChatCompletion.cs +++ b/src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMappers.StreamingChatCompletion.cs @@ -47,7 +47,7 @@ internal static partial class OpenAIModelMappers yield return OpenAIChatModelFactory.StreamingChatCompletionUpdate( completionId: chatCompletionUpdate.CompletionId ?? CreateCompletionId(), - model: chatCompletionUpdate.ModelId ?? string.Empty, + model: chatCompletionUpdate.ModelId, createdAt: chatCompletionUpdate.CreatedAt ?? DateTimeOffset.UtcNow, role: ToOpenAIChatRole(chatCompletionUpdate.Role), finishReason: chatCompletionUpdate.FinishReason is null ? null : ToOpenAIFinishReason(chatCompletionUpdate.FinishReason),