- 
        Couldn't load subscription status. 
- Fork 840
Fix schema generation for Nullable<T> function parameters. #6596
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
Fix schema generation for Nullable<T> function parameters. #6596
Conversation
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 ensures that functions with Nullable<T> parameters generate correct JSON schemas (including null types and defaults) and updates related tests and utilities.
- Added a new test for nullable int?andDateTime?parameters inAIFunctionFactoryTest.
- Refactored AssertExtensionsto centralize JSON schema/value comparisons.
- Extended the schema generator in AIJsonUtilitiesto includenullin type arrays forNullable<T>and skip uninitialized objects forNullable<T>defaults.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description | 
|---|---|
| test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs | New AIFunctionFactory_NullableParameterstest and addedJsonSerializableforint?/DateTime?. | 
| test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/AssertExtensions.cs | Introduced EqualJsonValuesand updated helper for JSON comparisons. | 
| src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs | Updated schema exporter to handle Nullable<T>types and default values correctly. | 
Comments suppressed due to low confidence (3)
src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs:292
- There’s logic to handle nullable enum types but no corresponding unit test. Consider adding a test for a Nullable<YourEnum>parameter to verify enum schemas include["string","null"].
                if (Nullable.GetUnderlyingType(ctx.TypeInfo.Type) is Type nullableElement)
test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs:861
- [nitpick] This assertion about StructWithDefaultCtorisn’t related to validating nullable-parameter schemas and may be confusing. Consider removing or moving it to a dedicated test for struct default behavior.
        Assert.NotEqual(new StructWithDefaultCtor().Value, default(StructWithDefaultCtor).Value);
src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs:300
- The typeValuecomes from a non-nullableGetValue<string>()!call, so this null check is redundant and can be removed to simplify the logic.
                        if (typeValue is not null)
        
          
                src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
      …IJsonUtilities.Schema.Create.cs Co-authored-by: Copilot <[email protected]>
        
          
                src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
      …IJsonUtilities.Schema.Create.cs
        
          
                src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs
          
            Show resolved
            Hide resolved
        
              
          
                src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs
          
            Show resolved
            Hide resolved
        
              
          
                src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs
          
            Show resolved
            Hide resolved
        
              
          
                src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs
          
            Show resolved
            Hide resolved
        
              
          
                src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs
          
            Show resolved
            Hide resolved
        
      …IJsonUtilities.Schema.Create.cs
| @jeffhandley we need a second approval from dotnet/dotnet-extensions-fundamentals because this PR is touching "shared" components. | 
| I connected with @rosebyte to request a review from them or someone else on @dotnet/dotnet-extensions-fundamentals. | 
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.
Approve for Fundamentals
* Fix schema generation for Nullable<T> function parameters. * Update src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs Co-authored-by: Copilot <[email protected]> * Update src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs * Incorporate fix from dotnet/runtime#117493. * Update src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs * Extend fix to include AllowReadingFromString. --------- Co-authored-by: Copilot <[email protected]>
* Fix schema generation for Nullable<T> function parameters. * Update src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs Co-authored-by: Copilot <[email protected]> * Update src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs * Incorporate fix from dotnet/runtime#117493. * Update src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs * Extend fix to include AllowReadingFromString. --------- Co-authored-by: Copilot <[email protected]>
Addresses modelcontextprotocol/csharp-sdk#601
Microsoft Reviewers: Open in CodeFlow