-
Notifications
You must be signed in to change notification settings - Fork 849
Use JsonElement.Parse for JsonElement conversions in AI libraries #7067
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
…to JsonElement conversions Co-authored-by: stephentoub <[email protected]>
|
@copilot, please do another search. I think you missed some, such as in OpenAIConversionTests.cs |
…or byte spans Co-authored-by: stephentoub <[email protected]>
You're right, I missed 3 additional instances. Fixed in commit a44e0bc:
I initially only looked for string-to-JsonElement conversions, but JsonElement.Parse also supports ReadOnlySpan overloads. |
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 modernizes JSON parsing in the AI libraries by migrating from JsonSerializer.Deserialize<JsonElement>() to the newer, more efficient JsonElement.Parse() method introduced in System.Text.Json 10.0.0.
Key changes:
- Replaced 8 instances across test and production code where string or byte span inputs were being deserialized to JsonElement
- Removed unnecessary
AIJsonUtilities.DefaultOptionsparameters since JsonSerializerOptions don't affect JsonElement parsing behavior - Improved code efficiency by using the purpose-built parsing API
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| OpenAIConversionTests.cs | Updated 2 test assertions that parse function arguments from byte spans to JsonElement |
| ChatClientIntegrationTests.cs | Updated CustomAIFunction's JsonSchema property initialization to use JsonElement.Parse |
| AzureAIInferenceChatClientTests.cs | Updated JSON schema parsing in response format test to use JsonElement.Parse |
| CitationAnnotationTests.cs | Updated serialization roundtrip test assertion to use JsonElement.Parse |
| AIAnnotationTests.cs | Updated serialization roundtrip test assertion to use JsonElement.Parse |
| ChatResponseFormatTests.cs | Updated JSON schema creation in serialization test to use JsonElement.Parse |
| MicrosoftExtensionsAIChatExtensions.cs | Updated production code that converts tool result content from byte span to JsonElement |
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.
@copilot I recall that at some point I added an internal helper method in AIJsonUtilities that replicates the behavior of the new JsonElement.Parse method. If it still exists, please remove it and update all consuming code.
Co-authored-by: eiriktsarpalis <[email protected]>
Head branch was pushed to by a user without write access
Found and removed the |
Replace
JsonSerializer.DeserializewithJsonElement.Parsethroughout Microsoft.Extensions.AI libraries and tests.JsonElement.Parseis the canonical API for parsing JSON toJsonElementsince System.Text.Json 10.0.0, and supports multiple input types including strings, byte spans, and streams.Changes
ParseJsonElementhelper method)Examples
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
Microsoft Reviewers: Open in CodeFlow