‼️Found issues‼️

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

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

‼️Found issues‼️

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

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

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

@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 providesAIFunctionMetadata, or aMethodInfo which we convert toAIFunctionMetadata
  • IChatClient implementation receivesAIFunctionMetadata and converts it to its own wire format, usually JSON schema
• After:
  • App provides JSON schema, or aMethodInfo 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 thatAIFunction would gainName andDescription 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 containsname anddescription, so I'm guessing we could eliminateName andDescription as separate properties.

Can you summarize the design intent?

One use case forAIFunction 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 symbolicAIFunction 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 ofAIFunctionMetadata 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 theAIFunctionParameterMetadata.Type property; even though it's optional they didn't understand the ramifications of not using something there.

Given thatAIFunctionMetadata is really a proxy forParameterInfo, we initially debated using anIReadOnlyList<ParameterInfo> but ultimately ended up onMethodInfo?. It more clearly communicates that this particular instance is backed by a .NET method, provides richer metadata, and it substantially simplifies function implementations. BecauseMethodInfo 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 anobject? Schema property scoped toAIFunctionParameterMetadata that we were documenting as being a JSON schema. Every component in our infrastructure and leaf client implementations expected this to beJsonElement. That PR changed the schema to explicitly be aJsonElement scoped to the function level as opposed to the parameter level. It came followingcustomer 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, theMethodInfo 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 inenvelopes 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 seeName (and possibleDescription?) lifted to to theAITool base class since he needs to introduce a new derived tool type that needs this information as well.

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 theserver end of anIChatClient 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 instancesare 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 inenvelopes 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 onlyName andDescription, but alsoParameters. So how does it follow that we can removeParameters but notName andDescription?

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'mguessing 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.

You mention that the instancesare 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 onlyName andDescription, but alsoParameters. So how does it follow that we can removeParameters but notName andDescription?

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:

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 theAIFunctionMetadata.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'mguessing 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.

We wouldn'tneed 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.

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 thatAIFunctionMetadata 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!

‼️Found issues‼️

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

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

‼️Found issues‼️

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

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

‼️Found issues‼️

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

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