microsoft / semantic-kernel

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

.Net: Bug: Invalid channel Key for aggregation Channel #9072

Open mickaelropars opened 2 days ago

mickaelropars commented 2 days ago

``Describe the bug the workflow when having 2 aggregate agents in a agent group chat, because the aggregate channel key is always the same : set to AggregatorChannel full name value

To Reproduce Steps to reproduce the behavior:

[Fact]
public async Task TestMultipleAggregatorAgent()
{
    const string UserInput = "User Input";

    // Arrange
    Agent agent1Exec = CreateMockAgent("agent1 exec");
    Agent agent1Review = CreateMockAgent("agent1 [OK]");
    Agent agent2Exec = CreateMockAgent("agent2 exec");
    Agent agent2Review = CreateMockAgent("agent2 [OK]");

    AgentGroupChat agent1Chat =
        new(agent1Exec, agent1Review)
        {
            ExecutionSettings =
                new()
                {
                    TerminationStrategy = new ApprovalTerminationStrategy()
                    {
                        Agents = [agent1Review],
                        MaximumIterations = 3,
                        AutomaticReset = true,
                    }
                }
        };
    AgentGroupChat agent2Chat =
        new(agent2Exec, agent2Review)
        {
            ExecutionSettings =
                new()
                {
                    TerminationStrategy = new ApprovalTerminationStrategy()
                    {
                        Agents = [agent2Review],
                        MaximumIterations = 4,
                        AutomaticReset = false,
                    }
                }
        };

    AggregatorAgent agent1 = new(() => agent1Chat) { Mode = AggregatorMode.Flat, Name = "agent1" };
    AggregatorAgent agent2 = new(() => agent2Chat) { Mode = AggregatorMode.Flat, Name = "agent2" };
    AgentGroupChat userChat = new(agent1, agent2)
    {
        ExecutionSettings =
            new()
            {
                TerminationStrategy = new AgentTerminationStrategy(agent2)
                {
                    MaximumIterations = 8,
                    AutomaticReset = true
                }
            }
    };

    userChat.AddChatMessage(new ChatMessageContent(AuthorRole.User, UserInput));

    await foreach (var t in userChat.InvokeAsync())
    {
        Console.WriteLine(t);
    }
}

    private static MockAgent CreateMockAgent(string agentName) => new() { Name = agentName, Response = [new(AuthorRole.Assistant, $"{agentName} -> test") { AuthorName = agentName }] };

    private sealed class ApprovalTerminationStrategy : TerminationStrategy
    {
        protected override Task<bool> ShouldAgentTerminateAsync(Agent agent, IReadOnlyList<ChatMessageContent> history, CancellationToken cancellationToken)
            => Task.FromResult(history[history.Count - 1].Content?.Contains("[OK]", StringComparison.OrdinalIgnoreCase) ?? false);
    }

    private sealed class AgentTerminationStrategy(Agent lastAgent) : TerminationStrategy
    {
        protected override Task<bool> ShouldAgentTerminateAsync(Agent agent, IReadOnlyList<ChatMessageContent> history, CancellationToken cancellationToken = default)
        {
            return Task.FromResult(agent == lastAgent);
        }
    }

at the end of the process the output is

Expected behavior

the output history should be :

Platform

Additional context this is due to the fact that the channel key is based on Agregator class (so when moving to agent2 , the channel executed is the one from agent1 (firstly created)

    protected internal override IEnumerable<string> GetChannelKeys()
    {
        yield return typeof(AggregatorChannel).FullName!;
    }

Potential solution (this solution works)

    protected internal override IEnumerable<string> GetChannelKeys()
    {
        yield return chatProvider().GetHashCode().ToString();
    }
mickaelropars commented 2 days ago

NB: I think that's why I needed to set AutomaticReset to true, because the chat was already completed.