Skip to content

Commit f34f54f

Browse files
authored
Address PR feedback: DiagnosticIds merge, netfx fix, duplicate detection, test improvements
1 parent 6da64b7 commit f34f54f

6 files changed

Lines changed: 195 additions & 15 deletions

File tree

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,11 @@ private ReflectionAIFunctionDescriptor(DescriptorKey key, JsonSerializerOptions
808808
pType != typeof(IServiceProvider) &&
809809
!string.IsNullOrEmpty(parameters[i].Name))
810810
{
811-
_ = expectedArgumentNames.Add(AIJsonUtilities.GetParameterSchemaName(parameters[i]));
811+
string effectiveName = AIJsonUtilities.GetParameterSchemaName(parameters[i]);
812+
if (!expectedArgumentNames.Add(effectiveName))
813+
{
814+
Throw.ArgumentException(nameof(key.Method), $"Multiple parameters are mapped to the same name '{effectiveName}'. Ensure that any {nameof(AIParameterNameAttribute)} values do not collide with each other or with other parameter names.");
815+
}
812816
}
813817
}
814818

@@ -1220,9 +1224,11 @@ private static bool IsAIContentRelatedType(Type type) =>
12201224
{
12211225
return method.ReturnParameter.GetCustomAttribute<DescriptionAttribute>(inherit: true)?.Description;
12221226
}
1223-
catch (Exception e) when (e is ArgumentNullException or NullReferenceException)
1227+
catch (Exception e) when (e is ArgumentNullException or NullReferenceException or IndexOutOfRangeException)
12241228
{
1225-
// DynamicMethod return parameters don't support GetCustomAttribute.
1229+
// DynamicMethod return parameters don't support GetCustomAttribute. Additionally, on .NET Framework,
1230+
// querying inherited attributes on the return parameter of an overriding method can throw
1231+
// IndexOutOfRangeException. In either case, treat the description as absent.
12261232
return null;
12271233
}
12281234
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace Microsoft.Extensions.AI;
1414
/// By default this is inferred from .NET metadata. Apply this attribute to use a different identifier.
1515
/// </remarks>
1616
[AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
17-
[Experimental(DiagnosticIds.Experiments.AIFunctionName, UrlFormat = DiagnosticIds.UrlFormat)]
17+
[Experimental(DiagnosticIds.Experiments.AIFunctionAndParameterName, UrlFormat = DiagnosticIds.UrlFormat)]
1818
public sealed class AIFunctionNameAttribute : Attribute
1919
{
2020
/// <summary>Initializes a new instance of the <see cref="AIFunctionNameAttribute"/> class.</summary>

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace Microsoft.Extensions.AI;
1414
/// By default this is inferred from .NET metadata. Apply this attribute to use a different identifier.
1515
/// </remarks>
1616
[AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false, Inherited = true)]
17-
[Experimental(DiagnosticIds.Experiments.AIParameterName, UrlFormat = DiagnosticIds.UrlFormat)]
17+
[Experimental(DiagnosticIds.Experiments.AIFunctionAndParameterName, UrlFormat = DiagnosticIds.UrlFormat)]
1818
public sealed class AIParameterNameAttribute : Attribute
1919
{
2020
/// <summary>Initializes a new instance of the <see cref="AIParameterNameAttribute"/> class.</summary>

src/Shared/DiagnosticIds/DiagnosticIds.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,7 @@ internal static class Experiments
5454
internal const string AIMcpServers = AIExperiments;
5555
internal const string AIFunctionApprovals = AIExperiments;
5656
internal const string AIApprovalsInvocationRequired = AIExperiments;
57-
internal const string AIFunctionName = AIExperiments;
58-
internal const string AIParameterName = AIExperiments;
57+
internal const string AIFunctionAndParameterName = AIExperiments;
5958

6059
internal const string AIChatReduction = AIExperiments;
6160
internal const string AIToolSearch = AIExperiments;

test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Utilities/AIJsonUtilitiesTests.cs

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -584,18 +584,18 @@ static void TestMethod()
584584
[Fact]
585585
public static void CreateFunctionJsonSchema_AIParameterNameAttribute_UsedForPropertyName()
586586
{
587-
Delegate method = ([AIParameterName("$select")] string select, int top) =>
587+
Delegate method = ([AIParameterName("custom_property_name")] string select, int top) =>
588588
{
589589
};
590590

591591
JsonElement schema = AIJsonUtilities.CreateFunctionJsonSchema(method.Method);
592592

593593
JsonElement properties = schema.GetProperty("properties");
594-
Assert.True(properties.TryGetProperty("$select", out _));
594+
Assert.True(properties.TryGetProperty("custom_property_name", out _));
595595
Assert.False(properties.TryGetProperty("select", out _));
596596

597597
string[] required = schema.GetProperty("required").EnumerateArray().Select(e => e.GetString()!).ToArray();
598-
Assert.Contains("$select", required);
598+
Assert.Contains("custom_property_name", required);
599599
Assert.Contains("top", required);
600600
}
601601

@@ -627,6 +627,26 @@ public static void CreateFunctionJsonSchema_AIParameterNameAttribute_EscapesJson
627627
Assert.DoesNotContain("#/properties/a/b~c", schema);
628628
}
629629

630+
[Fact]
631+
public static void CreateFunctionJsonSchema_AIFunctionNameAttribute_NotUsedForSchemaTitle()
632+
{
633+
// AIFunctionNameAttribute overrides the AI-facing function name but does not affect the JSON schema title.
634+
Delegate method = [AIFunctionName("my_tool")] (string param) => { };
635+
636+
JsonElement schema = AIJsonUtilities.CreateFunctionJsonSchema(method.Method);
637+
638+
// The schema title is not derived from AIFunctionNameAttribute.
639+
Assert.False(schema.TryGetProperty("title", out JsonElement titleElement) && titleElement.GetString() == "my_tool");
640+
}
641+
642+
[Fact]
643+
public static void CreateFunctionJsonSchema_AIFunctionNameAttribute_HonoredByAIFunctionFactory()
644+
{
645+
Func<string> funcWithAttribute = [AIFunctionName("get_user")] () => "test";
646+
AIFunction func = AIFunctionFactory.Create(funcWithAttribute);
647+
Assert.Equal("get_user", func.Name);
648+
}
649+
630650
[Fact]
631651
public static void CreateJsonSchema_CanBeBoolean()
632652
{

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

Lines changed: 160 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using System.Text.Json;
1111
using System.Text.Json.Nodes;
1212
using System.Text.Json.Serialization;
13+
using System.Text.Json.Serialization.Metadata;
1314
using System.Threading;
1415
using System.Threading.Tasks;
1516
using Microsoft.Extensions.DependencyInjection;
@@ -18,6 +19,7 @@
1819
#pragma warning disable IDE0004 // Remove Unnecessary Cast
1920
#pragma warning disable S103 // Lines should not be too long
2021
#pragma warning disable S107 // Methods should not have too many parameters
22+
#pragma warning disable S1144 // Unused private types or members should be removed (accessed via reflection)
2123
#pragma warning disable S2760 // Sequential tests should not check the same condition
2224
#pragma warning disable S3358 // Ternary operators should not be nested
2325
#pragma warning disable S5034 // "ValueTask" should be consumed correctly
@@ -338,16 +340,154 @@ public async Task Parameters_MappedByAIParameterNameAttribute_Async()
338340
{
339341
AIFunction func = AIFunctionFactory.Create(([AIParameterName("$select")] string select, int top) => select + top);
340342

341-
AssertExtensions.EqualFunctionCallResults("Name2", await func.InvokeAsync(new()
342-
{
343-
["$select"] = "Name",
344-
["top"] = 2,
345-
}));
343+
AssertExtensions.EqualFunctionCallResults("Name2", await func.InvokeAsync(new() { ["$select"] = "Name", ["top"] = 2 }));
346344

347345
ArgumentException ex = await Assert.ThrowsAsync<ArgumentException>(() => func.InvokeAsync(new() { ["top"] = 2 }).AsTask());
348346
Assert.Contains("$select", ex.Message);
349347
}
350348

349+
[Fact]
350+
public void AIParameterNameAttribute_OverridesSchemaPropertyName()
351+
{
352+
AIFunction func = AIFunctionFactory.Create(
353+
([AIParameterName("my_param")] string myParam, int top) => myParam + top);
354+
355+
JsonElement expectedSchema = JsonDocument.Parse("""
356+
{
357+
"type": "object",
358+
"properties": {
359+
"my_param": { "type": "string" },
360+
"top": { "type": "integer" }
361+
},
362+
"required": ["my_param", "top"]
363+
}
364+
""").RootElement;
365+
366+
AssertExtensions.EqualJsonValues(expectedSchema, func.JsonSchema);
367+
}
368+
369+
[Fact]
370+
public async Task AIParameterNameAttribute_BindsArgumentByOverriddenName_Async()
371+
{
372+
AIFunction func = AIFunctionFactory.Create(
373+
([AIParameterName("$select")] string select,
374+
[AIParameterName("$expand")] string expand,
375+
string filter) =>
376+
$"select='{select}', expand='{expand}', filter='{filter}'");
377+
378+
object? result = await func.InvokeAsync(new()
379+
{
380+
["$select"] = "Name,Id",
381+
["$expand"] = "Orders",
382+
["filter"] = "Active",
383+
});
384+
385+
AssertExtensions.EqualFunctionCallResults("select='Name,Id', expand='Orders', filter='Active'", result);
386+
}
387+
388+
[Fact]
389+
public async Task AIParameterNameAttribute_MissingRequiredArgument_ReportsSchemaName_Async()
390+
{
391+
AIFunction func = AIFunctionFactory.Create(
392+
([AIParameterName("my_param")] string myParam) => myParam);
393+
394+
ArgumentException ex = await Assert.ThrowsAsync<ArgumentException>(() => func.InvokeAsync().AsTask());
395+
396+
Assert.Contains("my_param", ex.Message);
397+
}
398+
399+
[Fact]
400+
public async Task AIParameterNameAttribute_HonoredByStrictUnmappedMemberHandling_Async()
401+
{
402+
JsonSerializerOptions strictOptions = new(AIJsonUtilities.DefaultOptions)
403+
{
404+
UnmappedMemberHandling = JsonUnmappedMemberHandling.Disallow,
405+
};
406+
407+
AIFunction func = AIFunctionFactory.Create(
408+
([AIParameterName("my_param")] string myParam) => myParam,
409+
new AIFunctionFactoryOptions { SerializerOptions = strictOptions });
410+
411+
// The overridden name is "expected", so it passes strict validation.
412+
AssertExtensions.EqualFunctionCallResults("Name", await func.InvokeAsync(new() { ["my_param"] = "Name" }));
413+
414+
// The underlying C# name is now an unexpected argument.
415+
ArgumentException ex = await Assert.ThrowsAsync<ArgumentException>("arguments", async () =>
416+
await func.InvokeAsync(new() { ["myParam"] = "Name" }));
417+
Assert.Contains("myParam", ex.Message);
418+
}
419+
420+
[Fact]
421+
public async Task AIParameterNameAttribute_InheritedByOverride_Async()
422+
{
423+
MethodInfo overrideMethod = typeof(MyDerivedType).GetMethod(nameof(MyDerivedType.Method))!;
424+
AIFunction func = AIFunctionFactory.Create(overrideMethod, new MyDerivedType());
425+
426+
Assert.Contains("my_param", func.JsonSchema.ToString());
427+
Assert.DoesNotContain("\"myParam\"", func.JsonSchema.ToString());
428+
429+
AssertExtensions.EqualFunctionCallResults("param='Name'", await func.InvokeAsync(new() { ["my_param"] = "Name" }));
430+
}
431+
432+
[Fact]
433+
public async Task AIFunctionAndParameterNameAttributes_BothHonored_Async()
434+
{
435+
AIFunction func = AIFunctionFactory.Create([AIFunctionName("my_tool")] ([AIParameterName("my_param")] string myParam) => myParam);
436+
437+
Assert.Equal("my_tool", func.Name);
438+
Assert.Contains("my_param", func.JsonSchema.ToString());
439+
440+
AssertExtensions.EqualFunctionCallResults("Name", await func.InvokeAsync(new() { ["my_param"] = "Name" }));
441+
}
442+
443+
[Fact]
444+
public void AIFunctionAndParameterNameAttributes_NameAndParameterSchema_PreservedByAsDeclarationOnly()
445+
{
446+
AIFunction func = AIFunctionFactory.Create([AIFunctionName("my_tool")] ([AIParameterName("my_param")] string myParam) => myParam);
447+
448+
AIFunctionDeclaration declaration = func.AsDeclarationOnly();
449+
450+
Assert.Equal("my_tool", declaration.Name);
451+
Assert.Equal(func.JsonSchema.ToString(), declaration.JsonSchema.ToString());
452+
Assert.Contains("my_param", declaration.JsonSchema.ToString());
453+
Assert.IsNotAssignableFrom<AIFunction>(declaration);
454+
}
455+
456+
[Fact]
457+
public void AIParameterNameAttribute_EscapesNameInJsonPointerRef()
458+
{
459+
JsonSerializerOptions options = new(AIJsonUtilities.DefaultOptions) { TypeInfoResolver = new DefaultJsonTypeInfoResolver() };
460+
461+
AIFunction func = AIFunctionFactory.Create(
462+
([AIParameterName("a/b~c")] AIParameterNameAttributeRecursiveNode node) => node.ToString(),
463+
new AIFunctionFactoryOptions { SerializerOptions = options });
464+
465+
string schema = func.JsonSchema.ToString();
466+
467+
Assert.Contains("#/properties/a~1b~0c", schema);
468+
Assert.DoesNotContain("#/properties/a/b~c", schema);
469+
}
470+
471+
[Fact]
472+
public void AIParameterNameAttribute_DuplicateNames_Throw()
473+
{
474+
ArgumentException ex = Assert.Throws<ArgumentException>(() => AIFunctionFactory.Create(
475+
([AIParameterName("dup")] string first, [AIParameterName("dup")] string second) => first + second));
476+
Assert.Contains("dup", ex.Message);
477+
478+
ArgumentException ex2 = Assert.Throws<ArgumentException>(() => AIFunctionFactory.Create(
479+
([AIParameterName("filter")] string select, string filter) => select + filter));
480+
Assert.Contains("filter", ex2.Message);
481+
}
482+
483+
[Fact]
484+
public void AIFunctionNameAttribute_DisplayNameAttribute_StillHonoredWhenNoAIFunctionNameAttribute()
485+
{
486+
AIFunction func = AIFunctionFactory.Create([DisplayName("from-display-name")] () => "result");
487+
488+
Assert.Equal("from-display-name", func.Name);
489+
}
490+
351491
[Fact]
352492
public void AIFunctionFactoryCreateOptions_ValuesPropagateToAIFunction()
353493
{
@@ -1625,4 +1765,19 @@ public async Task Parameters_UnmappedMemberHandlingDisallow_CustomBindParameter_
16251765
[JsonSerializable(typeof(int?))]
16261766
[JsonSerializable(typeof(DateTime?))]
16271767
private partial class JsonContext : JsonSerializerContext;
1768+
1769+
private abstract class MyBaseType
1770+
{
1771+
public abstract string Method([AIParameterName("my_param")] string myParam);
1772+
}
1773+
1774+
private sealed class MyDerivedType : MyBaseType
1775+
{
1776+
public override string Method(string myParam) => $"param='{myParam}'";
1777+
}
1778+
1779+
private sealed class AIParameterNameAttributeRecursiveNode
1780+
{
1781+
public AIParameterNameAttributeRecursiveNode? Next { get; set; }
1782+
}
16281783
}

0 commit comments

Comments
 (0)