Move AIFunction parameter schematization from parameter level to function level. #5826
Conversation
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunctionMetadata.cs
Show resolved
Hide resolved
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=933837&view=codecoverage-tab |
eiriktsarpalis
force-pushed
the
feature/function-metadata
branch
from
0a3b415 to
ec8d3ab
Compare
src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.cs
Show resolved
Hide resolved
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=938548&view=codecoverage-tab |
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunctionMetadata.cs
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunctionMetadata.cs
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.AzureAIInference/AzureAIInferenceChatClient.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.Ollama/OllamaChatClient.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIModelMapper.ChatCompletion.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIRealtimeExtensions.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIRealtimeExtensions.cs
Outdated
Show resolved
Hide resolved
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=939708&view=codecoverage-tab |
| /// When no schema is specified, consuming chat clients should assume the "{}" or "true" schema, indicating that any JSON input is admissible. | ||
| /// </para> | ||
| /// </remarks> | ||
| public JsonElement Schema |
There was a problem hiding this comment.
@stephentoub one consequence of making the schema self-contained and never-null is that leaf clients no longer need to consult AIFunctionParameterMetadata or AIFunctionReturnParameterMetadata information. Neither they, nor the AIFunctionMetadata.GetParameter helper function is being used in product code today.
While I can see the informational value in exposing this metadata, it is all entirely redundant today and might in fact completely diverge from both the schema and the actual requirements of the underlying AIFunction.InvokeAsync method. It is also forcing more boilerplate on users implementing AIFunction types of their own. Is it something we could consider removing?
There was a problem hiding this comment.
How would SK layer on top if it lacked that information?
Is there not still value / scenarios in having the .NET type information for parameters exposed, so that another consumer of the AIFunction can use that to understand what .NET types it's actually expecting? Seems like this is inching back towards having the arguments dictionary being strongly typed as JsonElement.
And as we spoke about offline, we've seen folks take the return schema information and add it into function call results to help the LLM better understand what the return information represents. Unless we incorporate the return schema into the function schema, we'll be cutting off that possibility, no?
I'm all for simplifying, but I worry this would be cutting off useful information.
Fix #5655.