Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Starting from Filters to talk about invoke and ChatCompletionService #6181

Closed
guru98 opened this issue May 10, 2024 · 3 comments
Closed

Starting from Filters to talk about invoke and ChatCompletionService #6181

guru98 opened this issue May 10, 2024 · 3 comments
Labels

Comments

@guru98
Copy link

guru98 commented May 10, 2024

Filters rapresents a great value for the framework and I appreciate the effort to extend to auto functions agens and so on. Still I think that there is an issue when. for example you use ChatCompletionServices directly instead use Kernel.Invoke. In my mind the framework should have a preferred way to use it and should have all necessary feature to use it at the best (e.g. history management). Call directly a service or a specific function could be possible but not recomended as you loose consistency. From this point of view all the examples that use ChatCompletitionServices, for example, could generate confusion about if filters are available in those scenario and when to use invoke instead ChatCompletionService

@dmytrostruk
Copy link
Member

@guru98 Thank you very much for feedback!
The problem with applying filters on ChatCompletionService level is that we can do that for chat completion services that exist today (e.g. OpenAI, Google, Hugging Face), but it's also possible to inject your custom chat completion service. In this case, you will have to apply filtering logic manually, while if you invoke chat completion through kernel, filters will be invoked automatically, no matter which actual chat completion service you use.

It's still possible to achieve similar behavior, but we will have to re-think ChatCompletionService abstraction (e.g. use abstract class and provide some default implementation that will trigger filters), but we should think if this is really the problem we want to resolve.

Maybe we should understand why there is a need to call ChatCompletionService directly instead of Kernel.InvokeAsync, for example if it's for chat history management, then maybe we should improve it from this perspective.

Also, current filters provide a context with information like KernelFunction, KernelArguments, FunctionResult etc., so it is specific to kernel/function invocation. While in chat completion services we operate with different type of data - LLM-related settings, text content, audio content, etc. So, even if we make a decision to apply filtering on chat completion service level, there should be new type of filters added which will receive information specific to chat completion service.

@guru98
Copy link
Author

guru98 commented May 10, 2024 via email

@dmytrostruk
Copy link
Member

dmytrostruk commented May 10, 2024

@guru98 That makes sense and thanks a lot for this feedback, it's really helpful! I agree, we should minimize the need to use chat completion service directly. There are other benefits of using kernel instead of chat completion service such as telemetry, prompt templating etc.

@microsoft microsoft locked and limited conversation to collaborators May 10, 2024
@matthewbolanos matthewbolanos converted this issue into discussion #6185 May 10, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Projects
None yet
Development

No branches or pull requests

3 participants