There doesn't seem to be a way to set the context.Terminate in a AIFunction call

[gyankov]

From what I see in the source the context is created internally and by default it will continue the calls:

• extensions/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs

  Line 529 in d8e2cd2

  FunctionInvocationContext context = new(chatMessages, functionCallContent, function)

Perhaps it would be better if the return type of the AIFunction is void to set the FunctionInvocationContext.Terminate prop to true by default. Or somehow allow users to set it manually when needed.

[SteveSandersonMS]

@gyankov Thanks for pointing this out.

Perhaps it would be better if the return type of the AIFunction is void to set the FunctionInvocationContext.Terminate prop to true by default

That's not the semantics that are intended here. Even if an AIFunction is void, the LLM should be allowed to invoke subsequent functions if it deems that applicable (and if other stopping conditions, such as the maximum number of calls, are not met).

The design we had in mind is using the FunctionInvocationContext object. This has a public bool Terminate { get; set; } property that can be set and this will cause the function invocation loop to end. However, this can't currently be used in reality because we don't pass the FunctionInvocationContext object into the AIFunction. As I recall, we were planning to have some way to receive that object in the AIFunction code, e.g. something like:

void MyFunction(FunctionInvocationContext context, string otherParam1, int otherParam2) { ... }

... matching on the object type.

We can treat this issue as tracking the need to complete this functionality. Does this match your expectations, @stephentoub?

[stephentoub]

However, this can't currently be used in reality because we don't pass the FunctionInvocationContext object into the AIFunction

It can be used, but not via AIFunction, as you call out. Right now you need to derive from FunctionInvokingChatClient and override the InvokeFunctionAsync method. That lets you add logic like "if the method being invoked is ABC, then set Terminate = true".

protected override async Task<object?> InvokeFunctionAsync(FunctionInvocationContext context, CancellationToken cancellationToken)
{
    try
    {
        return await base.InvokeFunctionAsync(context, cancellationToken);
    }
    finally
    {
        if (context.Function.Metadata.Name == "ABC") context.Terminate = true;
    }
}

We'd need to think more about a design that would actually funnel that FunctionInvocationContext into the AIFunction itself.

[gyankov]

Thank you @SteveSandersonMS and @stephentoub! Overriding the InvokeFunctionAsync works unless I add the UseFunctionInvocation as it always creates a FunctionInvokingChatClient instance:

• extensions/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClientBuilderExtensions.cs

  Line 35 in d8e2cd2

  var chatClient = new FunctionInvokingChatClient(innerClient, loggerFactory?.CreateLogger(typeof(FunctionInvokingChatClient)));

[stephentoub]

Overriding the InvokeFunctionAsync works

Great!

unless I add the UseFunctionInvocation as it always creates a FunctionInvokingChatClient instance

Yes, if you're using your own type, you'd need to use the Use method to add it to the pipeline rather than using the UseFunctionInvocation helper.

[gyankov]

Got you! Thanks once again @stephentoub.

[MackinnonBuck]

I can think of a few possible directions to take this.

Option 1: Terminate via context argument

This would be something like what @SteveSandersonMS mentioned earlier in this issue:

void MyFunction(
  FunctionInvokingChatClient.FunctionInvocationContext context,
  string otherParam1,
  int otherParam2)
{
  // ...
}

Challenges

The current signature of the AIFunction.InvokeAsync method looks like this:

public Task<object?> InvokeAsync(
  IEnumerable<KeyValuePair<string, object?>>? arguments = null,
  CancellationToken cancellationToken = default)

This doesn't really provide a nice way for the caller (e.g., FunctionInvokingChatClient) to add its own context argument, unless it does so by adding it as an entry in the arguments argument. Without changing anything else in M.E.AI, this would require the developer's AI function to:

1. Use a magic parameter name to receive the FunctionInvocationContext argument (which is inconsistent with how AIFunctionContext already works)
2. Potentially rely on multiple "context" objects (the AIFunctionContext and the FunctionInvocationContext)

We could fix these issues by:

1. Creating a new type that subclasses AIFunctionContext and adds an InvocationContext property of type FunctionInvocationContext so that you only ever need one context object
   • Note that this would introduce an extra allocation on each call to AI functions that don't accept a context object (since FunctionInvokingChatClient would create the new context object unconditionally).
2. Adding some extra logic in ReflectionAIFunction to detect when a value assignable to AIFunctionContext is provided via arguments, using that as the context object instead of constructing a new AIFunctionContext.

An alternative to 2.) might be adding an AIFunction.InvokeAsync() overload that accepts an explicit AIFunctionContext argument. This is a bit cleaner from the caller's perspective, but this makes AIFunction itself a bit hairy:

