From b88611fc88e09147e3b89bb2f903147b35b478a0 Mon Sep 17 00:00:00 2001 From: Devis Lucato Date: Mon, 8 May 2023 16:52:13 -0700 Subject: [PATCH] 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 | 1 + .../SKContextExtensionsTests.cs | 16 +--- .../SequentialPlanParserTests.cs | 18 +--- .../SequentialPlannerTests.cs | 18 +--- .../SKContextSequentialPlannerExtensions.cs | 9 +- .../SequentialPlanParser.cs | 3 +- .../IReadOnlySkillCollection.cs | 76 +-------------- .../SkillDefinition/ISkillCollection.cs | 11 +-- .../SemanticKernel.UnitTests/KernelTests.cs | 9 +- .../Planning/PlanSerializationTests.cs | 9 +- .../SkillDefinition/SKContextTests.cs | 4 +- dotnet/src/SemanticKernel/Kernel.cs | 6 +- .../Orchestration/SKContextExtensions.cs | 40 -------- dotnet/src/SemanticKernel/Planning/Plan.cs | 16 +++- .../ReadOnlySkillCollection.cs | 36 +------ .../SkillDefinition/SkillCollection.cs | 94 +++---------------- .../TemplateEngine/Blocks/CodeBlock.cs | 8 +- 17 files changed, 76 insertions(+), 298 deletions(-) delete mode 100644 dotnet/src/SemanticKernel/Orchestration/SKContextExtensions.cs diff --git a/dotnet/SK-dotnet.sln.DotSettings b/dotnet/SK-dotnet.sln.DotSettings index cf0ebe4e28de..11f3c7637560 100644 --- a/dotnet/SK-dotnet.sln.DotSettings +++ b/dotnet/SK-dotnet.sln.DotSettings @@ -193,6 +193,7 @@ public void It$SOMENAME$() True True True + True True True True diff --git a/dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SKContextExtensionsTests.cs b/dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SKContextExtensionsTests.cs index 8649699a7a66..adada01a7519 100644 --- a/dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SKContextExtensionsTests.cs +++ b/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(); - var mockNativeFunction = new Mock(); + var functionMock = new Mock(); var functionsView = new FunctionsView(); var functionView = new FunctionView("functionName", "skillName", "description", new List(), true, false); var nativeFunctionView = new FunctionView("nativeFunctionName", "skillName", "description", new List(), false, false); @@ -94,10 +93,7 @@ public async Task CanCallGetAvailableFunctionsWithFunctionsAsync() .Returns(asyncEnumerable); skills.Setup(x => x.TryGetFunction(It.IsAny(), It.IsAny(), out It.Ref.IsAny)).Returns(true); - skills.Setup(x => x.TryGetSemanticFunction(It.IsAny(), It.IsAny(), out It.Ref.IsAny)).Returns(true); - skills.Setup(x => x.TryGetNativeFunction(It.IsAny(), It.IsAny(), out It.Ref.IsAny)).Returns(true); - skills.Setup(x => x.GetSemanticFunction(It.IsAny(), It.IsAny())).Returns(mockSemanticFunction.Object); - skills.Setup(x => x.GetNativeFunction(It.IsAny(), It.IsAny())).Returns(mockNativeFunction.Object); + skills.Setup(x => x.GetFunction(It.IsAny(), It.IsAny())).Returns(functionMock.Object); skills.Setup(x => x.GetFunctionsView(It.IsAny(), It.IsAny())).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(); - var mockNativeFunction = new Mock(); + var functionMock = new Mock(); var functionsView = new FunctionsView(); var functionView = new FunctionView("functionName", "skillName", "description", new List(), true, false); var nativeFunctionView = new FunctionView("nativeFunctionName", "skillName", "description", new List(), false, false); @@ -164,10 +159,7 @@ public async Task CanCallGetAvailableFunctionsWithFunctionsWithRelevancyAsync() .Returns(asyncEnumerable); skills.Setup(x => x.TryGetFunction(It.IsAny(), It.IsAny(), out It.Ref.IsAny)).Returns(true); - skills.Setup(x => x.TryGetSemanticFunction(It.IsAny(), It.IsAny(), out It.Ref.IsAny)).Returns(true); - skills.Setup(x => x.TryGetNativeFunction(It.IsAny(), It.IsAny(), out It.Ref.IsAny)).Returns(true); - skills.Setup(x => x.GetSemanticFunction(It.IsAny(), It.IsAny())).Returns(mockSemanticFunction.Object); - skills.Setup(x => x.GetNativeFunction(It.IsAny(), It.IsAny())).Returns(mockNativeFunction.Object); + skills.Setup(x => x.GetFunction(It.IsAny(), It.IsAny())).Returns(functionMock.Object); skills.Setup(x => x.GetFunctionsView(It.IsAny(), It.IsAny())).Returns(functionsView); skills.SetupGet(x => x.ReadOnlySkillCollection).Returns(skills.Object); diff --git a/dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SequentialPlanParserTests.cs b/dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SequentialPlanParserTests.cs index 437cedd60208..f3b9fd67988c 100644 --- a/dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SequentialPlanParserTests.cs +++ b/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(s => s == skillName), It.Is(s => s == name))) - .Returns(mockFunction.Object); - ISKFunction? outFunc = mockFunction.Object; - skills.Setup(x => x.TryGetSemanticFunction(It.Is(s => s == skillName), It.Is(s => s == name), out outFunc)).Returns(true); - } - else - { - skills.Setup(x => x.GetNativeFunction(It.Is(s => s == skillName), It.Is(s => s == name))) - .Returns(mockFunction.Object); - ISKFunction? outFunc = mockFunction.Object; - skills.Setup(x => x.TryGetNativeFunction(It.Is(s => s == skillName), It.Is(s => s == name), out outFunc)).Returns(true); - } + skills.Setup(x => x.GetFunction(It.Is(s => s == skillName), It.Is(s => s == name))) + .Returns(mockFunction.Object); + ISKFunction? outFunc = mockFunction.Object; + skills.Setup(x => x.TryGetFunction(It.Is(s => s == skillName), It.Is(s => s == name), out outFunc)).Returns(true); } } diff --git a/dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SequentialPlannerTests.cs b/dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SequentialPlannerTests.cs index 6d97a0912a60..743c0c3d6f75 100644 --- a/dotnet/src/Extensions/Extensions.UnitTests/Planning/SequentialPlanner/SequentialPlannerTests.cs +++ b/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(s => s == skillName), It.Is(s => s == name))) - .Returns(mockFunction.Object); - ISKFunction? outFunc = mockFunction.Object; - skills.Setup(x => x.TryGetSemanticFunction(It.Is(s => s == skillName), It.Is(s => s == name), out outFunc)).Returns(true); - } - else - { - skills.Setup(x => x.GetNativeFunction(It.Is(s => s == skillName), It.Is(s => s == name))) - .Returns(mockFunction.Object); - ISKFunction? outFunc = mockFunction.Object; - skills.Setup(x => x.TryGetNativeFunction(It.Is(s => s == skillName), It.Is(s => s == name), out outFunc)).Returns(true); - } + skills.Setup(x => x.GetFunction(It.Is(s => s == skillName), It.Is(s => s == name))) + .Returns(mockFunction.Object); + ISKFunction? outFunc = mockFunction.Object; + skills.Setup(x => x.TryGetFunction(It.Is(s => s == skillName), It.Is(s => s == name), out outFunc)).Returns(true); } skills.Setup(x => x.GetFunctionsView(It.IsAny(), It.IsAny())).Returns(functionsView); diff --git a/dotnet/src/Extensions/Planning.SequentialPlanner/SKContextSequentialPlannerExtensions.cs b/dotnet/src/Extensions/Planning.SequentialPlanner/SKContextSequentialPlannerExtensions.cs index 2fb86261309f..8bbd57866b04 100644 --- a/dotnet/src/Extensions/Planning.SequentialPlanner/SKContextSequentialPlannerExtensions.cs +++ b/dotnet/src/Extensions/Planning.SequentialPlanner/SKContextSequentialPlannerExtensions.cs @@ -54,9 +54,14 @@ public static async Task> 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) diff --git a/dotnet/src/Extensions/Planning.SequentialPlanner/SequentialPlanParser.cs b/dotnet/src/Extensions/Planning.SequentialPlanner/SequentialPlanParser.cs index 883df05294ea..c3d0ac131e95 100644 --- a/dotnet/src/Extensions/Planning.SequentialPlanner/SequentialPlanParser.cs +++ b/dotnet/src/Extensions/Planning.SequentialPlanner/SequentialPlanParser.cs @@ -15,6 +15,7 @@ internal static class SequentialPlanParser { /// /// The tag name used in the plan xml for the user's goal/ask. + /// TODO: never used /// 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); diff --git a/dotnet/src/SemanticKernel.Abstractions/SkillDefinition/IReadOnlySkillCollection.cs b/dotnet/src/SemanticKernel.Abstractions/SkillDefinition/IReadOnlySkillCollection.cs index 3e829f8cfcd3..77e383222f7e 100644 --- a/dotnet/src/SemanticKernel.Abstractions/SkillDefinition/IReadOnlySkillCollection.cs +++ b/dotnet/src/SemanticKernel.Abstractions/SkillDefinition/IReadOnlySkillCollection.cs @@ -28,7 +28,7 @@ public interface IReadOnlySkillCollection ISKFunction GetFunction(string skillName, string functionName); /// - /// Gets the function stored in the collection. + /// Check if a function is available in the current context, and return it. /// /// The name of the function to retrieve. /// When this method returns, the function that was retrieved if one with the specified name was found; otherwise, . @@ -36,81 +36,13 @@ public interface IReadOnlySkillCollection bool TryGetFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance); /// - /// Gets the function stored in the collection. - /// - /// The name of the skill with which the function is associated. - /// The name of the function to retrieve. - /// When this method returns, the function that was retrieved if one with the specified name was found; otherwise, . - /// if the function was found; otherwise, . - bool TryGetFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance); - - /// - /// Gets the semantic function stored in the collection. - /// - /// The name of the function to retrieve. - /// The function retrieved from the collection. - /// The specified function could not be found in the collection. - ISKFunction GetSemanticFunction(string functionName); - - /// - /// Gets the semantic function stored in the collection. - /// - /// The name of the skill with which the function is associated. - /// The name of the function to retrieve. - /// The function retrieved from the collection. - /// The specified function could not be found in the collection. - ISKFunction GetSemanticFunction(string skillName, string functionName); - - /// - /// Gets the semantic function stored in the collection. - /// - /// The name of the function to retrieve. - /// When this method returns, the function that was retrieved if one with the specified name was found; otherwise, . - /// if the function was found; otherwise, . - bool TryGetSemanticFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance); - - /// - /// Gets the semantic function stored in the collection. + /// Check if a function is available in the current context, and return it. /// /// The name of the skill with which the function is associated. /// The name of the function to retrieve. - /// When this method returns, the function that was retrieved if one with the specified name was found; otherwise, . - /// if the function was found; otherwise, . - bool TryGetSemanticFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance); - - /// - /// Gets the native function stored in the collection. - /// - /// The name of the function to retrieve. - /// The function retrieved from the collection. - /// The specified function could not be found in the collection. - ISKFunction GetNativeFunction(string functionName); - - /// - /// Gets the native function stored in the collection. - /// - /// The name of the skill with which the function is associated. - /// The name of the function to retrieve. - /// The function retrieved from the collection. - /// The specified function could not be found in the collection. - ISKFunction GetNativeFunction(string skillName, string functionName); - - /// - /// Gets the native function stored in the collection. - /// - /// The name of the function to retrieve. - /// When this method returns, the function that was retrieved if one with the specified name was found; otherwise, . - /// if the function was found; otherwise, . - bool TryGetNativeFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance); - - /// - /// Gets the native function stored in the collection. - /// - /// The name of the skill with which the function is associated. - /// The name of the function to retrieve. - /// When this method returns, the function that was retrieved if one with the specified name was found; otherwise, . + /// When this method returns, the function that was retrieved if one with the specified name was found; otherwise, . /// if the function was found; otherwise, . - bool TryGetNativeFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance); + bool TryGetFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? availableFunction); /// /// Get all registered functions details, minus the delegates diff --git a/dotnet/src/SemanticKernel.Abstractions/SkillDefinition/ISkillCollection.cs b/dotnet/src/SemanticKernel.Abstractions/SkillDefinition/ISkillCollection.cs index 5ff79d9edb63..744f22a7421a 100644 --- a/dotnet/src/SemanticKernel.Abstractions/SkillDefinition/ISkillCollection.cs +++ b/dotnet/src/SemanticKernel.Abstractions/SkillDefinition/ISkillCollection.cs @@ -16,16 +16,9 @@ public interface ISkillCollection : IReadOnlySkillCollection IReadOnlySkillCollection ReadOnlySkillCollection { get; } /// - /// Add a semantic function to the collection + /// Add a function to the collection /// /// Function delegate /// Self instance - ISkillCollection AddSemanticFunction(ISKFunction functionInstance); - - /// - /// Add a native function to the collection - /// - /// Wrapped function delegate - /// Self instance - ISkillCollection AddNativeFunction(ISKFunction functionInstance); + ISkillCollection AddFunction(ISKFunction functionInstance); } diff --git a/dotnet/src/SemanticKernel.UnitTests/KernelTests.cs b/dotnet/src/SemanticKernel.UnitTests/KernelTests.cs index 491775458ceb..9b6b6fc61b34 100644 --- a/dotnet/src/SemanticKernel.UnitTests/KernelTests.cs +++ b/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 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> list in procMem.SemanticFunctions) { diff --git a/dotnet/src/SemanticKernel.UnitTests/Planning/PlanSerializationTests.cs b/dotnet/src/SemanticKernel.UnitTests/Planning/PlanSerializationTests.cs index c3b515397bf7..3da05d985a1a 100644 --- a/dotnet/src/SemanticKernel.UnitTests/Planning/PlanSerializationTests.cs +++ b/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() + Parameters = new List { - new ParameterView() { Name = "variables" } + new() { Name = "variables" } } }); ISKFunction? outFunc = mockFunction.Object; - skills.Setup(x => x.TryGetNativeFunction(It.IsAny(), It.IsAny(), out outFunc)).Returns(true); - skills.Setup(x => x.GetNativeFunction(It.IsAny(), It.IsAny())).Returns(mockFunction.Object); + skills.Setup(x => x.TryGetFunction(It.IsAny(), out outFunc)).Returns(true); + skills.Setup(x => x.TryGetFunction(It.IsAny(), It.IsAny(), out outFunc)).Returns(true); + skills.Setup(x => x.GetFunction(It.IsAny(), It.IsAny())).Returns(mockFunction.Object); plan.AddSteps(mockFunction.Object, mockFunction.Object); diff --git a/dotnet/src/SemanticKernel.UnitTests/SkillDefinition/SKContextTests.cs b/dotnet/src/SemanticKernel.UnitTests/SkillDefinition/SKContextTests.cs index 5455530119d5..1c34a426abe6 100644 --- a/dotnet/src/SemanticKernel.UnitTests/SkillDefinition/SKContextTests.cs +++ b/dotnet/src/SemanticKernel.UnitTests/SkillDefinition/SKContextTests.cs @@ -52,12 +52,12 @@ public async Task ItHasHelpersForSkillCollectionAsync() { // Arrange IDictionary 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 diff --git a/dotnet/src/SemanticKernel/Kernel.cs b/dotnet/src/SemanticKernel/Kernel.cs index 062bb68ec8fd..2218dddcce39 100644 --- a/dotnet/src/SemanticKernel/Kernel.cs +++ b/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 ImportSkill(object skillInstance, string foreach (KeyValuePair 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; } diff --git a/dotnet/src/SemanticKernel/Orchestration/SKContextExtensions.cs b/dotnet/src/SemanticKernel/Orchestration/SKContextExtensions.cs deleted file mode 100644 index 7b79b8df8d92..000000000000 --- a/dotnet/src/SemanticKernel/Orchestration/SKContextExtensions.cs +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System.Diagnostics.CodeAnalysis; -using Microsoft.SemanticKernel.SkillDefinition; - -// ReSharper disable once CheckNamespace - Using NS of SKContext -namespace Microsoft.SemanticKernel.Orchestration; - -internal static class SKContextExtensions -{ - /// - /// Simple extension method to check if a function is registered in the SKContext. - /// - /// The SKContext to check - /// The skill name - /// The function name - /// The registered function, if found - internal static bool IsFunctionRegistered(this SKContext context, string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? registeredFunction) - { - context.ThrowIfSkillCollectionNotSet(); - - return context.Skills!.TryGetNativeFunction(skillName, functionName, out registeredFunction) || - context.Skills.TryGetNativeFunction(functionName, out registeredFunction) || - context.Skills.TryGetSemanticFunction(skillName, functionName, out registeredFunction); - } - - /// - /// Ensures the context has a skill collection available - /// - /// SK execution context - internal static void ThrowIfSkillCollectionNotSet(this SKContext context) - { - if (context.Skills == null) - { - throw new KernelException( - KernelException.ErrorCodes.SkillCollectionNotSet, - "Skill collection not found in the context"); - } - } -} diff --git a/dotnet/src/SemanticKernel/Planning/Plan.cs b/dotnet/src/SemanticKernel/Planning/Plan.cs index 1e92376b6d34..9a9b6de8b811 100644 --- a/dotnet/src/SemanticKernel/Planning/Plan.cs +++ b/dotnet/src/SemanticKernel/Planning/Plan.cs @@ -158,6 +158,7 @@ public Plan( /// /// Deserialize a JSON string into a Plan object. + /// TODO: the context should never be null, it's required internally /// /// JSON string representation of a Plan /// The context to use for function registrations. @@ -169,7 +170,7 @@ public static Plan FromJson(string json, SKContext? context = null) if (context != null) { - plan = SetRegisteredFunctions(plan, context); + plan = SetAvailableFunctions(plan, context); } return plan; @@ -401,11 +402,18 @@ internal string ExpandFromVariables(ContextVariables variables, string input) /// Plan to set functions for. /// Context to use. /// The plan with functions set. - private static Plan SetRegisteredFunctions(Plan plan, SKContext context) + private static Plan SetAvailableFunctions(Plan plan, SKContext context) { if (plan.Steps.Count == 0) { - if (context.IsFunctionRegistered(plan.SkillName, plan.Name, out var skillFunction)) + if (context.Skills == null) + { + throw new KernelException( + KernelException.ErrorCodes.SkillCollectionNotSet, + "Skill collection not found in the context"); + } + + if (context.Skills.TryGetFunction(plan.SkillName, plan.Name, out var skillFunction)) { plan.SetFunction(skillFunction); } @@ -414,7 +422,7 @@ private static Plan SetRegisteredFunctions(Plan plan, SKContext context) { foreach (var step in plan.Steps) { - SetRegisteredFunctions(step, context); + SetAvailableFunctions(step, context); } } diff --git a/dotnet/src/SemanticKernel/SkillDefinition/ReadOnlySkillCollection.cs b/dotnet/src/SemanticKernel/SkillDefinition/ReadOnlySkillCollection.cs index 0e05114c9225..7f1356b735d6 100644 --- a/dotnet/src/SemanticKernel/SkillDefinition/ReadOnlySkillCollection.cs +++ b/dotnet/src/SemanticKernel/SkillDefinition/ReadOnlySkillCollection.cs @@ -27,40 +27,8 @@ public bool TryGetFunction(string functionName, [NotNullWhen(true)] out ISKFunct this._skillCollection.TryGetFunction(functionName, out functionInstance); /// - public bool TryGetFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance) => - this._skillCollection.TryGetFunction(skillName, functionName, out functionInstance); - - /// - public ISKFunction GetSemanticFunction(string functionName) => - this._skillCollection.GetSemanticFunction(functionName); - - /// - public ISKFunction GetSemanticFunction(string skillName, string functionName) => - this._skillCollection.GetSemanticFunction(skillName, functionName); - - /// - public bool TryGetSemanticFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance) => - this._skillCollection.TryGetSemanticFunction(functionName, out functionInstance); - - /// - public bool TryGetSemanticFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance) => - this._skillCollection.TryGetSemanticFunction(skillName, functionName, out functionInstance); - - /// - public ISKFunction GetNativeFunction(string functionName) => - this._skillCollection.GetNativeFunction(functionName); - - /// - public ISKFunction GetNativeFunction(string skillName, string functionName) => - this._skillCollection.GetNativeFunction(skillName, functionName); - - /// - public bool TryGetNativeFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance) => - this._skillCollection.TryGetNativeFunction(functionName, out functionInstance); - - /// - public bool TryGetNativeFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance) => - this._skillCollection.TryGetNativeFunction(skillName, functionName, out functionInstance); + public bool TryGetFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? availableFunction) => + this._skillCollection.TryGetFunction(skillName, functionName, out availableFunction); /// public FunctionsView GetFunctionsView(bool includeSemantic = true, bool includeNative = true) => diff --git a/dotnet/src/SemanticKernel/SkillDefinition/SkillCollection.cs b/dotnet/src/SemanticKernel/SkillDefinition/SkillCollection.cs index 4249ee888105..ec5bd354db8a 100644 --- a/dotnet/src/SemanticKernel/SkillDefinition/SkillCollection.cs +++ b/dotnet/src/SemanticKernel/SkillDefinition/SkillCollection.cs @@ -33,13 +33,15 @@ public SkillCollection(ILogger? log = null) this._skillCollection = new(StringComparer.OrdinalIgnoreCase); } - /// - public ISkillCollection AddSemanticFunction(ISKFunction functionInstance) => - this.AddFunction(functionInstance); + public ISkillCollection AddFunction(ISKFunction functionInstance) + { + Verify.NotNull(functionInstance, "The function is NULL"); - /// - public ISkillCollection AddNativeFunction(ISKFunction functionInstance) => - this.AddFunction(functionInstance); + ConcurrentDictionary skill = this._skillCollection.GetOrAdd(functionInstance.SkillName, static _ => new(StringComparer.OrdinalIgnoreCase)); + skill.TryAdd(functionInstance.Name, functionInstance); + + return this; + } /// public ISKFunction GetFunction(string functionName) => @@ -61,81 +63,17 @@ public bool TryGetFunction(string functionName, [NotNullWhen(true)] out ISKFunct this.TryGetFunction(GlobalSkill, functionName, out functionInstance); /// - public bool TryGetFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance) + public bool TryGetFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? availableFunction) { Verify.NotNull(skillName, nameof(skillName)); Verify.NotNull(functionName, nameof(functionName)); if (this._skillCollection.TryGetValue(skillName, out ConcurrentDictionary? skill)) { - return skill.TryGetValue(functionName, out functionInstance); - } - - functionInstance = null; - return false; - } - - /// - public ISKFunction GetSemanticFunction(string functionName) => - this.GetSemanticFunction(GlobalSkill, functionName); - - /// - public ISKFunction GetSemanticFunction(string skillName, string functionName) - { - if (!this.TryGetSemanticFunction(skillName, functionName, out ISKFunction? functionInstance)) - { - this.ThrowFunctionNotAvailable(skillName, functionName); - } - - return functionInstance; - } - - /// - public bool TryGetSemanticFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance) => - this.TryGetSemanticFunction(GlobalSkill, functionName, out functionInstance); - - /// - public bool TryGetSemanticFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance) - { - if (this.TryGetFunction(skillName, functionName, out ISKFunction? func) && func.IsSemantic) - { - functionInstance = func; - return true; + return skill.TryGetValue(functionName, out availableFunction); } - functionInstance = null; - return false; - } - - /// - public ISKFunction GetNativeFunction(string functionName) => - this.GetNativeFunction(GlobalSkill, functionName); - - /// - public ISKFunction GetNativeFunction(string skillName, string functionName) - { - if (!this.TryGetNativeFunction(skillName, functionName, out ISKFunction? functionInstance)) - { - this.ThrowFunctionNotAvailable(skillName, functionName); - } - - return functionInstance; - } - - /// - public bool TryGetNativeFunction(string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance) => - this.TryGetNativeFunction(GlobalSkill, functionName, out functionInstance); - - /// - public bool TryGetNativeFunction(string skillName, string functionName, [NotNullWhen(true)] out ISKFunction? functionInstance) - { - if (this.TryGetFunction(skillName, functionName, out ISKFunction? func) && !func.IsSemantic) - { - functionInstance = func; - return true; - } - - functionInstance = null; + availableFunction = null; return false; } @@ -163,16 +101,6 @@ public FunctionsView GetFunctionsView(bool includeSemantic = true, bool includeN #region private ================================================================================ - private SkillCollection AddFunction(ISKFunction functionInstance) - { - Verify.NotNull(functionInstance, "The function is NULL"); - - ConcurrentDictionary skill = this._skillCollection.GetOrAdd(functionInstance.SkillName, static _ => new(StringComparer.OrdinalIgnoreCase)); - skill.TryAdd(functionInstance.Name, functionInstance); - - return this; - } - [DoesNotReturn] private void ThrowFunctionNotAvailable(string skillName, string functionName) { diff --git a/dotnet/src/SemanticKernel/TemplateEngine/Blocks/CodeBlock.cs b/dotnet/src/SemanticKernel/TemplateEngine/Blocks/CodeBlock.cs index 4cd9bba5f5c8..f5f737182055 100644 --- a/dotnet/src/SemanticKernel/TemplateEngine/Blocks/CodeBlock.cs +++ b/dotnet/src/SemanticKernel/TemplateEngine/Blocks/CodeBlock.cs @@ -98,7 +98,13 @@ public async Task RenderCodeAsync(SKContext context) private async Task RenderFunctionCallAsync(FunctionIdBlock fBlock, SKContext context) { - context.ThrowIfSkillCollectionNotSet(); + if (context.Skills == null) + { + throw new KernelException( + KernelException.ErrorCodes.SkillCollectionNotSet, + "Skill collection not found in the context"); + } + if (!this.GetFunctionFromSkillCollection(context.Skills!, fBlock, out ISKFunction? function)) { var errorMsg = $"Function `{fBlock.Content}` not found";