FunctionApprovalRequest get applied to all functions although expected to only one

[Lanayx]

I'm using 1.0.0-preview.251219.1 library version and exploring examples. When running Step14 I've noticed that approval is asked when agent tries calling both functions (so it's asked two times), although only one of them (GetWeather) is wrapped with ApprovalRequiredAIFunction

[stephentoub]

This is by design, assuming the calls are being made in the same response from the model. The function calling middleware can't call back to the model with only a subset of the requested calls, so it's forced to request approval for all the calls so that they can roundtrip. If you want to avoid that, you can disable parallel function calling.

[Lanayx]

@stephentoub I'm sorry, but it doesn't seem right to me. The problem is not with the middleware implemented in the example, but the result of the agent invocation, currently it returns both contents as required approval

Right after response none of functions require approval, debugging inside FromOpenAIResponse

So there is some internal mechanism (I haven't yet figured out where) that transforms function call into required for approval. And that should only transform functions that were wrapped with ApprovalRequiredAIFunction

[stephentoub]

The middleware I was referring to is the function invoking support built into the agent. But I can describe it again just taking about the agent. The model knows nothing about approvals; that's a concept manufactured by the agent. If the model requests two functuon calls as part of a single response ("parallel tool calls"), the model requires that both results to both calls come back in the next request. But if one of those calls is for a tool that requires approval, the agent can't send a request back to the model with just a result for the other call, and it has to return control to the user to get approval for the one requiring approval, so it has no choice but to send both function call requests back to the caller, and the cleanest way it has to do so is to require approval for all function calls in a request of any if them require approval.

https://github.com/dotnet/extensions/blob/main/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs#L48-L53

[Lanayx]

@stephentoub Thank you for referencing the FunctionInvokingChatClient, now I see how the transformation and tool calling is implemented.

But if one of those calls is for a tool that requires approval, the agent can't send a request back to the model with just a result for the other call, and it has to return control to the user to get approval for the one requiring approval

Fully agree with that.

so it has no choice but to send both function call requests back to the caller, and the cleanest way it has to do so is to require approval for all function calls in a request of any if them require approval.

I still don't see why it is required. From my point of view the correct flow should be the following:

1. Before the request CreateToolsMap should include "requiresApproval" flag to the dictionary for each function, rather than storing in in shared field.
2. When response mapping occurs, if flag is set for function - transform it into FunctionApprovalRequestContent, when there is no flag - execute the function and set FunctionResultContent
3. User will only give consent for FunctionApprovalRequestContent

[stephentoub]

But the FunctionInvokingChatClient can't store anything between invocations, and because there's an approval required for something, it can't send anything back to the model directly: all state must be returned to the client. And because there are services (like OpenAI Responses) that store all conversation state server side, there's no requirement that the client send back in the next request anything that's returned in the previous request; in fact doing so could break things. So there needs to be a special indication that the FunctionCallContent must be roundtripped to the FICC but not beyond, and specially handled to invoke the FICC's corresponding tool when it comes back (or alternatively a special indication that a FunctionResultContent created by the FCC must be roundtripped). That's effectively what the approval content does.

@westey-m

[Lanayx]

But the FunctionInvokingChatClient can't store anything between invocations, and because there's an approval required for something, it can't send anything back to the model directly: all state must be returned to the client.

There is no need to store anything between invocations in FunctionInvokingChatClient. Functions results are already stored as ChatMessage in its Contents.

And because there are services (like OpenAI Responses) that store all conversation state server side

It's not relevant as the process we are currently discussing is fully executed on client side

there's no requirement that the client send back in the next request anything that's returned in the previous request; in fact doing so could break things

Current approval logic already does that - user receives two function requests and has to manually construct FunctionApprovalResponseContent. For one function requiring approval he still requires to do that. For the second function he would need to copy FunctionResultContent back to the content list.

[stephentoub]

It's not relevant as the process we are currently discussing is fully executed on client side

It is relevant. If working with a service that stores the conversation server side, nothing previously yielded should be sent back to the server... if it is, it'll be considered to be in addition to everything previously yielded, which is problematic. As such, the FICC can't expect that FunctionCallContent or FunctionResultContent it yields back to the caller will be sent back to it on a subsequent request.

Current approval logic already does that

No, the caller sends back a new approval response content it creates, not an pproval request or response content it received.

For the second function he would need to copy FunctionResultContent back to the content list.

But the caller shouldn't do that in the general case. That's the point. If it sends back an FRC when working with a service that already has that result stored, it's erroneous.

[Lanayx]

No, the caller sends back the approval response content, not the approval request content.

I mean caller receives FunctionApprovalRequestContent and caller needs to manually transform it to FunctionApprovalResponseContent. Here is a simplified flow for the case where one function requires approval and another doesn't:
Currently:

1. FICC recieves 2 function calls from model, it transform both into FunctionApprovalRequestContent
2. Caller receives 2 approval requests, manually constructs two FunctionApprovalResponseContent and sends back to FICC
3. FICC unpacks approval responses, executes functions and sends request to the model

Suggested approach:

1. FICC recieves 2 function calls from model, it transform only one into FunctionApprovalRequestContent, another to FunctionResultContent (by executing second function)
2. Caller receives 1 approval request, manually constructs one FunctionApprovalResponseContent, copies FunctionResultContent for second function and sends back to FICC
3. FICC unpacks one approval response, executes function, adds second FunctionResultContent that user copied and sends request to the model

As you can see this is irrelevant to the message history storage, because in both cases caller sends only one user ChatMessage, but constructs its contents list manually. As an alternative - it's possible to leave FunctionCall for the second function rather than executing it immediately and storing FunctionResultContent, so it will be executed together with approved function after user's response.

[stephentoub]

copies FunctionResultContent for second function and sends back to FICC

As I've stated, it shouldn't do this. FICC sends back FunctionResultContents representing results already exposed to the model / possibly already in server side history. The caller would need to be able to differentiate FRCs representing results already exposed to the model from those that aren't.

[Lanayx]

The caller would need to be able to differentiate FRCs representing results already exposed to the model from those that aren't.

This depends on the API. Currently it is expected that caller analyzes response.UserInputRequests collection and FunctionApprovalRequestContent is present there while FunctionResultContent is not present there.

FICC sends back FunctionResultContents representing results already exposed to the model / possibly already in server side history

Here I assume that UserInputRequests only contains contents that are not yet exposed to the model.

So, we just need to add one more type derived from UserInputRequestContent (something like FunctionExecutedRequestContent) there so caller will be able to figure out that it should be copied to user message along with approval response by filtering out that new type. But as I think about it more, it looks like there might be a better way.

Currently FunctionApprovalRequestContent is constructed separately for each function, which is cumbersome for user and really is inconvenient to work with (and it also doesn't make much sense since "requiresApproval" field is already shared). Alternatively, we can construct it only once for all the "parallel" functions in the model response, inside it the array with approvals for each functions will be stored. In this way user will be able to approve all requested function approvals at once (if more than one of them requires approval)

In our example case, it will store two function contents inside, one FunctionApprovalRequestContent and one FunctionResultContent, as soon as caller executes functionApprovalRequest.CreateResponse(true), approval object will copy the underlying structure of FunctionApprovalRequestContent, so FICC will see both approved content and already present result.

[stephentoub]

add one more type derived from UserInputRequestContent

Right, it can't just be the raw FunctionCallContent/FunctionResultContent, and instead needs to be wrapped in some UserInputRequestContent so that the caller knows it needs to be handled specially. Which is exactly what happens today, it gets wrapped in a FunctionApprovalRequestContent. We made the explicit decision to just do that rather than introduce yet a other type that would force special handling. If the caller wants to match those against functions that don't actually require approval, it can; they can treat them like any other approval-required content, or they can special-case them, but they're not forced to special-case them. As noted, this can also be avoided entirely by ensuring parallel tool calling is disabled.

and it also doesn't make much sense since "requiresApproval" field is already shared

I don't know what you mean by this. Any AIFunctionDeclaration sent into the ChatClientAgent that's actually an Approval Required AIFunction will require approval.

and it also doesn't make much sense since "requiresApproval" field is already shared

Eye of the beholder. I don't see it as being cumbersome nor do I see how having all the unrelated FCC's grouped into a single instance is any less cumbersome than having one approval content for each. And it's perfectly valid for some function invocations to be approved while others are rejected;; leading the user to need to treat them as a single unit is a pit of failure.

[Lanayx]

If the caller wants to match those against functions that don't actually require approval, it can; they can treat them like any other approval-required content, or they can special-case them, but they're not forced to special-case them.

This is a problem - developer (who handles response processing) can't actually tell if this function really requires approval, or it was added because another function requested it. So at the very least there should be an additional field inside FunctionApprovalRequestContent telling that approval for the particular funciton is not actually required (although if feels hacky), for example CanBeAutoApproved.

I don't know what you mean by this.

I meant anyToolsRequireApproval field inside FICC

Eye of the beholder. I don't see it as being cumbersome nor do I see how having all the unrelated FCC's grouped into a single instance is any less cumbersome than having one approval content for each. And it's perfectly valid for some function invocations to be approved while others are rejected;; leading the user to need to treat them as a single unit is a pit of failure.

Let's say I'm using copilot and I'm requesting some task, also giving it 50 tools, 45 of which don't require approval and 5 do. Model returns 5 calls, 4 don't require approvals and 1 does. As a copilot developer I'd like only to ask user about 1 approval rather than 5. With current implementation the only option is to maintain own copy of tool map and match functions by name, while ideally I should receive this information from the MAF, that only one of 5 calls requires approval. Grouping into single instance will give visibility, so developer can clearly see which functions have already executed, which are yet to be executed and which require explicit approval.

[stephentoub]

As a copilot developer I'd like only to ask user about 1 approval rather than 5. With current implementation the only option is to maintain own copy of tool map and match functions by name

Any such AI tool is almost certainly going to need to be doing further filtering, eg by arguments to the functions. The approval at the tool level is thus a min bar and frequently the consumer will be automatically approving some portion of those that come back requesting approval.

(Such tools frequently also disable parallel tool calling, at which point, again, this is a non-issue.)

I meant anyToolsRequireApproval field inside FICC

That is an implementation detail which has no bearing on how things are exposed.

[Lanayx]

Any such AI tool is almost certainly going to need to be doing further filtering, eg by arguments to the functions. The approval at the tool level is thus a min bar and frequently the consumer will be automatically approving some portion of those that come back requesting approval.

Well, it might require more filtering or not, it's up to developer and use case. Other tools also provide per-tool approval with ability to switch based on invocation state - example. The behavior where framework returns incorrect information about required tool approval shouldn't be regarded correct, but rather buggy. I hope more users will find this issue and vote for fixing it.

(Such tools frequently also disable parallel tool calling, at which point, again, this is a non-issue.)

Disabling parallel tool calling eats more tokens and is slower from UX perspective, so I'd regard as a poor-man workaround rather than recommended default.