Skip to content

Commit b88611f

Browse files
authored
Simplify orchestration API (#848)
### Motivation and Context Complete some early work from Feb, simplifying the orchestration API, removing redundant code in the function registry. ### Description Merge registry methods used for native and semantic functions, given that the underlying implementation was refactored and the distinction is not required anymore.
1 parent 67a4854 commit b88611f

File tree

17 files changed

+76
-298
lines changed

17 files changed

+76
-298
lines changed

dotnet/SK-dotnet.sln.DotSettings

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ public void It$SOMENAME$()
193193
<s:Boolean x:Key="/Default/UserDictionary/Words/=greaterthan/@EntryIndexedValue">True</s:Boolean>
194194
<s:Boolean x:Key="/Default/UserDictionary/Words/=Joinable/@EntryIndexedValue">True</s:Boolean>
195195
<s:Boolean x:Key="/Default/UserDictionary/Words/=keyvault/@EntryIndexedValue">True</s:Boolean>
196+
<s:Boolean x:Key="/Default/UserDictionary/Words/=Kitto/@EntryIndexedValue">True</s:Boolean>
196197
<s:Boolean x:Key="/Default/UserDictionary/Words/=lessthan/@EntryIndexedValue">True</s:Boolean>
197198
<s:Boolean x:Key="/Default/UserDictionary/Words/=mergeresults/@EntryIndexedValue">True</s:Boolean>
198199
<s:Boolean x:Key="/Default/UserDictionary/Words/=myfile/@EntryIndexedValue">True</s:Boolean>

dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SKContextExtensionsTests.cs

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ public async Task CanCallGetAvailableFunctionsWithFunctionsAsync()
6666
var cancellationToken = default(CancellationToken);
6767

6868
// Arrange FunctionView
69-
var mockSemanticFunction = new Mock<ISKFunction>();
70-
var mockNativeFunction = new Mock<ISKFunction>();
69+
var functionMock = new Mock<ISKFunction>();
7170
var functionsView = new FunctionsView();
7271
var functionView = new FunctionView("functionName", "skillName", "description", new List<ParameterView>(), true, false);
7372
var nativeFunctionView = new FunctionView("nativeFunctionName", "skillName", "description", new List<ParameterView>(), false, false);
@@ -94,10 +93,7 @@ public async Task CanCallGetAvailableFunctionsWithFunctionsAsync()
9493
.Returns(asyncEnumerable);
9594

9695
skills.Setup(x => x.TryGetFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
97-
skills.Setup(x => x.TryGetSemanticFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
98-
skills.Setup(x => x.TryGetNativeFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
99-
skills.Setup(x => x.GetSemanticFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockSemanticFunction.Object);
100-
skills.Setup(x => x.GetNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockNativeFunction.Object);
96+
skills.Setup(x => x.GetFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(functionMock.Object);
10197
skills.Setup(x => x.GetFunctionsView(It.IsAny<bool>(), It.IsAny<bool>())).Returns(functionsView);
10298
skills.SetupGet(x => x.ReadOnlySkillCollection).Returns(skills.Object);
10399

@@ -136,8 +132,7 @@ public async Task CanCallGetAvailableFunctionsWithFunctionsWithRelevancyAsync()
136132
var cancellationToken = default(CancellationToken);
137133

138134
// Arrange FunctionView
139-
var mockSemanticFunction = new Mock<ISKFunction>();
140-
var mockNativeFunction = new Mock<ISKFunction>();
135+
var functionMock = new Mock<ISKFunction>();
141136
var functionsView = new FunctionsView();
142137
var functionView = new FunctionView("functionName", "skillName", "description", new List<ParameterView>(), true, false);
143138
var nativeFunctionView = new FunctionView("nativeFunctionName", "skillName", "description", new List<ParameterView>(), false, false);
@@ -164,10 +159,7 @@ public async Task CanCallGetAvailableFunctionsWithFunctionsWithRelevancyAsync()
164159
.Returns(asyncEnumerable);
165160

166161
skills.Setup(x => x.TryGetFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
167-
skills.Setup(x => x.TryGetSemanticFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
168-
skills.Setup(x => x.TryGetNativeFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
169-
skills.Setup(x => x.GetSemanticFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockSemanticFunction.Object);
170-
skills.Setup(x => x.GetNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockNativeFunction.Object);
162+
skills.Setup(x => x.GetFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(functionMock.Object);
171163
skills.Setup(x => x.GetFunctionsView(It.IsAny<bool>(), It.IsAny<bool>())).Returns(functionsView);
172164
skills.SetupGet(x => x.ReadOnlySkillCollection).Returns(skills.Object);
173165

dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SequentialPlanParserTests.cs

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -90,20 +90,10 @@ private void CreateKernelAndFunctionCreateMocks(List<(string name, string skillN
9090
}
9191
else
9292
{
93-
if (isSemantic)
94-
{
95-
skills.Setup(x => x.GetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
96-
.Returns(mockFunction.Object);
97-
ISKFunction? outFunc = mockFunction.Object;
98-
skills.Setup(x => x.TryGetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
99-
}
100-
else
101-
{
102-
skills.Setup(x => x.GetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
103-
.Returns(mockFunction.Object);
104-
ISKFunction? outFunc = mockFunction.Object;
105-
skills.Setup(x => x.TryGetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
106-
}
93+
skills.Setup(x => x.GetFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
94+
.Returns(mockFunction.Object);
95+
ISKFunction? outFunc = mockFunction.Object;
96+
skills.Setup(x => x.TryGetFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
10797
}
10898
}
10999

dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SequentialPlannerTests.cs

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -53,20 +53,10 @@ public async Task ItCanCreatePlanAsync(string goal)
5353
return Task.FromResult(context);
5454
});
5555

56-
if (isSemantic)
57-
{
58-
skills.Setup(x => x.GetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
59-
.Returns(mockFunction.Object);
60-
ISKFunction? outFunc = mockFunction.Object;
61-
skills.Setup(x => x.TryGetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
62-
}
63-
else
64-
{
65-
skills.Setup(x => x.GetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
66-
.Returns(mockFunction.Object);
67-
ISKFunction? outFunc = mockFunction.Object;
68-
skills.Setup(x => x.TryGetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
69-
}
56+
skills.Setup(x => x.GetFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
57+
.Returns(mockFunction.Object);
58+
ISKFunction? outFunc = mockFunction.Object;
59+
skills.Setup(x => x.TryGetFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
7060
}
7161

7262
skills.Setup(x => x.GetFunctionsView(It.IsAny<bool>(), It.IsAny<bool>())).Returns(functionsView);

dotnet/src/Extensions/Planning.SequentialPlanner/SKContextSequentialPlannerExtensions.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,14 @@ public static async Task<IOrderedEnumerable<FunctionView>> GetAvailableFunctions
5454
var excludedFunctions = config.ExcludedFunctions ?? new();
5555
var includedFunctions = config.IncludedFunctions ?? new();
5656

57-
context.ThrowIfSkillCollectionNotSet();
57+
if (context.Skills == null)
58+
{
59+
throw new KernelException(
60+
KernelException.ErrorCodes.SkillCollectionNotSet,
61+
"Skill collection not found in the context");
62+
}
5863

59-
var functionsView = context.Skills!.GetFunctionsView();
64+
var functionsView = context.Skills.GetFunctionsView();
6065

6166
var availableFunctions = functionsView.SemanticFunctions
6267
.Concat(functionsView.NativeFunctions)

dotnet/src/Extensions/Planning.SequentialPlanner/SequentialPlanParser.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ internal static class SequentialPlanParser
1515
{
1616
/// <summary>
1717
/// The tag name used in the plan xml for the user's goal/ask.
18+
/// TODO: never used
1819
/// </summary>
1920
internal const string GoalTag = "goal";
2021

@@ -87,7 +88,7 @@ internal static Plan ToPlanFromXml(this string xmlString, string goal, SKContext
8788
var skillFunctionName = childNode.Name.Split(s_functionTagArray, StringSplitOptions.None)?[1] ?? string.Empty;
8889
GetSkillFunctionNames(skillFunctionName, out var skillName, out var functionName);
8990

90-
if (!string.IsNullOrEmpty(functionName) && context.IsFunctionRegistered(skillName, functionName, out var skillFunction))
91+
if (!string.IsNullOrEmpty(functionName) && context.Skills!.TryGetFunction(skillName, functionName, out var skillFunction))
9192
{
9293
var planStep = new Plan(skillFunction);
9394

dotnet/src/SemanticKernel.Abstractions/SkillDefinition/IReadOnlySkillCollection.cs

Lines changed: 4 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -28,89 +28,21 @@ public interface IReadOnlySkillCollection
2828
ISKFunction GetFunction(string skillName, string functionName);
2929

3030
/// <summary>
31-
/// Gets the function stored in the collection.
31+
/// Check if a function is available in the current context, and return it.
3232
/// </summary>
3333
/// <param name="functionName">The name of the function to retrieve.</param>
3434
/// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
3535
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
3636
bool TryGetFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
3737

3838
/// <summary>
39-
/// Gets the function stored in the collection.
40-
/// </summary>
41-
/// <param name="skillName">The name of the skill with which the function is associated.</param>
42-
/// <param name="functionName">The name of the function to retrieve.</param>
43-
/// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
44-
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
45-
bool TryGetFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
46-
47-
/// <summary>
48-
/// Gets the semantic function stored in the collection.
49-
/// </summary>
50-
/// <param name="functionName">The name of the function to retrieve.</param>
51-
/// <returns>The function retrieved from the collection.</returns>
52-
/// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
53-
ISKFunction GetSemanticFunction(string functionName);
54-
55-
/// <summary>
56-
/// Gets the semantic function stored in the collection.
57-
/// </summary>
58-
/// <param name="skillName">The name of the skill with which the function is associated.</param>
59-
/// <param name="functionName">The name of the function to retrieve.</param>
60-
/// <returns>The function retrieved from the collection.</returns>
61-
/// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
62-
ISKFunction GetSemanticFunction(string skillName, string functionName);
63-
64-
/// <summary>
65-
/// Gets the semantic function stored in the collection.
66-
/// </summary>
67-
/// <param name="functionName">The name of the function to retrieve.</param>
68-
/// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
69-
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
70-
bool TryGetSemanticFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
71-
72-
/// <summary>
73-
/// Gets the semantic function stored in the collection.
39+
/// Check if a function is available in the current context, and return it.
7440
/// </summary>
7541
/// <param name="skillName">The name of the skill with which the function is associated.</param>
7642
/// <param name="functionName">The name of the function to retrieve.</param>
77-
/// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
78-
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
79-
bool TryGetSemanticFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
80-
81-
/// <summary>
82-
/// Gets the native function stored in the collection.
83-
/// </summary>
84-
/// <param name="functionName">The name of the function to retrieve.</param>
85-
/// <returns>The function retrieved from the collection.</returns>
86-
/// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
87-
ISKFunction GetNativeFunction(string functionName);
88-
89-
/// <summary>
90-
/// Gets the native function stored in the collection.
91-
/// </summary>
92-
/// <param name="skillName">The name of the skill with which the function is associated.</param>
93-
/// <param name="functionName">The name of the function to retrieve.</param>
94-
/// <returns>The function retrieved from the collection.</returns>
95-
/// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
96-
ISKFunction GetNativeFunction(string skillName, string functionName);
97-
98-
/// <summary>
99-
/// Gets the native function stored in the collection.
100-
/// </summary>
101-
/// <param name="functionName">The name of the function to retrieve.</param>
102-
/// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
103-
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
104-
bool TryGetNativeFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
105-
106-
/// <summary>
107-
/// Gets the native function stored in the collection.
108-
/// </summary>
109-
/// <param name="skillName">The name of the skill with which the function is associated.</param>
110-
/// <param name="functionName">The name of the function to retrieve.</param>
111-
/// <param name="functionInstance">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
43+
/// <param name="availableFunction">When this method returns, the function that was retrieved if one with the specified name was found; otherwise, <see langword="null"/>.</param>
11244
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
113-
bool TryGetNativeFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
45+
bool TryGetFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? availableFunction);
11446

11547
/// <summary>
11648
/// Get all registered functions details, minus the delegates

dotnet/src/SemanticKernel.Abstractions/SkillDefinition/ISkillCollection.cs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,9 @@ public interface ISkillCollection : IReadOnlySkillCollection
1616
IReadOnlySkillCollection ReadOnlySkillCollection { get; }
1717

1818
/// <summary>
19-
/// Add a semantic function to the collection
19+
/// Add a function to the collection
2020
/// </summary>
2121
/// <param name="functionInstance">Function delegate</param>
2222
/// <returns>Self instance</returns>
23-
ISkillCollection AddSemanticFunction(ISKFunction functionInstance);
24-
25-
/// <summary>
26-
/// Add a native function to the collection
27-
/// </summary>
28-
/// <param name="functionInstance">Wrapped function delegate</param>
29-
/// <returns>Self instance</returns>
30-
ISkillCollection AddNativeFunction(ISKFunction functionInstance);
23+
ISkillCollection AddFunction(ISKFunction functionInstance);
3124
}

dotnet/src/SemanticKernel.UnitTests/KernelTests.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public void ItAllowsToImportSkillsInTheGlobalNamespace()
132132

133133
// Assert
134134
Assert.Equal(3, skill.Count);
135-
Assert.True(kernel.Skills.TryGetNativeFunction("GetAnyValue", out ISKFunction? functionInstance));
135+
Assert.True(kernel.Skills.TryGetFunction("GetAnyValue", out ISKFunction? functionInstance));
136136
Assert.NotNull(functionInstance);
137137
}
138138

@@ -177,9 +177,12 @@ public async Task<SKContext> ReadSkillCollectionAsync(SKContext context)
177177
{
178178
await Task.Delay(0);
179179

180-
context.ThrowIfSkillCollectionNotSet();
180+
if (context.Skills == null)
181+
{
182+
Assert.Fail("Skills collection is missing");
183+
}
181184

182-
FunctionsView procMem = context.Skills!.GetFunctionsView();
185+
FunctionsView procMem = context.Skills.GetFunctionsView();
183186

184187
foreach (KeyValuePair<string, List<FunctionView>> list in procMem.SemanticFunctions)
185188
{

dotnet/src/SemanticKernel.UnitTests/Planning/PlanSerializationTests.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -435,15 +435,16 @@ public async Task CanStepAndSerializeAndDeserializePlanWithStepsAndContextAsync(
435435
.Returns(() => Task.FromResult(returnContext));
436436
mockFunction.Setup(x => x.Describe()).Returns(new FunctionView()
437437
{
438-
Parameters = new List<ParameterView>()
438+
Parameters = new List<ParameterView>
439439
{
440-
new ParameterView() { Name = "variables" }
440+
new() { Name = "variables" }
441441
}
442442
});
443443

444444
ISKFunction? outFunc = mockFunction.Object;
445-
skills.Setup(x => x.TryGetNativeFunction(It.IsAny<string>(), It.IsAny<string>(), out outFunc)).Returns(true);
446-
skills.Setup(x => x.GetNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockFunction.Object);
445+
skills.Setup(x => x.TryGetFunction(It.IsAny<string>(), out outFunc)).Returns(true);
446+
skills.Setup(x => x.TryGetFunction(It.IsAny<string>(), It.IsAny<string>(), out outFunc)).Returns(true);
447+
skills.Setup(x => x.GetFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockFunction.Object);
447448

448449
plan.AddSteps(mockFunction.Object, mockFunction.Object);
449450

0 commit comments

Comments
 (0)