Replace AIFunctionParameterMetadata with MethodInfo

[eiriktsarpalis]

Removes both AIFunctionParameterMetadata and AIFunctionReturnParameterMetadata, replacing both with an optional MethodInfo? UnderlyingMethod property.

Me and @stephentoub discussed possibly removing AIFunctionMetadata altogether and flattening all its remaining properties on AIFunction itself. This PR stops short of that so we can gather feedback on the basis of this initial removal.

Microsoft Reviewers: Open in CodeFlow

[Copilot]

Copilot reviewed 5 out of 15 changed files in this pull request and generated no comments.

Files not reviewed (10)

• test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Functions/AIFunctionParameterMetadataTests.cs: Evaluated as low risk
• src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunctionParameterMetadata.cs: Evaluated as low risk
• src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunctionReturnParameterMetadata.cs: Evaluated as low risk
• test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Functions/AIFunctionReturnParameterMetadataTests.cs: Evaluated as low risk
• src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMapper.ChatCompletion.cs: Evaluated as low risk
• src/Libraries/Microsoft.Extensions.AI/Functions/AIFunctionFactoryCreateOptions.cs: Evaluated as low risk
• test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/FunctionCallContentTests..cs: Evaluated as low risk
• test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAISerializationTests.cs: Evaluated as low risk
• test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Functions/AIFunctionMetadataTests.cs: Evaluated as low risk
• src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunctionMetadata.cs: Evaluated as low risk

Comments suppressed due to low confidence (8)

src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.cs:84

• The defaultValue is being serialized without checking if it's null. Add a null check before serializing the defaultValue.

defaultValue: parameter.HasDefaultValue ? parameter.DefaultValue : null,

src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.cs:81

• Ensure that the parameterName is not null or empty before using it. This will prevent any potential runtime errors.

parameterName: parameter.Name,

src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.cs:82

• Ensure that the description is not null or empty before using it. This will prevent any potential runtime errors.

description: parameter.GetCustomAttribute<DescriptionAttribute>(inherit: true)?.Description,

test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs:136

• [nitpick] The variable name 'dotnetFunc' is not descriptive. It should be renamed to something more meaningful, such as 'testFunction'.

Func<string> dotnetFunc = () => "test";

test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs:142

• [nitpick] The variable name 'dotnetFunc2' is not descriptive. It should be renamed to something more meaningful, such as 'concatenateFunction'.

Func<string, string> dotnetFunc2 = (string a) => a + " " + a;

test/Libraries/Microsoft.Extensions.AI.Tests/Functions/AIFunctionFactoryTest.cs:148

• [nitpick] The variable name 'dotnetFunc3' is not descriptive. It should be renamed to something more meaningful, such as 'descriptionFunction'.

Func<string, string, string> dotnetFunc3 = [Description("This is a test function")] ([Description("This is A")] string a, [Description("This is B")] string b) => b + " " + a;

src/Libraries/Microsoft.Extensions.AI/Functions/AIFunctionFactory.cs:367

• The new implementation does not include parameter descriptions in the GetParameterMarshaller method.

return (IReadOnlyDictionary<string, object?> arguments, AIFunctionContext? _) =>

src/Libraries/Microsoft.Extensions.AI/Functions/AIFunctionFactory.cs:264

• The return type description is not explicitly handled in the new implementation.

UnderlyingMethod = method,

[dotnet-comment-bot]

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Microsoft.Gen.MetadataExtractor	Line	98	57.35 🔻
Microsoft.Gen.MetadataExtractor	Branch	98	62.5 🔻
Microsoft.Extensions.AI.OpenAI	Line	77	68.26 🔻
Microsoft.Extensions.AI.OpenAI	Branch	77	50.2 🔻
Microsoft.Extensions.AI.Ollama	Line	80	78.2 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project	Expected	Actual
Microsoft.Extensions.AI.AzureAIInference	91	92
Microsoft.Extensions.Caching.Hybrid	86	87
Microsoft.Extensions.AI.Abstractions	83	84

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=950025&view=codecoverage-tab

