Skip to content

.Net: Function calling: Add function author role and name property - #3302

Closed
Gina Triolo (gitri-ms) wants to merge 6 commits into
microsoft:mainfrom
gitri-ms:author-role-function
Closed

Gina Triolo (gitri-ms) wants to merge 6 commits into
microsoft:mainfrom
gitri-ms:author-role-function

Conversation

@gitri-ms

Copy link
Copy Markdown
Contributor

Motivation and Context

Resolves #3017 and enables the full function calling scenario as described in the OpenAI function calling documentation.

Description

Adds the "function" author role to allow adding function messages to chat history. Function messages require a "name" parameter, so this change also adds that to chat message model.

Contribution Checklist

@gitri-ms Gina Triolo (gitri-ms) added the PR: ready for review All feedback addressed, ready for reviews label Oct 26, 2023
@gitri-ms
Gina Triolo (gitri-ms) requested a review from a team as a code owner October 26, 2023 06:24
@shawncal Shawn Callegari (shawncal) added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel kernel.core labels Oct 26, 2023
@github-actions github-actions Bot changed the title Function calling: Add function author role and name property .Net: Function calling: Add function author role and name property Oct 26, 2023
/// <summary>
/// If role is function, the name of the function that produced the content
/// </summary>
public string? Name { get; set; } = null;

@gitri-ms Gina Triolo (gitri-ms) Oct 26, 2023 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surfacing Stephen Toub (@stephentoub) 's suggestion from the issue (#3017):

If we're not comfortable adding a name property to a chat message in support of this, we could also just add a dictionary to the message so that additional context can be provided and interpreted by the chat completion service however it likes:
main...stephentoub:semantic-kernel:fixfunctioncalling

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gina Triolo (@gitri-ms) I would not change Abstractions regarding FunctionCalling support, instead I would think of having a Specialized ChatHistory class for OpenAI where you can add that function.
On the examples where we know you are dealing with a OpenAIModel, you can check if the ChatHistory is an OpenAIChatHistory and play with the detailed function data.

SKChatMessage of a OpenAIChatHistory will have the name.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the concern here, but if we don't have a way to support additional fields like this in the base object, then devs will have to downcast the ChatHistory whenever they want to interact with OpenAI features, which makes for a really clunky experience. Talking with Ben Thomas (@alliscode), we thought that Stephen Toub (@stephentoub)'s approach would be most flexible and future-proof, so I updated this PR to add an optional dictionary to ChatMessageBase.

@leungkimming

Michael Leung (leungkimming) commented Oct 26, 2023 •

Copy link
Copy Markdown

I found that your version has an issue. I have a CheckStock function and my prompt is "How many tables and chairs are there in stock?". Note that LLM should issue 2 function calling messages (I have added a do-while loop in Example59). However, with your version, LLM only called CheckStock once. In my original working version using native OpenAI, the Messages in the ChatCompletionsOptions keeps both the function calling assistance message and the function return user message. In your solution, only function return user
messages are kept in the abstracted ChatMessageBase. Hence, in ClientBase.cs, when CreateChatCompletionsOptions populate ChatCompletionsOptions using the abstracted ChatMessageBase, there are no function calling assistance message histories passing back to the LLM. Hence, LLM failed to relate the function result message with the original function calling message.

Below is my Example59 source code. I asked for 2 stock items but LLM only call CheckStock once. The reason is because the function calling message was missing in the chat history.

Example59_OpenAIFunctionCalling.zip

/// <summary>
/// If role is function, the name of the function that produced the content
/// </summary>
public string? Name { get; set; } = null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gina Triolo (@gitri-ms) I would not change Abstractions regarding FunctionCalling support, instead I would think of having a Specialized ChatHistory class for OpenAI where you can add that function.
On the examples where we know you are dealing with a OpenAIModel, you can check if the ChatHistory is an OpenAIChatHistory and play with the detailed function data.

SKChatMessage of a OpenAIChatHistory will have the name.

