-
Notifications
You must be signed in to change notification settings - Fork 841
Support DefaultValueAttribute in AIFunctionFactory parameter handling #6947
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
Conversation
- Check for DefaultValueAttribute when determining parameter optionality - Use DefaultValueAttribute value as default when parameter not provided - Add helper methods HasEffectiveDefaultValue and GetEffectiveDefaultValue - Add comprehensive tests for DefaultValue attribute support Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Verify that when both DefaultValue attribute and C# default are present, the DefaultValue attribute takes precedence. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
- Add test verifying DefaultValue attribute takes precedence over C# default - Add .nuget directory to gitignore to exclude build artifacts Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for the [DefaultValue] attribute in AIFunctionFactory, ensuring parameters decorated with this attribute are treated as optional in the same way as parameters with C# default value syntax.
Key Changes:
- Added helper methods to detect and retrieve default values from both
[DefaultValue]attribute and C# default syntax - Updated schema generation to correctly mark parameters with
[DefaultValue]as optional - Updated parameter marshaling to use default values from
[DefaultValue]attribute
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| AIJsonUtilities.Schema.Create.cs | Added HasEffectiveDefaultValue() and GetEffectiveDefaultValue() helpers to check for both C# defaults and [DefaultValue] attribute; updated schema generation logic to use these helpers |
| AIFunctionFactory.cs | Added duplicate helper methods and updated parameter marshaling to respect [DefaultValue] attribute |
| AIFunctionFactoryTest.cs | Added comprehensive test coverage for [DefaultValue] attribute functionality including null values, non-null values, and precedence scenarios |
src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunctionFactory.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs
Outdated
Show resolved
Hide resolved
stephentoub
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot, please fix the formatting diagnostics:
Build FAILED.
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(69,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(75,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(78,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(81,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(87,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(90,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(96,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(99,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(105,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net9.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(69,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(75,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(78,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(81,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(87,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(90,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(96,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(99,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]
/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs(105,1): error IDE0055: Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055) [/mnt/vss/_work/1/s/test/Libraries/Microsoft.Extensions.AI.Tests/Microsoft.Extensions.AI.Tests.csproj::TargetFramework=net8.0]
0 Warning(s)
18 Error(s)
src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs
Show resolved
Hide resolved
- 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>
test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs
Outdated
Show resolved
Hide resolved
Use a longer, more descriptive parameter name to avoid false positives when searching for the parameter name in string assertions. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Replace HasEffectiveDefaultValue and GetEffectiveDefaultValue with a single TryGetEffectiveDefaultValue method to avoid resolving the DefaultValueAttribute multiple times, improving performance. Co-authored-by: eiriktsarpalis <2813363+eiriktsarpalis@users.noreply.github.com>
[DefaultValue(null)]are treated as required instead of optionalParameterInfo.IsOptionalandParameterInfo.HasDefaultValueare false for[DefaultValue]attributesSummary
Successfully implemented support for the
[DefaultValue]attribute in AIFunctionFactory. Parameters decorated with[DefaultValue]are now treated as optional in the JSON schema and function invocation, matching the behavior of C# default values.Changes Made:
TryGetEffectiveDefaultValuehelper method to check for and retrieve default values from both C# defaults and DefaultValueAttribute in a single callSecurity Considerations:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Microsoft Reviewers: Open in CodeFlow