[dotnet-comment-bot]

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Microsoft.Extensions.AI.OpenAI	Line	77	68.26 🔻
Microsoft.Extensions.AI.OpenAI	Branch	77	50.2 🔻
Microsoft.Extensions.AI.Ollama	Line	80	78.2 🔻
Microsoft.Gen.MetadataExtractor	Line	98	57.35 🔻
Microsoft.Gen.MetadataExtractor	Branch	98	62.5 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project	Expected	Actual
Microsoft.Extensions.Caching.Hybrid	86	87
Microsoft.Extensions.AI.Abstractions	83	84
Microsoft.Extensions.AI.AzureAIInference	91	92
Microsoft.Extensions.AI	88	89

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=950291&view=codecoverage-tab

[eiriktsarpalis]

@SteveSandersonMS what do you think? Is this a step in the right direction?

[SteveSandersonMS]

@eiriktsarpalis @stephentoub Can you summarize the design intent?

I've read through the implementation and simplifying things certainly looks good and beneficial. But it's not obvious initially why we can get away with just removing all the metadata, unless it was just not serving a purpose before.

My guess is that you've identified that it was an unnecessary extra level of indirection, e.g.:

• Before:
  • App provides AIFunctionMetadata, or a MethodInfo which we convert to AIFunctionMetadata
  • IChatClient implementation receives AIFunctionMetadata and converts it to its own wire format, usually JSON schema
• After:
  • App provides JSON schema, or a MethodInfo which we convert to JSON schema
  • IChatClient uses that JSON schema directly or, in the unusual case where it needs a different format, needs to parse the JSON schema and convert that to its own other wire format

At least, this lines up with my memory of the initial design, though I know that may have already changed. I know there was initially a lot of debate about to what extent we're willing to make a strong assumption that JSON schema is the one true format for function descriptions.

Me and @stephentoub discussed possibly removing AIFunctionMetadata altogether and flattening all its remaining properties on AIFunction itself.

It's not clear to me which properties would remain. In this PR you've removed our abstract representation for parameter metadata, but left the abstract representation for function-level metadata (name, description).

Is the intent that AIFunction would gain Name and Description properties? If so, can you clarify why we would need those if we don't need metadata about parameters? In the OpenAI function metadata format at least, the schema innately contains name and description, so I'm guessing we could eliminate Name and Description as separate properties.

[eiriktsarpalis]

Can you summarize the design intent?

One use case for AIFunction that has come up both in my own Copilot extensions work and in feedback received from partner teams is being able to deserialize a "tools" payload into a symbolic AIFunction instance (aka an instance that declares a signature but does not encapsulate behavior). Instances like that are meant to be forwarded to other chat services as opposed to being executed by a local function invoking middleware.

The presence of AIFunctionMetadata in this case confuses implementers, compelling them to populate them with unnecessary information. The specifc feedback we got from the VS Copilot team was that they didn't know what to use in the AIFunctionParameterMetadata.Type property; even though it's optional they didn't understand the ramifications of not using something there.

Given that AIFunctionMetadata is really a proxy for ParameterInfo, we initially debated using an IReadOnlyList<ParameterInfo> but ultimately ended up on MethodInfo?. It more clearly communicates that this particular instance is backed by a .NET method, provides richer metadata, and it substantially simplifies function implementations. Because MethodInfo can be inherited, it doesn't prevent users from defining synthetic metadata if they so choose.

I know there was initially a lot of debate about to what extent we're willing to make a strong assumption that JSON schema is the one true format for function descriptions.

