Skip to content
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

Add support for "functions" in OpenAI chat completion APIs. #78

Merged
merged 3 commits into from
Jun 20, 2023

Conversation

ljoukov
Copy link
Contributor

@ljoukov ljoukov commented Jun 14, 2023

What

Add support for "function calling" in chat completion API: https://platform.openai.com/docs/guides/gpt/function-calling

Chat.content type was changed from String to String? as it is now optional and is actually null for function calls.

Why

OpenAI added support for function calling which need explicit API support.

Affected Areas

Added:
ChatFunctionCall
ChatFunctionDeclaration

Updated:
ChatQuery (describes what function calls are available)
Chat (responses from OpenAI calling functions)

Added example to README.md

@ljoukov
Copy link
Contributor Author

ljoukov commented Jun 14, 2023

This should close the issue: #76

@ljoukov
Copy link
Contributor Author

ljoukov commented Jun 15, 2023

Switched to encoding JSONSchema as swift structs (as opposed to dictionaries or arbitrary values), code is not better type checked and much easier to read.

See the diff:
eddd69c

@jakemor
Copy link

jakemor commented Jun 15, 2023

Tysm!

Copy link
Collaborator

@Krivoblotsky Krivoblotsky left a comment

Choose a reason for hiding this comment

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

hey, @ljoukov!
Thanks for the contribution!
Looks good to me, I'll do some tests in a while, and merge it.

@fiatdv
Copy link

fiatdv commented Jun 16, 2023

Seems like we need a bunch of good tests or some sample code of how to use functions in order to be truly usable? @ljoukov can you pls add some additional support for functions? Trying to get the name of the function to be called but it's not working.

@ljoukov
Copy link
Contributor Author

ljoukov commented Jun 16, 2023

Seems like we need a bunch of good tests or some sample code of how to use functions in order to be truly usable? @ljoukov can you pls add some additional support for functions? Trying to get the name of the function to be called but it's not working.

Hey @fiatdv did you check the example in README.md? It shows the response OpenAI gives you. Please make sure there is no misconception: the "functions" are not really functions are are not "called", OpenAI just returns a response with function name and parameter values:

https://github.com/MacPaw/OpenAI/pull/78/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5

@fiatdv
Copy link

fiatdv commented Jun 16, 2023

@ljoukov I am trying to run the weather example as shown here: https://platform.openai.com/docs/guides/gpt/function-calling
I am failing to see the response:

Step 2, check if the model wants to call a function

if message.get("function_call"):
    function_name = message["function_call"]["name"]
    function_args = json.loads(message["function_call"]["arguments"])

What I see is:

finishReason: Optional("function_call")

But no way to get the function_name or function_args because they are nil.

So, am wondering if this is working for you, or if you tried these steps. I know it is not going to call anything, but you should at least see these elements, right?

Maybe I am doing something wrong. Can you check? Thanks.

@fiatdv
Copy link

fiatdv commented Jun 16, 2023

Added a PR for testing purposes, here: ljoukov#1
to support testing of this function. It would be great to have a way to define functions and provide callbacks to them when triggered in the chat.

Update ChatStreamResult.swift
Make JSONSchema properties "let" and "public", also make "init" methods public.
Update Demo app to use function calls (adapted from fiatdv@)
@ljoukov
Copy link
Contributor Author

ljoukov commented Jun 16, 2023

Added a PR for testing purposes, here: ljoukov#1 to support testing of this function. It would be great to have a way to define functions and provide callbacks to them when triggered in the chat.

Great find! Indeed streaming was not working properly with functions. Also made JSONSchema members public so we no longer need to manually write JSON as strings and we can instead use strongly typed Swift structs:)
2620356

@fiatdv
Copy link

fiatdv commented Jun 16, 2023

Thanks @ljoukov !! Works like a charm! Maybe as another PR we can add a mechanism to provide functions and call them when matched?

@fiatdv
Copy link

fiatdv commented Jun 16, 2023

Actually... looking at it closer, we should not have the Weather Code inside the ChatStore. Perhaps it should be injected into the store and when a function match is issued, the function would be called and the result would be pasted into the messageText? This is will make it more expandable... maybe we can have a FunctionStore with a Dictionary of function names and callbacks. Not sure if there is a better way @ljoukov ?

@ljoukov
Copy link
Contributor Author

ljoukov commented Jun 16, 2023

Actually... looking at it closer, we should not have the Weather Code inside the ChatStore. Perhaps it should be injected into the store and when a function match is issued, the function would be called and the result would be pasted into the messageText? This is will make it more expandable... maybe we can have a FunctionStore with a Dictionary of function names and callbacks. Not sure if there is a better way @ljoukov ?

IMHO the API part is already at the right level, do you agree?
The demo app of course might be improved and real function calls added, for example, to provide current time, so it is easy to implement. Again this is supposed to be a demo for how to use the APIs, we probably want to postpone improvements to a separate PR.

@fiatdv
Copy link

fiatdv commented Jun 16, 2023 via email

@ljoukov
Copy link
Contributor Author

ljoukov commented Jun 17, 2023

Sure, agreed, many thanks for your contribution. The only thing, is please comment out the weather function and pass an empty functions array to avoid adding this to chat requests. Thanks again!

commenting out the code is likely not the best way to go. we should have a demo a function in a demo app, I can make it actually do something like current time so folks can use that. does this make sense?

@fiatdv
Copy link

fiatdv commented Jun 17, 2023 via email

@pgorzelany
Copy link

Any chance this will get merge and released anytime soon?

@Krivoblotsky Krivoblotsky merged commit b116159 into MacPaw:main Jun 20, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants