Skip to content

Commit 81953f2

Browse files
authored
.Net Agents - Align with expectations when arguments override specified. (microsoft#9096)
### Motivation and Context <!-- Thank you for your contribution to the semantic-kernel repo! Please help reviewers and future users, providing the following information: 1. Why is this change required? 2. What problem does it solve? 3. What scenario does it contribute to? 4. If it fixes an open issue, please link to the issue here. --> When default `KernelArguments` are specified for `KernelAgent.Arguments`, it is likely expected that when override `arguments` parameter is provided that this override doesn't necessarily invalidate the default `KernelArguments.ExecutionSettings`. ### Description <!-- Describe your changes, the overall approach, the underlying design. These notes will help understanding how your code works. Thanks! --> Provide performant merge logic for _default_ and _override_ `KernelArguments` for all invocations. This merge preserves any _default_ values not specifically included in the _override_...including both `ExecutionSettings` and parameters. Solves this (common) pattern: ```c# ChatCompletionAgent agent = new() { Name = "SampleAssistantAgent", Instructions = """ Something something. Something dynamic: {{$topic}} The current date and time is: {{$now}}. """, Kernel = kernel, Arguments = new KernelArguments(new AzureOpenAIPromptExecutionSettings() { FunctionChoiceBehavior = FunctionChoiceBehavior.Auto() }) { { "topic", "default topic" } } }; KernelArguments arguments = new() { { "now", $"{now.ToShortDateString()} {now.ToShortTimeString()}" } }; await foreach (ChatMessageContent response in agent.InvokeAsync(history, arguments)) { ... } ``` ### Contribution Checklist <!-- Before submitting this PR, please make sure: --> - [X] The code builds clean without any errors or warnings - [X] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations - [X] All unit tests pass, and I have added new tests where possible - [X] I didn't break anyone 😄
1 parent 27fa987 commit 81953f2

File tree

6 files changed

+179
-14
lines changed

6 files changed

+179
-14
lines changed

dotnet/src/Agents/Abstractions/KernelAgent.cs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
// Copyright (c) Microsoft. All rights reserved.
2+
using System.Collections.Generic;
3+
using System.Linq;
24
using System.Threading;
35
using System.Threading.Tasks;
46

@@ -62,4 +64,48 @@ public abstract class KernelAgent : Agent
6264

6365
return await this.Template.RenderAsync(kernel, arguments, cancellationToken).ConfigureAwait(false);
6466
}
67+
68+
/// <summary>
69+
/// Provide a merged instance of <see cref="KernelArguments"/> with precedence for override arguments.
70+
/// </summary>
71+
/// <param name="arguments">The override arguments</param>
72+
/// <remarks>
73+
/// This merge preserves original <see cref="PromptExecutionSettings"/> and <see cref="KernelArguments"/> parameters.
74+
/// and allows for incremental addition or replacement of specific parameters while also preserving the ability
75+
/// to override the execution settings.
76+
/// </remarks>
77+
protected KernelArguments? MergeArguments(KernelArguments? arguments)
78+
{
79+
// Avoid merge when default arguments are not set.
80+
if (this.Arguments == null)
81+
{
82+
return arguments;
83+
}
84+
85+
// Avoid merge when override arguments are not set.
86+
if (arguments == null)
87+
{
88+
return this.Arguments;
89+
}
90+
91+
// Both instances are not null, merge with precedence for override arguments.
92+
93+
// Merge execution settings with precedence for override arguments.
94+
Dictionary<string, PromptExecutionSettings>? settings =
95+
(arguments.ExecutionSettings ?? s_emptySettings)
96+
.Concat(this.Arguments.ExecutionSettings ?? s_emptySettings)
97+
.GroupBy(entry => entry.Key)
98+
.ToDictionary(entry => entry.Key, entry => entry.First().Value);
99+
100+
// Merge parameters with precedence for override arguments.
101+
Dictionary<string, object?>? parameters =
102+
arguments
103+
.Concat(this.Arguments)
104+
.GroupBy(entry => entry.Key)
105+
.ToDictionary(entry => entry.Key, entry => entry.First().Value);
106+
107+
return new KernelArguments(parameters, settings);
108+
}
109+
110+
private static readonly Dictionary<string, PromptExecutionSettings> s_emptySettings = [];
65111
}

