Skip to content

Commit

Permalink
Simplify orchestration API
Browse files Browse the repository at this point in the history
  • Loading branch information
@dluc @shawncal
dluc authored and shawncal committed May 8, 2023
1 parent 67a4854 commit 1ec6326
Show file tree
Hide file tree
Showing 17 changed files with 76 additions and 298 deletions.
1 change: 1 addition & 0 deletions dotnet/SK-dotnet.sln.DotSettings
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ public void It$SOMENAME$()
<s:Boolean x:Key="/Default/UserDictionary/Words/=greaterthan/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Joinable/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=keyvault/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Kitto/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=lessthan/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=mergeresults/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=myfile/@EntryIndexedValue">True</s:Boolean>
Expand Down
16 changes: 4 additions & 12 deletions ...rc/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SKContextExtensionsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ public async Task CanCallGetAvailableFunctionsWithFunctionsAsync()
var cancellationToken = default(CancellationToken);

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

skills.Setup(x => x.TryGetFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
skills.Setup(x => x.TryGetSemanticFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
skills.Setup(x => x.TryGetNativeFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
skills.Setup(x => x.GetSemanticFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockSemanticFunction.Object);
skills.Setup(x => x.GetNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockNativeFunction.Object);
skills.Setup(x => x.GetFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(functionMock.Object);
skills.Setup(x => x.GetFunctionsView(It.IsAny<bool>(), It.IsAny<bool>())).Returns(functionsView);
skills.SetupGet(x => x.ReadOnlySkillCollection).Returns(skills.Object);

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

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

skills.Setup(x => x.TryGetFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
skills.Setup(x => x.TryGetSemanticFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
skills.Setup(x => x.TryGetNativeFunction(It.IsAny<string>(), It.IsAny<string>(), out It.Ref<ISKFunction?>.IsAny)).Returns(true);
skills.Setup(x => x.GetSemanticFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockSemanticFunction.Object);
skills.Setup(x => x.GetNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockNativeFunction.Object);
skills.Setup(x => x.GetFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(functionMock.Object);
skills.Setup(x => x.GetFunctionsView(It.IsAny<bool>(), It.IsAny<bool>())).Returns(functionsView);
skills.SetupGet(x => x.ReadOnlySkillCollection).Returns(skills.Object);

Expand Down
18 changes: 4 additions & 14 deletions ...c/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SequentialPlanParserTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,10 @@ private static Mock<ISKFunction> CreateMockFunction(FunctionView functionView, s
}
else
{
if (isSemantic)
{
skills.Setup(x => x.GetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
.Returns(mockFunction.Object);
ISKFunction? outFunc = mockFunction.Object;
skills.Setup(x => x.TryGetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
}
else
{
skills.Setup(x => x.GetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
.Returns(mockFunction.Object);
ISKFunction? outFunc = mockFunction.Object;
skills.Setup(x => x.TryGetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
}
skills.Setup(x => x.GetFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
.Returns(mockFunction.Object);
ISKFunction? outFunc = mockFunction.Object;
skills.Setup(x => x.TryGetFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
}
}

Expand Down
18 changes: 4 additions & 14 deletions .../src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SequentialPlannerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,20 +53,10 @@ public async Task ItCanCreatePlanAsync(string goal)
return Task.FromResult(context);
});

if (isSemantic)
{
skills.Setup(x => x.GetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
.Returns(mockFunction.Object);
ISKFunction? outFunc = mockFunction.Object;
skills.Setup(x => x.TryGetSemanticFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
}
else
{
skills.Setup(x => x.GetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
.Returns(mockFunction.Object);
ISKFunction? outFunc = mockFunction.Object;
skills.Setup(x => x.TryGetNativeFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
}
skills.Setup(x => x.GetFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name)))
.Returns(mockFunction.Object);
ISKFunction? outFunc = mockFunction.Object;
skills.Setup(x => x.TryGetFunction(It.Is<string>(s => s == skillName), It.Is<string>(s => s == name), out outFunc)).Returns(true);
}

skills.Setup(x => x.GetFunctionsView(It.IsAny<bool>(), It.IsAny<bool>())).Returns(functionsView);
Expand Down
9 changes: 7 additions & 2 deletions dotnet/src/Extensions/Planning.SequentialPlanner/SKContextSequentialPlannerExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,14 @@ public static class SKContextSequentialPlannerExtensions
var excludedFunctions = config.ExcludedFunctions ?? new();
var includedFunctions = config.IncludedFunctions ?? new();

context.ThrowIfSkillCollectionNotSet();
if (context.Skills == null)
{
throw new KernelException(
KernelException.ErrorCodes.SkillCollectionNotSet,
"Skill collection not found in the context");
}

var functionsView = context.Skills!.GetFunctionsView();
var functionsView = context.Skills.GetFunctionsView();

var availableFunctions = functionsView.SemanticFunctions
.Concat(functionsView.NativeFunctions)
Expand Down
3 changes: 2 additions & 1 deletion dotnet/src/Extensions/Planning.SequentialPlanner/SequentialPlanParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ internal static class SequentialPlanParser
{
/// <summary>
/// The tag name used in the plan xml for the user's goal/ask.
/// TODO: never used
/// </summary>
internal const string GoalTag = "goal";

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

if (!string.IsNullOrEmpty(functionName) && context.IsFunctionRegistered(skillName, functionName, out var skillFunction))
if (!string.IsNullOrEmpty(functionName) && context.Skills!.TryGetFunction(skillName, functionName, out var skillFunction))
{
var planStep = new Plan(skillFunction);

Expand Down
76 changes: 4 additions & 72 deletions dotnet/src/SemanticKernel.Abstractions/SkillDefinition/IReadOnlySkillCollection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,89 +28,21 @@ public interface IReadOnlySkillCollection
ISKFunction GetFunction(string skillName, string functionName);

/// <summary>
/// Gets the function stored in the collection.
/// Check if a function is available in the current context, and return it.
/// </summary>
/// <param name="functionName">The name of the function to retrieve.</param>
/// <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>
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
bool TryGetFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);

/// <summary>
/// Gets the function stored in the collection.
/// </summary>
/// <param name="skillName">The name of the skill with which the function is associated.</param>
/// <param name="functionName">The name of the function to retrieve.</param>
/// <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>
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
bool TryGetFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);

/// <summary>
/// Gets the semantic function stored in the collection.
/// </summary>
/// <param name="functionName">The name of the function to retrieve.</param>
/// <returns>The function retrieved from the collection.</returns>
/// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
ISKFunction GetSemanticFunction(string functionName);

/// <summary>
/// Gets the semantic function stored in the collection.
/// </summary>
/// <param name="skillName">The name of the skill with which the function is associated.</param>
/// <param name="functionName">The name of the function to retrieve.</param>
/// <returns>The function retrieved from the collection.</returns>
/// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
ISKFunction GetSemanticFunction(string skillName, string functionName);

/// <summary>
/// Gets the semantic function stored in the collection.
/// </summary>
/// <param name="functionName">The name of the function to retrieve.</param>
/// <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>
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
bool TryGetSemanticFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);

/// <summary>
/// Gets the semantic function stored in the collection.
/// Check if a function is available in the current context, and return it.
/// </summary>
/// <param name="skillName">The name of the skill with which the function is associated.</param>
/// <param name="functionName">The name of the function to retrieve.</param>
/// <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>
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
bool TryGetSemanticFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);

/// <summary>
/// Gets the native function stored in the collection.
/// </summary>
/// <param name="functionName">The name of the function to retrieve.</param>
/// <returns>The function retrieved from the collection.</returns>
/// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
ISKFunction GetNativeFunction(string functionName);

/// <summary>
/// Gets the native function stored in the collection.
/// </summary>
/// <param name="skillName">The name of the skill with which the function is associated.</param>
/// <param name="functionName">The name of the function to retrieve.</param>
/// <returns>The function retrieved from the collection.</returns>
/// <exception cref="KernelException">The specified function could not be found in the collection.</exception>
ISKFunction GetNativeFunction(string skillName, string functionName);

/// <summary>
/// Gets the native function stored in the collection.
/// </summary>
/// <param name="functionName">The name of the function to retrieve.</param>
/// <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>
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
bool TryGetNativeFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);

/// <summary>
/// Gets the native function stored in the collection.
/// </summary>
/// <param name="skillName">The name of the skill with which the function is associated.</param>
/// <param name="functionName">The name of the function to retrieve.</param>
/// <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>
/// <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>
/// <returns><see langword="true"/> if the function was found; otherwise, <see langword="false"/>.</returns>
bool TryGetNativeFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance);
bool TryGetFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? availableFunction);

/// <summary>
/// Get all registered functions details, minus the delegates
Expand Down
11 changes: 2 additions & 9 deletions dotnet/src/SemanticKernel.Abstractions/SkillDefinition/ISkillCollection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,9 @@ public interface ISkillCollection : IReadOnlySkillCollection
IReadOnlySkillCollection ReadOnlySkillCollection { get; }

/// <summary>
/// Add a semantic function to the collection
/// Add a function to the collection
/// </summary>
/// <param name="functionInstance">Function delegate</param>
/// <returns>Self instance</returns>
ISkillCollection AddSemanticFunction(ISKFunction functionInstance);

/// <summary>
/// Add a native function to the collection
/// </summary>
/// <param name="functionInstance">Wrapped function delegate</param>
/// <returns>Self instance</returns>
ISkillCollection AddNativeFunction(ISKFunction functionInstance);
ISkillCollection AddFunction(ISKFunction functionInstance);
}
9 changes: 6 additions & 3 deletions dotnet/src/SemanticKernel.UnitTests/KernelTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public void ItAllowsToImportSkillsInTheGlobalNamespace()

// Assert
Assert.Equal(3, skill.Count);
Assert.True(kernel.Skills.TryGetNativeFunction("GetAnyValue", out ISKFunction? functionInstance));
Assert.True(kernel.Skills.TryGetFunction("GetAnyValue", out ISKFunction? functionInstance));
Assert.NotNull(functionInstance);
}

Expand Down Expand Up @@ -177,9 +177,12 @@ public async Task<SKContext> ReadSkillCollectionAsync(SKContext context)
{
await Task.Delay(0);

context.ThrowIfSkillCollectionNotSet();
if (context.Skills == null)
{
Assert.Fail("Skills collection is missing");
}

FunctionsView procMem = context.Skills!.GetFunctionsView();
FunctionsView procMem = context.Skills.GetFunctionsView();

foreach (KeyValuePair<string, List<FunctionView>> list in procMem.SemanticFunctions)
{
Expand Down
9 changes: 5 additions & 4 deletions dotnet/src/SemanticKernel.UnitTests/Planning/PlanSerializationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,15 +435,16 @@ public async Task CanStepAndSerializeAndDeserializePlanWithStepsAndContextAsync(
.Returns(() => Task.FromResult(returnContext));
mockFunction.Setup(x => x.Describe()).Returns(new FunctionView()
{
Parameters = new List<ParameterView>()
Parameters = new List<ParameterView>
{
new ParameterView() { Name = "variables" }
new() { Name = "variables" }
}
});

ISKFunction? outFunc = mockFunction.Object;
skills.Setup(x => x.TryGetNativeFunction(It.IsAny<string>(), It.IsAny<string>(), out outFunc)).Returns(true);
skills.Setup(x => x.GetNativeFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockFunction.Object);
skills.Setup(x => x.TryGetFunction(It.IsAny<string>(), out outFunc)).Returns(true);
skills.Setup(x => x.TryGetFunction(It.IsAny<string>(), It.IsAny<string>(), out outFunc)).Returns(true);
skills.Setup(x => x.GetFunction(It.IsAny<string>(), It.IsAny<string>())).Returns(mockFunction.Object);

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

Expand Down
4 changes: 2 additions & 2 deletions dotnet/src/SemanticKernel.UnitTests/SkillDefinition/SKContextTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ public async Task ItHasHelpersForSkillCollectionAsync()
{
// Arrange
IDictionary<string, ISKFunction> skill = KernelBuilder.Create().ImportSkill(new Parrot(), "test");
this._skills.Setup(x => x.GetNativeFunction("func")).Returns(skill["say"]);
this._skills.Setup(x => x.GetFunction("func")).Returns(skill["say"]);
var target = new SKContext(new ContextVariables(), NullMemory.Instance, this._skills.Object, this._log.Object);
Assert.NotNull(target.Skills);

// Act
var say = target.Skills.GetNativeFunction("func");
var say = target.Skills.GetFunction("func");
SKContext result = await say.InvokeAsync("ciao");

// Assert
Expand Down
6 changes: 3 additions & 3 deletions dotnet/src/SemanticKernel/Kernel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public ISKFunction RegisterSemanticFunction(string skillName, string functionNam
Verify.ValidFunctionName(functionName);

ISKFunction function = this.CreateSemanticFunction(skillName, functionName, functionConfig);
this._skillCollection.AddSemanticFunction(function);
this._skillCollection.AddFunction(function);

return function;
}
Expand All @@ -112,7 +112,7 @@ public ISKFunction RegisterSemanticFunction(string skillName, string functionNam
foreach (KeyValuePair<string, ISKFunction> f in skill)
{
f.Value.SetDefaultSkillCollection(this.Skills);
this._skillCollection.AddNativeFunction(f.Value);
this._skillCollection.AddFunction(f.Value);
}

return skill;
Expand All @@ -126,7 +126,7 @@ public ISKFunction RegisterCustomFunction(string skillName, ISKFunction customFu
Verify.NotNull(customFunction);

customFunction.SetDefaultSkillCollection(this.Skills);
this._skillCollection.AddSemanticFunction(customFunction);
this._skillCollection.AddFunction(customFunction);

return customFunction;
}
Expand Down

0 comments on commit 1ec6326

Please sign in to comment.