Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.Net InputVariable.Default type is changed from string to object? #4345

Merged
merged 14 commits into from
Dec 18, 2023
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 @@ -22,9 +22,10 @@ public static OpenAIFunction ToOpenAIFunction(this KernelFunctionMetadata metada
for (int i = 0; i < openAIParams.Length; i++)
{
var param = metadataParams[i];

openAIParams[i] = new OpenAIFunctionParameter(
param.Name,
string.IsNullOrEmpty(param.DefaultValue) ? param.Description : $"{param.Description} (default value: {param.DefaultValue})",
GetDescription(param),
param.IsRequired,
param.ParameterType,
param.Schema);
Expand All @@ -39,5 +40,15 @@ public static OpenAIFunction ToOpenAIFunction(this KernelFunctionMetadata metada
metadata.ReturnParameter.Description,
metadata.ReturnParameter.ParameterType,
metadata.ReturnParameter.Schema));

static string GetDescription(KernelParameterMetadata param)
{
if (InternalTypeConverter.ConvertToString(param.DefaultValue) is string stringValue && !string.IsNullOrEmpty(stringValue))
{
return $"{param.Description} (default value: {stringValue})";
}

return param.Description;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,12 @@ private KernelArguments GetVariables(KernelArguments? arguments)

foreach (var p in this._promptModel.InputVariables)
{
if (!string.IsNullOrEmpty(p.Default))
if (p.Default == null || (p.Default is string stringDefault && stringDefault.Length == 0))
{
result[p.Name] = p.Default;
continue;
}

result[p.Name] = p.Default;
}

if (arguments is not null)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) Microsoft. All rights reserved.

using System;
using Microsoft.SemanticKernel;
using Microsoft.SemanticKernel.Connectors.OpenAI;
using Xunit;
Expand All @@ -10,6 +11,15 @@ namespace SemanticKernel.Functions.UnitTests.Yaml;

public class KernelFunctionYamlTests
{
private readonly ISerializer _serializer;

public KernelFunctionYamlTests()
{
this._serializer = new SerializerBuilder()
.WithNamingConvention(UnderscoredNamingConvention.Instance)
.Build();
}

[Fact]
public void ItShouldCreateFunctionFromPromptYamlWithNoExecutionSettings()
{
Expand Down Expand Up @@ -72,78 +82,118 @@ public void ItShouldSupportCreatingOpenAIExecutionSettings()
Assert.Equal(0.0, executionSettings.TopP);
}

[Fact]
public void ItShouldCreateFunctionWithDefaultValueOfStringType()
{
// Act
var function = KernelFunctionYaml.FromPromptYaml(this._yamlWithCustomSettings);

// Assert
Assert.NotNull(function?.Metadata?.Parameters);
Assert.Equal("John", function?.Metadata?.Parameters[0].DefaultValue);
Assert.Equal("English", function?.Metadata?.Parameters[1].DefaultValue);
}

[Fact]
// This test checks that the logic of imposing a temporary limitation on the default value being a string is in place and works as expected.
public void ItShouldThrowExceptionWhileCreatingFunctionWithDefaultValueOtherThanString()
{
string CreateYaml(object defaultValue)
{
var obj = new
{
description = "function description",
input_variables = new[]
{
new
{
name = "name",
description = "description",
@default = defaultValue,
isRequired = true
}
}
};

return this._serializer.Serialize(obj);
}

// Act
Assert.Throws<NotSupportedException>(() => KernelFunctionYaml.FromPromptYaml(CreateYaml(new { p1 = "v1" })));
}

private readonly string _yamlNoExecutionSettings = @"
template_format: semantic-kernel
template: Say hello world to {{$name}} in {{$language}}
description: Say hello to the specified person using the specified language
name: SayHello
input_variables:
- name: name
description: The name of the person to greet
default: John
- name: language
description: The language to generate the greeting in
default: English
";
template_format: semantic-kernel
template: Say hello world to {{$name}} in {{$language}}
description: Say hello to the specified person using the specified language
name: SayHello
input_variables:
- name: name
description: The name of the person to greet
default: John
- name: language
description: The language to generate the greeting in
default: English
";

private readonly string _yaml = @"
template_format: semantic-kernel
template: Say hello world to {{$name}} in {{$language}}
description: Say hello to the specified person using the specified language
name: SayHello
input_variables:
- name: name
description: The name of the person to greet
default: John
- name: language
description: The language to generate the greeting in
default: English
execution_settings:
service1:
model_id: gpt-4
temperature: 1.0
top_p: 0.0
presence_penalty: 0.0
frequency_penalty: 0.0
max_tokens: 256
stop_sequences: []
service2:
model_id: gpt-3.5
temperature: 1.0
top_p: 0.0
presence_penalty: 0.0
frequency_penalty: 0.0
max_tokens: 256
stop_sequences: [ ""foo"", ""bar"", ""baz"" ]
";
template_format: semantic-kernel
template: Say hello world to {{$name}} in {{$language}}
description: Say hello to the specified person using the specified language
name: SayHello
input_variables:
- name: name
description: The name of the person to greet
default: John
- name: language
description: The language to generate the greeting in
default: English
execution_settings:
service1:
model_id: gpt-4
temperature: 1.0
top_p: 0.0
presence_penalty: 0.0
frequency_penalty: 0.0
max_tokens: 256
stop_sequences: []
service2:
model_id: gpt-3.5
temperature: 1.0
top_p: 0.0
presence_penalty: 0.0
frequency_penalty: 0.0
max_tokens: 256
stop_sequences: [ ""foo"", ""bar"", ""baz"" ]
";

private readonly string _yamlWithCustomSettings = @"
template_format: semantic-kernel
template: Say hello world to {{$name}} in {{$language}}
description: Say hello to the specified person using the specified language
name: SayHello
input_variables:
- name: name
description: The name of the person to greet
default: John
- name: language
description: The language to generate the greeting in
default: English
execution_settings:
service1:
model_id: gpt-4
temperature: 1.0
top_p: 0.0
presence_penalty: 0.0
frequency_penalty: 0.0
max_tokens: 256
stop_sequences: []
service2:
model_id: random-model
temperaturex: 1.0
top_q: 0.0
rando_penalty: 0.0
max_token_count: 256
stop_sequences: [ ""foo"", ""bar"", ""baz"" ]
";
template_format: semantic-kernel
template: Say hello world to {{$name}} in {{$language}}
description: Say hello to the specified person using the specified language
name: SayHello
input_variables:
- name: name
description: The name of the person to greet
default: John
- name: language
description: The language to generate the greeting in
default: English
execution_settings:
service1:
model_id: gpt-4
temperature: 1.0
top_p: 0.0
presence_penalty: 0.0
frequency_penalty: 0.0
max_tokens: 256
stop_sequences: []
service2:
model_id: random-model
temperaturex: 1.0
top_q: 0.0
rando_penalty: 0.0
max_token_count: 256
stop_sequences: [ ""foo"", ""bar"", ""baz"" ]
";
}
15 changes: 15 additions & 0 deletions dotnet/src/Functions/Functions.Yaml/KernelFunctionYaml.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) Microsoft. All rights reserved.

