From 46a117d58195255a7d49e2d32954974a58396f34 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Tue, 10 Jun 2025 15:51:37 +0100 Subject: [PATCH] Remove create/delete from AgentThread to avoid needing a thread implementation for each service. (#64) * Remove create/delete from thread to avoid needing a thread implementation for each service. * Address pr feedback. --- .../Microsoft.Agents.Abstractions/Agent.cs | 50 ++--- .../AgentThread.cs | 103 +---------- .../AgentTests.cs | 49 +++-- .../AgentThreadTests.cs | 171 ------------------ 4 files changed, 58 insertions(+), 315 deletions(-) delete mode 100644 dotnet/tests/Microsoft.Agents.Abstractions.UnitTests/AgentThreadTests.cs diff --git a/dotnet/src/Microsoft.Agents.Abstractions/Agent.cs b/dotnet/src/Microsoft.Agents.Abstractions/Agent.cs index 6fbd00add8..ba452532c0 100644 --- a/dotnet/src/Microsoft.Agents.Abstractions/Agent.cs +++ b/dotnet/src/Microsoft.Agents.Abstractions/Agent.cs @@ -39,14 +39,19 @@ public abstract class Agent public virtual string? Instructions { get; } /// - /// Create a new that is compatible with the agent. + /// Get a new instance that is compatible with the agent. /// - /// A new instance that is in the created state. + /// A new instance. /// + /// /// If an agent supports multiple thread types, this method should return the default thread /// type for the agent or whatever the agent was configured to use. + /// + /// + /// If the thread needs to be created via a service call it would be created on first use. + /// /// - public abstract Task CreateThreadAsync(); + public abstract AgentThread GetNewThread(); /// /// Run the agent with no message assuming that all required instructions are already provided to the agent or on the thread. @@ -189,23 +194,17 @@ public abstract class Agent CancellationToken cancellationToken = default); /// - /// Ensures that the thread exists, is of the expected type, and is active, plus adds the provided message to the thread. + /// Checks that the thread is of the expected type, or if null, creates the default thread type. /// /// The expected type of the thead. - /// 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. + /// The thread to create if it's null and validate its type if not null. /// 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 virtual async Task EnsureThreadExistsWithMessagesAsync( - IReadOnlyCollection messages, + protected virtual TThreadType ValidateOrCreateThreadType( AgentThread? thread, - Func constructThread, - CancellationToken cancellationToken) + Func constructThread) where TThreadType : AgentThread { - Throw.IfNull(messages); - thread ??= constructThread is not null ? constructThread() : throw new ArgumentNullException(nameof(constructThread)); if (thread is not TThreadType concreteThreadType) @@ -213,23 +212,11 @@ public abstract class Agent throw new NotSupportedException($"{this.GetType().Name} currently only supports agent threads of type {nameof(TThreadType)}."); } - // We have to explicitly call create here to ensure that the thread is created - // before we run using the thread. While threads will be created when - // notified of new messages, some agents support invoking without a message, - // and in that case no messages will be sent in the next step. - await thread.CreateAsync(cancellationToken).ConfigureAwait(false); - - // Notify the thread that new messages are available. - foreach (var message in messages) - { - await this.NotifyThreadOfNewMessage(thread, message, cancellationToken).ConfigureAwait(false); - } - return concreteThreadType; } /// - /// Notfiy the given thread that a new message is available. + /// Notfiy the given thread that new messages are available. /// /// /// @@ -248,12 +235,15 @@ public abstract class Agent /// require the message. /// /// - /// The thread to notify of the new message. - /// The message to pass to the thread. + /// The thread to notify of the new messages. + /// The messages to pass to the thread. /// The to monitor for cancellation requests. The default is . /// An async task that completes once the notification is complete. - protected Task NotifyThreadOfNewMessage(AgentThread thread, ChatMessage message, CancellationToken cancellationToken) + protected async Task NotifyThreadOfNewMessagesAsync(AgentThread thread, IReadOnlyCollection messages, CancellationToken cancellationToken) { - return thread.OnNewMessageAsync(message, cancellationToken); + if (messages.Count > 0) + { + await thread.OnNewMessagesAsync(messages, cancellationToken).ConfigureAwait(false); + } } } diff --git a/dotnet/src/Microsoft.Agents.Abstractions/AgentThread.cs b/dotnet/src/Microsoft.Agents.Abstractions/AgentThread.cs index 36a6516292..5cd5181c6e 100644 --- a/dotnet/src/Microsoft.Agents.Abstractions/AgentThread.cs +++ b/dotnet/src/Microsoft.Agents.Abstractions/AgentThread.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. using System; +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.AI; @@ -11,113 +12,25 @@ namespace Microsoft.Agents; /// Base abstraction for all agent threads. /// A thread represents a specific conversation with an agent. /// -/// -/// This class is used to manage the lifecycle of an agent thread. -/// The thread can be not-start, started or ended. -/// -public abstract class AgentThread +public class AgentThread { /// - /// Gets the id of the current thread. + /// Gets or sets the id of the current thread. /// - public string? Id { get; protected set; } + public string? Id { get; set; } /// - /// Gets a value indicating whether the thread has been deleted. - /// - public bool IsDeleted { get; protected set; } = false; - - /// - /// Creates the thread and returns the thread id. - /// - /// The to monitor for cancellation requests. The default is . - /// A task that completes when the thread has been created. - /// The thread has been deleted. - internal 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 = await this.CreateCoreAsync(cancellationToken: cancellationToken).ConfigureAwait(false); - } - - /// - /// Deletes the current thread. - /// - /// The to monitor for cancellation requests. The default is . - /// A task that completes when the thread has been deleted. - /// The thread was never created. - public async Task DeleteAsync(CancellationToken cancellationToken = default) - { - if (this.IsDeleted) - { - return; - } - - if (this.Id is null) - { - throw new InvalidOperationException("This thread cannot be deleted, since it has not been created."); - } - - await this.DeleteCoreAsync(cancellationToken).ConfigureAwait(false); - - this.IsDeleted = true; - } - - /// - /// This method is called when a new message has been contributed to the chat by any participant. + /// This method is called when new messages have been contributed to the chat by any participant. /// /// /// Inheritors can use this method to update their context based on the new message. /// - /// The new message. + /// The new messages. /// The to monitor for cancellation requests. The default is . /// A task that completes when the context has been updated. /// The thread has been deleted. - internal async Task OnNewMessageAsync(ChatMessage newMessage, CancellationToken cancellationToken = default) + protected internal virtual Task OnNewMessagesAsync(IReadOnlyCollection newMessages, 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); - } - - await this.OnNewMessageCoreAsync(newMessage, cancellationToken).ConfigureAwait(false); + return Task.CompletedTask; } - - /// - /// Creates the thread and returns the thread id. - /// Checks have already been completed in the method to ensure that the thread can be created. - /// - /// The to monitor for cancellation requests. The default is . - /// The id of the thread that was created if one is available. - protected abstract Task CreateCoreAsync(CancellationToken cancellationToken); - - /// - /// Deletes the current thread. - /// Checks have already been completed in the method to ensure that the thread can be deleted. - /// - /// The to monitor for cancellation requests. The default is . - /// A task that completes when the thread has been deleted. - protected abstract Task DeleteCoreAsync(CancellationToken cancellationToken); - - /// - /// This method is called when a new message has been contributed to the chat by any participant. - /// Checks have already been completed in the method to ensure that the thread can be updated. - /// - /// The new message. - /// The to monitor for cancellation requests. The default is . - /// A task that completes when the context has been updated. - protected abstract Task OnNewMessageCoreAsync(ChatMessage newMessage, CancellationToken cancellationToken = default); } diff --git a/dotnet/tests/Microsoft.Agents.Abstractions.UnitTests/AgentTests.cs b/dotnet/tests/Microsoft.Agents.Abstractions.UnitTests/AgentTests.cs index b0347c9491..8f3ed1ab58 100644 --- a/dotnet/tests/Microsoft.Agents.Abstractions.UnitTests/AgentTests.cs +++ b/dotnet/tests/Microsoft.Agents.Abstractions.UnitTests/AgentTests.cs @@ -212,25 +212,35 @@ public class AgentTests } [Fact] - public async Task EnsureThreadExistsWithMessagesVerifiesAndCreatesThreadAndNotifiesThreadAsync() + public void ValidateOrCreateThreadTypeVerifiesAndCreatesThread() { - var messages = new[] { new ChatMessage(ChatRole.User, "msg1"), new ChatMessage(ChatRole.User, "msg2") }; - var cancellationToken = new CancellationToken(); - // Custom thread type for type checking var threadMock = new Mock() { CallBase = true }; var agent = new MockAgent(); - // Should create and notify - var result = await agent.EnsureThreadExistsWithMessagesAsync(messages, null, () => threadMock.Object, cancellationToken); + // Should create + var result = agent.ValidateOrCreateThreadType(null, () => threadMock.Object); Assert.Same(threadMock.Object, result); - threadMock.Protected().Verify("OnNewMessageCoreAsync", Times.Once(), messages[0], CancellationToken.None); - threadMock.Protected().Verify("OnNewMessageCoreAsync", Times.Once(), messages[1], CancellationToken.None); // Should throw if wrong type var wrongThread = new Mock().Object; - await Assert.ThrowsAsync(() => agent.EnsureThreadExistsWithMessagesAsync(messages, wrongThread, () => threadMock.Object, cancellationToken)); + Assert.Throws(() => agent.ValidateOrCreateThreadType(wrongThread, () => threadMock.Object)); + } + + [Fact] + public async Task NotifyThreadOfNewMessagesNotifiesThreadAsync() + { + var cancellationToken = new CancellationToken(); + + var messages = new[] { new ChatMessage(ChatRole.User, "msg1"), new ChatMessage(ChatRole.User, "msg2") }; + + var threadMock = new Mock() { CallBase = true }; + var agent = new MockAgent(); + + await agent.NotifyThreadOfNewMessagesAsync(threadMock.Object, messages, cancellationToken); + + threadMock.Protected().Verify("OnNewMessagesAsync", Times.Once(), messages, cancellationToken); } /// @@ -241,25 +251,26 @@ public class AgentTests } /// - /// Mock class to test the method. + /// Mock class to test the method. /// private sealed class MockAgent : Agent { - public new Task EnsureThreadExistsWithMessagesAsync( - IReadOnlyCollection messages, + public new TThreadType ValidateOrCreateThreadType( AgentThread? thread, - Func constructThread, - CancellationToken cancellationToken) + Func constructThread) where TThreadType : AgentThread { - return base.EnsureThreadExistsWithMessagesAsync( - messages, + return base.ValidateOrCreateThreadType( thread, - constructThread, - cancellationToken); + constructThread); } - public override Task CreateThreadAsync() + public new Task NotifyThreadOfNewMessagesAsync(AgentThread thread, IReadOnlyCollection messages, CancellationToken cancellationToken) + { + return base.NotifyThreadOfNewMessagesAsync(thread, messages, cancellationToken); + } + + public override AgentThread GetNewThread() { throw new System.NotImplementedException(); } diff --git a/dotnet/tests/Microsoft.Agents.Abstractions.UnitTests/AgentThreadTests.cs b/dotnet/tests/Microsoft.Agents.Abstractions.UnitTests/AgentThreadTests.cs deleted file mode 100644 index 331a6d6604..0000000000 --- a/dotnet/tests/Microsoft.Agents.Abstractions.UnitTests/AgentThreadTests.cs +++ /dev/null @@ -1,171 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.Extensions.AI; - -namespace Microsoft.Agents.Abstractions.UnitTests; - -/// -/// Contains tests for the class. -/// -public class AgentThreadTests -{ - /// - /// Tests that the CreateAsync method sets the Id and invokes CreateInternalAsync once. - /// - [Fact] - public async Task CreateShouldSetIdAndInvokeCreateInternalOnceAsync() - { - // Arrange - var thread = new TestAgentThread(); - - // Act - await thread.CreateAsync(); - await thread.CreateAsync(); - - // Assert - Assert.Equal("test-thread-id", thread.Id); - Assert.Equal(1, thread.CreateInternalAsyncCount); - } - - /// - /// Tests that the CreateAsync method throws an InvalidOperationException if the thread is deleted. - /// - [Fact] - public async Task CreateShouldThrowIfThreadDeletedAsync() - { - // Arrange - var thread = new TestAgentThread(); - await thread.CreateAsync(); - await thread.DeleteAsync(); - - // Act & Assert - await Assert.ThrowsAsync(() => thread.CreateAsync()); - Assert.Equal(1, thread.CreateInternalAsyncCount); - Assert.Equal(1, thread.DeleteInternalAsyncCount); - } - - /// - /// Tests that the DeleteAsync method sets IsDeleted and invokes DeleteInternalAsync. - /// - [Fact] - public async Task DeleteShouldSetIsDeletedAndInvokeDeleteInternalAsync() - { - // Arrange - var thread = new TestAgentThread(); - await thread.CreateAsync(); - - // Act - await thread.DeleteAsync(); - - // Assert - Assert.True(thread.IsDeleted); - Assert.Equal(1, thread.CreateInternalAsyncCount); - Assert.Equal(1, thread.DeleteInternalAsyncCount); - } - - /// - /// Tests that the DeleteAsync method does not invoke DeleteInternalAsync if the thread is already deleted. - /// - [Fact] - public async Task DeleteShouldNotInvokeDeleteInternalIfAlreadyDeletedAsync() - { - // Arrange - var thread = new TestAgentThread(); - await thread.CreateAsync(); - await thread.DeleteAsync(); - - // Act - await thread.DeleteAsync(); - - // Assert - Assert.True(thread.IsDeleted); - Assert.Equal(1, thread.CreateInternalAsyncCount); - Assert.Equal(1, thread.DeleteInternalAsyncCount); - } - - /// - /// Tests that the DeleteAsync method throws an InvalidOperationException if the thread was never created. - /// - [Fact] - public async Task DeleteShouldThrowIfNeverCreatedAsync() - { - // Arrange - var thread = new TestAgentThread(); - - // Act & Assert - await Assert.ThrowsAsync(() => thread.DeleteAsync()); - Assert.Equal(0, thread.CreateInternalAsyncCount); - Assert.Equal(0, thread.DeleteInternalAsyncCount); - } - - /// - /// Tests that the OnNewMessageAsync method creates the thread if it is not already created. - /// - [Fact] - public async Task OnNewMessageShouldCreateThreadIfNotCreatedAsync() - { - // Arrange - var thread = new TestAgentThread(); - var message = new ChatMessage(); - - // Act - await thread.OnNewMessageAsync(message); - - // Assert - Assert.Equal("test-thread-id", thread.Id); - Assert.Equal(1, thread.CreateInternalAsyncCount); - Assert.Equal(1, thread.OnNewMessageInternalAsyncCount); - } - - /// - /// Tests that the OnNewMessageAsync method throws an InvalidOperationException if the thread is deleted. - /// - [Fact] - public async Task OnNewMessageShouldThrowIfThreadDeletedAsync() - { - // Arrange - var thread = new TestAgentThread(); - await thread.CreateAsync(); - await thread.DeleteAsync(); - var message = new ChatMessage(); - - // Act & Assert - await Assert.ThrowsAsync(() => thread.OnNewMessageAsync(message)); - Assert.Equal(1, thread.CreateInternalAsyncCount); - Assert.Equal(1, thread.DeleteInternalAsyncCount); - Assert.Equal(0, thread.OnNewMessageInternalAsyncCount); - } - - private sealed class TestAgentThread : AgentThread - { - public int CreateInternalAsyncCount { get; private set; } - public int DeleteInternalAsyncCount { get; private set; } - public int OnNewMessageInternalAsyncCount { get; private set; } - - public new Task CreateAsync(CancellationToken cancellationToken = default) - { - return base.CreateAsync(cancellationToken); - } - - protected override Task CreateCoreAsync(CancellationToken cancellationToken) - { - this.CreateInternalAsyncCount++; - return Task.FromResult("test-thread-id"); - } - - protected override Task DeleteCoreAsync(CancellationToken cancellationToken) - { - this.DeleteInternalAsyncCount++; - return Task.CompletedTask; - } - - protected override Task OnNewMessageCoreAsync(ChatMessage newMessage, CancellationToken cancellationToken = default) - { - this.OnNewMessageInternalAsyncCount++; - return Task.CompletedTask; - } - } -}