From 938d91d1dbb91bd60509a6e9e7ba724d4948e26e Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Wed, 27 Aug 2025 16:27:41 +0100 Subject: [PATCH] .NET: Fixes for ChatClientAgent to improve MEAI static analysis compliance (#502) * Fixes for ChatClientAgent to improve MEAI static analysis compliance * Improve instruction handling and fix unit tests. * Address PR comments. --- .../NewPersistentAgentsChatClient.cs | 1 + .../AgentChatClientBuilderExtensions.cs | 6 +- ...hatClient.cs => AgentInvokedChatClient.cs} | 6 +- .../ChatCompletion/ChatClientAgent.cs | 60 ++++++++++--------- .../ChatCompletion/ChatClientAgentOptions.cs | 14 +++++ .../ChatClientAgentRunOptions.cs | 8 +-- .../ChatCompletion/ChatClientExtensions.cs | 6 +- .../ChatClientAgentExtensionsTests.cs | 7 +-- .../ChatCompletion/ChatClientAgentTests.cs | 46 ++++++++------ 9 files changed, 90 insertions(+), 64 deletions(-) rename dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/{AgentInvokingChatClient.cs => AgentInvokedChatClient.cs} (63%) diff --git a/dotnet/src/Microsoft.Extensions.AI.Agents.AzureAI/NewPersistentAgentsChatClient.cs b/dotnet/src/Microsoft.Extensions.AI.Agents.AzureAI/NewPersistentAgentsChatClient.cs index 943f01846d..af5f1ca273 100644 --- a/dotnet/src/Microsoft.Extensions.AI.Agents.AzureAI/NewPersistentAgentsChatClient.cs +++ b/dotnet/src/Microsoft.Extensions.AI.Agents.AzureAI/NewPersistentAgentsChatClient.cs @@ -256,6 +256,7 @@ namespace Azure.AI.Agents.Persistent // Populate the run options from the ChatOptions, if provided. if (options is not null) { + runOptions.OverrideInstructions ??= options.Instructions ?? _agent.Instructions; runOptions.MaxCompletionTokens ??= options.MaxOutputTokens; runOptions.OverrideModelName ??= options.ModelId; runOptions.TopP ??= options.TopP; diff --git a/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/AgentChatClientBuilderExtensions.cs b/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/AgentChatClientBuilderExtensions.cs index e8ad0a8e53..15433dbd68 100644 --- a/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/AgentChatClientBuilderExtensions.cs +++ b/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/AgentChatClientBuilderExtensions.cs @@ -5,13 +5,13 @@ using Microsoft.Shared.Diagnostics; namespace Microsoft.Extensions.AI.Agents; -/// Provides extensions for configuring instances. +/// Provides extensions for configuring instances. public static class AgentChatClientBuilderExtensions { /// /// Enables automatic function call invocation on the chat pipeline. /// - /// This works by adding an instance of with default options. + /// This works by adding an instance of with default options. /// The being used to build the chat pipeline. /// The supplied . /// is . @@ -22,7 +22,7 @@ public static class AgentChatClientBuilderExtensions return builder.Use((innerClient, services) => { - return new AgentInvokingChatClient(innerClient); + return new AgentInvokedChatClient(innerClient); }); } } diff --git a/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/AgentInvokingChatClient.cs b/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/AgentInvokedChatClient.cs similarity index 63% rename from dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/AgentInvokingChatClient.cs rename to dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/AgentInvokedChatClient.cs index a7e29f67cf..67212c2143 100644 --- a/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/AgentInvokingChatClient.cs +++ b/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/AgentInvokedChatClient.cs @@ -5,13 +5,13 @@ namespace Microsoft.Extensions.AI.Agents; /// /// Internal chat client that handle agent invocation details for the chat client pipeline. /// -internal sealed class AgentInvokingChatClient : DelegatingChatClient +internal sealed class AgentInvokedChatClient : DelegatingChatClient { /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// /// The chat client to invoke agents. - internal AgentInvokingChatClient(IChatClient chatClient) + internal AgentInvokedChatClient(IChatClient chatClient) : base(chatClient) { } diff --git a/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientAgent.cs b/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientAgent.cs index 7025fb122c..f53b6248a8 100644 --- a/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientAgent.cs +++ b/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientAgent.cs @@ -9,6 +9,8 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Shared.Diagnostics; +#pragma warning disable S3358 // Ternary operators should not be nested + namespace Microsoft.Extensions.AI.Agents; /// @@ -33,12 +35,12 @@ public sealed class ChatClientAgent : AIAgent public ChatClientAgent(IChatClient chatClient, string? instructions = null, string? name = null, string? description = null, IList? tools = null, ILoggerFactory? loggerFactory = null) : this( chatClient, - new ChatClientAgentOptions() + new ChatClientAgentOptions { Name = name, Description = description, Instructions = instructions, - ChatOptions = tools is null ? null : new ChatOptions() + ChatOptions = tools is null ? null : new ChatOptions { Tools = tools, } @@ -55,7 +57,7 @@ public sealed class ChatClientAgent : AIAgent /// Optional logger factory to use for logging. public ChatClientAgent(IChatClient chatClient, ChatClientAgentOptions options, ILoggerFactory? loggerFactory = null) { - Throw.IfNull(chatClient); + _ = Throw.IfNull(chatClient); // Options must be cloned since ChatClientAgentOptions is mutable. this._agentOptions = options?.Clone(); @@ -65,13 +67,14 @@ public sealed class ChatClientAgent : AIAgent // Get the type of the chat client before wrapping it as an agent invoking chat client. this._chatClientType = chatClient.GetType(); - this.ChatClient = chatClient.AsAgentInvokingChatClient(); + // If the user has not opted out of using our default decorators, we wrap the chat client. + this.ChatClient = options?.UseProvidedChatClientAsIs is true ? chatClient : chatClient.AsAgentInvokedChatClient(); this._logger = (loggerFactory ?? chatClient.GetService() ?? NullLoggerFactory.Instance).CreateLogger(); } /// - /// The underlying chat client used by the agent to invoke chat completions. + /// Gets the underlying chat client used by the agent to invoke chat completions. /// public IChatClient ChatClient { get; } @@ -101,7 +104,7 @@ public sealed class ChatClientAgent : AIAgent AgentRunOptions? options = null, CancellationToken cancellationToken = default) { - Throw.IfNull(messages); + _ = Throw.IfNull(messages); (AgentThread safeThread, ChatOptions? chatOptions, List threadMessages) = await this.PrepareThreadAndMessagesAsync(thread, messages, options, cancellationToken).ConfigureAwait(false); @@ -190,15 +193,17 @@ public sealed class ChatClientAgent : AIAgent /// public override object? GetService(Type serviceType, object? serviceKey = null) - => base.GetService(serviceType, serviceKey) - ?? (serviceType == typeof(AIAgentMetadata) ? this._agentMetadata - : serviceType == typeof(IChatClient) ? this.ChatClient - : this.ChatClient.GetService(serviceType, serviceKey)); + { + return base.GetService(serviceType, serviceKey) + ?? (serviceType == typeof(AIAgentMetadata) ? this._agentMetadata + : serviceType == typeof(IChatClient) ? this.ChatClient + : this.ChatClient.GetService(serviceType, serviceKey)); + } /// public override AgentThread GetNewThread() { - var thread = new AgentThread() { MessageStore = this._agentOptions?.ChatMessageStoreFactory?.Invoke() }; + var thread = new AgentThread { MessageStore = this._agentOptions?.ChatMessageStoreFactory?.Invoke() }; return thread; } @@ -235,6 +240,7 @@ public sealed class ChatClientAgent : AIAgent requestChatOptions.AllowMultipleToolCalls ??= this._agentOptions.ChatOptions.AllowMultipleToolCalls; requestChatOptions.ConversationId ??= this._agentOptions.ChatOptions.ConversationId; requestChatOptions.FrequencyPenalty ??= this._agentOptions.ChatOptions.FrequencyPenalty; + requestChatOptions.Instructions ??= this._agentOptions.ChatOptions.Instructions; requestChatOptions.MaxOutputTokens ??= this._agentOptions.ChatOptions.MaxOutputTokens; requestChatOptions.ModelId ??= this._agentOptions.ChatOptions.ModelId; requestChatOptions.PresencePenalty ??= this._agentOptions.ChatOptions.PresencePenalty; @@ -250,7 +256,7 @@ public sealed class ChatClientAgent : AIAgent { foreach (var propertyKey in this._agentOptions.ChatOptions.AdditionalProperties.Keys) { - requestChatOptions.AdditionalProperties.TryAdd(propertyKey, this._agentOptions.ChatOptions.AdditionalProperties[propertyKey]); + _ = requestChatOptions.AdditionalProperties.TryAdd(propertyKey, this._agentOptions.ChatOptions.AdditionalProperties[propertyKey]); } } else @@ -326,7 +332,7 @@ public sealed class ChatClientAgent : AIAgent /// Optional parameters for agent invocation. /// The cancellation token. /// A tuple containing the thread, chat options, and thread messages. - private async Task<(AgentThread, ChatOptions?, List)> PrepareThreadAndMessagesAsync( + private async Task<(AgentThread AgentThread, ChatOptions? ChatOptions, List ThreadMessages)> PrepareThreadAndMessagesAsync( AgentThread? thread, IReadOnlyCollection inputMessages, AgentRunOptions? runOptions, @@ -343,21 +349,27 @@ public sealed class ChatClientAgent : AIAgent threadMessages.Add(message); } - // Update the messages with agent instructions. - this.UpdateThreadMessagesWithAgentInstructions(threadMessages, runOptions); - // Add the input messages to the end of thread messages. threadMessages.AddRange(inputMessages); // If a user provided two different thread ids, via the thread object and options, we should throw // since we don't know which one to use. - if (!string.IsNullOrWhiteSpace(thread.ConversationId) && !string.IsNullOrWhiteSpace(chatOptions?.ConversationId) && thread.ConversationId != chatOptions.ConversationId) + if (!string.IsNullOrWhiteSpace(thread.ConversationId) && !string.IsNullOrWhiteSpace(chatOptions?.ConversationId) && thread.ConversationId != chatOptions!.ConversationId) { throw new InvalidOperationException( - $"The {nameof(chatOptions.ConversationId)} provided via {nameof(Microsoft.Extensions.AI.ChatOptions)} is different to the id of the provided {nameof(AgentThread)}. Only one thread id can be used for a run."); + $""" + The {nameof(chatOptions.ConversationId)} provided via {nameof(Microsoft.Extensions.AI.ChatOptions)} is different to the id of the provided {nameof(AgentThread)}. + Only one id can be used for a run. + """); } - // Only clone and update ChatOptions if we have an id on the thread and we don't have the same one already in ChatOptions. + if (!string.IsNullOrWhiteSpace(this.Instructions)) + { + chatOptions ??= new(); + chatOptions.Instructions = string.IsNullOrWhiteSpace(chatOptions.Instructions) ? this.Instructions : $"{this.Instructions}\n{chatOptions.Instructions}"; + } + + // Only create or update ChatOptions if we have an id on the thread and we don't have the same one already in ChatOptions. if (!string.IsNullOrWhiteSpace(thread.ConversationId) && thread.ConversationId != chatOptions?.ConversationId) { chatOptions ??= new(); @@ -373,7 +385,9 @@ public sealed class ChatClientAgent : AIAgent { // We were passed a thread that is service managed, but we got no conversation id back from the chat client, // meaning the service doesn't support service managed threads, so the thread cannot be used with this service. +#pragma warning disable S2302 // "nameof" should be used - False positive. throw new InvalidOperationException("Service did not return a valid conversation id when using a service managed thread."); +#pragma warning restore S2302 // "nameof" should be used } if (!string.IsNullOrWhiteSpace(responseConversationId)) @@ -391,14 +405,6 @@ public sealed class ChatClientAgent : AIAgent } } - private void UpdateThreadMessagesWithAgentInstructions(List threadMessages, AgentRunOptions? options) - { - if (!string.IsNullOrWhiteSpace(this.Instructions)) - { - threadMessages.Insert(0, new(ChatRole.System, this.Instructions) { AuthorName = this.Name }); - } - } - private string GetLoggingAgentName() => this.Name ?? "UnnamedAgent"; #endregion } diff --git a/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientAgentOptions.cs b/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientAgentOptions.cs index 6c5cee3ab7..2690539db1 100644 --- a/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientAgentOptions.cs +++ b/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientAgentOptions.cs @@ -79,6 +79,20 @@ public class ChatClientAgentOptions /// public Func? ChatMessageStoreFactory { get; set; } = null; + /// + /// Gets or sets a value indicating whether to use the provided instance as is, + /// without applying any default decorators. + /// + /// + /// By default the applies decorators to the provided + /// for doing for example automatic function invocation. Setting this property to + /// disables adding these default decorators. + /// Disabling is recommended if you want to decorate the with different decorators + /// than the default ones. The provided instance should then already be decorated + /// with the desired decorators. + /// + public bool UseProvidedChatClientAsIs { get; set; } = false; + /// /// Creates a new instance of with the same values as this instance. /// diff --git a/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientAgentRunOptions.cs b/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientAgentRunOptions.cs index 46ecbe3a5a..e7d13e9b70 100644 --- a/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientAgentRunOptions.cs +++ b/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientAgentRunOptions.cs @@ -11,8 +11,8 @@ public sealed class ChatClientAgentRunOptions : AgentRunOptions /// Initializes a new instance of the class. /// /// Optional chat options to pass to the agent's invocation. - public ChatClientAgentRunOptions(ChatOptions? chatOptions = null) : - this(null, chatOptions) + public ChatClientAgentRunOptions(ChatOptions? chatOptions = null) + : this(null, chatOptions) { } @@ -26,8 +26,6 @@ public sealed class ChatClientAgentRunOptions : AgentRunOptions this.ChatOptions = chatOptions; } - /// - /// Gets or sets optional chat options to pass to the agent's invocation - /// + /// Gets or sets optional chat options to pass to the agent's invocation. public ChatOptions? ChatOptions { get; set; } } diff --git a/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientExtensions.cs b/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientExtensions.cs index c65a0d9a69..4577946889 100644 --- a/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientExtensions.cs +++ b/dotnet/src/Microsoft.Extensions.AI.Agents/ChatCompletion/ChatClientExtensions.cs @@ -8,18 +8,18 @@ namespace Microsoft.Extensions.AI.Agents; internal static class ChatClientExtensions { - internal static IChatClient AsAgentInvokingChatClient(this IChatClient chatClient) + internal static IChatClient AsAgentInvokedChatClient(this IChatClient chatClient) { var chatBuilder = chatClient.AsBuilder(); - if (chatClient is not AgentInvokingChatClient agentInvokingChatClient) + if (chatClient is not AgentInvokedChatClient agentInvokedChatClient) { chatBuilder.UseAgentInvocation(); } if (chatClient.GetService() is null) { - chatBuilder.Use((IChatClient innerClient, IServiceProvider services) => + _ = chatBuilder.Use((IChatClient innerClient, IServiceProvider services) => { var loggerFactory = services.GetService(); diff --git a/dotnet/tests/Microsoft.Extensions.AI.Agents.UnitTests/ChatCompletion/ChatClientAgentExtensionsTests.cs b/dotnet/tests/Microsoft.Extensions.AI.Agents.UnitTests/ChatCompletion/ChatClientAgentExtensionsTests.cs index 52ec5f74f2..6302bdeaa6 100644 --- a/dotnet/tests/Microsoft.Extensions.AI.Agents.UnitTests/ChatCompletion/ChatClientAgentExtensionsTests.cs +++ b/dotnet/tests/Microsoft.Extensions.AI.Agents.UnitTests/ChatCompletion/ChatClientAgentExtensionsTests.cs @@ -125,9 +125,9 @@ public class ChatClientAgentExtensionsTests await ChatClientAgentExtensions.RunAsync(agent, messages, agentRunOptions: runOptions); // Assert - Assert.Contains(capturedMessages, m => m.Text == "base instructions" && m.Role == ChatRole.System); Assert.Contains(capturedMessages, m => m.Text == "test" && m.Role == ChatRole.User); - Assert.All(capturedChatOptions, Assert.Null); + Assert.Single(capturedChatOptions); + Assert.Equal("base instructions", capturedChatOptions[0].Instructions); } /// @@ -296,7 +296,6 @@ public class ChatClientAgentExtensionsTests // Assert Assert.Contains(capturedMessages, m => m.Text == "test prompt" && m.Role == ChatRole.User); - Assert.Contains(capturedMessages, m => m.Text == "test instructions" && m.Role == ChatRole.System); } /// @@ -355,7 +354,6 @@ public class ChatClientAgentExtensionsTests await ChatClientAgentExtensions.RunAsync(agent, TestPrompt, agentRunOptions: runOptions); // Assert - Assert.Contains(capturedMessages, m => m.Text == "base instructions" && m.Role == ChatRole.System); Assert.Contains(capturedMessages, m => m.Text == "test prompt" && m.Role == ChatRole.User); } @@ -749,7 +747,6 @@ public class ChatClientAgentExtensionsTests // Assert Assert.Single(updates); Assert.Contains(capturedMessages, m => m.Text == "test prompt" && m.Role == ChatRole.User); - Assert.Contains(capturedMessages, m => m.Text == "test instructions" && m.Role == ChatRole.System); } /// diff --git a/dotnet/tests/Microsoft.Extensions.AI.Agents.UnitTests/ChatCompletion/ChatClientAgentTests.cs b/dotnet/tests/Microsoft.Extensions.AI.Agents.UnitTests/ChatCompletion/ChatClientAgentTests.cs index 5433f69b0c..666777a9b6 100644 --- a/dotnet/tests/Microsoft.Extensions.AI.Agents.UnitTests/ChatCompletion/ChatClientAgentTests.cs +++ b/dotnet/tests/Microsoft.Extensions.AI.Agents.UnitTests/ChatCompletion/ChatClientAgentTests.cs @@ -36,7 +36,7 @@ public class ChatClientAgentTests Assert.Equal("test description", agent.Description); Assert.Equal("test instructions", agent.Instructions); Assert.NotNull(agent.ChatClient); - Assert.Equal("AgentInvokingChatClient", agent.ChatClient.GetType().Name); + Assert.Equal("AgentInvokedChatClient", agent.ChatClient.GetType().Name); } /// @@ -139,7 +139,7 @@ public class ChatClientAgentTests null, It.IsAny())).ReturnsAsync(new ChatResponse([new(ChatRole.Assistant, "response")])); - ChatClientAgent agent = new(mockService.Object, options: new() { Instructions = "test instructions" }); + ChatClientAgent agent = new(mockService.Object); var runOptions = new AgentRunOptions(); // Act @@ -158,7 +158,7 @@ public class ChatClientAgentTests /// Verify that RunAsync includes base instructions in messages. /// [Fact] - public async Task RunAsyncIncludesBaseInstructionsAsync() + public async Task RunAsyncIncludesBaseInstructionsInOptionsAsync() { // Arrange Mock mockService = new(); @@ -166,7 +166,7 @@ public class ChatClientAgentTests mockService.Setup( s => s.GetResponseAsync( It.IsAny>(), - It.IsAny(), + It.Is(x => x.Instructions == "base instructions"), It.IsAny())) .Callback, ChatOptions, CancellationToken>((msgs, opts, ct) => capturedMessages.AddRange(msgs)) @@ -179,7 +179,6 @@ public class ChatClientAgentTests await agent.RunAsync([new(ChatRole.User, "test")], options: runOptions); // Assert - Assert.Contains(capturedMessages, m => m.Text == "base instructions" && m.Role == ChatRole.System); Assert.Contains(capturedMessages, m => m.Text == "test" && m.Role == ChatRole.User); } @@ -238,8 +237,7 @@ public class ChatClientAgentTests await agent.RunAsync([new(ChatRole.User, "new message")], thread: thread); // Assert - // Should contain: instructions + new message - Assert.Contains(capturedMessages, m => m.Text == "test instructions"); + // Should contain: new message Assert.Contains(capturedMessages, m => m.Text == "new message"); } @@ -298,9 +296,7 @@ public class ChatClientAgentTests // Assert // Should only contain the instructions - Assert.Single(capturedMessages); - Assert.Equal("test instructions", capturedMessages[0].Text); - Assert.Equal(ChatRole.System, capturedMessages[0].Role); + Assert.Empty(capturedMessages); } /// @@ -324,7 +320,8 @@ public class ChatClientAgentTests AgentThread thread = new() { ConversationId = "ConvId" }; // Act & Assert - await agent.RunAsync([new(ChatRole.User, "test")], thread, chatOptions: chatOptions); + var response = await agent.RunAsync([new(ChatRole.User, "test")], thread, chatOptions: chatOptions); + Assert.NotNull(response); } /// @@ -424,6 +421,7 @@ public class ChatClientAgentTests // Act & Assert Assert.NotNull(agent.Id); Assert.NotEmpty(agent.Id); + // Base implementation returns a GUID, so it should be parseable as a GUID Assert.True(Guid.TryParse(agent.Id, out _)); } @@ -442,6 +440,7 @@ public class ChatClientAgentTests // Act & Assert Assert.NotNull(agent.Id); Assert.NotEmpty(agent.Id); + // Base implementation returns a GUID, so it should be parseable as a GUID Assert.True(Guid.TryParse(agent.Id, out _)); } @@ -732,6 +731,7 @@ public class ChatClientAgentTests Assert.NotNull(capturedChatOptions); Assert.Equal(100, capturedChatOptions.MaxOutputTokens); Assert.Equal(0.7f, capturedChatOptions.Temperature); + Assert.Equal("test instructions", capturedChatOptions.Instructions); } /// @@ -741,7 +741,7 @@ public class ChatClientAgentTests public async Task ChatOptionsMergingUsesRequestOptionsWhenAgentHasNoneAsync() { // Arrange - var requestChatOptions = new ChatOptions { MaxOutputTokens = 200, Temperature = 0.3f }; + var requestChatOptions = new ChatOptions { MaxOutputTokens = 200, Temperature = 0.3f, Instructions = "test instructions" }; Mock mockService = new(); ChatOptions? capturedChatOptions = null; mockService.Setup( @@ -753,7 +753,7 @@ public class ChatClientAgentTests capturedChatOptions = opts) .ReturnsAsync(new ChatResponse([new(ChatRole.Assistant, "response")])); - ChatClientAgent agent = new(mockService.Object, options: new() { Instructions = "test instructions" }); + ChatClientAgent agent = new(mockService.Object); var messages = new List { new(ChatRole.User, "test") }; // Act @@ -764,6 +764,7 @@ public class ChatClientAgentTests Assert.Equivalent(requestChatOptions, capturedChatOptions); // Should be the same instance since no merging needed Assert.Equal(200, capturedChatOptions.MaxOutputTokens); Assert.Equal(0.3f, capturedChatOptions.Temperature); + Assert.Equal("test instructions", capturedChatOptions.Instructions); } /// @@ -785,7 +786,8 @@ public class ChatClientAgentTests { MaxOutputTokens = 200, Temperature = 0.3f, - AdditionalProperties = new AdditionalPropertiesDictionary() { ["key"] = "request-value" } + AdditionalProperties = new AdditionalPropertiesDictionary() { ["key"] = "request-value" }, + Instructions = "request instructions" // TopP and ModelId not set, should use agent values }; var expectedChatOptionsMerge = new ChatOptions @@ -794,7 +796,8 @@ public class ChatClientAgentTests Temperature = 0.3f, // Request value takes priority AdditionalProperties = new AdditionalPropertiesDictionary() { ["key"] = "request-value" }, // Request value takes priority TopP = 0.9f, // Agent value used when request doesn't specify - ModelId = "agent-model" // Agent value used when request doesn't specify + ModelId = "agent-model", // Agent value used when request doesn't specify + Instructions = "test instructions\nrequest instructions" // Request is in addition to agent instructions }; Mock mockService = new(); @@ -847,7 +850,7 @@ public class ChatClientAgentTests capturedChatOptions = opts) .ReturnsAsync(new ChatResponse([new(ChatRole.Assistant, "response")])); - ChatClientAgent agent = new(mockService.Object, options: new() { Instructions = "test instructions" }); + ChatClientAgent agent = new(mockService.Object); var messages = new List { new(ChatRole.User, "test") }; // Act @@ -1014,6 +1017,7 @@ public class ChatClientAgentTests TopK = 50, PresencePenalty = 0.1f, FrequencyPenalty = 0.2f, + Instructions = "agent instructions", ModelId = "agent-model", Seed = 12345, ConversationId = "agent-conversation", @@ -1024,6 +1028,7 @@ public class ChatClientAgentTests { MaxOutputTokens = 200, Temperature = 0.3f, + Instructions = "request instructions", // Other properties not set, should use agent values StopSequences = ["request-stop"] }; @@ -1038,6 +1043,7 @@ public class ChatClientAgentTests TopK = 50, PresencePenalty = 0.1f, FrequencyPenalty = 0.2f, + Instructions = "test instructions\nrequest instructions", ModelId = "agent-model", Seed = 12345, ConversationId = "agent-conversation", @@ -1142,8 +1148,8 @@ public class ChatClientAgentTests // Assert Assert.NotNull(result); Assert.IsAssignableFrom(result); - // Note: The result will be the AgentInvokingChatClient wrapper, not the original mock - Assert.Equal("AgentInvokingChatClient", result.GetType().Name); + // Note: The result will be the AgentInvokedChatClient wrapper, not the original mock + Assert.Equal("AgentInvokedChatClient", result.GetType().Name); } /// @@ -1383,6 +1389,7 @@ public class ChatClientAgentTests // Assert Assert.NotNull(result); Assert.Same(agent, result); + // Verify that the ChatClient's GetService was not called for this type since base.GetService() handled it mockChatClient.Verify(c => c.GetService(typeof(ChatClientAgent), null), Times.Never); } @@ -1406,6 +1413,7 @@ public class ChatClientAgentTests // Assert Assert.NotNull(result); Assert.Same(agent, result); + // Verify that the ChatClient's GetService was not called for this type since base.GetService() handled it mockChatClient.Verify(c => c.GetService(typeof(AIAgent), null), Times.Never); } @@ -1430,6 +1438,7 @@ public class ChatClientAgentTests // Assert Assert.NotNull(result); Assert.IsAssignableFrom(result); + // Verify that the ChatClient's GetService was NOT called because IChatClient is handled by the agent itself mockChatClient.Verify(c => c.GetService(typeof(IChatClient), "some-key"), Times.Never); } @@ -1454,6 +1463,7 @@ public class ChatClientAgentTests // Assert Assert.NotNull(result); Assert.Equal("test-result", result); + // Verify that the ChatClient's GetService was called after base.GetService() returned null mockChatClient.Verify(c => c.GetService(typeof(string), "some-key"), Times.Once); }