microsoft / semantic-kernel

Integrate cutting-edge LLM technology quickly and easily into your apps
https://aka.ms/semantic-kernel
MIT License
21.97k stars 3.27k forks source link

.Net: RunAsync Return SKContext Rather than KernelResult #3242

Closed jickler-o closed 10 months ago

jickler-o commented 1 year ago

I am requesting that the RunAsync function should once again return an SKContext or equivalent when running a semantic function. Before you could pass in a context and store relevant information(such as sources or the info used to answer a questions) as context variables and access them from the SKContext. Now you can only get the result out and it makes preserving that info for each call a bit more inefficient.

For example, a function that used to take a list of SKContext's and also an ISKFunction now requires: List<(string Source, ContextVariables Context, KernelResult? Result, bool IsAnswer)> answerList, ISKFunction function

R0315 commented 1 year ago

I've been collaborating with @jickler-o on a project and noticed that the recent changes appear to have diverged from our project's vision. I'm certainly open to feedback and guidance if I'm missing an alternative solution. Below is some context from our codebase which might be helpful.

// Initiate a native function, starting a chain of functions leading to an answer for a user query.
var answer = await KernelCreator.SK!.RunAsync(KernelSkills.DocQAFunction!["FindAnswers"], context);

// Extract the context from the answer to retrieve a context variable used in BuildCitedAnswer to construct a cited response.
await Conversation.AddItem("AI", RequestHandler.BuildCitedAnswer(answer), jsRuntime);

I am going to focus on these lines of code here, however, it should be noted that the native function being called relies on other functions being called to analyze and further refine the ultimate response. The v1 changes have made this process slightly more difficult.

The primary challenge with the v1 changes is that SKContext is no longer the return type for RunAsync. Consequently, accessing context variables has become challenging. Our vision for this project has been to maintain granular control over the steps involved in formulating a reply to a user query. Previously, this involved effortlessly accessing, manipulating, and passing along context variables and model results together. However, the immutability of KernelResult as a sealed class, coupled with the internal setting of the Value (which represents the model's response or user's answer), limits our ability to craft bespoke answers. This necessitates more intricate handling that wasn't required before or, as is the case here, makes certain steps that were previously quite simple impossible, necessitating verbose refactoring.

This shift has transformed what was once a streamlined multi-model framework approach into a more verbose and complex process. I'd kindly request reconsideration of these changes, possibly reverting back to the simpler SKContext structure. This would greatly enhance flexibility for developers, ensuring projects operate smoothly without unnecessary complications.

dmytrostruk commented 1 year ago

@jickler-o @R0315 Thanks for reporting! This feedback is greatly appreciated! We are collecting user feedback to understand how we can improve SK and what features are useful for developers.

Original goal of hiding SKContext was to make experience for developers closer to standard programming - it's the ability to return any Type from Function/Kernel, and not only SKContext, which at the moment works only with strings. For example, if you have Native Function that returns TypeA, if you run it as function.InvokeAsync or through kernel.RunAsync, you will be able to receive the same TypeA instead of SKContext. Also, if your Native Function supports streaming (e.g. returns IAsyncEnumerable<T>), it's also possible to project that streaming to the place where you invoke that function.

Also, together with actual value you are able to get Metadata. In case of Semantic Functions, this property may contain some useful information about your semantic function execution, like token usage, response from AI etc.

The wider vision would be to pass any kind of object as input to Function/Kernel and the ability to get any kind of object as response. We think that all SKContext and variables handling can be done internally inside Kernel, so users don't have to manage it manually. Although, it probably makes sense to provide this ability for developers for more granular control.

It's worth to mention that current implementation is definitely not a final one and we will make it better to cover all possible scenarios. That's why your feedback is very useful :)

It would be great to understand your scenario better, so we will be able to see if it's possible to achieve your scenario with current implementation or we will need to add/revert some functionality to cover it.

It looks like you are interested in accessing SKContext/ContextVariables after each function execution. Could you please share some scenarios when the information from SKContext/ContextVariables will be useful for you. Some code snippets would help as well. For example, if you have Function1, Function2 and Function3 in your pipeline, how you manage your data, pass it from one function to another, and how you work with result.

Based on your code snippet shared above, when you execute RequestHandler.BuildCitedAnswer(answer), what information is expected here from SKContext/ContextVariables, that is not available in actual result?

Any information will be greatly appreciated, thank you!

anthonypuppo commented 1 year ago

You can pass your own ContextVariables instance to the run. The kernel will use your instance to maintain state. You could effectively ignore the result the kernel passes back if all you're interested in is the intermediate variables.

using Microsoft.SemanticKernel;
using Microsoft.SemanticKernel.Orchestration;

var kernel = new KernelBuilder().Build();

kernel.ImportFunctions(new Plugin(), nameof(Plugin));

var variables = new ContextVariables();
var function = kernel.Functions.GetFunction(nameof(Plugin), nameof(Plugin.Function));
var kernelResult = await kernel.RunAsync(variables, function);

foreach (var variable in variables)
{
    Console.WriteLine($"{variable.Key}: {variable.Value}");
}

class Plugin
{
    [SKFunction]
    public SKContext Function(SKContext context)
    {
        context.Variables.Set("sideEffect", "Hello from side effect!");
        context.Variables.Update("Main result");

        return context;
    }
}

// Output
// sideEffect: Hello from side effect!
// INPUT: Main result
dmytrostruk commented 1 year ago

@anthonypuppo Thank you for this example!

