Skip to content

Commit c79b280

Browse files
Copilotstephentoub
andcommitted
Address PR feedback: consolidate helpers and fix formatting
- Revert changes to .gitignore - Fix formatting issues in test file (remove extra blank lines) - Make HasEffectiveDefaultValue and GetEffectiveDefaultValue internal in AIJsonUtilities - Remove duplicate helper methods from AIFunctionFactory and use the ones from AIJsonUtilities Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
1 parent da1832e commit c79b280

File tree

4 files changed

+14
-36
lines changed

4 files changed

+14
-36
lines changed

.gitignore

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,4 +319,3 @@ BenchmarkDotNet.artifacts/
319319

320320
# VERIFY
321321
!**/*.verified/**
322-
/.nuget/

src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunctionFactory.cs

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -803,27 +803,6 @@ static bool IsAsyncMethod(MethodInfo method)
803803
}
804804
}
805805

806-
/// <summary>
807-
/// Checks if a parameter has an effective default value, either through C# default value syntax or DefaultValueAttribute.
808-
/// </summary>
809-
private static bool HasEffectiveDefaultValue(ParameterInfo parameter) =>
810-
parameter.HasDefaultValue || parameter.GetCustomAttribute<DefaultValueAttribute>(inherit: true) is not null;
811-
812-
/// <summary>
813-
/// Gets the effective default value for a parameter, checking both C# default value and DefaultValueAttribute.
814-
/// </summary>
815-
private static object? GetEffectiveDefaultValue(ParameterInfo parameter)
816-
{
817-
// First check for DefaultValueAttribute
818-
if (parameter.GetCustomAttribute<DefaultValueAttribute>(inherit: true) is { } attr)
819-
{
820-
return attr.Value;
821-
}
822-
823-
// Fall back to the parameter's declared default value
824-
return parameter.DefaultValue;
825-
}
826-
827806
/// <summary>
828807
/// Gets a delegate for handling the marshaling of a parameter.
829808
/// </summary>
@@ -868,7 +847,7 @@ private static bool HasEffectiveDefaultValue(ParameterInfo parameter) =>
868847
return (arguments, _) =>
869848
{
870849
IServiceProvider? services = arguments.Services;
871-
if (!HasEffectiveDefaultValue(parameter) && services is null)
850+
if (!AIJsonUtilities.HasEffectiveDefaultValue(parameter) && services is null)
872851
{
873852
ThrowNullServices(parameter.Name);
874853
}
@@ -928,13 +907,13 @@ private static bool HasEffectiveDefaultValue(ParameterInfo parameter) =>
928907
}
929908

930909
// If the parameter is required and there's no argument specified for it, throw.
931-
if (!HasEffectiveDefaultValue(parameter))
910+
if (!AIJsonUtilities.HasEffectiveDefaultValue(parameter))
932911
{
933912
Throw.ArgumentException(nameof(arguments), $"The arguments dictionary is missing a value for the required parameter '{parameter.Name}'.");
934913
}
935914

936915
// Otherwise, use the optional parameter's default value.
937-
return GetEffectiveDefaultValue(parameter);
916+
return AIJsonUtilities.GetEffectiveDefaultValue(parameter);
938917
};
939918

940919
// Throws an ArgumentNullException indicating that AIFunctionArguments.Services must be provided.

src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -764,13 +764,13 @@ private static JsonElement ParseJsonElement(ReadOnlySpan<byte> utf8Json)
764764
/// <summary>
765765
/// Checks if a parameter has an effective default value, either through C# default value syntax or DefaultValueAttribute.
766766
/// </summary>
767-
private static bool HasEffectiveDefaultValue(ParameterInfo parameterInfo) =>
767+
internal static bool HasEffectiveDefaultValue(ParameterInfo parameterInfo) =>
768768
parameterInfo.HasDefaultValue || parameterInfo.GetCustomAttribute<DefaultValueAttribute>(inherit: true) is not null;
769769

770770
/// <summary>
771771
/// Gets the effective default value for a parameter, checking both C# default value and DefaultValueAttribute.
772772
/// </summary>
773-
private static object? GetEffectiveDefaultValue(ParameterInfo parameterInfo)
773+
internal static object? GetEffectiveDefaultValue(ParameterInfo parameterInfo)
774774
{
775775
// First check for DefaultValueAttribute
776776
if (parameterInfo.GetCustomAttribute<DefaultValueAttribute>(inherit: true) is { } attr)

test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -66,43 +66,43 @@ public async Task Parameters_DefaultValueAttributeIsRespected_Async()
6666
{
6767
// Test with null default value
6868
AIFunction funcNull = AIFunctionFactory.Create(([DefaultValue(null)] string? s) => s ?? "was null");
69-
69+
7070
// Schema should not list 's' as required and should have default value
7171
string schema = funcNull.JsonSchema.ToString();
7272
Assert.Contains("\"s\"", schema);
7373
Assert.DoesNotContain("\"required\"", schema);
7474
Assert.Contains("\"default\":null", schema);
75-
75+
7676
// Should be invocable without providing the parameter
7777
AssertExtensions.EqualFunctionCallResults("was null", await funcNull.InvokeAsync());
78-
78+
7979
// Should be overridable
8080
AssertExtensions.EqualFunctionCallResults("hello", await funcNull.InvokeAsync(new() { ["s"] = "hello" }));
81-
81+
8282
// Test with non-null default value
8383
AIFunction funcValue = AIFunctionFactory.Create(([DefaultValue("default")] string s) => s);
8484
schema = funcValue.JsonSchema.ToString();
8585
Assert.DoesNotContain("\"required\"", schema);
8686
Assert.Contains("\"default\":\"default\"", schema);
87-
87+
8888
AssertExtensions.EqualFunctionCallResults("default", await funcValue.InvokeAsync());
8989
AssertExtensions.EqualFunctionCallResults("custom", await funcValue.InvokeAsync(new() { ["s"] = "custom" }));
90-
90+
9191
// Test with int default value
9292
AIFunction funcInt = AIFunctionFactory.Create(([DefaultValue(42)] int x) => x * 2);
9393
schema = funcInt.JsonSchema.ToString();
9494
Assert.DoesNotContain("\"required\"", schema);
9595
Assert.Contains("\"default\":42", schema);
96-
96+
9797
AssertExtensions.EqualFunctionCallResults(84, await funcInt.InvokeAsync());
9898
AssertExtensions.EqualFunctionCallResults(10, await funcInt.InvokeAsync(new() { ["x"] = 5 }));
99-
99+
100100
// Test that DefaultValue attribute takes precedence over C# default value
101101
AIFunction funcBoth = AIFunctionFactory.Create(([DefaultValue(100)] int y = 50) => y);
102102
schema = funcBoth.JsonSchema.ToString();
103103
Assert.DoesNotContain("\"required\"", schema);
104104
Assert.Contains("\"default\":100", schema); // DefaultValue should take precedence
105-
105+
106106
AssertExtensions.EqualFunctionCallResults(100, await funcBoth.InvokeAsync()); // Should use DefaultValue, not C# default
107107
}
108108

0 commit comments

Comments
 (0)