dotnet/src/Agents/Core/ChatCompletionAgent.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public override async IAsyncEnumerable<ChatMessageContent> InvokeAsync(
5050
[EnumeratorCancellation] CancellationToken cancellationToken = default)
5151
{
5252
kernel ??= this.Kernel;
53-
arguments ??= this.Arguments;
53+
arguments = this.MergeArguments(arguments);
5454

5555
(IChatCompletionService chatCompletionService, PromptExecutionSettings? executionSettings) = GetChatCompletionService(kernel, arguments);
5656

@@ -95,7 +95,7 @@ public override async IAsyncEnumerable<StreamingChatMessageContent> InvokeStream
9595
[EnumeratorCancellation] CancellationToken cancellationToken = default)
9696
{
9797
kernel ??= this.Kernel;
98-
arguments ??= this.Arguments;
98+
arguments = this.MergeArguments(arguments);
9999

100100
(IChatCompletionService chatCompletionService, PromptExecutionSettings? executionSettings) = GetChatCompletionService(kernel, arguments);
101101

dotnet/src/Agents/OpenAI/Internal/AssistantToolResourcesFactory.cs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,20 @@ internal static class AssistantToolResourcesFactory
2626

2727
if (hasVectorStore || hasCodeInterpreterFiles)
2828
{
29-
var fileSearch = hasVectorStore
30-
? new FileSearchToolResources
31-
{
32-
VectorStoreIds = { vectorStoreId! }
33-
}
34-
: null;
35-
36-
var codeInterpreter = hasCodeInterpreterFiles
37-
? new CodeInterpreterToolResources()
38-
: null;
29+
FileSearchToolResources? fileSearch =
30+
hasVectorStore ?
31+
new()
32+
{
33+
VectorStoreIds = { vectorStoreId! }
34+
} :
35+
null;
3936

37+
CodeInterpreterToolResources? codeInterpreter =
38+
hasCodeInterpreterFiles ?
39+
new() :
40+
null;
4041
codeInterpreter?.FileIds.AddRange(codeInterpreterFileIds!);
42+
4143
toolResources = new ToolResources
4244
{
4345
FileSearch = fileSearch,

dotnet/src/Agents/OpenAI/OpenAIAssistantAgent.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ public async IAsyncEnumerable<ChatMessageContent> InvokeAsync(
341341
this.ThrowIfDeleted();
342342

343343
kernel ??= this.Kernel;
344-
arguments ??= this.Arguments;
344+
arguments = this.MergeArguments(arguments);
345345

346346
await foreach ((bool isVisible, ChatMessageContent message) in AssistantThreadActions.InvokeAsync(this, this._client, threadId, options, this.Logger, kernel, arguments, cancellationToken).ConfigureAwait(false))
347347
{
@@ -396,7 +396,7 @@ public IAsyncEnumerable<StreamingChatMessageContent> InvokeStreamingAsync(
396396
this.ThrowIfDeleted();
397397

398398
kernel ??= this.Kernel;
399-
arguments ??= this.Arguments;
399+
arguments = this.MergeArguments(arguments);
400400

401401
return AssistantThreadActions.InvokeStreamingAsync(this, this._client, threadId, messages, options, this.Logger, kernel, arguments, cancellationToken);
402402
}
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
// Copyright (c) Microsoft. All rights reserved.
2+
using System.Linq;
3+
using Microsoft.SemanticKernel;
4+
using Microsoft.SemanticKernel.Agents;
5+
using Xunit;
6+
7+
namespace SemanticKernel.Agents.UnitTests;
8+
9+
/// <summary>
10+
/// Verify behavior of <see cref="KernelAgent"/> base class.
11+
/// </summary>
12+
public class KernelAgentTests
13+
{
14+
/// <summary>
15+
/// Verify ability to merge null <see cref="KernelArguments"/>.
16+
/// </summary>
17+
[Fact]
18+
public void VerifyNullArgumentMerge()
19+
{
20+
// Arrange
21+
MockAgent agentWithNullArguments = new();
22+
// Act
23+
KernelArguments? arguments = agentWithNullArguments.MergeArguments(null);
24+
// Assert
25+
Assert.Null(arguments);
26+
27+
// Arrange
28+
KernelArguments overrideArguments = [];
29+
// Act
30+
arguments = agentWithNullArguments.MergeArguments(overrideArguments);
31+
// Assert
32+
Assert.NotNull(arguments);
33+
Assert.StrictEqual(overrideArguments, arguments);
34+
35+
// Arrange
36+
MockAgent agentWithEmptyArguments = new() { Arguments = new() };
37+
// Act
38+
arguments = agentWithEmptyArguments.MergeArguments(null);
39+
// Assert
40+
Assert.NotNull(arguments);
41+
Assert.StrictEqual(agentWithEmptyArguments.Arguments, arguments);
42+
}
43+
44+
/// <summary>
45+
/// Verify ability to merge <see cref="KernelArguments"/> parameters.
46+
/// </summary>
47+
[Fact]
48+
public void VerifyArgumentParameterMerge()
49+
{
50+
// Arrange
51+
MockAgent agentWithArguments = new() { Arguments = new() { { "a", 1 } } };
52+
KernelArguments overrideArguments = new() { { "b", 2 } };
53+
54+
// Act
55+
KernelArguments? arguments = agentWithArguments.MergeArguments(overrideArguments);
56+
57+
// Assert
58+
Assert.NotNull(arguments);
59+
Assert.Equal(2, arguments.Count);
60+
Assert.Equal(1, arguments["a"]);
61+
Assert.Equal(2, arguments["b"]);
62+
63+
// Arrange
64+
overrideArguments["a"] = 11;
65+
overrideArguments["c"] = 3;
66+
67+
// Act
68+
arguments = agentWithArguments.MergeArguments(overrideArguments);
69+
70+
// Assert
71+
Assert.NotNull(arguments);
72+
Assert.Equal(3, arguments.Count);
73+
Assert.Equal(11, arguments["a"]);
74+
Assert.Equal(2, arguments["b"]);
75+
Assert.Equal(3, arguments["c"]);
76+
}
77+
78+
/// <summary>
79+
/// Verify ability to merge <see cref="KernelArguments.ExecutionSettings"/>.
80+
/// </summary>
81+
[Fact]
82+
public void VerifyArgumentSettingsMerge()
83+
{
84+
// Arrange
85+
FunctionChoiceBehavior autoInvoke = FunctionChoiceBehavior.Auto();
86+
MockAgent agentWithSettings = new() { Arguments = new(new PromptExecutionSettings() { FunctionChoiceBehavior = autoInvoke }) };
87+
KernelArguments overrideArgumentsNoSettings = new();
88+
89+
// Act
90+
KernelArguments? arguments = agentWithSettings.MergeArguments(overrideArgumentsNoSettings);
91+
92+
// Assert
93+
Assert.NotNull(arguments);
94+
Assert.NotNull(arguments.ExecutionSettings);
95+
Assert.Single(arguments.ExecutionSettings);
96+
Assert.StrictEqual(autoInvoke, arguments.ExecutionSettings.First().Value.FunctionChoiceBehavior);
97+
98+
// Arrange
99+
FunctionChoiceBehavior noInvoke = FunctionChoiceBehavior.None();
100+
KernelArguments overrideArgumentsWithSettings = new(new PromptExecutionSettings() { FunctionChoiceBehavior = noInvoke });
101+
102+
// Act
103+
arguments = agentWithSettings.MergeArguments(overrideArgumentsWithSettings);
104+
105+
// Assert
106+
Assert.NotNull(arguments);
107+
Assert.NotNull(arguments.ExecutionSettings);
108+
Assert.Single(arguments.ExecutionSettings);
109+
Assert.StrictEqual(noInvoke, arguments.ExecutionSettings.First().Value.FunctionChoiceBehavior);
110+
}
111+
}

dotnet/src/Agents/UnitTests/MockAgent.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,10 @@ public override IAsyncEnumerable<StreamingChatMessageContent> InvokeStreamingAsy
3737
this.InvokeCount++;
3838
return this.Response.Select(m => new StreamingChatMessageContent(m.Role, m.Content)).ToAsyncEnumerable();
3939
}
40+
41+
// Expose protected method for testing
42+
public new KernelArguments? MergeArguments(KernelArguments? arguments)
43+
{
44+
return base.MergeArguments(arguments);
45+
}
4046
}

0 commit comments

Comments
 (0)