While this implementation should work, we are trying to understand if this way of using SKContext is the right approach. Because, currently it looks like SKContext/ContextVariables objects are kind of global variables, which are mutable, and as soon as you have big list of functions in pipeline and they are able to modify this information, things may be uncontrollable. We also consider some scenarios when in the future it will be possible to run some functions in parallel (e.g. when these functions are not related to each other and don't need an input from previous functions).

Also, instead of SKContext it should be possible to work with your custom types as well. That will mean that instead of working with dictionary of properties, where value is string, you should be able to work with strong types.

internal class Program
{
    static async Task Main(string[] args)
    {
        var kernel = new KernelBuilder().Build();

        var functions = kernel.ImportFunctions(new MyPlugin(), MyPlugin.PluginName);

        var result = await kernel.RunAsync(functions["Function1"]);
        var customType = result.GetValue<MyCustomType>()!;

        Console.WriteLine(customType.Number); // 2
        Console.WriteLine(customType.Text); // From Function1 + From Function2
    }
}

public class MyPlugin
{
    public const string PluginName = "MyPlugin";

    [SKFunction, SKName("Function1")]
    public static async Task<MyCustomType> Function1(SKContext context)
    {
        // Execute another function
        var result = await context.Runner.RunAsync(PluginName, "Function2");
        var customType = result.GetValue<MyCustomType>()!;

        return new MyCustomType
        {
            Number = 2 * customType.Number,
            Text = "From Function1 + " + customType.Text
        };
    }

    [SKFunction, SKName("Function2")]
    public static MyCustomType Function2()
    {
        return new MyCustomType
        {
            Number = 1,
            Text = "From Function2",
        };
    }
}

We would recommend using your custom types instead of SKContext, since it will allow to develop your functions in natural way and avoid coupling to SKContext implementation.

Please let us know if this example is helpful. As I mentioned earlier, it would be nice to understand what benefits SKContext provides over other types in user applications. Thanks again!

dmytrostruk commented 1 year ago

In previous example, we triggered Function2 from Function1 manually. But it's also possible to execute them in pipeline, so Kernel will do that automatically and will pass appropriate information as needed:

internal class Program
{
    static async Task Main(string[] args)
    {
        var kernel = new KernelBuilder().Build();

        var functions = kernel.ImportFunctions(new MyPlugin(), MyPlugin.PluginName);

        var result = await kernel.RunAsync(functions["Function1"], functions["Function2"]);
        var customType = result.GetValue<MyCustomType>()!;

        Console.WriteLine(customType.Number); // 2
        Console.WriteLine(customType.Text); // From Function2 + From Function1
    }
}

public class MyPlugin
{
    public const string PluginName = "MyPlugin";

    [SKFunction, SKName("Function1")]
    public static MyCustomType Function1()
    {
        return new MyCustomType
        {
            Number = 1,
            Text = "From Function1",
        };
    }

    [SKFunction, SKName("Function2")]
    public static MyCustomType Function2(MyCustomType customType)
    {
        return new MyCustomType
        {
            Number = 2 * customType.Number,
            Text = "From Function2 + " + customType.Text
        };
    }
}

Although, keep in mind that currently for MyCustomType you should define your TypeConverter, so on Kernel side we will know how to work with it as string (to be able to use it in conversation with AI). But this is something that we want to improve in the future to be able to use custom types without type converters:

[TypeConverter(typeof(MyCustomTypeConverter))]
public sealed class MyCustomType
{
    public int Number { get; set; }

    public string Text { get; set; }
}

public sealed class MyCustomTypeConverter : TypeConverter
{
    public override bool CanConvertFrom(ITypeDescriptorContext? context, Type sourceType) => true;

    public override object? ConvertFrom(ITypeDescriptorContext? context, CultureInfo? culture, object value)
    {
        return JsonSerializer.Deserialize<MyCustomType>((string)value);
    }

    public override object? ConvertTo(ITypeDescriptorContext? context, CultureInfo? culture, object? value, Type destinationType)
    {
        return JsonSerializer.Serialize(value);
    }
}
R0315 commented 1 year ago

@anthonypuppo, thank you for the detailed explanation. With your guidance, I managed to achieve my objectives by passing a reference to the initial context variables at every step of our docQA plugin. While this method works, I believe it would be more development-friendly not to have to go through this route. However, if the overarching goal is to enable native functions to return any type, then I anticipate that we could eventually return exactly what we need. This would be an ideal solution.

@dmytrostruk:

@jickler-o @R0315 Thanks for reporting! This feedback is greatly appreciated! We are collecting user feedback to understand how we can improve SK and what features are useful for developers.

Original goal of hiding SKContext was to make experience for developers closer to standard programming - it's the ability to return any Type from Function/Kernel, and not only SKContext, which at the moment works only with strings. For example, if you have Native Function that returns TypeA, if you run it as function.InvokeAsync or through kernel.RunAsync, you will be able to receive the same TypeA instead of SKContext. Also, if your Native Function supports streaming (e.g. returns IAsyncEnumerable<T>), it's also possible to project that streaming to the place where you invoke that function.

Being able to return any type from a native function we've designed would perfectly address the challenges I encountered. In fact, I believe it would be a superior solution compared to the original SKContext, as it would grant us the flexibility to precisely tailor our outputs.

It looks like you are interested in accessing SKContext/ContextVariables after each function execution. Could you please share some scenarios when the information from SKContext/ContextVariables will be useful for you. Some code snippets would help as well. For example, if you have Function1, Function2 and Function3 in your pipeline, how you manage your data, pass it from one function to another, and how you work with result.

Our multi-model approach enables us to construct highly specific answers for users. We have skills that ascertain the user's intent behind their queries and evaluate each model's response against that intent, categorizing them as TRUE (fulfills intent) or FALSE (doesn't fulfill intent). The ability to seamlessly pass references to the same context and update it with results—which we could later extract for further processing or to craft a final reply—was incredibly efficient. That said, if we can soon return any type from the native functions, that would likely resolve our challenges. I'll look to see if I've overlooked this capability or if it's still in the pipeline.

Based on your code snippet shared above, when you execute RequestHandler.BuildCitedAnswer(answer), what information is expected here from SKContext/ContextVariables, that is not available in actual result?

The KernelResult currently doesn't provide access to the context variables. Anthony separately highlighted that I could pass a reference to the initialized context variables, which would essentially allow for a similar functionality. By integrating this approach and passing an additional parameter to BuildCitedAnswer, I was able to restore functionality to our project.

dmytrostruk commented 1 year ago

Being able to return any type from a native function we've designed would perfectly address the challenges I encountered. In fact, I believe it would be a superior solution compared to the original SKContext, as it would grant us the flexibility to precisely tailor our outputs.

@R0315 Thanks for your reply! Could you please check two examples that I shared above and provide your feedback if it's something that you would like to use in your scenario? I just checked these examples in console application, and they work as expected.

The KernelResult currently doesn't provide access to the context variables. Anthony separately highlighted that I could pass a reference to the initialized context variables, which would essentially allow for a similar functionality. By integrating this approach and passing an additional parameter to BuildCitedAnswer, I was able to restore functionality to our project.

I'm sorry to hear that new SK version didn't align with your current functionality. In the future, we will try to deliver new features in a way, that will allow users to migrate to the new version smoother, including more documentation and migration guides.

As for now, it's great that you were able to restore your functionality with ContextVariables. If you could take a look at my examples above and provide some feedback about it, that would be super helpful for us. Thanks again and sorry for any inconvenience.

lemillermicrosoft commented 1 year ago

We would recommend using your custom types instead of SKContext, since it will allow to develop your functions in natural way and avoid coupling to SKContext implementation.

This can be difficult though if we don't know what the return type is going to be -- i.e. if my function is calling kernel.RunAsync on a Plan object, for example and I want to return the results of that execution as the result of my function call. If I could return FunctionResult I would but this isn't supported at present. Before when it was all SKContext, I didn't have to do any handling of the results.

R0315 commented 1 year ago

Being able to return any type from a native function we've designed would perfectly address the challenges I encountered. In fact, I believe it would be a superior solution compared to the original SKContext, as it would grant us the flexibility to precisely tailor our outputs.

@R0315 Thanks for you reply! Could you please check two examples that I shared above and provide your feedback if it's something that you would like to use in your scenario? I just checked these examples in console application, and they work as expected.

The KernelResult currently doesn't provide access to the context variables. Anthony separately highlighted that I could pass a reference to the initialized context variables, which would essentially allow for a similar functionality. By integrating this approach and passing an additional parameter to BuildCitedAnswer, I was able to restore functionality to our project.

I'm sorry to hear that new SK version didn't align with your current functionality. In the future, we will try to deliver new features in a way, that will allow users to migrate to the new version smoother, including more documentation and migration guides.

As for now, it's great that you were able to restore your functionality with ContextVariables. If you could take a look at my examples above and provide some feedback about it, that would be super helpful for us. Thanks again and sorry for any inconvenience.

Absolutely. I am at the end of my day now. However, I will be looking to work this in tomorrow and see how it goes. I'll get back to you!

dmytrostruk commented 1 year ago

This can be difficult though if we don't know what the return type is going to be -- i.e. if my function is calling kernel.RunAsync on a Plan object, for example and I want to return the results of that execution as the result of my function call.

@lemillermicrosoft I think it worth to discuss planning functionality in separate thread, but in general, when AI generates a plan, you don't know what the last function in that plan would be, which means that you don't know what type it's going to return. But in all cases, either it's ContextVariables or KernelResult, you should be able to know how to process your result. By that I mean - if you have context.Variables[key], you should know what key to use to get specific information from variables. Same for KernelResult, but instead of requirement to know key, you need to know the result type. And as soon as you know the result type, it's possible to process your plan execution result using strong types, which should be better compile-time experience than operating with dictionary.

In theory, if you want to proceed with dictionary, you should be able to implement your own MySKContext, which will contain dictionary of properties and return it from your functions. Your potential custom MySKContext, which is basically your custom type, can hold dictionary of properties together with strongly typed properties, to mix both approaches. But the main point here, that it's your type, and you are able to define it in a way that covers your scenario. So, you are not forced to use just Microsoft.SemanticKernel.Orchestration.SKContext in your functions.

Please let me know if I'm missing something. I would be happy to discuss it further, as we want to make sure that previous and new scenarios can be supported with current version of SK. Thanks a lot!

R0315 commented 1 year ago

@dmytrostruk

Also, instead of SKContext it should be possible to work with your custom types as well. That will mean that instead of working with dictionary of properties, where value is string, you should be able to work with strong types.

internal class Program
{
    static async Task Main(string[] args)
    {
        var kernel = new KernelBuilder().Build();

        var functions = kernel.ImportFunctions(new MyPlugin(), MyPlugin.PluginName);

        var result = await kernel.RunAsync(functions["Function1"]);
        var customType = result.GetValue<MyCustomType>()!;

        Console.WriteLine(customType.Number); // 2
        Console.WriteLine(customType.Text); // From Function1 + From Function2
    }
}

public class MyPlugin
{
    public const string PluginName = "MyPlugin";

    [SKFunction, SKName("Function1")]
    public static async Task<MyCustomType> Function1(SKContext context)
    {
        // Execute another function
        var result = await context.Runner.RunAsync(PluginName, "Function2");
        var customType = result.GetValue<MyCustomType>()!;

        return new MyCustomType
        {
            Number = 2 * customType.Number,
            Text = "From Function1 + " + customType.Text
        };
    }

    [SKFunction, SKName("Function2")]
    public static MyCustomType Function2()
    {
        return new MyCustomType
        {
            Number = 1,
            Text = "From Function2",
        };
    }
}

We would recommend using your custom types instead of SKContext, since it will allow to develop your functions in natural way and avoid coupling to SKContext implementation.

Please let us know if this example is helpful. As I mentioned earlier, it would be nice to understand what benefits SKContext provides over other types in user applications. Thanks again!

I'm working on trying to build in this example. We aren't using planners or a pipeline, but rather hardcoding our steps. I've defined my own custom type and i'm trying to get the native function to return that custom type now. So, I have this:

var answer = await KernelCreator.SK!.RunAsync(KernelSkills.DocQAFunction!["FindAnswers"], contextVariables);

await Conversation.AddItem("AI", RequestHandler.BuildCitedAnswer(answer.GetValue<Spoonful>()!), jsRuntime);

Where the FindAnswers is now trying to return a custom type "Spoonful" (we have a whole theme going, don't ask haha). When I run this, though, I immediately see an error:

[2023-10-20T22:17:47.773Z] Error: Microsoft.SemanticKernel.Diagnostics.SKException: Function 'FindAnswers' is not supported by the kernel. Unknown return type System.Threading.Tasks.Task`1[Spoonful]

I have a habit of overthinking things, so I feel like either what I'm doing incorrectly is staring me in the face or I can't actually get the native function to return a custom type yet.

dmytrostruk commented 1 year ago

@R0315 Thank you for reply! Please see the following note that I mentioned in one of my previous comments: image

For your type Spoonful you should define TypeConverter which will convert your type to string representation. In theory, you could define some general TypeConverter and use it for multiple types that you want to use with Kernel.

Please let me know if this will resolve the problem. Thank you!

anthonypuppo commented 1 year ago

While this implementation should work, we are trying to understand if this way of using SKContext is the right approach. Because, currently it looks like SKContext/ContextVariables objects are kind of global variables, which are mutable, and as soon as you have big list of functions in pipeline and they are able to modify this information, things may be uncontrollable. We also consider some scenarios when in the future it will be possible to run some functions in parallel (e.g. when these functions are not related to each other and don't need an input from previous functions).

@dmytrostruk Would it not make more sense to refactor the pipeline run to pass each function it's own local context subset (ie. new type SKFunctionContext)? A "scratchpad" of sorts. Its life would be locked to that of the function so all the flexibility of the current SKContext could be maintained while giving consumers an options to play around and reduce the risk of impacting other things or exposing sensitive information. This could even just be a property that exists on SKContext but I personally think a dedicated parameter makes the intention more clear. Would be way more simple then expecting the consumer to implement their own type (though I really like that idea of having that as an additional option).

class Plugin
{
    [SKFunction]
    public SKContext Function(SKContext context, SKFunctionContext functionContext)
    {
        functionContext.Variables.Set("sideEffect", "Hello from side effect!");
        context.Variables.Update("Main result");

        return context;
    }
}

// Output
// INPUT: Main result
[TypeConverter(typeof(MyCustomTypeConverter))]
public sealed class MyCustomType
{
    public int Number { get; set; }

    public string Text { get; set; }
}

public sealed class MyCustomTypeConverter : TypeConverter
{
    public override bool CanConvertFrom(ITypeDescriptorContext? context, Type sourceType) => true;

    public override object? ConvertFrom(ITypeDescriptorContext? context, CultureInfo? culture, object value)
    {
        return JsonSerializer.Deserialize<MyCustomType>((string)value);
    }

    public override object? ConvertTo(ITypeDescriptorContext? context, CultureInfo? culture, object? value, Type destinationType)
    {
        return JsonSerializer.Serialize(value);
    }
}

Does an example of using TypeConverter exist in the samples? This, along with the auto conversion between context variables and most primitive types, should really be in there. You'd be surprised how often people I talk to people who realize the below works without having to pass as string and do the conversion inline.

class Plugin
{
    [SKFunction]
    public SKContext Function(SKContext context, string input, int var1, double var2)
    {
        // ...

        return context;
    }
}
dmytrostruk commented 1 year ago

Would it not make more sense to refactor the pipeline run to pass each function it's own local context subset (ie. new type SKFunctionContext)? A "scratchpad" of sorts. Its life would be locked to that of the function so all the flexibility of the current SKContext could be maintained while giving consumers an options to play around and reduce the risk of impacting other things or exposing sensitive information. This could even just be a property that exists on SKContext but I personally think a dedicated parameter makes the intention more clear. Would be way more simple then expecting the consumer to implement their own type (though I really like that idea of having that as an additional option).

@anthonypuppo I think that SKContext/ContextVariables will be hidden from public surface in the future. We already noticed that these types are used in a way that is different from initial ideas and design, and not only by SK consumers but SK developers as well. We will need to redesign it or replace with new mechanism, that will be internal only. SK consumers should develop native functions in the same way, as they develop regular .NET functions, by using primitive and custom types.

Let's imagine there is already existing user application that has a lot of classes and methods implemented. The goal is to make these methods accessible by AI. We don't want to force users to modify their existing methods or create a new one to accept SKContext as parameter or return it as result type. Users should be able to import their existing functions and use it with AI. Even attributes like SKFunction or SKName should not be required, although with their usage it will be easier for Planners to understand the purpose of each function and where it can be used when generating a plan.

Let's take another example, when you use already existing .NET SDK of your favorite social network. It has some methods like SendMessage, AddFriend etc., and you want to use these methods with AI. If SK will be coupled to SKContext, all these methods should be updated to accept or return SKContext (which is not possible, because that social network won't update their methods to support SK), or for all these methods users will have to implement adapters/wrappers that will wrap the logic with things like SKContext. It will be hard to achieve, because there are a lot of great SDKs already available, which would be nice to support out of the box. The ideal scenario would be, when you have a list of already implemented methods from some SDK or your own library, and you import it to Kernel as they are, so they will be accessible from semantic functions or for generating plans.

These capabilities are not ready yet, but we are trying to refactor current functionality to make it possible in the future.

Does an example of using TypeConverter exist in the samples? This, along with the auto conversion between context variables and most primitive types, should really be in there. You'd be surprised how often people I talk to people who realize the below works without having to pass as string and do the conversion inline.

I created PR with these examples and we will try to merge it as soon as possible: https://github.com/microsoft/semantic-kernel/pull/3255

Thank you!

matthewbolanos commented 10 months ago

This should now be fixed with v1.0.1; instead of returning KernelResult, we now return back FunctionResult again.