Skip to content
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

Remove JsonSchema, use System.Text.Json.Schema instead #164

Merged
merged 3 commits into from
Jan 8, 2025

Conversation

Joy-less
Copy link
Contributor

@Joy-less Joy-less commented Jan 6, 2025

Support for JSON schemas was recently added by @mili-tan in #150.
What I wasn't aware of is that C# already contains a built-in way of generating JSON schemas, fully compatible with System.Text.Json and its various attributes:
https://devblogs.microsoft.com/dotnet/system-text-json-in-dotnet-9

Schemas generated this way are already compatible with OllamaSharp since format is of type object?.

This pull request removes the JsonSchema class (which doesn't contain support for enums, fields, etc) and encourages the user to use JsonSerializerOptions.Default.GetJsonSchemaAsNode instead.

Previously:

public class Dialog {
    public DialogLine[] DialogLines { get; set; } = [];
}
public class DialogLine {
    public string Name { get; set; }
    public string Speech { get; set; }
}
OllamaSharp.JsonSchema format = JsonSchema.ToJsonSchema(typeof(Dialog));

Now:

public class Dialog {
    public required DialogLine[] DialogLines { get; set; } = [];
}
public class DialogLine {
    public required string Name { get; set; }
    public required string Speech { get; set; }
}
System.Text.Json.Nodes.JsonNode format = JsonSerializerOptions.Default.GetJsonSchemaAsNode(typeof(Dialog));

System.Text.Json.Schema can be used in frameworks prior to .NET 9 by installing the latest official System.Text.Json NuGet package.

@awaescher
Copy link
Owner

I'd love to merge this, but would you please provide one or more unit tests to make sure everything works as expected?

@Joy-less
Copy link
Contributor Author

Joy-less commented Jan 7, 2025

Hi @awaescher, I'm not sure how this could be tested without calling the AI so I added a functional test which passed 5 times in a row:
image

One note is that the AI sometimes seemed to return null when I didn't instruct it to return valid JSON.

Another note is this API is added in .NET 9.0 so I needed to bump the test framework (or add a package, but we are already bumping the test framework in #162.)

@awaescher awaescher merged commit fcba491 into awaescher:main Jan 8, 2025
1 check passed
@awaescher
Copy link
Owner

Looks good to me, awesome work once again, @Joy-less

@Joy-less Joy-less deleted the replace-JsonSchema branch January 8, 2025 22:44
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