using System;
using Microsoft.Extensions.Logging;
using YamlDotNet.Serialization;
using YamlDotNet.Serialization.NamingConventions;
Expand Down Expand Up @@ -33,6 +34,20 @@ public static KernelFunction FromPromptYaml(

var promptTemplateConfig = deserializer.Deserialize<PromptTemplateConfig>(text);

// Prevent the default value from being any type other than a string.
// It's a temporary limitation that helps shape the public API surface
// (changing the type of the Default property to object) now, before the release.
// This helps avoid a breaking change while a proper solution for
// dealing with the different deserialization outputs of JSON/YAML prompt configurations is being evaluated.
foreach (var inputVariable in promptTemplateConfig.InputVariables)
{
if (inputVariable.Default is not null && inputVariable.Default is not string)
{
throw new NotSupportedException($"Default value for input variable '{inputVariable.Name}' must be a string. " +
$"This is a temporary limitation; future updates are expected to remove this constraint. Prompt function - '{promptTemplateConfig.Name ?? promptTemplateConfig.Description}'.");
}
}

return KernelFunctionFactory.CreateFromPrompt(
promptTemplateConfig,
promptTemplateFactory,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,14 @@ internal static string ToManualString(this KernelFunctionMetadata function)
{
var inputs = string.Join("\n", function.Parameters.Select(parameter =>
{
var defaultValueString = string.IsNullOrEmpty(parameter.DefaultValue) ? string.Empty : $" (default value: {parameter.DefaultValue})";
var defaultValueString = InternalTypeConverter.ConvertToString(parameter.DefaultValue);
defaultValueString = string.IsNullOrEmpty(defaultValueString) ? string.Empty : $" (default value: {defaultValueString})";
return $" - {parameter.Name}: {parameter.Description}{defaultValueString}";
}));

// description and inputs are indented by 2 spaces
// While each parameter in inputs is indented by 4 spaces
return $@"{function.ToFullyQualifiedName()}:
description: {function.Description}
inputs:
{inputs}";
return $"{function.ToFullyQualifiedName()}: description: {function.Description} inputs:{inputs}";
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ internal static class InternalTypeConverter
/// <param name="value">The object to convert.</param>
/// <param name="culture">The CultureInfo to consider during conversion.</param>
/// <returns>A string representation of the object value, considering the specified CultureInfo.</returns>
public static string? ConvertToString(object? value, CultureInfo culture)
public static string? ConvertToString(object? value, CultureInfo? culture = null)
{
if (value == null) { return null; }

Expand All @@ -28,7 +28,7 @@ internal static class InternalTypeConverter

return converterDelegate == null
? value.ToString()
: converterDelegate(value, culture);
: converterDelegate(value, culture ?? CultureInfo.InvariantCulture);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public sealed class KernelParameterMetadata
/// <summary>The description of the parameter.</summary>
private string _description = string.Empty;
/// <summary>The default value of the parameter.</summary>
private string? _defaultValue;
private object? _defaultValue;
/// <summary>The .NET type of the parameter.</summary>
private Type? _parameterType;
/// <summary>The schema of the parameter, potentially lazily-initialized.</summary>
Expand Down Expand Up @@ -72,7 +72,7 @@ public string Description
}

/// <summary>Gets the default value of the parameter.</summary>
public string? DefaultValue
public object? DefaultValue
{
get => this._defaultValue;
init
Expand Down Expand Up @@ -113,7 +113,7 @@ public KernelJsonSchema? Schema
/// <param name="parameterType">The parameter type. If null, no schema can be inferred.</param>
/// <param name="defaultValue">The parameter's default value, if any.</param>
/// <param name="description">The parameter description. If null, it won't be included in the schema.</param>
internal static InitializedSchema InferSchema(Type? parameterType, string? defaultValue, string? description)
internal static InitializedSchema InferSchema(Type? parameterType, object? defaultValue, string? description)
{
KernelJsonSchema? schema = null;

Expand All @@ -134,10 +134,10 @@ internal static InitializedSchema InferSchema(Type? parameterType, string? defau
{
try
{
if (!string.IsNullOrWhiteSpace(defaultValue))
if (InternalTypeConverter.ConvertToString(defaultValue) is string stringDefault && !string.IsNullOrWhiteSpace(stringDefault))
{
bool needsSpace = !string.IsNullOrWhiteSpace(description);
description += $"{(needsSpace ? " " : "")}(default value: {defaultValue})";
description += $"{(needsSpace ? " " : "")}(default value: {stringDefault})";
}

var builder = new JsonSchemaBuilder().FromType(parameterType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public string Description
/// Gets or sets a default value for the variable.
/// </summary>
[JsonPropertyName("default")]
public string? Default { get; set; }
public object? Default { get; set; }

/// <summary>
/// Gets or sets whether the variable is considered required (rather than optional).
Expand Down
Loading
Loading