I think this has always been the assumption, what we've been changing recently (particularly with #5826) is how explicit this is being made in the API contract. We originally exposed an object? Schema property scoped to AIFunctionParameterMetadata that we were documenting as being a JSON schema. Every component in our infrastructure and leaf client implementations expected this to be JsonElement. That PR changed the schema to explicitly be a JsonElement scoped to the function level as opposed to the parameter level. It came following customer feedback that it wasn't possible to simply specify a self-contained JSON schema, and users could only work with schema fragments scoped to individual parameters.

Assuming a new schematization format does come around in the future, the MethodInfo should contain all metadata necessary to reconstitute that on demand. If a new format proves ubiquitous enough, we can add it as a separate (strongly typed) schema property.

Is the intent that AIFunction would gain Name and Description properties? If so, can you clarify why we would need those if we don't need metadata about parameters? In the OpenAI function metadata format at least, the schema innately contains name and description, so I'm guessing we could eliminate Name and Description as separate properties.

I've kept those since they are required values in envelopes that contain the function schema. The "description" keyword is optional in JSON schema and "name" does not exist per se -- the canonical keyword in this case is "title".

@stephentoub made the point that he would like to see Name (and possible Description?) lifted to to the AITool base class since he needs to introduce a new derived tool type that needs this information as well.

[SteveSandersonMS]

One use case for AIFunction that has come up both in my own Copilot extensions work and in feedback received from partner teams is being able to deserialize a "tools" payload into a symbolic AIFunction instance (aka an instance that declares a signature but does not encapsulate behavior). Instances like that are meant to be forwarded to other chat services as opposed to being executed by a local function invoking middleware.

I guess I'm less clear now on how to understand the situation. Could you give an outline of what's happening in this situation? Who's providing data to who else and what are they going to do with it?

My guess is that, if you're on the server end of an IChatClient interaction (e.g., implementing an OpenAI-style REST API) then you'd receive incoming /chat calls that contain tool descriptors. I would have expected that you need all the metadata, including the parameter types, in order to be able to generate valid calls to those tools.

You mention that the instances are meant to be forwarded to other chat services but surely somewhere at the end of this line, the final recipient needs the full metadata (including parameter types) in order to be able to generate valid calls to those tools.

In what situation do you need a name and description but no definition of the parameter schema?

I've kept those since they are required values in envelopes that contain the function schema. The "description" keyword is optional in JSON schema and "name" does not exist per se -- the canonical keyword in this case is "title".

Not sure how to interpret the example given (OllamaFunctionTool). That class requires not only Name and Description, but also Parameters. So how does it follow that we can remove Parameters but not Name and Description?

The "description" keyword is optional in JSON schema and "name" does not exist per se -- the canonical keyword in this case is "title".

This sounds like JSON schema has an official spec for how functions would be represented. Is that the case? I'm guessing not.

So if this is an informally-defined way to represent functions, basically just taking the de-facto standard from OpenAI, what difference does it make what is canonical for JSON schema?

Apologies if my lack of context here is slowing things down. Please don't interpret any of it as blocking this change, which almost certainly is a good thing since it's removing complexity.

[eiriktsarpalis]

You mention that the instances are meant to be forwarded to other chat services but surely somewhere at the end of this line, the final recipient needs the full metadata (including parameter types) in order to be able to generate valid calls to those tools.

Why would it need the .NET types to do so? The final recipient needn't be a .NET process; the contract is to generate argument data based on a schema that has been specified on the wire. Only an AIFunctoin that needs to invoke an underlying .NET method requires type information so that it can marshal and unmarshal data.

Not sure how to interpret the example given (OllamaFunctionTool). That class requires not only Name and Description, but also Parameters. So how does it follow that we can remove Parameters but not Name and Description?

The "parameters" property used in Ollama (and OpenAI as well) is a bit of a misnomer since it actually contains a complete JSON schema and not just parameter keywords. Here's what the corresponding model in our code looks like:

extensions/src/Libraries/Microsoft.Extensions.AI.Ollama/OllamaFunctionToolParameters.cs