if (kernel.Functions.TryGetFunctionAndContext(functionResponse, out ISKFunction? func, out ContextVariables? context))
{
var kernelResult = await kernel.RunAsync(func, context);
var result = (await kernel.RunAsync(func, context)).GetValue<object>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we intentionally skip adding an LLM function call request to the chat history before adding the function execution result?

image Link - https://platform.openai.com/docs/guides/gpt/function-calling

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I think i just overlooked that. However, now that you've pointed it out, it's not as simple as it seems to add the OpenAI message back into chat history. We can access the message via IChatResult.ModelResult.GetResult<ChatModelResult>().Choice.Message, which gives us back an Azure.AI.OpenAI.ChatMessage object. However, I dont believe we have the capability to add that back into chat history without losing some information in the translation (would convert to our ChatMessage to add to ChatHistory, and then back to Azure.AI.OpenAI.ChatMessage in ClientBase.cs)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually.... I think we are able to handle this with the dictionary approach. When adding this message back into chat history, would need to add key/value for "function_call" into the dictionary and make sure that gets copied back into the OAI message. I will update the PR with this.

@gitri-ms Gina Triolo (gitri-ms) added PR: feedback to address Waiting for PR owner to address comments/questions and removed PR: ready for review All feedback addressed, ready for reviews labels Oct 30, 2023
@gitri-ms Gina Triolo (gitri-ms) added PR: in progress Under development and/or addressing feedback and removed PR: feedback to address Waiting for PR owner to address comments/questions labels Oct 31, 2023
validRole != ChatRole.Function)
{
throw new ArgumentException($"Invalid chat message author role: {role}");
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should consider removing this role validation altogether. If the role is invalid according to OpenAI, it'll eventually tell us that. The problem with this validation is if OpenAI adds more valid roles, this validation will proactively block their use and will require changes to SK and picking up a new package in order to be able to use them, even if they'd otherwise work.

It's similar to how .NET used to proactively try to validate paths being passed to File/Path APIs, but now that validation has been removed and we just rely on the OS to say whether there was something wrong with the path.

/// If there is no plugin name, this is the same as the function name.
/// </summary>
public string FullyQualifiedName =>
this.PluginName.IsNullOrEmpty() ? this.FunctionName : string.Join(OpenAIFunction.NameSeparator, this.PluginName, this.FunctionName);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

string.Join(..., params string[]) is unnecessary expense here. This can instead be:

Suggested change
this.PluginName.IsNullOrEmpty() ? this.FunctionName : string.Join(OpenAIFunction.NameSeparator, this.PluginName, this.FunctionName);
this.PluginName.IsNullOrEmpty() ? this.FunctionName : $"{this.PluginName}{OpenAIFunction.NameSeparator}{this.FunctionName}";

or

Suggested change
this.PluginName.IsNullOrEmpty() ? this.FunctionName : string.Join(OpenAIFunction.NameSeparator, this.PluginName, this.FunctionName);
this.PluginName.IsNullOrEmpty() ? this.FunctionName : this.PluginName + OpenAIFunction.NameSeparator + this.FunctionName;

or

Suggested change
this.PluginName.IsNullOrEmpty() ? this.FunctionName : string.Join(OpenAIFunction.NameSeparator, this.PluginName, this.FunctionName);
this.PluginName.IsNullOrEmpty() ? this.FunctionName : string.Concat(this.PluginName, OpenAIFunction.NameSeparator, this.FunctionName);

all of which will compile down identically.

/// <summary>
/// The role that provides information about a function call result.
/// </summary>
public static readonly AuthorRole Function = new("function");

@stephentoub Stephen Toub (stephentoub) Nov 1, 2023 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these System/Assistant/User/Tool/Function fields should be changed to properties, e.g.

public static AuthorRole Function { get; } = new("function");

/// <summary>
/// Dictionary for any additional message properties
/// </summary>
public IDictionary<string, string>? AdditionalContext { get; set; }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize I came up with the AdditionalContext name, but we might want to consider alternatives, like AdditionalProperties.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like AdditionalProperties -- avoids confusion about the term "context" with SKContext / context variables

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing this.

@leungkimming

Copy link
Copy Markdown

Gina Triolo (@gitri-ms) As mentioned above, we need to add LLM function call request to the chat history before adding the function execution result. Please consider my suggestions below:

  1. Connectors.AI.OpenAI\AzureSdk\ClientBase.cs
foreach (var message in chatHistory)
{
    var azureMessage = new ChatMessage(GetValidChatRole(message.Role), message.Content);
    string functionName = "";
    if (message.AdditionalContext?.TryGetValue("Name", out string? name) is true)
    {
        azureMessage.Name = name;
        functionName = name;
    }

    if (message.AdditionalContext?.TryGetValue("Arguments", out string? arguments) is true)
    {
        azureMessage.FunctionCall = new FunctionCall(functionName, arguments);
    }

    options.Messages.Add(azureMessage);
}
  1. Connectors\Connectors.AI.OpenAI\ChatCompletion\ChatHistoryExtensions.cs
public static void AddAssistantMessage(this ChatHistory chatHistory, IChatResult chatResult)
{
    FunctionCall fc = chatResult.ModelResult.GetOpenAIChatResult().Choice.Message.FunctionCall;
    chatHistory.AddMessage(AuthorRole.Assistant, "", new Dictionary<string, string> {
        { "Name", fc.Name },
        { "Arguments", fc.Arguments }
    });
}
  1. Example59_OpenAIFunctionCalling.cs
// Check for message response
var chatMessage = await chatResult.GetChatMessageAsync();
if (!string.IsNullOrEmpty(chatMessage.Content))
{
    Console.WriteLine(chatMessage.Content);

    // Add the response to chat history
    chatHistory.AddAssistantMessage(chatMessage.Content);
}
// Check for function response
OpenAIFunctionResponse? functionResponse = chatResult.GetFunctionResponse();
if (functionResponse is not null)
{
    chatHistory.AddAssistantMessage(chatResult);

@gitri-ms

Copy link
Copy Markdown
Contributor Author

Michael Leung (@leungkimming) - Thanks for the suggestions, I am incorporating them and will have updated changes soon!

@leungkimming

Copy link
Copy Markdown

Gina Triolo (@gitri-ms) I am sorry that there is a minor bug in my suggested coding. In the original OpenAI function calling assistance message, the Name field should be null but in my coding, the Name field contains the function name. Only the function execution result message should have a value in the Name field. Below is the correction in the ClientBase.cs file.

foreach (var message in chatHistory)
{
    var azureMessage = new ChatMessage(new ChatRole(message.Role.Label), message.Content);
    string? functionName = null;

    if (message.AdditionalProperties?.TryGetValue("Name", out string? name) is true)
    {
        functionName = name;
    }

    if (message.AdditionalProperties?.TryGetValue("Arguments", out string? arguments) is true)
    {
        azureMessage.FunctionCall = new FunctionCall(functionName, arguments);
    } else
    {
        azureMessage.Name = functionName;
    }

    options.Messages.Add(azureMessage);
}

In addition, to demonstrate multiple function calling, I would like to suggest below changes to the Example59_OpenAIFunctionCalling.cs file.

  1. Make the second prompt to call both the TimePlugin-Date and KlarnaShoppingPlugin
  2. Add a do loop until CompletionsFinishReason.Stopped
  3. When passing function execution result, change to FunctionCallAuto instead of FunctionCallNone.
// Set FunctionCall to the name of a specific function to force the model to use that function.
requestSettings.FunctionCall = "TimePlugin-Date";
await CompleteChatWithFunctionsAsync("What day is today?", chatHistory, chatCompletion, kernel, requestSettings);

// Set FunctionCall to auto to let the model choose the best function to use.
requestSettings.FunctionCall = OpenAIRequestSettings.FunctionCallAuto;
await CompleteChatWithFunctionsAsync("What day is today and What computer tablets are available for under $200?", chatHistory, chatCompletion, kernel, requestSettings);

...

private static async Task CompleteChatWithFunctionsAsync(string ask, ChatHistory chatHistory, IChatCompletion chatCompletion, IKernel kernel, OpenAIRequestSettings requestSettings)
{
    Console.WriteLine($"User message: {ask}");
    chatHistory.AddUserMessage(ask);
    IChatResult chatResult;

    // Send request and add response to chat history
    chatResult = (await chatCompletion.GetChatCompletionsAsync(chatHistory, requestSettings))[0];
    chatHistory.AddAssistantMessage(chatResult);

    await PrintChatResultAsync(chatResult);
    do
    {
        // Check for function response
        OpenAIFunctionResponse? functionResponse = chatResult.GetFunctionResponse();
        if (functionResponse is not null)
        {
            // If the function returned by OpenAI is an SKFunction registered with the kernel,
            // you can invoke it using the following code.
            if (kernel.Functions.TryGetFunctionAndContext(functionResponse, out ISKFunction? func, out ContextVariables? context))
            {
                var result = (await kernel.RunAsync(func, context)).GetValue<object>();

                string? resultContent = null;
                if (result is RestApiOperationResponse apiResponse)
                {
                    resultContent = apiResponse.Content?.ToString();
                }
                else if (result is string str)
                {
                    resultContent = str;
                }

                if (!string.IsNullOrEmpty(resultContent))
                {
                    // Add the function result to chat history
                    chatHistory.AddFunctionMessage(resultContent, functionResponse.FullyQualifiedName);

                    //Get another completion
                    requestSettings.FunctionCall = OpenAIRequestSettings.FunctionCallAuto;
                    chatResult = (await chatCompletion.GetChatCompletionsAsync(chatHistory, requestSettings))[0];
                    chatHistory.AddAssistantMessage(chatResult);

                    await PrintChatResultAsync(chatResult);
                }
            }
            else
            {
                Console.WriteLine($"Error: Function {functionResponse.PluginName}.{functionResponse.FunctionName} not found.");
            }
        }
    } while (chatResult.ModelResult.GetOpenAIChatResult().Choice.FinishReason != CompletionsFinishReason.Stopped);
}

{
var chatMessage = chatResult.ModelResult.GetOpenAIChatResult().Choice.Message;

if (!chatMessage.Content.IsNullOrEmpty())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can a chat message have both text and a function call? The code suggests that it's possible.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SergeyMenshykh As you have quoted from OpenAI documentation,

messages.append(response_message)  # extend conversation with assistant's reply

I believe that we should simply extend the response_message from LLM, regardless of its content to the chat history. The response_message may contain message or function call or both because OpenAI's API reference does not explicitly mention that they cannot co-exist. You are correct that if they both exist, the code will generate two messages instead of one. Therefore, I suggest below changes:

public static void AddAssistantMessage(this ChatHistory chatHistory, IChatResult chatResult)
{
    var chatMessage = chatResult.ModelResult.GetOpenAIChatResult().Choice.Message;

    var functionCall = chatMessage.FunctionCall;
    if (functionCall is not null)
    {
        chatHistory.AddMessage(AuthorRole.Assistant, chatMessage.Content, new Dictionary<string, string>
        {
            { "Name", functionCall.Name },
            { "Arguments", functionCall.Arguments }
        });
    } else
    {
        chatHistory.AddAssistantMessage(chatMessage.Content);
    }
}

@markwallace-microsoft

Copy link
Copy Markdown
Contributor

Created a new PR here: #3421

@markwallace-microsoft

Copy link
Copy Markdown
Contributor

Michael Leung (@leungkimming) I have created a new PR here: #3421 to continue this work. Could you try out the latest changes (just minor updates) and let me know if it works for your scenarios. Thanks

@leungkimming

Copy link
Copy Markdown

Michael Leung (@leungkimming) I have created a new PR here: #3421 to continue this work. Could you try out the latest changes (just minor updates) and let me know if it works for your scenarios. Thanks

@ markwallace-microsoft Thankyou. It works for my scenario. However, I have a minor suggestion. Using chatResult.GetOpenAIFunctionResponse to check if not null to execute functions may have problem if OpenAI set the CompletionsFinishReason other than "stop" or "function_call". Shall we add a "GetOpenAIFinishReason" in ChatResultExtensions.cs and let us properly check below values to determine the next action to take?
private const string StoppedValue = "stop";
private const string TokenLimitReachedValue = "length";
private const string ContentFilteredValue = "content_filter";
private const string FunctionCallValue = "function_call";

I am using chatResult.ModelResult.GetOpenAIChatResult().Choice.FinishReason but not quite elegant!

github-merge-queue Bot pushed a commit that referenced this pull request Nov 9, 2023
…3421)

### Motivation and Context

Replaces #3302

Resolves #3017 and enables the full function calling scenario as
described in the [OpenAI function calling
documentation](https://platform.openai.com/docs/guides/gpt/function-calling).

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->
Adds the "function" author role to allow adding function messages to
chat history. Function messages require a "name" parameter, so this
change also adds that to chat message model.

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

---------

Co-authored-by: Gina Triolo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code PR: in progress Under development and/or addressing feedback

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

.Net: Function calling: Update ChatMessage to support 'name'

7 participants