-
Notifications
You must be signed in to change notification settings - Fork 588
Fix ToJsonObject serialization failure with anonymous types and add support for custom JsonSerializerOptions #1113
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
982138e
6a070b3
57b4a93
bcf2e24
7041a89
867d044
bd598ea
32f2549
cb95813
a3cb705
111616b
c1212c6
f46474b
8c94a23
46425b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -139,7 +139,7 @@ public static class AIContentExtensions | |
|
|
||
| /// <summary>Converts the specified dictionary to a <see cref="JsonObject"/>.</summary> | ||
| internal static JsonObject? ToJsonObject(this IReadOnlyDictionary<string, object?> properties) => | ||
| JsonSerializer.SerializeToNode(properties, McpJsonUtilities.JsonContext.Default.IReadOnlyDictionaryStringObject) as JsonObject; | ||
| JsonSerializer.SerializeToNode(properties, McpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(IReadOnlyDictionary<string, object?>))) as JsonObject; | ||
|
|
||
| internal static AdditionalPropertiesDictionary ToAdditionalProperties(this JsonObject obj) | ||
| { | ||
|
|
@@ -271,7 +271,7 @@ public static IList<PromptMessage> ToPromptMessages(this ChatMessage chatMessage | |
| EmbeddedResourceBlock resourceContent => resourceContent.Resource.ToAIContent(), | ||
|
|
||
| ToolUseContentBlock toolUse => FunctionCallContent.CreateFromParsedArguments(toolUse.Input, toolUse.Id, toolUse.Name, | ||
| static json => JsonSerializer.Deserialize(json, McpJsonUtilities.JsonContext.Default.IDictionaryStringObject)), | ||
| static json => JsonSerializer.Deserialize(json, McpJsonUtilities.DefaultOptions.GetTypeInfo<IDictionary<string, object?>>())), | ||
|
||
|
|
||
| ToolResultContentBlock toolResult => new FunctionResultContent( | ||
| toolResult.ToolUseId, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,167 @@ | ||
| using Microsoft.Extensions.AI; | ||
stephentoub marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| using ModelContextProtocol.Protocol; | ||
| using System.Text.Json; | ||
|
|
||
| namespace ModelContextProtocol.Tests; | ||
|
|
||
| /// <summary> | ||
| /// Tests for AIContentExtensions with anonymous types in AdditionalProperties. | ||
| /// This validates the fix for the sampling pipeline regression in 0.5.0-preview.1. | ||
| /// These tests require reflection-based serialization and will be skipped when reflection is disabled. | ||
| /// </summary> | ||
| public class AIContentExtensionsAnonymousTypeTests | ||
| { | ||
| [Fact] | ||
| public void ToContentBlock_WithAnonymousTypeInAdditionalProperties_DoesNotThrow() | ||
| { | ||
| if (!JsonSerializer.IsReflectionEnabledByDefault) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // This is the minimal repro from the issue | ||
| AIContent c = new() | ||
| { | ||
| AdditionalProperties = new() | ||
| { | ||
| ["data"] = new { X = 1.0, Y = 2.0 } | ||
| } | ||
| }; | ||
|
|
||
| // Should not throw NotSupportedException | ||
| var contentBlock = c.ToContentBlock(); | ||
|
|
||
| Assert.NotNull(contentBlock); | ||
| Assert.NotNull(contentBlock.Meta); | ||
| Assert.True(contentBlock.Meta.ContainsKey("data")); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ToContentBlock_WithMultipleAnonymousTypes_DoesNotThrow() | ||
| { | ||
| if (!JsonSerializer.IsReflectionEnabledByDefault) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| AIContent c = new() | ||
| { | ||
| AdditionalProperties = new() | ||
| { | ||
| ["point"] = new { X = 1.0, Y = 2.0 }, | ||
| ["metadata"] = new { Name = "Test", Id = 42 }, | ||
| ["config"] = new { Enabled = true, Timeout = 30 } | ||
| } | ||
| }; | ||
|
|
||
| var contentBlock = c.ToContentBlock(); | ||
|
|
||
| Assert.NotNull(contentBlock); | ||
| Assert.NotNull(contentBlock.Meta); | ||
| Assert.Equal(3, contentBlock.Meta.Count); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ToContentBlock_WithNestedAnonymousTypes_DoesNotThrow() | ||
| { | ||
| if (!JsonSerializer.IsReflectionEnabledByDefault) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| AIContent c = new() | ||
| { | ||
| AdditionalProperties = new() | ||
| { | ||
| ["outer"] = new | ||
| { | ||
| Inner = new { Value = "test" }, | ||
| Count = 5 | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| var contentBlock = c.ToContentBlock(); | ||
|
|
||
| Assert.NotNull(contentBlock); | ||
| Assert.NotNull(contentBlock.Meta); | ||
| Assert.True(contentBlock.Meta.ContainsKey("outer")); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ToContentBlock_WithMixedTypesInAdditionalProperties_DoesNotThrow() | ||
| { | ||
| if (!JsonSerializer.IsReflectionEnabledByDefault) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| AIContent c = new() | ||
| { | ||
| AdditionalProperties = new() | ||
| { | ||
| ["anonymous"] = new { X = 1.0, Y = 2.0 }, | ||
| ["string"] = "test", | ||
| ["number"] = 42, | ||
| ["boolean"] = true, | ||
| ["array"] = new[] { 1, 2, 3 } | ||
| } | ||
| }; | ||
|
|
||
| var contentBlock = c.ToContentBlock(); | ||
|
|
||
| Assert.NotNull(contentBlock); | ||
| Assert.NotNull(contentBlock.Meta); | ||
| Assert.Equal(5, contentBlock.Meta.Count); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void TextContent_ToContentBlock_WithAnonymousTypeInAdditionalProperties_PreservesData() | ||
| { | ||
| if (!JsonSerializer.IsReflectionEnabledByDefault) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| TextContent textContent = new("Hello, world!") | ||
| { | ||
| AdditionalProperties = new() | ||
| { | ||
| ["location"] = new { Lat = 40.7128, Lon = -74.0060 } | ||
| } | ||
| }; | ||
|
|
||
| var contentBlock = textContent.ToContentBlock(); | ||
| var textBlock = Assert.IsType<TextContentBlock>(contentBlock); | ||
|
|
||
| Assert.Equal("Hello, world!", textBlock.Text); | ||
| Assert.NotNull(textBlock.Meta); | ||
| Assert.True(textBlock.Meta.ContainsKey("location")); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void DataContent_ToContentBlock_WithAnonymousTypeInAdditionalProperties_PreservesData() | ||
| { | ||
| if (!JsonSerializer.IsReflectionEnabledByDefault) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| byte[] imageData = [1, 2, 3, 4, 5]; | ||
| DataContent dataContent = new(imageData, "image/png") | ||
| { | ||
| AdditionalProperties = new() | ||
| { | ||
| ["dimensions"] = new { Width = 100, Height = 200 } | ||
| } | ||
| }; | ||
|
|
||
| var contentBlock = dataContent.ToContentBlock(); | ||
| var imageBlock = Assert.IsType<ImageContentBlock>(contentBlock); | ||
|
|
||
| Assert.Equal(Convert.ToBase64String(imageData), imageBlock.Data); | ||
| Assert.Equal("image/png", imageBlock.MimeType); | ||
| Assert.NotNull(imageBlock.Meta); | ||
| Assert.True(imageBlock.Meta.ContainsKey("dimensions")); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| using Microsoft.Extensions.AI; | ||
| using System.Text.Json; | ||
|
|
||
| namespace ModelContextProtocol.Tests; | ||
|
|
||
| /// <summary> | ||
| /// Regression tests for specific issues that were reported and fixed. | ||
| /// </summary> | ||
| public class RegressionTests | ||
| { | ||
| /// <summary> | ||
| /// Regression test for GitHub issue: ToJsonObject fails when dictionary values contain anonymous types. | ||
| /// This is a sampling pipeline regression from version 0.5.0-preview.1. | ||
| /// </summary> | ||
| [Fact] | ||
| public void Issue_AnonymousTypes_InAdditionalProperties_ShouldNotThrow() | ||
| { | ||
| // Anonymous types require reflection-based serialization | ||
| if (!JsonSerializer.IsReflectionEnabledByDefault) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // Exact minimal repro from the issue | ||
| AIContent c = new() | ||
| { | ||
| AdditionalProperties = new() | ||
| { | ||
| ["data"] = new { X = 1.0, Y = 2.0 } | ||
| } | ||
| }; | ||
|
|
||
| // This should not throw NotSupportedException | ||
| var exception = Record.Exception(() => c.ToContentBlock()); | ||
|
|
||
| Assert.Null(exception); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.