Lines 9 to 14 in 89b0d7e

	internal sealed class OllamaFunctionToolParameters
	{
	public string Type { get; set; } = "object";
	public required IDictionary<string, JsonElement> Properties { get; set; }
	public IList<string>? Required { get; set; }
	}

In other words, "parameters" should be thought of as the JSON schema for parameterizing the function, but the actual schemas for individual inputs are nested inside a "properties" keyword -- in accordance with JSON schema specification.

In what situation do you need a name and description but no definition of the parameter schema?

I haven't tested, but my expection is that it would always be mandatory. Since my previous change the AIFunctionMetadata.Schema is non-nullable and always returns a valid JSON schema for functions.

This sounds like JSON schema has an official spec for how functions would be represented. Is that the case? I'm guessing not.

Correct. De facto, schemas for functions model a pseudo-object that lists all function parameters as its properties. There are some deviations here, for example OpenAI has coined the "function" type although in my experiements it seems happy with "type" : "object" declarations as well.

So if this is an informally-defined way to represent functions, basically just taking the de-facto standard from OpenAI, what difference does it make what is canonical for JSON schema?

AIFunctionFactory will always populate the function schema with a valid and self-contained schema document per the latest spec. But we do apply transformations to it so that it does conform with OpenAI (and presumably other major vendor) restrictions.

[stephentoub]

We wouldn't need to have Name and Description on AIFunction: Schema is required, and name and description are part of the schema. Having them as first-class members on AIFunction highlights them as being something that might be desirable for consumption for reasons other than function calling, e.g. for diagnostics, for telemetry, for display in a UI when getting approval from the user whether it's ok to invoke a particular function, etc. They would be duplicative, and in that sense they could just be accelerators for pulling specific pieces of information out of the schema, rather than being actually duplicate information.

That's more valuable I think if we pull them down to the base AITool, where it seems like having such a Name and Description are useful for similar purposes, and where parameters aren't as applicable because there aren't necessarily parameters; the base type isn't directly invocable. As an example, OpenAI, Gemini, and Bedrock all support the concept of server-side code interpretation, so we'll likely want to expose a CodeInterpreterAITool : AITool that can be put into the Tools collection as a signal to the service that it can enable code interpretation. That won't have a schema, but it could still have a Name/Description we provide so that there's some human-consumable about it when enumerating Tools. AIFunction could override those members to pull out the relevant portions of the schema.

Again, we don't have to expose Name/Description. We just might want to.

[SteveSandersonMS]

Thanks @eiriktsarpalis and @stephentoub. Lots of it is clearer now. In particular the benefit of retaining name/description properties to simplify telemetry/debugging etc makes a lot of sense.

I'm understanding now that AIFunctionMetadata was itself already duplicative, in the sense that the JSON schema already carried all the information. Having the same info in a different form didn't really achieve anything, now we've settled on taking JSON schema as the standard.

So yes this all does seem like a good direction. Thanks for explaining the details!

[dotnet-comment-bot]

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Microsoft.Extensions.Caching.Hybrid	Line	86	82.92 🔻
Microsoft.Extensions.AI.Ollama	Line	80	78.2 🔻
Microsoft.Extensions.AI.Evaluation.Quality	Line	88	7.57 🔻
Microsoft.Extensions.AI.Evaluation.Quality	Branch	88	16.42 🔻
Microsoft.Extensions.AI.Evaluation.Console	Line	88	8.26 🔻
Microsoft.Extensions.AI.Evaluation.Console	Branch	88	17.07 🔻
Microsoft.Extensions.AI.OpenAI	Line	77	68.26 🔻
Microsoft.Extensions.AI.OpenAI	Branch	77	50.2 🔻
Microsoft.Extensions.AI.Evaluation	Line	88	58.67 🔻
Microsoft.Extensions.AI.Evaluation	Branch	88	56.67 🔻
Microsoft.Extensions.AI.Evaluation.Reporting	Line	88	72.06 🔻
Microsoft.Extensions.AI.Evaluation.Reporting	Branch	88	64.8 🔻
Microsoft.Gen.MetadataExtractor	Line	98	57.35 🔻
Microsoft.Gen.MetadataExtractor	Branch	98	62.5 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project	Expected	Actual
Microsoft.Extensions.AI	88	89
Microsoft.Extensions.AI.Abstractions	83	84
Microsoft.Extensions.AI.AzureAIInference	91	92

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=951531&view=codecoverage-tab

