-
Notifications
You must be signed in to change notification settings - Fork 22
fix: use Properties initializer for InputSchema in IChatClient extensions #83
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: next
Are you sure you want to change the base?
fix: use Properties initializer for InputSchema in IChatClient extensions #83
Conversation
* "Claude PR Assistant workflow" * "Claude Code Review workflow"
|
Have you tested this with the AIFunction tools created by AIFunctionFactory (and thus indirectly the MCP SDK)? Just to make sure it doesn't break those? |
17d974e to
f300e7b
Compare
|
@PederHP Yes - I've now validated with I've updated those tests to expect the correct JSON Schema format: - "input_schema": {
- "query": {
- "type": "string"
- },
- "type": "object",
- "required": ["query"]
- }
+ "input_schema": {
+ "type": "object",
+ "properties": {
+ "query": {
+ "type": "string"
+ }
+ },
+ "required": ["query"]
+ }All |
But that's strange, because AIFunctions created by AIFunctionFactory.Create currently work on the I am worried that AIFunctionFactory uses a different top level schema internally than the SDK and that's why this was working as it was - and that this fix will break the extensions when AIFunction objects not created manually are passed. Have you tried testing with a minimal reproducible console app that uses |
|
@PederHP I've created a minimal test to verify what So if (inputSchema.TryGetProperty("properties", out JsonElement propsElement))
{
foreach (JsonProperty p in propsElement.EnumerateObject())
{
properties[p.Name] = p.Value; // Extracts: { "query": { "type": "string" } }
}
}But the bug is in how it writes to InputSchema = new(properties) { Required = required }This constructor puts { "query": { "type": "string" }, "type": "object", "required": ["query"] }After the fix: InputSchema = new InputSchema() { Properties = properties, Required = required }This correctly produces: { "type": "object", "properties": { "query": { "type": "string" } }, "required": ["query"] }As for why your code might be working - are you perhaps:
The bug specifically affects the |
|
Here's the test code I used (with using System.Text.Json;
using Microsoft.Extensions.AI;
var func1 = AIFunctionFactory.Create(
(string query) => "result",
"test_function",
"A test function"
);
var func2 = AIFunctionFactory.Create(
(string location, string unit) => $"Weather in {location}",
"get_weather",
"Gets the weather for a location"
);
var func3 = AIFunctionFactory.Create(
() => DateTime.Now.ToString(),
"get_time",
"Gets the current time"
);
Console.WriteLine("=== AIFunctionFactory JsonSchema Output ===\n");
foreach (var func in new[] { func1, func2, func3 })
{
Console.WriteLine($"Function: {func.Name}");
Console.WriteLine($"JsonSchema:\n{JsonSerializer.Serialize(func.JsonSchema, new JsonSerializerOptions { WriteIndented = true })}\n");
var hasProperties = func.JsonSchema.TryGetProperty("properties", out var props);
Console.WriteLine($"Has 'properties' key: {hasProperties}");
if (hasProperties)
{
Console.WriteLine($"Properties content: {props}\n");
}
} |
I am using IChatClient and the next branch, so that's puzzling. All my tools are from the MCP SDK, though, but that also uses AIFunctionFactory. (You should rebase this fix onto next, as they don't accept PRs onto main except for releases). |
f300e7b to
7d2b4bc
Compare
|
Thanks for the heads up about targeting I've re-validated everything against the
The same bug appears to exist in |
…ions The InputSchema constructor was being passed properties as rawData, which caused them to be serialized at the top level of the JSON object instead of under a "properties" key. This resulted in invalid JSON Schema that failed Anthropic API validation with error: "tools.0.custom.input_schema: JSON schema is invalid" The fix uses property initializer syntax to correctly populate the Properties property, ensuring the schema is valid JSON Schema draft 2020-12. Also updates tests to expect the corrected schema format. Fixes anthropics#82
7d2b4bc to
c227354
Compare
|
The InputSchema ctor used to accept "properties". That changed in 75a2cce#diff-79ec764df52aaa105c06a11788463cf56f9aa0f6fcfb87d49e433f4b78d1654bL274 |
|
This causes issues only when the schema for the tool conflicts with the recognized json schema top level keywords - e.g. I wasn't aware of this PR and created another - where I added unit test that valides the produced schema (so it fails without the fix): |
d3bef74 to
9ad3dc0
Compare
|
@sd-st could we get this small fix merged? Thanks! |
|
Looks like there's an extra commit in here, could we get rid of the one that adds claude code? It should already be in repo |
|
Other than that it LGTM |
Summary
When using
IChatClientwithAIFunctiontools, theInputSchemawas incorrectly constructed by passing thepropertiesdictionary to the constructor asrawData. This caused property definitions to be serialized at the top level of the JSON schema object instead of being nested under the"properties"key.Before (invalid schema):
{ "format": { "type": "string", "description": "..." }, "type": "object" }After (valid schema):
{ "type": "object", "properties": { "format": { "type": "string", "description": "..." } } }This resulted in Anthropic API validation errors:
Fix
Changed from constructor-based initialization to property initializer syntax in both:
AnthropicClientExtensions.csAnthropicBetaClientExtensions.csFixes #82