-
Notifications
You must be signed in to change notification settings - Fork 772
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[AI] FunctionCallContent is not included in StreamingChatCompletionUpdate when KeepFunctionCallingMessages enabled #5909
Comments
This is by design, though of course we could revisit the design. The reason for it is called out here: extensions/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs Lines 344 to 349 in 1146e90
Essentially, a client that takes all the content and just adds it back into the history will end up with an erroneous history that includes FunctionCallContent without corresponding FunctionResultContent, or possibly duplicated FunctionCallContents. So the implementation is removing them from the stream but including them in the history (when KeepFunctionCallingMessages is true, which is the default). If we change that to stream the function call messages, we'd need to come up with some other workaround, and that can't just be telling developers not to include FRCs in the history. We have helpers like ToChatCompletion that will convert updates into complete messages, and such helpers shouldn't have baked in policy that excludes FunctionCallContent, because they have no knowledge of whether a particular FunctionInvokingChatClient is in use. Off the top of my head, one fallback could be to add this as another knob on FunctionInvokingChatClient, e.g. a StreamFunctionCallContent property, that defaults to false but that you could set to true if you want that behavior. Another option would be to yield the FunctionCallContent and then also yield the manufactured FunctionResultContent, either instead of or in addition to having it in the chat history. That way at least they'd still be paired up. I suspect some services would still balk, though, at ending up with the same tool call ID multiple times in the same history. We could explore other combinations, as well. cc: @SteveSandersonMS in case he has other ideas |
For this part specifically, though, it sounds like you actually want much more control over the invocation. Presumably you want to know not just that a function has been requested, but you'd want to display in your UI details of the invocation, that it's currently in-flight, when it's ended, maybe even details about what was computed, and I don't see FunctionInvokingChatClient evolving to serve all possible such cases. At some point, for specific scenarios where you want more control, you'll likely want to process the FunctionCallContent yourself. |
I appreciate your quick response. Thank you anyway. I will see if I can implement my own solution then. |
In terms of performing UI updates, the way I prefer to approach it is having the UI state computed from the message history (i.e., the As long as the UI knows to refresh when function calls are performed, it will have access to the
I find this code comment really confusing. Is there a typo in here making it say the opposite of what it means, or am I just failing the parse the sentence properly? It seems to say "we are going to emit the FCCs, because doing so can lead to a problem" (which is weird), and then the code proceeds to strip out and hence not emit the FCCs. Should the comment say "we are not going to emit the FCCs..."? I'm not even sure which direction is "upstream" (perhaps phrasing it as "back to the caller" or "to the inner client" would be clearer).
That sounds good. But why would we have the same tool call IDs multiple times? Wouldn't we also remove the existing code that adds FCCs/FRCs into history when streaming, so they only get added via the new code path? |
Oh wait. It seems you are already rephrasing that comment in #5910. Your updated phrasing is clearer. |
Interesting. So change streaming to never add the FCC/FRC content to the history, and KeepFunctionCallingMessages for streaming means whether they're yielded or not rather than whether they're stored or not? |
That is what I was thinking but haven't gone as far as to validate how the resulting consumer code would look. Thinking one step further, it might not work. We'd expect people to add the streamed response to history using We'd need to change Or perhaps replace void AppendToHistory(this IEnumerable<ChatResponseUpdate> updates, IList<ChatMessage> history); ... which could create as many I know for sure that doing this change would break some apps and invalidate some code that otherwise looks fine. For example the new chat template concatenates streamed updates manually because it was easier and shorter to do so, and that would start discarding the FCCs/FRCs unless the logic was updated. So there is a case that having |
Thanks. And yeah, those all relate to why I think the current behavior might be the safer default but have an additional knob to let you opt-into alternate behavior, such as what you suggest. |
Description
When using
Microsoft.Extensions.AI
with function calling and streaming, theFunctionCallContent
is not included, even withKeepFunctionCallingMessages
enabled.The reason I need this is to notify my front-end that a function call is busy. Of course, I could implement the whole thing myself, but what's the point of having it if it doesn't serve this purpose?
Reproduction Steps
Minimal example code to repreduce this.
Expected behavior
MessageUpdate with FunctionCallContent when KeepFunctionCallingMessages is enabled.
Actual behavior
FunctionCallContent, FunctionResultContent is not included in the updates
Regression?
No response
Known Workarounds
No response
Configuration
.net 9
Other information
No response
The text was updated successfully, but these errors were encountered: