-
Notifications
You must be signed in to change notification settings - Fork 827
Split AIFunction into a base class #6695
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
Conversation
src/Libraries/Microsoft.Extensions.AI.Abstractions/ChatCompletion/RequiredChatToolMode.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunction.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/DelegatingAIFunctionDefinition.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonSchemaTransformCache.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I think this is a perfectly clear distinction between definition/implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks much for adding! The only other thing that may be nice to include is a simple helper / API to construct definitions for non-invokable tools (such as server-side tools).
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunction.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunctionFactory.cs
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunctionDefinition.cs
Outdated
Show resolved
Hide resolved
7dfb1cf
to
c77ef04
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR splits AIFunction
into a base class hierarchy by introducing AIFunctionDefinition
as a new base class. The main purpose is to enable scenarios where function metadata is needed without invocability, allowing better separation of concerns between function definition and execution.
- Introduces
AIFunctionDefinition
as a base class containing only metadata properties (Name
,Description
,JsonSchema
,ReturnJsonSchema
) - Updates
AIFunction
to inherit fromAIFunctionDefinition
while maintaining invocation capabilities - Adds support for non-invocable function definitions in
FunctionInvokingChatClient
with newTerminateOnUnknownCalls
property
Reviewed Changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunctionDefinition.cs |
New base class containing function metadata properties |
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunction.cs |
Updated to inherit from AIFunctionDefinition and added AsDefinitionOnly() method |
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunctionFactory.cs |
Added CreateDefinition method for creating non-invocable function definitions |
src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs |
Enhanced to handle non-invocable tools with new TerminateOnUnknownCalls property |
Multiple OpenAI integration files | Updated method signatures to accept AIFunctionDefinition instead of AIFunction |
Test files | Added comprehensive test coverage for new functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/DelegatingAIFunctionDefinition.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs
Show resolved
Hide resolved
@SteveSandersonMS, would you mind taking another look? In particular at the FICC logic changes. Thanks. |
src/Libraries/Microsoft.Extensions.AI.Abstractions/Functions/AIFunctionDefinition.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than misgivings about the choice of name, this is a good change.
99a5e5e
to
492b838
Compare
Enable a representation that doesn't support invocation separate from one that does.
492b838
to
32df6fb
Compare
AIFunctionDefinition
as a base class forAIFunction
, whereAIFunction
layers on invocability over the metadata exposed by the base.AIFunctionFactory.CreateDefinition
andAIFunction.AsDefinitionOnly
for creatingAIFunctionDefinition
s that aren't invocable.FunctionInvokingChatClient
to allow non-AIFunction
call requests to pass through.FunctionInvokingChatClient.TerminateOnUnknownCalls
to control behavior when requests are made to unknown functions.Closes #6688
Microsoft Reviewers: Open in CodeFlow