microsoft / semantic-kernel

Integrate cutting-edge LLM technology quickly and easily into your apps
https://aka.ms/semantic-kernel
MIT License
22.1k stars 3.3k forks source link

.Net: Strongly Typed Agents #6018

Open crickman opened 7 months ago

crickman commented 7 months ago

Some users may prefer to DI agents based on type.

Since our concete agents are all sealed, customer are unable to creat strong-typed agents:

class ProbeAgent : ChatCompletionAgent
class SupervisorAgent : ChatCompletionAgent

Other DI pattern exist:

  1. Keyed Services
  2. Agent Provider
  3. Adapater Agent (unsealed)
glennc commented 6 months ago

Was going to log an issue on this but found this. IMO unsealing them and also adding a constructor option to pass Name and such in would help this pattern.

I recently have been using the agent types and my natural inclination was to create a class that represented my agent that encapsulates the configuration and functionality of my agent. For example what I would like to do is something like this:

internal class MyAgent : ChatCompletionAgent
{
    private const string INSTRUCTIONS = """
            you are a helpful assistant.
        """;
    public MyAgent(Kernel kernel) : 
        base(name: "MyAgent", kernel: kernel, instructions: INSTRUCTIONS)
    {
        kernel.ImportPluginFromObject(this);
    }

    [Description("usefull tool")]
    [KernelFunction()]
    public async Task<string> MyFunction()
    {
        return "Hello World!";
    }
}

I could now inject other things that my plugins might use (a HTTPClient instance with pre-configured tokens to be used for example). I feel good because all the code for this specific agent is in one place and I can refer to it by its type. I can register it as MyAgent and refer to it that way or even as IEnumerable<ChatCompletionAgent> to get all agents if I want to write the code that consumes the agents in that way.

The way it is now almost forces me to use keyed services. Which is a fine pattern, just not the only pattern I can think of and wasn't the way I naturally started with this. My natural inclination was to create a class that represents my thing not create a bundle of config in my Program.cs that represents the thing. Is there something wrong with what I am describing that means it shouldn't be possible?

stephentoub commented 6 months ago

I think it makes sense to derive from Agent (or KernelAgent), but it seems better to not derive from something like ChatCompletionAgent. That bakes into your public surface area what's actually an implementation detail, that you're using ChatCompletionAgent as the implementation. If tomorrow you decided you instead wanted to use an OpenAIAssistantAgent, it would be a breaking change, and also publicly visible, to change the base type.

I think better would instead to use a decorator pattern, both deriving from Agent and containing / delegating to a ChatCompletionAgent, e.g.

internal sealed class MyAgent : Agent
{
    private readonly ChatCompletionAgent _impl = new ChatCompletionAgent(...);

    ... override ...
}

delegating wherever relevant to _impl.

crickman commented 2 months ago

@glennc - I've worked with some customers who are subclassing ChatHistoryKernelAgent and using composition (an internal ChatCompletionAgent) to address custom requirements around telemetry and instruction management. This is simliar to what @stephentoub has described, but requires much less code to implement as the "channel" contracts are already addressed.