From 8874e5c9842a63398cf21a70b4ac75c0f8f693b9 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Fri, 21 Mar 2025 19:44:28 +0000 Subject: [PATCH 1/5] Move kernel and kernelarguments to options. --- dotnet/src/Agents/Abstractions/Agent.cs | 8 -------- dotnet/src/Agents/Abstractions/AgentInvokeOptions.cs | 10 ++++++++++ dotnet/src/Agents/Abstractions/AggregatorAgent.cs | 4 ---- dotnet/src/Agents/AzureAI/AzureAIAgent.cs | 12 ++++-------- dotnet/src/Agents/Bedrock/BedrockAgent.cs | 4 ---- dotnet/src/Agents/Core/ChatCompletionAgent.cs | 12 ++++-------- dotnet/src/Agents/OpenAI/OpenAIAssistantAgent.cs | 12 ++++-------- dotnet/src/Agents/UnitTests/MockAgent.cs | 4 ---- 8 files changed, 22 insertions(+), 44 deletions(-) diff --git a/dotnet/src/Agents/Abstractions/Agent.cs b/dotnet/src/Agents/Abstractions/Agent.cs index a742d7ce6767..ea6c248a5933 100644 --- a/dotnet/src/Agents/Abstractions/Agent.cs +++ b/dotnet/src/Agents/Abstractions/Agent.cs @@ -48,8 +48,6 @@ public abstract class Agent /// /// The message to pass to the agent. /// The conversation thread to continue with this invocation. If not provided, creates a new thread. - /// Optional arguments to pass to the agents's invocation, including any . - /// The containing services, plugins, and other state for use by the agent. /// Optional parameters for agent invocation. /// The to monitor for cancellation requests. The default is . /// An async list of response items that each contain a and an . @@ -59,8 +57,6 @@ public abstract class Agent public abstract IAsyncEnumerable> InvokeAsync( ChatMessageContent message, AgentThread? thread = null, - KernelArguments? arguments = null, - Kernel? kernel = null, AgentInvokeOptions? options = null, CancellationToken cancellationToken = default); @@ -69,8 +65,6 @@ public abstract IAsyncEnumerable> InvokeAs /// /// The message to pass to the agent. /// The conversation thread to continue with this invocation. If not provided, creates a new thread. - /// Optional arguments to pass to the agents's invocation, including any . - /// The containing services, plugins, and other state for use by the agent. /// Optional parameters for agent invocation. /// The to monitor for cancellation requests. The default is . /// An async list of response items that each contain a and an . @@ -80,8 +74,6 @@ public abstract IAsyncEnumerable> InvokeAs public abstract IAsyncEnumerable> InvokeStreamingAsync( ChatMessageContent message, AgentThread? thread = null, - KernelArguments? arguments = null, - Kernel? kernel = null, AgentInvokeOptions? options = null, CancellationToken cancellationToken = default); diff --git a/dotnet/src/Agents/Abstractions/AgentInvokeOptions.cs b/dotnet/src/Agents/Abstractions/AgentInvokeOptions.cs index e93045a5544f..6aff36e0fc64 100644 --- a/dotnet/src/Agents/Abstractions/AgentInvokeOptions.cs +++ b/dotnet/src/Agents/Abstractions/AgentInvokeOptions.cs @@ -7,6 +7,16 @@ namespace Microsoft.SemanticKernel.Agents; /// public class AgentInvokeOptions { + /// + /// Gets or sets optional arguments to pass to the agent's invocation, including any + /// + public KernelArguments? KernelArguments { get; init; } = null; + + /// + /// Gets or sets the containing services, plugins, and other state for use by the agent + /// + public Kernel? Kernel { get; init; } = null; + /// /// Gets or sets any instructions, in addition to those that were provided to the agent /// initially, that need to be added to the prompt for this invocation only. diff --git a/dotnet/src/Agents/Abstractions/AggregatorAgent.cs b/dotnet/src/Agents/Abstractions/AggregatorAgent.cs index e71b7ff03f1a..d546a9361fc8 100644 --- a/dotnet/src/Agents/Abstractions/AggregatorAgent.cs +++ b/dotnet/src/Agents/Abstractions/AggregatorAgent.cs @@ -48,8 +48,6 @@ public sealed class AggregatorAgent(Func chatProvider) : Agent public override IAsyncEnumerable> InvokeAsync( ChatMessageContent message, AgentThread? thread = null, - KernelArguments? arguments = null, - Kernel? kernel = null, AgentInvokeOptions? options = null, CancellationToken cancellationToken = default) { @@ -61,8 +59,6 @@ public override IAsyncEnumerable> InvokeAs public override IAsyncEnumerable> InvokeStreamingAsync( ChatMessageContent message, AgentThread? thread = null, - KernelArguments? arguments = null, - Kernel? kernel = null, AgentInvokeOptions? options = null, CancellationToken cancellationToken = default) { diff --git a/dotnet/src/Agents/AzureAI/AzureAIAgent.cs b/dotnet/src/Agents/AzureAI/AzureAIAgent.cs index 0ee773037885..61ce4e8ffca4 100644 --- a/dotnet/src/Agents/AzureAI/AzureAIAgent.cs +++ b/dotnet/src/Agents/AzureAI/AzureAIAgent.cs @@ -145,8 +145,6 @@ public IAsyncEnumerable InvokeAsync( public override async IAsyncEnumerable> InvokeAsync( ChatMessageContent message, AgentThread? thread = null, - KernelArguments? arguments = null, - Kernel? kernel = null, AgentInvokeOptions? options = null, [EnumeratorCancellation] CancellationToken cancellationToken = default) { @@ -168,8 +166,8 @@ public override async IAsyncEnumerable> In var invokeResults = this.InvokeAsync( azureAIAgentThread.Id!, internalOptions, - this.MergeArguments(arguments), - kernel ?? this.Kernel, + this.MergeArguments(options?.KernelArguments), + options?.Kernel ?? this.Kernel, cancellationToken); // Notify the thread of new messages and return them to the caller. @@ -223,8 +221,6 @@ async IAsyncEnumerable InternalInvokeAsync() public async override IAsyncEnumerable> InvokeStreamingAsync( ChatMessageContent message, AgentThread? thread = null, - KernelArguments? arguments = null, - Kernel? kernel = null, AgentInvokeOptions? options = null, [EnumeratorCancellation] CancellationToken cancellationToken = default) { @@ -247,8 +243,8 @@ public async override IAsyncEnumerable> InvokeAsync( ChatMessageContent message, AgentThread? thread = null, - KernelArguments? arguments = null, - Kernel? kernel = null, AgentInvokeOptions? options = null, CancellationToken cancellationToken = default) { @@ -90,8 +88,6 @@ public override IAsyncEnumerable> InvokeAs public override IAsyncEnumerable> InvokeStreamingAsync( ChatMessageContent message, AgentThread? thread = null, - KernelArguments? arguments = null, - Kernel? kernel = null, AgentInvokeOptions? options = null, CancellationToken cancellationToken = default) { diff --git a/dotnet/src/Agents/Core/ChatCompletionAgent.cs b/dotnet/src/Agents/Core/ChatCompletionAgent.cs index ddbe0bb5ac5c..aac955c78dfa 100644 --- a/dotnet/src/Agents/Core/ChatCompletionAgent.cs +++ b/dotnet/src/Agents/Core/ChatCompletionAgent.cs @@ -61,8 +61,6 @@ public ChatCompletionAgent( public override async IAsyncEnumerable> InvokeAsync( ChatMessageContent message, AgentThread? thread = null, - KernelArguments? arguments = null, - Kernel? kernel = null, AgentInvokeOptions? options = null, [EnumeratorCancellation] CancellationToken cancellationToken = default) { @@ -84,8 +82,8 @@ public override async IAsyncEnumerable> In var invokeResults = this.InternalInvokeAsync( agentName, chatHistory, - this.MergeArguments(arguments), - kernel ?? this.Kernel, + this.MergeArguments(options?.KernelArguments), + options?.Kernel ?? this.Kernel, options?.AdditionalInstructions, cancellationToken); @@ -116,8 +114,6 @@ public override IAsyncEnumerable InvokeAsync( public override async IAsyncEnumerable> InvokeStreamingAsync( ChatMessageContent message, AgentThread? thread = null, - KernelArguments? arguments = null, - Kernel? kernel = null, AgentInvokeOptions? options = null, [EnumeratorCancellation] CancellationToken cancellationToken = default) { @@ -140,8 +136,8 @@ public override async IAsyncEnumerable chatHistoryAgentThread.OnNewMessageAsync(newMessage), - this.MergeArguments(arguments), - kernel ?? this.Kernel, + this.MergeArguments(options?.KernelArguments), + options?.Kernel ?? this.Kernel, options?.AdditionalInstructions, cancellationToken); diff --git a/dotnet/src/Agents/OpenAI/OpenAIAssistantAgent.cs b/dotnet/src/Agents/OpenAI/OpenAIAssistantAgent.cs index e16d130d6e46..606b58207a60 100644 --- a/dotnet/src/Agents/OpenAI/OpenAIAssistantAgent.cs +++ b/dotnet/src/Agents/OpenAI/OpenAIAssistantAgent.cs @@ -364,8 +364,6 @@ public async Task DeleteAsync(CancellationToken cancellationToken = defaul public override async IAsyncEnumerable> InvokeAsync( ChatMessageContent message, AgentThread? thread = null, - KernelArguments? arguments = null, - Kernel? kernel = null, AgentInvokeOptions? options = null, [EnumeratorCancellation] CancellationToken cancellationToken = default) { @@ -387,8 +385,8 @@ public override async IAsyncEnumerable> In var invokeResults = this.InvokeAsync( openAIAssistantAgentThread.Id!, internalOptions, - this.MergeArguments(arguments), - kernel ?? this.Kernel, + this.MergeArguments(options?.KernelArguments), + options?.Kernel ?? this.Kernel, cancellationToken); // Notify the thread of new messages and return them to the caller. @@ -460,8 +458,6 @@ async IAsyncEnumerable InternalInvokeAsync() public async override IAsyncEnumerable> InvokeStreamingAsync( ChatMessageContent message, AgentThread? thread = null, - KernelArguments? arguments = null, - Kernel? kernel = null, AgentInvokeOptions? options = null, [EnumeratorCancellation] CancellationToken cancellationToken = default) { @@ -484,8 +480,8 @@ public async override IAsyncEnumerable> InvokeAsync( ChatMessageContent message, AgentThread? thread = null, - KernelArguments? arguments = null, - Kernel? kernel = null, AgentInvokeOptions? options = null, CancellationToken cancellationToken = default) { @@ -46,8 +44,6 @@ public override IAsyncEnumerable InvokeAsync( public override IAsyncEnumerable> InvokeStreamingAsync( ChatMessageContent message, AgentThread? thread = null, - KernelArguments? arguments = null, - Kernel? kernel = null, AgentInvokeOptions? options = null, CancellationToken cancellationToken = default) { From 567800028f5c3e0ae08b3c5ec7fe39b8ad6c4da6 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Fri, 21 Mar 2025 19:57:21 +0000 Subject: [PATCH 2/5] Add mult-message invoke overload --- dotnet/src/Agents/Abstractions/Agent.cs | 55 +++++++++++++++++-- .../Agents/Abstractions/AggregatorAgent.cs | 4 +- dotnet/src/Agents/AzureAI/AzureAIAgent.cs | 12 ++-- dotnet/src/Agents/Bedrock/BedrockAgent.cs | 4 +- dotnet/src/Agents/Core/ChatCompletionAgent.cs | 12 ++-- .../src/Agents/OpenAI/OpenAIAssistantAgent.cs | 12 ++-- dotnet/src/Agents/UnitTests/MockAgent.cs | 4 +- .../CommonInterfaceConformance/InvokeTests.cs | 2 +- 8 files changed, 74 insertions(+), 31 deletions(-) diff --git a/dotnet/src/Agents/Abstractions/Agent.cs b/dotnet/src/Agents/Abstractions/Agent.cs index ea6c248a5933..1dc1461f122f 100644 --- a/dotnet/src/Agents/Abstractions/Agent.cs +++ b/dotnet/src/Agents/Abstractions/Agent.cs @@ -54,10 +54,30 @@ public abstract class Agent /// /// To continue this thread in the future, use an returned in one of the response items. /// - public abstract IAsyncEnumerable> InvokeAsync( + public virtual IAsyncEnumerable> InvokeAsync( ChatMessageContent message, AgentThread? thread = null, AgentInvokeOptions? options = null, + CancellationToken cancellationToken = default) + { + return this.InvokeAsync(new[] { message }, thread, options, cancellationToken); + } + + /// + /// Invoke the agent with the provided message and arguments. + /// + /// The messages to pass to the agent. + /// The conversation thread to continue with this invocation. If not provided, creates a new thread. + /// Optional parameters for agent invocation. + /// The to monitor for cancellation requests. The default is . + /// An async list of response items that each contain a and an . + /// + /// To continue this thread in the future, use an returned in one of the response items. + /// + public abstract IAsyncEnumerable> InvokeAsync( + ICollection messages, + AgentThread? thread = null, + AgentInvokeOptions? options = null, CancellationToken cancellationToken = default); /// @@ -71,10 +91,30 @@ public abstract IAsyncEnumerable> InvokeAs /// /// To continue this thread in the future, use an returned in one of the response items. /// - public abstract IAsyncEnumerable> InvokeStreamingAsync( + public virtual IAsyncEnumerable> InvokeStreamingAsync( ChatMessageContent message, AgentThread? thread = null, AgentInvokeOptions? options = null, + CancellationToken cancellationToken = default) + { + return this.InvokeStreamingAsync(new[] { message }, thread, options, cancellationToken); + } + + /// + /// Invoke the agent with the provided message and arguments. + /// + /// The messages to pass to the agent. + /// The conversation thread to continue with this invocation. If not provided, creates a new thread. + /// Optional parameters for agent invocation. + /// The to monitor for cancellation requests. The default is . + /// An async list of response items that each contain a and an . + /// + /// To continue this thread in the future, use an returned in one of the response items. + /// + public abstract IAsyncEnumerable> InvokeStreamingAsync( + ICollection messages, + AgentThread? thread = null, + AgentInvokeOptions? options = null, CancellationToken cancellationToken = default); /// @@ -135,14 +175,14 @@ public abstract IAsyncEnumerable> /// Ensures that the thread exists, is of the expected type, and is active, plus adds the provided message to the thread. /// /// The expected type of the thead. - /// The message to add to the thread once it is setup. + /// The messages to add to the thread once it is setup. /// The thread to create if it's null, validate it's type if not null, and start if it is not active. /// A callback to use to construct the thread if it's null. /// The to monitor for cancellation requests. The default is . /// An async task that completes once all update are complete. /// protected async Task EnsureThreadExistsWithMessageAsync( - ChatMessageContent message, + ICollection messages, AgentThread? thread, Func constructThread, CancellationToken cancellationToken) @@ -160,8 +200,11 @@ protected async Task EnsureThreadExistsWithMessageAsync chatProvider) : Agent /// public override IAsyncEnumerable> InvokeAsync( - ChatMessageContent message, + ICollection messages, AgentThread? thread = null, AgentInvokeOptions? options = null, CancellationToken cancellationToken = default) @@ -57,7 +57,7 @@ public override IAsyncEnumerable> InvokeAs /// public override IAsyncEnumerable> InvokeStreamingAsync( - ChatMessageContent message, + ICollection messages, AgentThread? thread = null, AgentInvokeOptions? options = null, CancellationToken cancellationToken = default) diff --git a/dotnet/src/Agents/AzureAI/AzureAIAgent.cs b/dotnet/src/Agents/AzureAI/AzureAIAgent.cs index 61ce4e8ffca4..548fa1bea0f0 100644 --- a/dotnet/src/Agents/AzureAI/AzureAIAgent.cs +++ b/dotnet/src/Agents/AzureAI/AzureAIAgent.cs @@ -143,15 +143,15 @@ public IAsyncEnumerable InvokeAsync( /// public override async IAsyncEnumerable> InvokeAsync( - ChatMessageContent message, + ICollection messages, AgentThread? thread = null, AgentInvokeOptions? options = null, [EnumeratorCancellation] CancellationToken cancellationToken = default) { - Verify.NotNull(message); + Verify.NotNull(messages); var azureAIAgentThread = await this.EnsureThreadExistsWithMessageAsync( - message, + messages, thread, () => new AzureAIAgentThread(this.Client), cancellationToken).ConfigureAwait(false); @@ -219,15 +219,15 @@ async IAsyncEnumerable InternalInvokeAsync() /// public async override IAsyncEnumerable> InvokeStreamingAsync( - ChatMessageContent message, + ICollection messages, AgentThread? thread = null, AgentInvokeOptions? options = null, [EnumeratorCancellation] CancellationToken cancellationToken = default) { - Verify.NotNull(message); + Verify.NotNull(messages); var azureAIAgentThread = await this.EnsureThreadExistsWithMessageAsync( - message, + messages, thread, () => new AzureAIAgentThread(this.Client), cancellationToken).ConfigureAwait(false); diff --git a/dotnet/src/Agents/Bedrock/BedrockAgent.cs b/dotnet/src/Agents/Bedrock/BedrockAgent.cs index b9dcab681d89..a3762f00bcb1 100644 --- a/dotnet/src/Agents/Bedrock/BedrockAgent.cs +++ b/dotnet/src/Agents/Bedrock/BedrockAgent.cs @@ -75,7 +75,7 @@ public static string CreateSessionId() /// public override IAsyncEnumerable> InvokeAsync( - ChatMessageContent message, + ICollection messages, AgentThread? thread = null, AgentInvokeOptions? options = null, CancellationToken cancellationToken = default) @@ -86,7 +86,7 @@ public override IAsyncEnumerable> InvokeAs /// public override IAsyncEnumerable> InvokeStreamingAsync( - ChatMessageContent message, + ICollection messages, AgentThread? thread = null, AgentInvokeOptions? options = null, CancellationToken cancellationToken = default) diff --git a/dotnet/src/Agents/Core/ChatCompletionAgent.cs b/dotnet/src/Agents/Core/ChatCompletionAgent.cs index aac955c78dfa..271770646632 100644 --- a/dotnet/src/Agents/Core/ChatCompletionAgent.cs +++ b/dotnet/src/Agents/Core/ChatCompletionAgent.cs @@ -59,15 +59,15 @@ public ChatCompletionAgent( /// public override async IAsyncEnumerable> InvokeAsync( - ChatMessageContent message, + ICollection messages, AgentThread? thread = null, AgentInvokeOptions? options = null, [EnumeratorCancellation] CancellationToken cancellationToken = default) { - Verify.NotNull(message); + Verify.NotNull(messages); var chatHistoryAgentThread = await this.EnsureThreadExistsWithMessageAsync( - message, + messages, thread, () => new ChatHistoryAgentThread(), cancellationToken).ConfigureAwait(false); @@ -112,15 +112,15 @@ public override IAsyncEnumerable InvokeAsync( /// public override async IAsyncEnumerable> InvokeStreamingAsync( - ChatMessageContent message, + ICollection messages, AgentThread? thread = null, AgentInvokeOptions? options = null, [EnumeratorCancellation] CancellationToken cancellationToken = default) { - Verify.NotNull(message); + Verify.NotNull(messages); var chatHistoryAgentThread = await this.EnsureThreadExistsWithMessageAsync( - message, + messages, thread, () => new ChatHistoryAgentThread(), cancellationToken).ConfigureAwait(false); diff --git a/dotnet/src/Agents/OpenAI/OpenAIAssistantAgent.cs b/dotnet/src/Agents/OpenAI/OpenAIAssistantAgent.cs index 606b58207a60..48aba8edf415 100644 --- a/dotnet/src/Agents/OpenAI/OpenAIAssistantAgent.cs +++ b/dotnet/src/Agents/OpenAI/OpenAIAssistantAgent.cs @@ -362,15 +362,15 @@ public async Task DeleteAsync(CancellationToken cancellationToken = defaul /// public override async IAsyncEnumerable> InvokeAsync( - ChatMessageContent message, + ICollection messages, AgentThread? thread = null, AgentInvokeOptions? options = null, [EnumeratorCancellation] CancellationToken cancellationToken = default) { - Verify.NotNull(message); + Verify.NotNull(messages); var openAIAssistantAgentThread = await this.EnsureThreadExistsWithMessageAsync( - message, + messages, thread, () => new OpenAIAssistantAgentThread(this.Client), cancellationToken).ConfigureAwait(false); @@ -456,15 +456,15 @@ async IAsyncEnumerable InternalInvokeAsync() /// public async override IAsyncEnumerable> InvokeStreamingAsync( - ChatMessageContent message, + ICollection messages, AgentThread? thread = null, AgentInvokeOptions? options = null, [EnumeratorCancellation] CancellationToken cancellationToken = default) { - Verify.NotNull(message); + Verify.NotNull(messages); var openAIAssistantAgentThread = await this.EnsureThreadExistsWithMessageAsync( - message, + messages, thread, () => new OpenAIAssistantAgentThread(this.Client), cancellationToken).ConfigureAwait(false); diff --git a/dotnet/src/Agents/UnitTests/MockAgent.cs b/dotnet/src/Agents/UnitTests/MockAgent.cs index 8307056239d5..72bb3ed30509 100644 --- a/dotnet/src/Agents/UnitTests/MockAgent.cs +++ b/dotnet/src/Agents/UnitTests/MockAgent.cs @@ -20,7 +20,7 @@ internal sealed class MockAgent : ChatHistoryKernelAgent public IReadOnlyList Response { get; set; } = []; public override IAsyncEnumerable> InvokeAsync( - ChatMessageContent message, + ICollection messages, AgentThread? thread = null, AgentInvokeOptions? options = null, CancellationToken cancellationToken = default) @@ -42,7 +42,7 @@ public override IAsyncEnumerable InvokeAsync( /// public override IAsyncEnumerable> InvokeStreamingAsync( - ChatMessageContent message, + ICollection messages, AgentThread? thread = null, AgentInvokeOptions? options = null, CancellationToken cancellationToken = default) diff --git a/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/InvokeTests.cs b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/InvokeTests.cs index 1cee519982b4..3b1afcd796b9 100644 --- a/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/InvokeTests.cs +++ b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/InvokeTests.cs @@ -11,7 +11,7 @@ namespace SemanticKernel.IntegrationTests.Agents.CommonInterfaceConformance; /// -/// Base test class for testing the method of agents. +/// Base test class for testing the method of agents. /// Each agent type should have its own derived class. /// public abstract class InvokeTests(Func createAgentFixture) : IAsyncLifetime From e70956fcd40560a9f6d7e42ffbee3e877c4c4ee4 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Fri, 21 Mar 2025 20:14:54 +0000 Subject: [PATCH 3/5] Seal threads and autocreate on message received. --- dotnet/src/Agents/AzureAI/AzureAIAgentThread.cs | 4 ++-- dotnet/src/Agents/Core/ChatHistoryAgentThread.cs | 7 +++---- dotnet/src/Agents/OpenAI/OpenAIAssistantAgentThread.cs | 4 ++-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/dotnet/src/Agents/AzureAI/AzureAIAgentThread.cs b/dotnet/src/Agents/AzureAI/AzureAIAgentThread.cs index c5c0090ea170..ea6897fa44d1 100644 --- a/dotnet/src/Agents/AzureAI/AzureAIAgentThread.cs +++ b/dotnet/src/Agents/AzureAI/AzureAIAgentThread.cs @@ -12,7 +12,7 @@ namespace Microsoft.SemanticKernel.Agents.AzureAI; /// /// Represents a conversation thread for an Azure AI agent. /// -public class AzureAIAgentThread : AgentThread +public sealed class AzureAIAgentThread : AgentThread { private readonly AgentsClient _client; private string? _id = null; @@ -76,7 +76,7 @@ public override async Task OnNewMessageAsync(ChatMessageContent newMessage, Canc { if (this._id is null) { - throw new InvalidOperationException("Messages cannot be added to this thread, since the thread has not been started."); + await this.CreateAsync(cancellationToken).ConfigureAwait(false); } // If the message was generated by this agent, it is already in the thread and we shouldn't add it again. diff --git a/dotnet/src/Agents/Core/ChatHistoryAgentThread.cs b/dotnet/src/Agents/Core/ChatHistoryAgentThread.cs index cb13ead90e99..0e32af43bdfe 100644 --- a/dotnet/src/Agents/Core/ChatHistoryAgentThread.cs +++ b/dotnet/src/Agents/Core/ChatHistoryAgentThread.cs @@ -12,7 +12,7 @@ namespace Microsoft.SemanticKernel.Agents; /// /// Represents a conversation thread based on an instance of that is maanged inside this class. /// -public class ChatHistoryAgentThread : AgentThread +public sealed class ChatHistoryAgentThread : AgentThread { private readonly ChatHistory _chatHistory = new(); private string? _id = null; @@ -67,15 +67,14 @@ public override Task DeleteAsync(CancellationToken cancellationToken = default) } /// - public override Task OnNewMessageAsync(ChatMessageContent newMessage, CancellationToken cancellationToken = default) + public async override Task OnNewMessageAsync(ChatMessageContent newMessage, CancellationToken cancellationToken = default) { if (this._id is null) { - throw new InvalidOperationException("Messages cannot be added to this thread, since the thread has not been started."); + await this.CreateAsync(cancellationToken).ConfigureAwait(false); } this._chatHistory.Add(newMessage); - return Task.CompletedTask; } /// diff --git a/dotnet/src/Agents/OpenAI/OpenAIAssistantAgentThread.cs b/dotnet/src/Agents/OpenAI/OpenAIAssistantAgentThread.cs index f6be22a77c91..810e6ac449b8 100644 --- a/dotnet/src/Agents/OpenAI/OpenAIAssistantAgentThread.cs +++ b/dotnet/src/Agents/OpenAI/OpenAIAssistantAgentThread.cs @@ -12,7 +12,7 @@ namespace Microsoft.SemanticKernel.Agents.OpenAI; /// /// Represents a conversation thread for an Open AI Assistant agent. /// -public class OpenAIAssistantAgentThread : AgentThread +public sealed class OpenAIAssistantAgentThread : AgentThread { private readonly AssistantClient _client; private string? _id = null; @@ -76,7 +76,7 @@ public override async Task OnNewMessageAsync(ChatMessageContent newMessage, Canc { if (this._id is null) { - throw new InvalidOperationException("Messages cannot be added to this thread, since the thread has not been started."); + await this.CreateAsync(cancellationToken).ConfigureAwait(false); } // If the message was generated by this agent, it is already in the thread and we shouldn't add it again. From 26a6af94c4c315ed4a2f3451f3549c6e3498b02f Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Fri, 21 Mar 2025 21:02:36 +0000 Subject: [PATCH 4/5] Add improved threading logic and compliance tests. --- .../src/Agents/AzureAI/AzureAIAgentThread.cs | 46 ++++++++++++-- .../src/Agents/Core/ChatHistoryAgentThread.cs | 36 +++++++++-- .../OpenAI/OpenAIAssistantAgentThread.cs | 36 +++++++++-- .../AgentThreadTests.cs | 60 +++++++++++++++++++ .../AzureAIAgentThreadTests.cs | 7 +++ .../ChatCompletionAgentThreadTests.cs | 7 +++ .../OpenAIAssistantAgentThreadTests.cs | 7 +++ .../AzureAIAgentFixture.cs | 9 ++- .../OpenAIAssistantAgentFixture.cs | 10 +++- 9 files changed, 199 insertions(+), 19 deletions(-) create mode 100644 dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/AgentThreadTests.cs create mode 100644 dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/AzureAIAgentThreadTests.cs create mode 100644 dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/ChatCompletionAgentThreadTests.cs create mode 100644 dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/OpenAIAssistantAgentThreadTests.cs diff --git a/dotnet/src/Agents/AzureAI/AzureAIAgentThread.cs b/dotnet/src/Agents/AzureAI/AzureAIAgentThread.cs index ea6897fa44d1..140d2f37def1 100644 --- a/dotnet/src/Agents/AzureAI/AzureAIAgentThread.cs +++ b/dotnet/src/Agents/AzureAI/AzureAIAgentThread.cs @@ -2,8 +2,10 @@ using System; using System.Collections.Generic; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; +using Azure; using Azure.AI.Projects; using Microsoft.SemanticKernel.Agents.AzureAI.Internal; @@ -16,6 +18,7 @@ public sealed class AzureAIAgentThread : AgentThread { private readonly AgentsClient _client; private string? _id = null; + private bool _isDeleted = false; /// /// Initializes a new instance of the class. @@ -48,6 +51,11 @@ public AzureAIAgentThread(AgentsClient client, string id) /// public override async Task CreateAsync(CancellationToken cancellationToken = default) { + if (this._isDeleted) + { + throw new InvalidOperationException("This thread has been deleted and cannot be recreated."); + } + if (this._id is not null) { return this._id; @@ -62,18 +70,36 @@ public override async Task CreateAsync(CancellationToken cancellationTok /// public override async Task DeleteAsync(CancellationToken cancellationToken = default) { + if (this._isDeleted) + { + return; + } + if (this._id is null) { - throw new InvalidOperationException("This thread cannot be ended, since it has not been started."); + throw new InvalidOperationException("This thread cannot be deleted, since it has not been created."); + } + + try + { + await this._client.DeleteThreadAsync(this._id, cancellationToken).ConfigureAwait(false); + } + catch (RequestFailedException ex) when (ex.Status == 404) + { + // Do nothing, since the thread was already deleted. } - await this._client.DeleteThreadAsync(this._id, cancellationToken).ConfigureAwait(false); - this._id = null; + this._isDeleted = true; } /// public override async Task OnNewMessageAsync(ChatMessageContent newMessage, CancellationToken cancellationToken = default) { + if (this._isDeleted) + { + throw new InvalidOperationException("This thread has been deleted and cannot be used anymore."); + } + if (this._id is null) { await this.CreateAsync(cancellationToken).ConfigureAwait(false); @@ -87,13 +113,21 @@ public override async Task OnNewMessageAsync(ChatMessageContent newMessage, Canc } /// - public IAsyncEnumerable GetMessagesAsync(CancellationToken cancellationToken = default) + public async IAsyncEnumerable GetMessagesAsync([EnumeratorCancellation] CancellationToken cancellationToken = default) { + if (this._isDeleted) + { + throw new InvalidOperationException("This thread has been deleted and cannot be used anymore."); + } + if (this._id is null) { - throw new InvalidOperationException("The messages for this thread cannot be retrieved, since the thread has not been started."); + await this.CreateAsync(cancellationToken).ConfigureAwait(false); } - return AgentThreadActions.GetMessagesAsync(this._client, this._id!, ListSortOrder.Ascending, cancellationToken); + await foreach (var message in AgentThreadActions.GetMessagesAsync(this._client, this._id!, ListSortOrder.Ascending, cancellationToken).ConfigureAwait(false)) + { + yield return message; + } } } diff --git a/dotnet/src/Agents/Core/ChatHistoryAgentThread.cs b/dotnet/src/Agents/Core/ChatHistoryAgentThread.cs index 0e32af43bdfe..889cae3b1f92 100644 --- a/dotnet/src/Agents/Core/ChatHistoryAgentThread.cs +++ b/dotnet/src/Agents/Core/ChatHistoryAgentThread.cs @@ -2,7 +2,7 @@ using System; using System.Collections.Generic; -using System.Linq; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using Microsoft.SemanticKernel.ChatCompletion; @@ -16,6 +16,7 @@ public sealed class ChatHistoryAgentThread : AgentThread { private readonly ChatHistory _chatHistory = new(); private string? _id = null; + private bool _isDeleted = false; /// /// Initializes a new instance of the class. @@ -42,6 +43,11 @@ public ChatHistoryAgentThread(ChatHistory chatHistory, string? id = null) /// public override Task CreateAsync(CancellationToken cancellationToken = default) { + if (this._isDeleted) + { + throw new InvalidOperationException("This thread has been deleted and cannot be recreated."); + } + if (this._id is not null) { return Task.FromResult(this._id); @@ -55,13 +61,18 @@ public override Task CreateAsync(CancellationToken cancellationToken = d /// public override Task DeleteAsync(CancellationToken cancellationToken = default) { + if (this._isDeleted) + { + return Task.CompletedTask; + } + if (this._id is null) { - throw new InvalidOperationException("This thread cannot be ended, since it has not been started."); + throw new InvalidOperationException("This thread cannot be deleted, since it has not been created."); } this._chatHistory.Clear(); - this._id = null; + this._isDeleted = true; return Task.CompletedTask; } @@ -69,6 +80,11 @@ public override Task DeleteAsync(CancellationToken cancellationToken = default) /// public async override Task OnNewMessageAsync(ChatMessageContent newMessage, CancellationToken cancellationToken = default) { + if (this._isDeleted) + { + throw new InvalidOperationException("This thread has been deleted and cannot be used anymore."); + } + if (this._id is null) { await this.CreateAsync(cancellationToken).ConfigureAwait(false); @@ -78,13 +94,21 @@ public async override Task OnNewMessageAsync(ChatMessageContent newMessage, Canc } /// - public IAsyncEnumerable GetMessagesAsync(CancellationToken cancellationToken = default) + public async IAsyncEnumerable GetMessagesAsync([EnumeratorCancellation] CancellationToken cancellationToken = default) { + if (this._isDeleted) + { + throw new InvalidOperationException("This thread has been deleted and cannot be used anymore."); + } + if (this._id is null) { - throw new InvalidOperationException("The messages for this thread cannot be retrieved, since the thread has not been started."); + await this.CreateAsync(cancellationToken).ConfigureAwait(false); } - return this._chatHistory.ToAsyncEnumerable(); + foreach (var message in this._chatHistory) + { + yield return message; + } } } diff --git a/dotnet/src/Agents/OpenAI/OpenAIAssistantAgentThread.cs b/dotnet/src/Agents/OpenAI/OpenAIAssistantAgentThread.cs index 810e6ac449b8..bc504e934e8c 100644 --- a/dotnet/src/Agents/OpenAI/OpenAIAssistantAgentThread.cs +++ b/dotnet/src/Agents/OpenAI/OpenAIAssistantAgentThread.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using Microsoft.SemanticKernel.Agents.OpenAI.Internal; @@ -16,6 +17,7 @@ public sealed class OpenAIAssistantAgentThread : AgentThread { private readonly AssistantClient _client; private string? _id = null; + private bool _isDeleted = false; /// /// Initializes a new instance of the class. @@ -48,6 +50,11 @@ public OpenAIAssistantAgentThread(AssistantClient client, string id) /// public override async Task CreateAsync(CancellationToken cancellationToken = default) { + if (this._isDeleted) + { + throw new InvalidOperationException("This thread has been deleted and cannot be recreated."); + } + if (this._id is not null) { return this._id; @@ -62,18 +69,29 @@ public override async Task CreateAsync(CancellationToken cancellationTok /// public override async Task DeleteAsync(CancellationToken cancellationToken = default) { + if (this._isDeleted) + { + return; + } + if (this._id is null) { - throw new InvalidOperationException("This thread cannot be ended, since it has not been started."); + throw new InvalidOperationException("This thread cannot be deleted, since it has not been created."); } await this._client.DeleteThreadAsync(this._id, cancellationToken).ConfigureAwait(false); - this._id = null; + + this._isDeleted = true; } /// public override async Task OnNewMessageAsync(ChatMessageContent newMessage, CancellationToken cancellationToken = default) { + if (this._isDeleted) + { + throw new InvalidOperationException("This thread has been deleted and cannot be used anymore."); + } + if (this._id is null) { await this.CreateAsync(cancellationToken).ConfigureAwait(false); @@ -87,13 +105,21 @@ public override async Task OnNewMessageAsync(ChatMessageContent newMessage, Canc } /// - public IAsyncEnumerable GetMessagesAsync(CancellationToken cancellationToken = default) + public async IAsyncEnumerable GetMessagesAsync([EnumeratorCancellation] CancellationToken cancellationToken = default) { + if (this._isDeleted) + { + throw new InvalidOperationException("This thread has been deleted and cannot be used anymore."); + } + if (this._id is null) { - throw new InvalidOperationException("The messages for this thread cannot be retrieved, since the thread has not been started."); + await this.CreateAsync(cancellationToken).ConfigureAwait(false); } - return AssistantThreadActions.GetMessagesAsync(this._client, this._id!, MessageCollectionOrder.Ascending, cancellationToken); + await foreach (var message in AssistantThreadActions.GetMessagesAsync(this._client, this._id!, MessageCollectionOrder.Ascending, cancellationToken).ConfigureAwait(false)) + { + yield return message; + } } } diff --git a/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/AgentThreadTests.cs b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/AgentThreadTests.cs new file mode 100644 index 000000000000..1b71751eb35e --- /dev/null +++ b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/AgentThreadTests.cs @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Threading.Tasks; +using Microsoft.SemanticKernel; +using Xunit; + +namespace SemanticKernel.IntegrationTests.Agents.CommonInterfaceConformance.AgentThreadConformance; + +public abstract class AgentThreadTests(Func createAgentFixture) : IAsyncLifetime +{ +#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring as nullable. + private AgentFixture _agentFixture; +#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring as nullable. + + protected AgentFixture Fixture => this._agentFixture; + + [Fact] + public async Task DeletingThreadTwiceDoesNotThrowAsync() + { + await this.Fixture.AgentThread.CreateAsync(); + + await this.Fixture.AgentThread.DeleteAsync(); + await this.Fixture.AgentThread.DeleteAsync(); + } + + [Fact] + public async Task UsingThreadAfterDeleteThrowsAsync() + { + await this.Fixture.AgentThread.CreateAsync(); + await this.Fixture.AgentThread.DeleteAsync(); + + await Assert.ThrowsAsync(async () => await this.Fixture.AgentThread.CreateAsync()); + await Assert.ThrowsAsync(async () => await this.Fixture.AgentThread.OnNewMessageAsync(new ChatMessageContent())); + } + + [Fact] + public async Task DeleteThreadBeforeCreateThrowsAsync() + { + await Assert.ThrowsAsync(async () => await this.Fixture.AgentThread.DeleteAsync()); + } + + [Fact] + public async Task UsingThreadbeforeCreateCreatesAsync() + { + await this.Fixture.AgentThread.OnNewMessageAsync(new ChatMessageContent()); + Assert.NotNull(this.Fixture.AgentThread.Id); + } + + public Task InitializeAsync() + { + this._agentFixture = createAgentFixture(); + return this._agentFixture.InitializeAsync(); + } + + public Task DisposeAsync() + { + return this._agentFixture.DisposeAsync(); + } +} diff --git a/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/AzureAIAgentThreadTests.cs b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/AzureAIAgentThreadTests.cs new file mode 100644 index 000000000000..db2c20495580 --- /dev/null +++ b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/AzureAIAgentThreadTests.cs @@ -0,0 +1,7 @@ +// Copyright (c) Microsoft. All rights reserved. + +namespace SemanticKernel.IntegrationTests.Agents.CommonInterfaceConformance.AgentThreadConformance; + +public class AzureAIAgentThreadTests() : AgentThreadTests(() => new AzureAIAgentFixture()) +{ +} diff --git a/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/ChatCompletionAgentThreadTests.cs b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/ChatCompletionAgentThreadTests.cs new file mode 100644 index 000000000000..6c1b29269a98 --- /dev/null +++ b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/ChatCompletionAgentThreadTests.cs @@ -0,0 +1,7 @@ +// Copyright (c) Microsoft. All rights reserved. + +namespace SemanticKernel.IntegrationTests.Agents.CommonInterfaceConformance.AgentThreadConformance; + +public class ChatCompletionAgentThreadTests() : AgentThreadTests(() => new ChatCompletionAgentFixture()) +{ +} diff --git a/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/OpenAIAssistantAgentThreadTests.cs b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/OpenAIAssistantAgentThreadTests.cs new file mode 100644 index 000000000000..63aadf8aa54c --- /dev/null +++ b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AgentThreadConformance/OpenAIAssistantAgentThreadTests.cs @@ -0,0 +1,7 @@ +// Copyright (c) Microsoft. All rights reserved. + +namespace SemanticKernel.IntegrationTests.Agents.CommonInterfaceConformance.AgentThreadConformance; + +public class OpenAIAssistantAgentThreadTests() : AgentThreadTests(() => new OpenAIAssistantAgentFixture()) +{ +} diff --git a/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AzureAIAgentFixture.cs b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AzureAIAgentFixture.cs index e944d506af2a..9d61ff73e866 100644 --- a/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AzureAIAgentFixture.cs +++ b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/AzureAIAgentFixture.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. using System.Threading.Tasks; +using Azure; using Azure.Identity; using Microsoft.Extensions.Configuration; using Microsoft.SemanticKernel; @@ -49,7 +50,13 @@ public override async Task DisposeAsync() { if (this._thread!.Id is not null) { - await this._agentsClient!.DeleteThreadAsync(this._thread!.Id); + try + { + await this._agentsClient!.DeleteThreadAsync(this._thread!.Id); + } + catch (RequestFailedException ex) when (ex.Status == 404) + { + } } await this._agentsClient!.DeleteAgentAsync(this._aiAgent!.Id); diff --git a/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/OpenAIAssistantAgentFixture.cs b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/OpenAIAssistantAgentFixture.cs index c62d913a4b0f..bff7620e8d60 100644 --- a/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/OpenAIAssistantAgentFixture.cs +++ b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/OpenAIAssistantAgentFixture.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. using System; +using System.ClientModel; using System.Threading.Tasks; using Azure.Identity; using Microsoft.Extensions.Configuration; @@ -48,7 +49,14 @@ public override async Task DisposeAsync() { if (this._thread!.Id is not null) { - await this._assistantClient!.DeleteThreadAsync(this._thread!.Id); + try + { + await this._assistantClient!.DeleteThreadAsync(this._thread!.Id); + } + catch (ClientResultException ex) when (ex.Status == 404) + { + Console.WriteLine(ex); + } } await this._assistantClient!.DeleteAssistantAsync(this._assistant!.Id); From 445d5d6841c40f14fc85c2dcf35488d02a4994e1 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Fri, 21 Mar 2025 21:09:41 +0000 Subject: [PATCH 5/5] Remove console.writeline --- .../CommonInterfaceConformance/OpenAIAssistantAgentFixture.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/OpenAIAssistantAgentFixture.cs b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/OpenAIAssistantAgentFixture.cs index bff7620e8d60..9fdce790f7f5 100644 --- a/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/OpenAIAssistantAgentFixture.cs +++ b/dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/OpenAIAssistantAgentFixture.cs @@ -55,7 +55,6 @@ public override async Task DisposeAsync() } catch (ClientResultException ex) when (ex.Status == 404) { - Console.WriteLine(ex); } }