• The AIFunction abstraction lives in M.E.AI.Abstractions and is currently unaware of the concept of an AIFunctionContext, which lives with AIFunctionFactory in M.E.AI. So adding this overload requires moving existing abstractions (or introducing new ones).
• This new overload would require either changing the signature of the existing abstract AIFunction.InvokeCoreAsync(args, CancellationToken cancellationToken) method (big breaking change) or introducing a new virtual InvokeCoreAsync(args, AIFunctionContext functionContext) overload that calls the abstract InvokeCoreAsync overload by default.

Conclusion

I'm not a big fan of this general direction because:

1. The FunctionInvokingChatClient.FunctionInvocationContext type seems like it was primarily designed to be used by types deriving from FunctionInvokingChatClient, particularly when overriding the InvokeFunctionAsync() method. I'm inferring this primarily based on the fact that FunctionInvocationContext is a nested type with properties closely reflecting implementation details of FunctionInvokingChatClient.
2. It doesn't seem to fit cleanly with already-defined abstractions.

Therefore, it might be better to limit the scope of this feature to just terminating the function invocation loop rather than exposing the entire FunctionInvocationContext.

Option 2: Result type

Another option is to create some kind of result type for the caller to observe:

AIFunctionResult MyFunction(string otherParam1, int otherParam2)
{
  // ...
  return new AIFunctionResult()
  {
    Result = ...,
    RequestTermination = true,
    // ...
  }
}

AI functions could either return a result directly or return an AIFunctionResult.

The ReflectionAIFunction implementation (defined in AIFunctionFactory) would detect when the return value is an AIFunctionResult and return a new AIFunctionResult with a serialized Result property. The FunctionInvokingChatClient implementation would check if an AIFunction's return value is an AIFunctionResult and respect the RequestTermination property.

One potential problem with this is that existing external code might not expect an AIFunction to return an AIFunctionResult. We could partially mitigate this by adding a JSON converter for AIFunctionResult that forwards serialization to the Result object, so that directly serializing any value returned from InvokeAsync() works regardless of where the "actual result" lives. However, this probably falls apart when the developer brings their own JsonSerializerOptions. There might be other problems with this that I'm not thinking of (or solutions that alleviate this concern).

Option 3: Do nothing

Our stance could be that if you need to access any properties on the FunctionInvocationContext, you should derive FunctionInvokingChatClient and handle that logic yourself. This is reasonable if we think it's rare for an AI function to be the right place to access/manipulate state specific to an IChatClient implementation. We might need more real-world evidence that this scenario should be supported.

Thoughts, @SteveSandersonMS or @stephentoub?

[SteveSandersonMS]

Thanks for this analysis, @MackinnonBuck! This is very helpful and clarifies that there isn't just one obvious solution here.

The FunctionInvokingChatClient.FunctionInvocationContext type seems like it was primarily designed to be used by types deriving from FunctionInvokingChatClient

Agreed. FunctionInvocationContext also contains a bunch of mutable properties like Iteration, FunctionCallIndex, and FunctionCount. Mutating them from inside the function being executed wouldn't have any effect since the instance is discarded at the end of the call anyway, but it looks as if mutating them would do something. It would be confusing to expose these APIs to functions in general.

An alternative to 2.) might be adding an AIFunction.InvokeAsync() overload that accepts an explicit AIFunctionContext argument.

To me that does seem like a pretty decent approach. So there would be two overloads, i.e.:

    protected abstract Task<object?> InvokeCoreAsync(
        IEnumerable<KeyValuePair<string, object?>> arguments,
        CancellationToken cancellationToken);

    protected virtual Task<object?> InvokeCoreAsync(
        IEnumerable<KeyValuePair<string, object?>> arguments,
        AIFunctionContext context) => InvokeCoreAsync(arguments, context.CancellationToken);

Or perhaps better just a breaking change to have only the second signature, which is likely fine in practice as I don't expect the existing ecosystem to be subclassing AIFunction yet.

And:

• Move AIFunctionContext into MEAI.Abstractions
• Add a bool Terminate { get; set; } to AIFunctionContext

I think I prefer that over the result-based approach (option 2) since:

1. The result-based approach forces all future consumers of the AIFunction type to know about a magic result type that needs special-case handling. This contrasts with passing in AIFunctionContext which is a type of magic we already do and hence doesn't produce any new special cases.
2. Being able to supply extra info on a parameter could have other uses in the future. For example, say we also wanted to pass in some kind of activity ID or user ID or the IServiceProvider or whatever. That could be added to AIFunctionContext but couldn't be communicated through anything related to a return type.

We might need more real-world evidence that this scenario should be supported.

That's a very valid point to raise. Right now I only have two bits of supporting evidence:

1. SK has an equivalent to this "dynamic termination" feature, so presumably they added it for a reason
2. This issue itself suggests that @gyankov needed to set the flag (unless @gyankov was only reporting this for theoretical reasons and not due to a real-world requirement)

