Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify orchestration API #848

Merged
merged 1 commit into from
May 8, 2023
Merged

Simplify orchestration API #848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions dotnet/SK-dotnet.sln.DotSettings
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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