[dotnet-comment-bot]

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Microsoft.Extensions.AI.Evaluation	Line	88	58.67 🔻
Microsoft.Extensions.AI.Evaluation	Branch	88	56.67 🔻
Microsoft.Extensions.Caching.Hybrid	Line	86	82.69 🔻
Microsoft.Extensions.AI.Evaluation.Quality	Line	88	7.57 🔻
Microsoft.Extensions.AI.Evaluation.Quality	Branch	88	16.42 🔻
Microsoft.Extensions.AI.OpenAI	Line	77	68.23 🔻
Microsoft.Extensions.AI.OpenAI	Branch	77	50.2 🔻
Microsoft.Extensions.AI.Ollama	Line	80	78.2 🔻
Microsoft.Extensions.AI.Evaluation.Console	Line	88	8.26 🔻
Microsoft.Extensions.AI.Evaluation.Console	Branch	88	17.07 🔻
Microsoft.Extensions.AI.Evaluation.Reporting	Line	88	72.06 🔻
Microsoft.Extensions.AI.Evaluation.Reporting	Branch	88	64.8 🔻
Microsoft.Gen.MetadataExtractor	Line	98	57.35 🔻
Microsoft.Gen.MetadataExtractor	Branch	98	62.5 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project	Expected	Actual
Microsoft.Extensions.AI.AzureAIInference	91	92
Microsoft.Extensions.AI	88	89
Microsoft.Extensions.AI.Abstractions	83	84

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=951694&view=codecoverage-tab

[dotnet-comment-bot]

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Microsoft.Extensions.Caching.Hybrid	Line	86	82.77 🔻
Microsoft.Extensions.AI.Ollama	Line	80	78.2 🔻
Microsoft.Extensions.AI.OpenAI	Line	77	68.19 🔻
Microsoft.Extensions.AI.OpenAI	Branch	77	50.2 🔻
Microsoft.Extensions.AI.Evaluation.Reporting	Line	88	72.06 🔻
Microsoft.Extensions.AI.Evaluation.Reporting	Branch	88	64.8 🔻
Microsoft.Extensions.AI.Evaluation.Console	Line	88	8.26 🔻
Microsoft.Extensions.AI.Evaluation.Console	Branch	88	17.07 🔻
Microsoft.Extensions.AI.Evaluation.Quality	Line	88	7.57 🔻
Microsoft.Extensions.AI.Evaluation.Quality	Branch	88	16.42 🔻
Microsoft.Gen.MetadataExtractor	Line	98	57.35 🔻
Microsoft.Gen.MetadataExtractor	Branch	98	62.5 🔻
Microsoft.Extensions.AI.Evaluation	Line	88	58.67 🔻
Microsoft.Extensions.AI.Evaluation	Branch	88	56.67 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

Project	Expected	Actual
Microsoft.Extensions.AI.AzureAIInference	91	92
Microsoft.Extensions.AI	88	89

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=951788&view=codecoverage-tab

[stephentoub]

Would it make sense at this point for JsonSchemaCreateOptions and SerializerOptions to be nullable, and then just substitute the default values if they're null when actually constructing the AIFunction? It's not clear to me at this point why that's how Name/Description/AdditionalProperties work but not how these work.

[eiriktsarpalis]

We could do that. Making JSO non-nullable specifically is a holdover from when the factory APIs not accepting an explicit JSO were being marked RUC/RDC.