support for passing "functions" to chat completions

[ChuckJonas]

Open AI just released support for "Functions". This is a first pass at supporting that behavior.

https://openai.com/blog/function-calling-and-other-api-updates

NOTE: this has been lightly tested and seems to be working

[ChuckJonas]

These might not be the full set of supported types and some of the types might be missing optional properties... but it's a start

[lino-levan]

Good start, especially given the lack of docs on this. This seems fine to me.

[ChuckJonas]

Testing Array rn and seems to work... The docs also mention anyOf, but I assume they also support allOf, oneOf. Might just wait and add these in the future as we have time to test they actually work.

[ChuckJonas]

this is a somewhat "breaking" change, but with the introductions of functions, seems like this value can now be null

Open AI example response:

{
  "id": "chatcmpl-123",
  ...
  "choices": [{
    "index": 0,
    "message": {
      "role": "assistant",
      "content": null,
      "function_call": {
        "name": "get_current_weather",
        "arguments": "{ \"location\": \"Boston, MA\"}"
      }
    },
    "finish_reason": "function_call"
  }]
}

[lino-levan]

This feels really bad to me. hate to have to tell typescript to ignore the null every single time I need to extract the content.

I feel like we could do more intelligent typing by looking at the functions and function_call parameter to see if there is a possibility of the response being null. Would you be willing to try doing this?

[ChuckJonas]

ya, I had the same thought. Shouldn't be too hard to compose the types to get it to work that way

[ChuckJonas]

@lino-levan so... it is definitely possible, but I think it would require a much more complicated typings with generics and conditional types...

Typescript Playground showing a POC

A simpler solutions might just be to keep the type as is, but default the content property to an empty string if null...

[lino-levan]

I think part of the philosophy of this library is to try to pass the raw response as often as possible. Transforming the response even for something as minor as this would mean that people couldn't rely on open ai docs. The complex typings look nice, I'd accept this PR if it were to be added.

[ChuckJonas]

Accidentally edited the wrong comment (sorry for the mess). This previously said something like:

This is possible, but the deeply nested properties make the conditional typing very complex.

async createChatCompletion<T extends ChatCompletionOptions>(
    options: T
  ): Promise<
    T extends { functions: any }
      ? Omit<ChatCompletion, "choices"> & {
          choices: Omit<ChatCompletionChoice, "message"> & {
            message: Omit<ChatCompletionChoiceMessage, "content"> & {
              content: string | null;
            };
          };
        }
      : ChatCompletion
  > {

I think this would have a negative impact on developer experience. IMO, the "null coalesce" to "" is the superior solution, but I respect whatever you want to do.

I may actually abandon the PR anyway, because I'm having issues with the error handling (throwError) of this library swallowing status code and other raw request details.

[lino-levan]

IMO, the "null coalesce" to "" is the superior solution, but I respect whatever you want to do.

Thanks for the insight. I see where you're coming from on the typing front. Maybe worth revisiting in the future, but after further pondering I think a "null coalesce" is more than reasonable.

I may actually abandon the PR anyway, because I'm having issues with the error handling (throwError) of this library swallowing status code and other raw request details.

Error handling was kind of hacked in after the fact 😅. If you have any ideas on how to improve it, I'm all ears!

[ChuckJonas]

Created ticket #18 to track

[lino-levan]

Sounds good, thanks for the issue.

[load1n9]

LGTM can you add tests

[lino-levan]

Overall the code looks fine, but the typing could use some work. Please refer to my comment above.

[blakechambers]

Something to consider here: I played around with this some locally and ended up needing to extract a ChatCompletionMessage type. The problem I was seeing is there is some drift between the messages in ChatCompletion and ChatCompletionOptions – meaning I got some typing issues trying to pass message replies back in during subsequent requests.

[lino-levan]

I played around with this some locally and ended up needing to extract a ChatCompletionMessage type

Ah, very interesting. Seems like a good idea.

[ChuckJonas]

LGTM can you add tests

I don't see any actual unit tests in the project? I added an "example" file.

I also added the null coalesce... It's not the most beautiful solution, but as discussed, I'm not sure if there is a better option.

Let me know if there is anything else needed to get this out.

[lino-levan]

LGTM. Nice! Thanks for the comment explaining why we are null coalescing.

[load1n9]

LGTM

[lino-levan]

Thanks for the PR @ChuckJonas!