Skip to content

.Net [ChatRoles] 2. Completion service (ChatCompletion / TextCompletion) selection by SemanticFunction #3256

Closed

Description

@SergeyMenshykh

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:

  1. Create ADR and present it to the team.
  2. Collect feedback and implement the completion service selection functionality.
### Tasks
- [x] ADR
- [ ] Implement

Activity

  1. converted this from a draft issue on Oct 23, 2023
  2. added
    kernelIssues or pull requests impacting the core kernel
    and removed on Oct 23, 2023
  3. added this to the v1.0.0 milestone on Oct 23, 2023
  4. moved this from Sprint: Planned to Sprint: In Progress in Semantic Kernelon Oct 24, 2023
  5. added
    sk team issueA tag to denote issues that where created by the Semantic Kernel team (i.e., not the community)
    on Oct 24, 2023
  6. 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
  7. changed the title [-].Net Completion service (ChatCompletion / TextCompletion) selection by SemanticFunction[/-] [+].Net 2. Completion service (ChatCompletion / TextCompletion) selection by SemanticFunction[/+] on Oct 27, 2023
  8. 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
  9. moved this from Sprint: In Progress to Sprint: In Review in Semantic Kernelon Nov 8, 2023
  10. moved this from Sprint: In Review to Sprint: Done in Semantic Kernelon Nov 13, 2023
  11. stekiri commented on Mar 13, 2024

    @stekiri

    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.
  12. dmytrostruk commented on Mar 14, 2024

    @dmytrostruk
    Member

    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?

  13. stekiri commented on Mar 15, 2024

    @stekiri

    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.

  14. dmytrostruk commented on Mar 15, 2024

    @dmytrostruk
    Member

    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.

  15. stekiri commented on Mar 18, 2024

    @stekiri

    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 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 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

.NETIssue or Pull requests regarding .NET codekernelIssues or pull requests impacting the core kernelsk team issueA tag to denote issues that where created by the Semantic Kernel team (i.e., not the community)

Type

No type

Projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions