semantic-kernel icon indicating copy to clipboard operation
semantic-kernel copied to clipboard

IChatCompletion Multiple Results / New Interfaces

Open rogerbarreto opened this issue 2 years ago • 2 comments

Motivation and Context

Add potential for IChatCompletion with multiple results per prompt.

MultiChatResultPR

Description

  • Adds new Interfaces/Abstractions for Chat Results: IChatResult, IChatStreamingResult, IChatMessage
  • Adds new IChatCompletion interface methods for Streaming and Non-Streaming results
  • Moves old IChatCompletion interface methods IAsyncEnumerable<string> GenerateMessageStreamAsync and Task<string> GenerateMessageAsync as extensions for retro compatibility
  • Changed ChatAsText to use the new multiple results implementations
  • Makes ChatHistory.ChatMessage & ChatHistory.AuthorsRole types obsolete (discouraging usage of those implementation in favor of new IChatMessage
  • ChatHistory abstraction now extends from List<IChatMessage>
  • Updates all Samples using the new interfaces.

Paves the way to ... (Futures)

  • Remove ChatHistory.ChatMessage inner class
  • Remove ChatHistory.AuthorsRole enums
  • Make ChatHistory an abstract class or an IChatHistory interface.
  • Rename ITextCompletionResult to ITextResult
  • Rename ITextCompletionStreamingResult to ITextStreamingResult

Contribution Checklist

  • [x] The code builds clean without any errors or warnings
  • [x] The PR follows SK Contribution Guidelines (https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
  • [x] The code follows the .NET coding conventions (https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions) verified with dotnet format
  • [x] All unit tests pass, and I have added new tests where possible
  • [x] I didn't break anyone :smile:

rogerbarreto avatar May 19 '23 22:05 rogerbarreto

Moves old IChatCompletion interface methods IAsyncEnumerable GenerateMessageStreamAsync and Task GenerateMessageAsync as extensions for retro compatibility

What's our plan for tracking / removing such temporary measures? Do we actually need them?

stephentoub avatar May 22 '23 16:05 stephentoub

What's our plan for tracking / removing such temporary measures? Do we actually need them?

@stephentoub The plan here is to keep those as extensions while we still use it as part of the SK internal implementations.

Once we moved all the SK implementations to use the Result approach those extension methods will be moved to another package/namespace Extensions or flagged as Obsolete.

rogerbarreto avatar May 22 '23 18:05 rogerbarreto