You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
.Net [ChatRoles] 2. Completion service (ChatCompletion / TextCompletion) selection by SemanticFunction #3256
As an engineer working on SK,
I want to have a mechanism allowing semantic function to select either text or chat completion API/Connector for execution
So, text prompts are handled by text completion connectors
And chat prompts are handled by chat completion connectors
Or chat prompt handled by text completion connectors and the other way around for fallback scenarios
Scope:
Create ADR and present it to the team.
Collect feedback and implement the completion service selection functionality.
changed the title [-].Net Select completion service type based on prompt completion type[/-][+].Net Completion service (ChatCompletion / TextCompletion) selection by SemanticFunction[/+]on Oct 24, 2023
changed the title [-].Net Completion service (ChatCompletion / TextCompletion) selection by SemanticFunction[/-][+].Net 2. Completion service (ChatCompletion / TextCompletion) selection by SemanticFunction[/+]on Oct 27, 2023
changed the title [-].Net 2. Completion service (ChatCompletion / TextCompletion) selection by SemanticFunction[/-][+].Net [ChatRoles] 2. Completion service (ChatCompletion / TextCompletion) selection by SemanticFunction[/+]on Oct 27, 2023
Dmytro Struk (@dmytrostruk), I read the ADR and it seems that the described logic has not been implemented. A different logic seems to have been introduced in #4085. Is the ADR obsolete?
I stumbled upon it in the following scenario:
Custom service that implements both IChatCompletionService and ITextGenerationService.
Chat completion implementation is chosen without taking into account the logic of the ADR.
stekiri Thank you for this comment. You are correct, the ADR which we added previously is kind of obsolete, because later we decided to proceed with other approach, which wasn't described in the documentation. We created a new ADR that explains new approach and we updated status of previous ADR in this PR: #5479
Do you have a specific scenario that is not covered by current approach?
Yes Dmytro Struk (@dmytrostruk), my scenario is when a service implements both IChatCompletionService and ITextGenerationService.
My reasoning would be:
If a user implements ITextGenerationService for a service, then the user made a conscious choice that he considers it being important (and maybe better as chat completion?) for a given use case. Therefore, it should take precedence over IChatCompletionService if there is no chat in his prompt.
Some code is worth more than a thousand words, so I made this small draft PR to communicate what I had in mind. My logic might be flawed, so maybe I miss some use cases that would not be supported with such a change.
If a user implements ITextGenerationService for a service, then the user made a conscious choice that he considers it being important (and maybe better as chat completion?) for a given use case. Therefore, it should take precedence over IChatCompletionService if there is no chat in his prompt.
stekiri I think this scenario is supported, because user could place text generation logic in separate class and implement only ITextGenerationService interface. If user has some common logic for text generation and chat completion that should be re-used in both services, instead of implementing 2 interfaces in the same class, user could create 2 classes, implement both interfaces separately, and put common logic in base class. In order to be able to switch between different services, it's possible to create multiple kernel instances, where one of them will rely on text generation and another one on chat completion.
Some code is worth more than a thousand words, so I made this small draft PR to communicate what I had in mind. My logic might be flawed, so maybe I miss some use cases that would not be supported with such a change.
Thanks for the PR, it definitely helps to understand the idea. The scenario with fallback chat completion may work for now, but I think it will be hard to scale it as soon as we will add support for other types of services (image, audio etc). The solution with placing text generation and chat completion in separate classes should work as expected.
I agree that there are many ways to handle the case, so no worries, it is not blocking me in any way. I am a noobie exploring semantic kernel, I just had the feeling that the implemented logic is surprising.
I finally chose to call kernel.ServiceSelector.TrySelectAIService<ITextGenerationService>(...) + service.GetTextContentsAsync(...) to make it explicit and invoke the text generation service directly. Probably not ideal either, because I guess using kernel.InvokeAsync(...) should be preferred to be future proof.
I had another thought, but I'm not sure if it's worth much. I was wondering if it couldn't be useful to make the decision logic a bit more explicit (and maybe less surprising) by telling the kernel what we expect him to return. For example:
If we invoke kernel.InvokeStreamingAsync<StreamingChatMessageContent>, we want to target a IChatCompletionService.
If we invoke kernel.InvokeStreamingAsync<StreamingTextContent>, we want to target a ITextGenerationService.
This can easily be extended to other services (like IImageGenerationService etc) in the future.
But maybe you already have better strategies in mind for the future, so no worries if my comment is not on point 😄
Activity
SergeyMenshykh commentedon Oct 23, 2023
Parent - #2673
ADR - https://github.com/microsoft/semantic-kernel/blob/main/docs/decisions/0015-completion-service-selection.md
[-].Net Select completion service type based on prompt completion type[/-][+].Net Completion service (ChatCompletion / TextCompletion) selection by SemanticFunction[/+][-].Net Completion service (ChatCompletion / TextCompletion) selection by SemanticFunction[/-][+].Net 2. Completion service (ChatCompletion / TextCompletion) selection by SemanticFunction[/+][-].Net 2. Completion service (ChatCompletion / TextCompletion) selection by SemanticFunction[/-][+].Net [ChatRoles] 2. Completion service (ChatCompletion / TextCompletion) selection by SemanticFunction[/+]stekiri commentedon Mar 13, 2024
Dmytro Struk (@dmytrostruk), I read the ADR and it seems that the described logic has not been implemented. A different logic seems to have been introduced in #4085. Is the ADR obsolete?
I stumbled upon it in the following scenario:
IChatCompletionServiceandITextGenerationService.dmytrostruk commentedon Mar 14, 2024
stekiri Thank you for this comment. You are correct, the ADR which we added previously is kind of obsolete, because later we decided to proceed with other approach, which wasn't described in the documentation. We created a new ADR that explains new approach and we updated status of previous ADR in this PR: #5479
Do you have a specific scenario that is not covered by current approach?
stekiri commentedon Mar 15, 2024
Yes Dmytro Struk (@dmytrostruk), my scenario is when a service implements both
IChatCompletionServiceandITextGenerationService.My reasoning would be:
If a user implements
ITextGenerationServicefor a service, then the user made a conscious choice that he considers it being important (and maybe better as chat completion?) for a given use case. Therefore, it should take precedence overIChatCompletionServiceif there is no chat in his prompt.Some code is worth more than a thousand words, so I made this small draft PR to communicate what I had in mind. My logic might be flawed, so maybe I miss some use cases that would not be supported with such a change.
dmytrostruk commentedon Mar 15, 2024
stekiri I think this scenario is supported, because user could place text generation logic in separate class and implement only
ITextGenerationServiceinterface. If user has some common logic for text generation and chat completion that should be re-used in both services, instead of implementing 2 interfaces in the same class, user could create 2 classes, implement both interfaces separately, and put common logic in base class. In order to be able to switch between different services, it's possible to create multiple kernel instances, where one of them will rely on text generation and another one on chat completion.Thanks for the PR, it definitely helps to understand the idea. The scenario with fallback chat completion may work for now, but I think it will be hard to scale it as soon as we will add support for other types of services (image, audio etc). The solution with placing text generation and chat completion in separate classes should work as expected.
stekiri commentedon Mar 18, 2024
Thanks for taking the time to explain your thoughts in detail Dmytro Struk (@dmytrostruk) 🙏
I agree that there are many ways to handle the case, so no worries, it is not blocking me in any way. I am a noobie exploring semantic kernel, I just had the feeling that the implemented logic is surprising.
I finally chose to call
kernel.ServiceSelector.TrySelectAIService<ITextGenerationService>(...)+service.GetTextContentsAsync(...)to make it explicit and invoke the text generation service directly. Probably not ideal either, because I guess usingkernel.InvokeAsync(...)should be preferred to be future proof.I had another thought, but I'm not sure if it's worth much. I was wondering if it couldn't be useful to make the decision logic a bit more explicit (and maybe less surprising) by telling the kernel what we expect him to return. For example:
kernel.InvokeStreamingAsync<StreamingChatMessageContent>, we want to target aIChatCompletionService.kernel.InvokeStreamingAsync<StreamingTextContent>, we want to target aITextGenerationService.IImageGenerationServiceetc) in the future.But maybe you already have better strategies in mind for the future, so no worries if my comment is not on point 😄