@stephentoub Do you have a clear sense of the concrete real-world need for the capability?

One final argument for doing it is that, if the above suggestion was our preferred approach, the fact that it's a breaking change gives us reason for doing it now when the break is almost free, and not in the future when it would be painful.

[stephentoub]

Thanks for the thoughts.

For completeness, there are a few other options:

• Option 4. Use a special exception to indicate termination; the body of the function would throw a FunctionInvocationTerminationException or some such thing, which would be special-cased by the FunctionInvokingChatClient.
• Opion 5. Use ambient state to flow the additional context around.

Even as the nominal owner of AsyncLocal<T>, I'm generally opposed to designs that rely on ambient context, but in this case, I think it could work out well. We'd have something like this:

public class FunctionInvokingChatClient
{
    private static readonly AsyncLocal<FunctionInvocationContext>s_currentContext = new();

    public static FunctionInvocationContext? CurrentContext
    {
        get => s_currentContext.Value;
        set => s_currentContext.Value = value;
    }

    protected virtual async Task<object?> InvokeFunctionAsync(FunctionInvocationContext context, CancellationToken cancellationToken) // existing
    {
        ...
        CurrentContext = context; // new
        result = await context.Function.InvokeAsync(context.CallContent.Arguments, cancellationToken).ConfigureAwait(false); // existing
        ...
    }
}

Then any code can query FunctionInvokingChatClient.CurrentContext in order to get the current context for the most recent FunctionInvokingChatClient performing the invocation. That has the really nice property that it keeps any logic related to FunctionInvokingChatClient isolated to the FunctionInvokingChatClient implementation, rather than needing to teach other components in Microsoft.Extensions.AI (like AIFunctionFactory and AIFunction) about it; user code can just query if it's needed, e.g.

AIFunction func = AIFunctionFactory.Create((int arg) =>
{
    if (arg == 0)
    {
        FunctionInvokingChatClient.CurrentContext!.Terminate = true;
        return "done";
    }

    return Compute(arg);
});

Having it isolated like this so as to not infiltrate the rest of the system is nice as well because it means there's nothing "special" about our FunctionInvokingChatClient; anyone else can implement the same thing without needing to reimplement AIFunction{Factory}.

As for real-world need, you can see other examples by searching around the SK repo, where similar functionality is exposed via the IAutoFunctionInvocationFilter:
https://github.com/microsoft/semantic-kernel/issues?q=is%3Aissue%20terminate%20auto%20invocation%20
The reason we started with having folks derive from FunctionInvokingChatClient to enable this scenario is because it seemed like a corner case: important for some scenarios, but not mainstream. Isolating via ambient state like this has the benefit of not promoting such a corner case to being something prominent in the rest of the system.

The downsides are the same as with other ambient context, as with Activity.Current, Transaction.Current, HttpContext.Current, etc... discoverability, overhead, etc.

[SteveSandersonMS]

The AsyncLocal approach does look nice.

I'm not sure how to quantify how problematic its downsides are. We don't generally think of AIFunction as being something that runs at very high frequency (e.g. millions of times per second) since the only scenario we care about is calling it in reaction to some language model output, which is never going to be that fast. So maybe the overhead isn't an issue. Many of the issues with HttpContext.Current are to do with state leakage across users if not handled carefully, but it seems less likely to be an issue here - perhaps in theory somebody's AIFunction execution could either get terminated or not terminated unexpectedly due to some incorrectly-written async code getting the wrong FunctionInvokingChatClient.CurrentContext.

It is a bit easier to shoot your own foot off with any static value since people could access and mutate FunctionInvokingChatClient.CurrentContext from literally anywhere without knowing what context they were accessing. I guess that fits into your general pattern of "same as with other ambient context". The design is still valid regardless.

The exception approach looks like a worse version of the "special return type" approach but I understand you were listing options for completeness :)

@MackinnonBuck What's your take?

[MackinnonBuck]

Thanks for all the comments, @SteveSandersonMS and @stephentoub!

I'm currently leaning toward something like what @SteveSandersonMS indicated as being his preference earlier in this thread:

• Move AIFunctionContext into M.E.AI.Abstractions
• Add an AIFunctionContext.Terminate property
• Make a breaking change in AIFunction by replacing the CancellationToken argument with an AIFunctionContext argument in InvokeCoreAsync

If you/others agree, I'd be happy to move forward with this approach.

The AsyncLocal<T> approach also seems like it would work. Although, another possible downside is that it doesn't allow an AI function to terminate the function invocation loop in a manner agnostic to the chat client implementation. Whereas, adding a Terminate property to the existing AIFunctionContext provides this functionality without requiring an AI function to care about how it's being invoked.