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

.Net: Bug: Invalid channel Key for aggregation Channel

Open mickaelropars opened this issue 1 year ago • 1 comments

``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:

  • Create this unit test
[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

  • User Input
  • agent1 exec
  • agent1 [OK]
  • agent1 exec
  • agent1 [OK]

Expected behavior

the output history should be :

  • User Input
  • agent1 exec
  • agent1 [OK]
  • agent2 exec
  • agent2 [OK]

Platform

  • OS: Windows
  • IDE: Visual Studio
  • Language: C#
  • Source: latest version of semantic kernel

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 avatar Oct 03 '24 01:10 mickaelropars

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

mickaelropars avatar Oct 03 '24 02:10 mickaelropars

@mickaelropars - Thank you for filing this issue and please accept my sincere appreciation for how clearly you've identified the issue. I've copied your test direclty for validation.

Ultimately, this clarifies that different AggregatorAgent should never participate in the same channel. Great catch!

crickman avatar Oct 11 '24 21:10 crickman

@crickman always a pleasure for me to report enhancement or issue. I really appreciate the work you did .

mickaelropars avatar Oct 12 '24 17:10 mickaelropars