-
Notifications
You must be signed in to change notification settings - Fork 16
Replace JsonSchema.Net with NJsonSchema #295
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 all commits
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 |
|---|---|---|
|
|
@@ -3,12 +3,8 @@ | |
|
|
||
| using System.Reflection; | ||
| using System.Text.Json; | ||
| using System.Text.Json.Nodes; | ||
| using System.Text.Json.Serialization; | ||
|
|
||
| using Json.Schema; | ||
| using Json.Schema.Generation; | ||
|
|
||
| using Microsoft.Teams.AI.Annotations; | ||
| using Microsoft.Teams.AI.Messages; | ||
| using Microsoft.Teams.Common.Extensions; | ||
|
|
@@ -38,7 +34,7 @@ public interface IFunction | |
| /// the Json Schema representing what | ||
| /// parameters the function accepts | ||
| /// </summary> | ||
| public JsonSchema? Parameters { get; } | ||
| public IJsonSchema? Parameters { get; } | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -57,7 +53,7 @@ public class Function : IFunction | |
|
|
||
| [JsonPropertyName("parameters")] | ||
| [JsonPropertyOrder(2)] | ||
| public JsonSchema? Parameters { get; set; } | ||
| public IJsonSchema? Parameters { get; set; } | ||
|
Comment on lines
54
to
+56
|
||
|
|
||
| [JsonIgnore] | ||
| public Delegate Handler { get; set; } | ||
|
|
@@ -70,7 +66,7 @@ public Function(string name, string? description, Delegate handler) | |
| Parameters = GenerateParametersSchema(handler); | ||
| } | ||
|
|
||
| public Function(string name, string? description, JsonSchema parameters, Delegate handler) | ||
| public Function(string name, string? description, IJsonSchema parameters, Delegate handler) | ||
| { | ||
| Name = name; | ||
| Description = description; | ||
|
|
@@ -82,13 +78,13 @@ public Function(string name, string? description, JsonSchema parameters, Delegat | |
| { | ||
| if (call.Arguments is not null && Parameters is not null) | ||
| { | ||
| var valid = Parameters.Evaluate(JsonNode.Parse(call.Arguments), new() { EvaluateAs = SpecVersion.DraftNext }); | ||
| var result = Parameters.Validate(call.Arguments); | ||
|
|
||
| if (!valid.IsValid) | ||
| if (!result.IsValid) | ||
| { | ||
| Console.WriteLine(JsonSerializer.Serialize(valid)); | ||
| Console.WriteLine(string.Join("\n", result.Errors.Select(e => $"{e.Path} => {e.Message}"))); | ||
| throw new ArgumentException( | ||
| string.Join("\n", valid.Errors?.Select(e => $"{e.Key} => {e.Value}") ?? []) | ||
| string.Join("\n", result.Errors.Select(e => $"{e.Path} => {e.Message}")) | ||
| ); | ||
| } | ||
| } | ||
|
|
@@ -128,7 +124,7 @@ public override string ToString() | |
| /// <summary> | ||
| /// Generates a JsonSchema for the parameters of a delegate handler using reflection | ||
| /// </summary> | ||
| private static JsonSchema? GenerateParametersSchema(Delegate handler) | ||
| private static IJsonSchema? GenerateParametersSchema(Delegate handler) | ||
| { | ||
| var method = handler.GetMethodInfo(); | ||
| var methodParams = method.GetParameters(); | ||
|
|
@@ -141,15 +137,11 @@ public override string ToString() | |
| var parameters = methodParams.Select(p => | ||
| { | ||
| var paramName = p.GetCustomAttribute<ParamAttribute>()?.Name ?? p.Name ?? p.Position.ToString(); | ||
| var schema = new JsonSchemaBuilder().FromType(p.ParameterType).Build(); | ||
| var schema = JsonSchemaWrapper.FromType(p.ParameterType); | ||
| var required = !p.IsOptional; | ||
| return (paramName, schema, required); | ||
| }); | ||
| }).ToArray(); | ||
|
|
||
| return new JsonSchemaBuilder() | ||
| .Type(SchemaValueType.Object) | ||
| .Properties(parameters.Select(item => (item.paramName, item.schema)).ToArray()) | ||
| .Required(parameters.Where(item => item.required).Select(item => item.paramName)) | ||
| .Build(); | ||
| return JsonSchemaWrapper.CreateObjectSchema(parameters); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,6 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using Json.Schema; | ||
|
|
||
| namespace Microsoft.Teams.AI.Prompts; | ||
|
|
||
| public partial class ChatPrompt<TOptions> | ||
|
|
@@ -17,8 +15,8 @@ public ChatPrompt<TOptions> Chain(IChatPrompt<TOptions> prompt) | |
| Functions.Add(new Function( | ||
| prompt.Name, | ||
| prompt.Description, | ||
| new JsonSchemaBuilder().Properties( | ||
| ("text", new JsonSchemaBuilder().Type(SchemaValueType.String).Description("text to send")) | ||
| JsonSchemaWrapper.CreateObjectSchema( | ||
| ("text", JsonSchemaWrapper.String("text to send"), true) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The old code passed no |
||
| ), | ||
| async (string text) => | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| namespace Microsoft.Teams.AI; | ||
|
|
||
| /// <summary> | ||
| /// Abstraction for JSON schema validation, decoupling from specific schema library implementations. | ||
| /// </summary> | ||
| public interface IJsonSchema | ||
| { | ||
| /// <summary> | ||
| /// Validates a JSON string against this schema. | ||
| /// </summary> | ||
| JsonSchemaValidationResult Validate(string json); | ||
|
|
||
| /// <summary> | ||
| /// Serializes this schema to a JSON string. | ||
| /// </summary> | ||
| string ToJson(); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Result of validating JSON against a schema. | ||
| /// </summary> | ||
| public class JsonSchemaValidationResult | ||
| { | ||
| /// <summary> | ||
| /// Whether the JSON is valid against the schema. | ||
| /// </summary> | ||
| public bool IsValid { get; init; } | ||
|
|
||
| /// <summary> | ||
| /// List of validation errors, if any. | ||
| /// </summary> | ||
| public IReadOnlyList<JsonSchemaValidationError> Errors { get; init; } = []; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Represents a validation error from schema validation. | ||
| /// </summary> | ||
| /// <param name="Path">The JSON path where the error occurred.</param> | ||
| /// <param name="Message">The error message.</param> | ||
| public record JsonSchemaValidationError(string Path, string Message); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,89 @@ | ||||||||||||||||||||||||||||||||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||||||||||||||||||||||||||||||||
| // Licensed under the MIT License. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| using NJsonSchema; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| namespace Microsoft.Teams.AI; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||
| /// NJsonSchema-based implementation of IJsonSchema. | ||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||
| public class JsonSchemaWrapper : IJsonSchema | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| private readonly JsonSchema _schema; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| private JsonSchemaWrapper(JsonSchema schema) => _schema = schema; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||
| /// Creates a schema from a .NET type using reflection. | ||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||
| public static IJsonSchema FromType(Type type) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| var schema = JsonSchema.FromType(type); | ||||||||||||||||||||||||||||||||
| return new JsonSchemaWrapper(schema); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||
| /// Creates a schema from a JSON schema string. | ||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||
| public static IJsonSchema FromJson(string json) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| var schema = JsonSchema.FromJsonAsync(json).GetAwaiter().GetResult(); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
| var schema = JsonSchema.FromJsonAsync(json).GetAwaiter().GetResult(); | |
| var schema = JsonSchema.FromJsonAsync(json).ConfigureAwait(false).GetAwaiter().GetResult(); |
Copilot
AI
Jan 28, 2026
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.
The JsonSchemaProperty creation only copies Type and Description from the wrapped schema, but it doesn't copy other potentially important properties like Format, Pattern, MinLength, MaxLength, Minimum, Maximum, Enum values, etc. This could result in incomplete validation when complex schemas are used as properties. Consider either using the entire wrapped schema directly or copying all relevant schema properties.
| Description = wrapper._schema.Description | |
| }; | |
| Description = wrapper._schema.Description, | |
| Format = wrapper._schema.Format, | |
| Pattern = wrapper._schema.Pattern, | |
| MinLength = wrapper._schema.MinLength, | |
| MaxLength = wrapper._schema.MaxLength, | |
| Minimum = wrapper._schema.Minimum, | |
| Maximum = wrapper._schema.Maximum | |
| }; | |
| foreach (var enumValue in wrapper._schema.Enumeration) | |
| { | |
| property.Enumeration.Add(enumValue); | |
| } |
Copilot
AI
Jan 28, 2026
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.
The CreateObjectSchema method silently skips properties when propSchema is not a JsonSchemaWrapper instance. This could lead to incomplete schema definitions if an implementation of IJsonSchema other than JsonSchemaWrapper is passed. Consider either throwing an exception when propSchema is not a JsonSchemaWrapper, or handling all IJsonSchema implementations properly. Alternatively, document this limitation in the method's XML documentation.
Copilot
AI
Jan 28, 2026
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.
The CreateObjectSchema method adds properties to RequiredProperties regardless of whether the property was successfully added to Properties. If a property's schema is not a JsonSchemaWrapper (and is thus skipped at lines 44-52), it will still be added to RequiredProperties at line 56. This inconsistency could lead to validation errors where the schema requires properties that don't exist in the schema definition.
| } | |
| if (required) | |
| { | |
| resultSchema.RequiredProperties.Add(name); | |
| if (required) | |
| { | |
| resultSchema.RequiredProperties.Add(name); | |
| } |
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.
Function.Invoke used to validate with EvaluateAs = SpecVersion.DraftNext (Function.cs:85 on main). NJsonSchema's default validator picks a draft from the schema's $schema keyword, falling back to Draft-04/06/07 depending on shape. Schemas coming from MCP servers (McpClientPlugin.cs:221) and OpenAI tool calls don't declare a draft, so draft-2020-12 keywords (prefixItems, unevaluatedProperties, $dynamicRef) get silently ignored at validation time. Either set the draft explicitly when parsing, or extend the equivalence tests to exercise 2020-12 keywords so this doesn't regress quietly.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,4 +22,4 @@ | |
| await context.Send("hi from teams..."); | ||
| }); | ||
|
|
||
| app.Run(); | ||
| app.Run(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,4 +40,4 @@ | |
| state.Save(context); | ||
| }); | ||
|
|
||
| app.Run(); | ||
| app.Run(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -507,4 +507,4 @@ function cancelSettings() { | |
| </body> | ||
| </html> | ||
| """; | ||
| } | ||
| } | ||
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.
Was
JsonSerializer.Serialize(function.Parameters), nowfunction.Parameters.ToJson(). NJsonSchema'sToJson()includes"$schema": "http://json-schema.org/draft-04/schema#"and may includedefinitions/x-extensions. OpenAI tolerates$schemabut rejectsdefinitionsin strict mode. There's no test that round-tripsChatTool.ToOpenAI(...)output and asserts it's the JSON OpenAI actually accepts. Add one, or post-processToJson()to strip metadata.