MicrosoftExtensions: Normalize nullable union tool types

[PederHP]

See: modelcontextprotocol/csharp-sdk#1091

Tools with ["integer", "null"] discriminated union style types to provide additional information on default when non provided for non-required properties would cause the following error:

tools.0.custom.input_schema: JSON schema is invalid. It must match JSON Schema draft 2020-12 (https://json-schema.org/draft/2020-12). Learn more about tool use at https://docs.claude.com/en/docs/tool-use.

[PederHP]

@stephentoub Any comments on this one? I made it to unbreak use of MCP Server tools which have discriminated unions from default parameters (which are valid JSON schema but not supported by the Anthropic API). I assume the code owners will lean on your review of this as it relates only to MEAI.

It would be really nice to get this in as it as otherwise the risk of MCP tools breaking the client is quite high with the beta client - requiring the use of a forked SDK. If the stricter schema validation of the beta client makes it to the main client, I think this will affect almost anyone using MCP with the SDK and extensions.

[stephentoub]

@eiriktsarpalis, could you take a look at this one?

[eiriktsarpalis]

Is the form "type" : ["integer","null"] considered a union type? I would have guessed such a terminology would be reserved for schemas using "anyOf" or "oneOf". Is the trivial schema {} (which we generate for things like object or JsonElement) similarly considered a union type in that case, and would we need to similarly restrict those should they turn up?

[PederHP]

I will have to test if {} causes an error. Note that the API just rejects the request without more information than shown in the PR description. Calling it a (discriminated) union type is on me. If there’s a more accurate description I am happy to change the comment.

But the API definitely rejects requests if this pattern is present.

[eiriktsarpalis]

The fact that this is constrained to a particular chat client makes me feel less concerned, however I do think that a nullable/optional input parameter encodes important aspects of an AIFunction's contract, so silently erasing those is likely going to result in unexpected behavior.

I would much rather have the chat client fail with an explicit "not supported" error message and give the user to opportunity to model their functions in a way that remains compatible with the LLM in question.

[stephentoub]

I would much rather have the chat client fail with an explicit "not supported" error message and give the user to opportunity to model their functions in a way that remains compatible with the LLM in question.

Then why do we expose transforms at all? The fact is that clients are talking to any number of mcp servers they don't own, all that have varying forms of schema employed as the mcp spec says nothing about the format, and talking to llms that each have their own peculiarities. Failing is a bad option in my opinion, as it completely breaks being able to use a valid mcp server with a valid llm.

[eiriktsarpalis]

I would much rather have the chat client fail with an explicit "not supported" error message and give the user to opportunity to model their functions in a way that remains compatible with the LLM in question.

Then why do we expose transforms at all?

Marking all inputs as required or transforming "type" : "null" to "nullable" : "true" is one thing, silently restricting the function inputs is another. The point of the transform is to do just that, transform the advertised schema in a way that remains compatible with the underlying function contract. I could write a transform that collapses all schemas to accepting just one integer, but that wouldn't make it valid transform.

[stephentoub]

The point of the transform is to do just that, transform the advertised schema in a way that remains compatible with the underlying function contract.

Reducing the allowed inputs to a subset is still compatible. If the other excluded inputs are erroneous from the llms perspective, then it wouldn't generate those, anyway.

If there's a better way to express the transformation, great, let's do that. If there isn't, I fail to see the problem here.

[eiriktsarpalis]

Reducing the allowed inputs to a subset is still compatible. If the other excluded inputs are erroneous from the llms perspective, then it wouldn't generate those, anyway.

I'm not questioning the soundness of it, however my point is that it could be limiting the intended functionality of the passed function. Consider the following hypothetical method, which would lose the ability to enumerate all documents:

// If no owner id is specified, all documents are returned
IEnumerable<Document> GetDocuments(int? ownerId = null);

[stephentoub]

Consider the following hypothetical method, which would lose the ability to enumerate all documents:

If the LLM doesn't support producing null for the input, the transform isn't losing anything.

If the LLM does support producing null for the input, and there's some way to express in the schema to the LLM that the parameter may be null, then the transform should produce the right schema for it.

[eiriktsarpalis]

Consider the following hypothetical method, which would lose the ability to enumerate all documents:

If the LLM doesn't support producing null for the input, the transform isn't losing anything.

Yes? I think the point is that the user wants the LLM to possibly provide null and we're silently pushing that part of the contract under the rug. We should instead provide feedback prompting the user to remodel their function into something compatible with the constraints of the current LLM.

[stephentoub]

Yes? I think the point is that the user wants the LLM to possibly provide null and we're silently pushing that part of the contract under the rug.

No, we're not. We have no control over what an LLM does. An LLM that's asked for an integer might only be capable of producing the interger 42, and that's fine. The client is choosing to use that LLM. If a function says it can accept null and the LLM isn't capable of producing null, the LLM just won't produce null; that's not an error, that's implicit to the LLM being used, just as is any other aspect of how good or bad the LLM is at particular workloads. It is not our place to error out if the LLM doesn't meet a particular capability threshold.

[PederHP]

I think we should be able to have a client built using the Anthropic SDK and the IChatClient extension connect to arbitrary MCP servers without getting runtime errors due to the API not accepting discriminated unions.

LLMs consider unspecified parameters as something different than null. The LLM is perfectly capable of not passing an optional int parameter.

The user can't remodel functions because they're neither the client or server developer.