From f9ef92919a91fdd3d2129c7c8da2e573ee2eb124 Mon Sep 17 00:00:00 2001 From: Dmytro Struk Date: Tue, 28 Mar 2023 19:00:59 +0100 Subject: [PATCH 1/3] Added prompt normalization for OpenAI requests --- .../AI/AIServiceType.cs | 19 +++++ .../AI/OpenAICompletionTests.cs | 85 ++++++++++++++----- .../AI/OpenAI/Clients/OpenAIClientAbstract.cs | 12 +++ .../AI/OpenAI/Services/AzureTextCompletion.cs | 2 + .../OpenAI/Services/OpenAITextCompletion.cs | 2 + 5 files changed, 99 insertions(+), 21 deletions(-) create mode 100644 dotnet/src/SemanticKernel.IntegrationTests/AI/AIServiceType.cs diff --git a/dotnet/src/SemanticKernel.IntegrationTests/AI/AIServiceType.cs b/dotnet/src/SemanticKernel.IntegrationTests/AI/AIServiceType.cs new file mode 100644 index 000000000000..901903634e02 --- /dev/null +++ b/dotnet/src/SemanticKernel.IntegrationTests/AI/AIServiceType.cs @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft. All rights reserved. + +namespace SemanticKernel.IntegrationTests.AI; + +/// +/// Enumeration to run integration tests for different AI services +/// +public enum AIServiceType +{ + /// + /// Open AI service + /// + OpenAI = 0, + + /// + /// Azure Open AI service + /// + AzureOpenAI = 1 +} diff --git a/dotnet/src/SemanticKernel.IntegrationTests/AI/OpenAICompletionTests.cs b/dotnet/src/SemanticKernel.IntegrationTests/AI/OpenAICompletionTests.cs index 3b9c186c472a..63351caec1f9 100644 --- a/dotnet/src/SemanticKernel.IntegrationTests/AI/OpenAICompletionTests.cs +++ b/dotnet/src/SemanticKernel.IntegrationTests/AI/OpenAICompletionTests.cs @@ -31,6 +31,9 @@ public OpenAICompletionTests(ITestOutputHelper output) .AddEnvironmentVariables() .AddUserSecrets() .Build(); + + this._serviceConfiguration.Add(AIServiceType.OpenAI, this.ConfigureOpenAI); + this._serviceConfiguration.Add(AIServiceType.AzureOpenAI, this.ConfigureAzureOpenAI); } [Theory(Skip = "OpenAI will often throttle requests. This test is for manual verification.")] @@ -40,15 +43,7 @@ public async Task OpenAITestAsync(string prompt, string expectedAnswerContains) // Arrange IKernel target = Kernel.Builder.WithLogger(this._logger).Build(); - OpenAIConfiguration? openAIConfiguration = this._configuration.GetSection("OpenAI").Get(); - Assert.NotNull(openAIConfiguration); - - target.Config.AddOpenAITextCompletion( - serviceId: openAIConfiguration.Label, - modelId: openAIConfiguration.ModelId, - apiKey: openAIConfiguration.ApiKey); - - target.Config.SetDefaultTextCompletionService(openAIConfiguration.Label); + this.ConfigureOpenAI(target); IDictionary skill = TestHelpers.GetSkill("ChatSkill", target); @@ -66,18 +61,7 @@ public async Task AzureOpenAITestAsync(string prompt, string expectedAnswerConta // Arrange IKernel target = Kernel.Builder.WithLogger(this._logger).Build(); - // OpenAIConfiguration? openAIConfiguration = this._configuration.GetSection("OpenAI").Get(); - // Assert.NotNull(openAIConfiguration); - AzureOpenAIConfiguration? azureOpenAIConfiguration = this._configuration.GetSection("AzureOpenAI").Get(); - Assert.NotNull(azureOpenAIConfiguration); - - target.Config.AddAzureOpenAITextCompletion( - serviceId: azureOpenAIConfiguration.Label, - deploymentName: azureOpenAIConfiguration.DeploymentName, - endpoint: azureOpenAIConfiguration.Endpoint, - apiKey: azureOpenAIConfiguration.ApiKey); - - target.Config.SetDefaultTextCompletionService(azureOpenAIConfiguration.Label); + this.ConfigureAzureOpenAI(target); IDictionary skill = TestHelpers.GetSkill("ChatSkill", target); @@ -118,11 +102,41 @@ public async Task OpenAIHttpRetryPolicyTestAsync(string prompt, string expectedO Assert.Contains(expectedOutput, this._testOutputHelper.GetLogs(), StringComparison.InvariantCultureIgnoreCase); } + [Theory(Skip = "This test is for manual verification.")] + [InlineData("\n", AIServiceType.OpenAI)] + [InlineData("\r\n", AIServiceType.OpenAI)] + [InlineData("\n", AIServiceType.AzureOpenAI)] + [InlineData("\r\n", AIServiceType.AzureOpenAI)] + public async Task CompletionWithDifferentLineEndingsAsync(string lineEnding, AIServiceType service) + { + // Arrange + var prompt = + $"Given a json input and a request. Apply the request on the json input and return the result. " + + $"Put the result in between tags{lineEnding}" + + $"Input:{lineEnding}{{\"name\": \"John\", \"age\": 30}}{lineEnding}{lineEnding}Request:{lineEnding}name"; + + const string expectedAnswerContains = "John"; + + IKernel target = Kernel.Builder.WithLogger(this._logger).Build(); + + this._serviceConfiguration[service](target); + + IDictionary skill = TestHelpers.GetSkill("ChatSkill", target); + + // Act + SKContext actual = await target.RunAsync(prompt, skill["Chat"]); + + // Assert + Assert.Contains(expectedAnswerContains, actual.Result, StringComparison.InvariantCultureIgnoreCase); + } + #region internals private readonly XunitLogger _logger; private readonly RedirectOutput _testOutputHelper; + private readonly Dictionary> _serviceConfiguration = new(); + public void Dispose() { this.Dispose(true); @@ -143,5 +157,34 @@ private void Dispose(bool disposing) } } + private void ConfigureOpenAI(IKernel kernel) + { + var openAIConfiguration = this._configuration.GetSection("OpenAI").Get(); + + Assert.NotNull(openAIConfiguration); + + kernel.Config.AddOpenAITextCompletion( + serviceId: openAIConfiguration.Label, + modelId: openAIConfiguration.ModelId, + apiKey: openAIConfiguration.ApiKey); + + kernel.Config.SetDefaultTextCompletionService(openAIConfiguration.Label); + } + + private void ConfigureAzureOpenAI(IKernel kernel) + { + var azureOpenAIConfiguration = this._configuration.GetSection("AzureOpenAI").Get(); + + Assert.NotNull(azureOpenAIConfiguration); + + kernel.Config.AddAzureOpenAITextCompletion( + serviceId: azureOpenAIConfiguration.Label, + deploymentName: azureOpenAIConfiguration.DeploymentName, + endpoint: azureOpenAIConfiguration.Endpoint, + apiKey: azureOpenAIConfiguration.ApiKey); + + kernel.Config.SetDefaultTextCompletionService(azureOpenAIConfiguration.Label); + } + #endregion } diff --git a/dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs b/dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs index 944b8b2978e4..f6f27f25f9ac 100644 --- a/dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs +++ b/dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs @@ -207,6 +207,18 @@ protected async Task> ExecuteImageBase64GenerationRequestAsync( } } + /// + /// Performs prompt normalization + /// + /// Prompt to normalize + protected void NormalizePrompt(string prompt) + { + if (!string.IsNullOrWhiteSpace(prompt)) + { + prompt = prompt.Replace("\r\n", "\n", StringComparison.InvariantCultureIgnoreCase); + } + } + /// /// Explicit finalizer called by IDisposable /// diff --git a/dotnet/src/SemanticKernel/AI/OpenAI/Services/AzureTextCompletion.cs b/dotnet/src/SemanticKernel/AI/OpenAI/Services/AzureTextCompletion.cs index ab6431328e0a..3ced28f81922 100644 --- a/dotnet/src/SemanticKernel/AI/OpenAI/Services/AzureTextCompletion.cs +++ b/dotnet/src/SemanticKernel/AI/OpenAI/Services/AzureTextCompletion.cs @@ -72,6 +72,8 @@ public async Task CompleteAsync(string text, CompleteRequestSettings req $"MaxTokens {requestSettings.MaxTokens} is not valid, the value must be greater than zero"); } + this.NormalizePrompt(text); + var requestBody = Json.Serialize(new AzureTextCompletionRequest { Prompt = text, diff --git a/dotnet/src/SemanticKernel/AI/OpenAI/Services/OpenAITextCompletion.cs b/dotnet/src/SemanticKernel/AI/OpenAI/Services/OpenAITextCompletion.cs index 5194f1d6227b..142d0737be22 100644 --- a/dotnet/src/SemanticKernel/AI/OpenAI/Services/OpenAITextCompletion.cs +++ b/dotnet/src/SemanticKernel/AI/OpenAI/Services/OpenAITextCompletion.cs @@ -71,6 +71,8 @@ public Task CompleteAsync(string text, CompleteRequestSettings requestSe $"MaxTokens {requestSettings.MaxTokens} is not valid, the value must be greater than zero"); } + this.NormalizePrompt(text); + var requestBody = Json.Serialize(new OpenAITextCompletionRequest { Model = this._modelId, From 28a7ae1dde8b456aa2f0fb806834a7e74868d049 Mon Sep 17 00:00:00 2001 From: Dmytro Struk Date: Tue, 28 Mar 2023 19:55:15 +0100 Subject: [PATCH 2/3] Fixed normalization function --- .../SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs | 5 ++++- .../SemanticKernel/AI/OpenAI/Services/AzureTextCompletion.cs | 4 +--- .../AI/OpenAI/Services/OpenAITextCompletion.cs | 4 +--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs b/dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs index f6f27f25f9ac..49ec02a7fecc 100644 --- a/dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs +++ b/dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs @@ -211,12 +211,15 @@ protected async Task> ExecuteImageBase64GenerationRequestAsync( /// Performs prompt normalization /// /// Prompt to normalize - protected void NormalizePrompt(string prompt) + /// Normalized prompt + protected string NormalizePrompt(string prompt) { if (!string.IsNullOrWhiteSpace(prompt)) { prompt = prompt.Replace("\r\n", "\n", StringComparison.InvariantCultureIgnoreCase); } + + return prompt; } /// diff --git a/dotnet/src/SemanticKernel/AI/OpenAI/Services/AzureTextCompletion.cs b/dotnet/src/SemanticKernel/AI/OpenAI/Services/AzureTextCompletion.cs index 3ced28f81922..e20b41ddfd0d 100644 --- a/dotnet/src/SemanticKernel/AI/OpenAI/Services/AzureTextCompletion.cs +++ b/dotnet/src/SemanticKernel/AI/OpenAI/Services/AzureTextCompletion.cs @@ -72,11 +72,9 @@ public async Task CompleteAsync(string text, CompleteRequestSettings req $"MaxTokens {requestSettings.MaxTokens} is not valid, the value must be greater than zero"); } - this.NormalizePrompt(text); - var requestBody = Json.Serialize(new AzureTextCompletionRequest { - Prompt = text, + Prompt = this.NormalizePrompt(text), Temperature = requestSettings.Temperature, TopP = requestSettings.TopP, PresencePenalty = requestSettings.PresencePenalty, diff --git a/dotnet/src/SemanticKernel/AI/OpenAI/Services/OpenAITextCompletion.cs b/dotnet/src/SemanticKernel/AI/OpenAI/Services/OpenAITextCompletion.cs index 142d0737be22..089fd2e2726f 100644 --- a/dotnet/src/SemanticKernel/AI/OpenAI/Services/OpenAITextCompletion.cs +++ b/dotnet/src/SemanticKernel/AI/OpenAI/Services/OpenAITextCompletion.cs @@ -71,12 +71,10 @@ public Task CompleteAsync(string text, CompleteRequestSettings requestSe $"MaxTokens {requestSettings.MaxTokens} is not valid, the value must be greater than zero"); } - this.NormalizePrompt(text); - var requestBody = Json.Serialize(new OpenAITextCompletionRequest { Model = this._modelId, - Prompt = text, + Prompt = this.NormalizePrompt(text), Temperature = requestSettings.Temperature, TopP = requestSettings.TopP, PresencePenalty = requestSettings.PresencePenalty, From aac6c2b5f428693626217f9cf17d7bcc18e75e18 Mon Sep 17 00:00:00 2001 From: Dmytro Struk Date: Tue, 28 Mar 2023 20:19:03 +0100 Subject: [PATCH 3/3] Put normalization function to string extensions. Added unit tests. --- .../Text/StringExtensionsTests.cs | 27 +++++++++++++++++++ .../AI/OpenAI/Clients/OpenAIClientAbstract.cs | 15 ----------- .../AI/OpenAI/Services/AzureTextCompletion.cs | 2 +- .../OpenAI/Services/OpenAITextCompletion.cs | 2 +- .../SemanticKernel/Text/StringExtensions.cs | 5 ++++ 5 files changed, 34 insertions(+), 17 deletions(-) create mode 100644 dotnet/src/SemanticKernel.UnitTests/Text/StringExtensionsTests.cs diff --git a/dotnet/src/SemanticKernel.UnitTests/Text/StringExtensionsTests.cs b/dotnet/src/SemanticKernel.UnitTests/Text/StringExtensionsTests.cs new file mode 100644 index 000000000000..c5b7a1cd5889 --- /dev/null +++ b/dotnet/src/SemanticKernel.UnitTests/Text/StringExtensionsTests.cs @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using Microsoft.SemanticKernel.Text; +using Xunit; + +namespace SemanticKernel.UnitTests.Text; + +/// +/// Unit tests for +/// +public class StringExtensionsTests +{ + [Theory] + [InlineData("\r\n", "\n")] + [InlineData("Test string\r\n", "Test string\n")] + [InlineData("\r\nTest string", "\nTest string")] + [InlineData("\r\nTest string\r\n", "\nTest string\n")] + public void ItNormalizesLineEndingsCorrectly(string input, string expectedString) + { + // Act + input = input.NormalizeLineEndings(); + + // Assert + Assert.Equal(expectedString, input); + } +} diff --git a/dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs b/dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs index 49ec02a7fecc..944b8b2978e4 100644 --- a/dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs +++ b/dotnet/src/SemanticKernel/AI/OpenAI/Clients/OpenAIClientAbstract.cs @@ -207,21 +207,6 @@ protected async Task> ExecuteImageBase64GenerationRequestAsync( } } - /// - /// Performs prompt normalization - /// - /// Prompt to normalize - /// Normalized prompt - protected string NormalizePrompt(string prompt) - { - if (!string.IsNullOrWhiteSpace(prompt)) - { - prompt = prompt.Replace("\r\n", "\n", StringComparison.InvariantCultureIgnoreCase); - } - - return prompt; - } - /// /// Explicit finalizer called by IDisposable /// diff --git a/dotnet/src/SemanticKernel/AI/OpenAI/Services/AzureTextCompletion.cs b/dotnet/src/SemanticKernel/AI/OpenAI/Services/AzureTextCompletion.cs index e20b41ddfd0d..ad3d31614649 100644 --- a/dotnet/src/SemanticKernel/AI/OpenAI/Services/AzureTextCompletion.cs +++ b/dotnet/src/SemanticKernel/AI/OpenAI/Services/AzureTextCompletion.cs @@ -74,7 +74,7 @@ public async Task CompleteAsync(string text, CompleteRequestSettings req var requestBody = Json.Serialize(new AzureTextCompletionRequest { - Prompt = this.NormalizePrompt(text), + Prompt = text.NormalizeLineEndings(), Temperature = requestSettings.Temperature, TopP = requestSettings.TopP, PresencePenalty = requestSettings.PresencePenalty, diff --git a/dotnet/src/SemanticKernel/AI/OpenAI/Services/OpenAITextCompletion.cs b/dotnet/src/SemanticKernel/AI/OpenAI/Services/OpenAITextCompletion.cs index 089fd2e2726f..96b7ac1f8052 100644 --- a/dotnet/src/SemanticKernel/AI/OpenAI/Services/OpenAITextCompletion.cs +++ b/dotnet/src/SemanticKernel/AI/OpenAI/Services/OpenAITextCompletion.cs @@ -74,7 +74,7 @@ public Task CompleteAsync(string text, CompleteRequestSettings requestSe var requestBody = Json.Serialize(new OpenAITextCompletionRequest { Model = this._modelId, - Prompt = this.NormalizePrompt(text), + Prompt = text.NormalizeLineEndings(), Temperature = requestSettings.Temperature, TopP = requestSettings.TopP, PresencePenalty = requestSettings.PresencePenalty, diff --git a/dotnet/src/SemanticKernel/Text/StringExtensions.cs b/dotnet/src/SemanticKernel/Text/StringExtensions.cs index fcf60d771127..f94636ff8d27 100644 --- a/dotnet/src/SemanticKernel/Text/StringExtensions.cs +++ b/dotnet/src/SemanticKernel/Text/StringExtensions.cs @@ -12,4 +12,9 @@ internal static bool EqualsIgnoreCase(this string src, string value) { return src.Equals(value, StringComparison.InvariantCultureIgnoreCase); } + + internal static string NormalizeLineEndings(this string src) + { + return src.Replace("\r\n", "\n", StringComparison.InvariantCultureIgnoreCase); + } }