Skip to content

Make enums serialize as strings if using the reflection-based serializer. #473

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

eiriktsarpalis
Copy link
Contributor

While triaging #460 it occurred to me that Microsoft.Extensions.AI is enabling string-based enum serialization only when reflection-based serialization is turned on. This PR enables that behavior for MCP as well, however I'm not sure if it should be merged: it does introduce inconsistency between the reflection and source gen worlds after all.

@eiriktsarpalis eiriktsarpalis force-pushed the reflection-use-string-enums branch from b152524 to b51df8a Compare May 30, 2025 17:27
@eiriktsarpalis eiriktsarpalis force-pushed the reflection-use-string-enums branch from b51df8a to b2d6481 Compare June 3, 2025 11:13
@halter73
Copy link
Contributor

halter73 commented Jun 3, 2025

Microsoft.Extensions.AI is enabling string-based enum serialization only when reflection-based serialization is turned on.

Will these changes to M.E.AI have any impact on users of the MCP SDK independently of the change in the PR? And does this mean we'd be inconsistent with using M.E.AI directly if we don't take this PR?

it does introduce inconsistency between the reflection and source gen worlds after all.

Can we somehow flip the default for the purposes of the MCP SDK? Could we make it an error either at build time or run time not to use JsonStringEnumConverter when using source generation unless you make some gesture to opt in to the previously default numeric behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants