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.

dotnet/SK-dotnet.sln.DotSettings

@@ -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>

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

@@ -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);
@@ -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);
 
@@ -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);
@@ -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);
 

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

@@ -90,20 +90,10 @@ private void CreateKernelAndFunctionCreateMocks(List<(string name, string skillN
             }
             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);
             }
         }
 

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

@@ -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);

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

@@ -54,9 +54,14 @@ public static async Task<IOrderedEnumerable<FunctionView>> GetAvailableFunctions
         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)

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

@@ -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";
 
@@ -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);
 

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

@@ -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

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

@@ -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);
 }

dotnet/src/SemanticKernel.UnitTests/KernelTests.cs

@@ -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);
     }
 
@@ -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)
             {

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

@@ -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);
 

dotnet/src/SemanticKernel.UnitTests/SkillDefinition/SKContextTests.cs

@@ -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

dotnet/src/SemanticKernel/Kernel.cs

@@ -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;
     }
@@ -112,7 +112,7 @@ public IDictionary<string, ISKFunction> ImportSkill(object skillInstance, string
         foreach (KeyValuePair<string, ISKFunction> f in skill)
         {
             f.Value.SetDefaultSkillCollection(this.Skills);
-            this._skillCollection.AddNativeFunction(f.Value);
+            this._skillCollection.AddFunction(f.Value);
         }
 
         return skill;
@@ -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;
     }