-
Notifications
You must be signed in to change notification settings - Fork 849
Fix operator precedence bug in ValidateSchemaDocument causing rejection of valid boolean schemas #7066
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
Co-authored-by: stephentoub <[email protected]>
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Utilities/AIJsonUtilitiesTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: stephentoub <[email protected]>
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 fixes a critical operator precedence bug in the ValidateSchemaDocument method that was incorrectly rejecting valid boolean JSON schemas (true/false). The issue stemmed from C#'s operator precedence rules incorrectly parsing the validation condition.
Key changes:
- Fixed the validation logic by adding parentheses to correctly express the intended condition
- Added comprehensive test coverage for boolean schema validation using the public
TransformSchemaAPI
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.Create.cs | Corrected operator precedence in ValidateSchemaDocument by wrapping the or-pattern in parentheses |
| test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Utilities/AIJsonUtilitiesTests.cs | Added test to verify boolean schemas (true/false) are correctly accepted and transformed |
eiriktsarpalis
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.
Ugh.
…on of valid boolean schemas (dotnet#7066) * Initial plan * Fix operator precedence bug in ValidateSchemaDocument and add tests Co-authored-by: stephentoub <[email protected]> * Replace reflection-based tests with public API tests Co-authored-by: stephentoub <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: stephentoub <[email protected]>
…on of valid boolean schemas (#7066) * Initial plan * Fix operator precedence bug in ValidateSchemaDocument and add tests Co-authored-by: stephentoub <[email protected]> * Replace reflection-based tests with public API tests Co-authored-by: stephentoub <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: stephentoub <[email protected]>
The
ValidateSchemaDocumentmethod incorrectly rejected valid boolean schemas (trueandfalse) due to operator precedence in the pattern matching expression.Problem
This evaluated to
truewhen the schema wasFalseorTrue, throwing exceptions for valid JSON schemas per the spec.Changes
Fixed operator precedence with explicit grouping:
Added tests using the public
TransformSchemaAPI to verify top-level boolean schemas (true,false) are correctly accepted and transformed. The new testTransformJsonSchema_BooleanSchemas_Successvalidates that:trueschemas are converted to empty objects{}falseschemas are converted to{"not": true}Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Microsoft Reviewers: Open in CodeFlow