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

Adding support for new OpanAI API #25

Merged
merged 7 commits into from
Jul 13, 2024
Merged

Adding support for new OpanAI API #25

merged 7 commits into from
Jul 13, 2024

Conversation

benitezhm
Copy link
Contributor

Trying to add new OpenAI API endpoints that are marked with "beta" as of right now

Hello, first of all, thanks for the library, it saved some time for me already, however, I noticed there is no support for the new OpenAI API, therefore I followed your instructions to update the docs.yaml and also found some problems with the auto-generation so, this is my attempt to fix it, can you please have a look and advise, I'm afraid I may have missed something, any feedback is welcome I can take care of the changes.

Also, I added a new configuration http_client to facilitate testing, in my case I use mox.

@dvcrn
Copy link
Owner

dvcrn commented Jun 26, 2024

Thanks for taking the time to look into the generation!

I'm assuming that OpenAI changed the docs format again, that's why the conflicts. Let me take a look!

@dvcrn
Copy link
Owner

dvcrn commented Jun 26, 2024

It looks like the tests are breaking, could you take a look?

@benitezhm
Copy link
Contributor Author

I fixed the tests, however, I noticed that for allOf a better implementation is needed when converting the response, sadly my priorities changed right now, so I do not have much time to dedicate to this at the moment, but I did a quick fix, essentially I'm using the first component from the list of possible components to build the struct and this is the part that needs to be improved, if someone else can take it, it would be nice, if not perhaps I can retake it in the future.

@dvcrn
Copy link
Owner

dvcrn commented Jun 30, 2024

Thanks for taking the time to fix this! I'll take a look and take it over from here :)

lib/ex_openai/codegen.ex Outdated Show resolved Hide resolved
Co-authored-by: Aleksandr <aleksandr.mosel@gmail.com>
@@ -35,6 +35,6 @@ defmodule ExOpenAI.JasonEncoderTest do
]

assert encode_and_return(msgs) ==
"[{\"content\":\"Hello!\",\"role\":\"user\"},{\"content\":\"What's up?\",\"role\":\"assistant\"},{\"content\":\"What ist the color of the sky?\",\"role\":\"user\"}]"
"[{\"role\":\"user\",\"content\":\"Hello!\"},{\"role\":\"assistant\",\"content\":\"What's up?\"},{\"role\":\"user\",\"content\":\"What ist the color of the sky?\"}]"
Copy link
Contributor

@bulld0zer bulld0zer Jul 1, 2024

Choose a reason for hiding this comment

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

I guess this test might be flaky
So to avoid these changes in order of parameters, it's possible to do something like this:

res = encode_and_return(msgs)
assert res =~ "{\"content\":\"Hello!\",\"role\":\"user\"}"
assert res =~ "{\"role\":\"user\",\"content\":\"Hello!\"}"
assert res =~ "{\"role\":\"user\",\"content\":\"What ist the color of the sky?\"}"

@dbrody
Copy link

dbrody commented Jul 11, 2024

This will support the beta endpoints such as File Search and Vector Stores?
https://platform.openai.com/docs/assistants/tools/file-search/vector-stores

I would love to see that in this library!

@dvcrn
Copy link
Owner

dvcrn commented Jul 12, 2024

Apologies for the delay. I've already started working on this, but it's usually after work in the evenings. Hope to have this merged by weekend :)

@dvcrn dvcrn merged commit 985a1bf into dvcrn:main Jul 13, 2024
1 check failed
@dvcrn
Copy link
Owner

dvcrn commented Jul 13, 2024

I'll fix the remaining items (parsing of types, assistant API in a separate PR).

I noticed that for allOf a better implementation is needed when converting the response

@benitezhm do you have an example of this happening?

@dvcrn dvcrn mentioned this pull request Jul 13, 2024
%{"$ref" => ref} ->
{:component, String.replace(ref, "#/components/schemas/", "")}

%{"oneOf" => list} ->
Copy link
Owner

Choose a reason for hiding this comment

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

ah I'm seeing what you mean, it's this part here

@dvcrn
Copy link
Owner

dvcrn commented Jul 13, 2024

Okay I fixed handling of oneOf types in return spec over in #26

iex(36)> h ExOpenAI.Audio.create_transcription

               def create_transcription(file, model, opts \\ [])                

  @spec create_transcription(
          bitstring() | {String.t(), bitstring()},
          :"whisper-1" | String.t(),
          openai_organization_key: String.t(),
          openai_api_key: String.t(),
          "timestamp_granularities[]": [:segment | :word],
          temperature: float(),
          response_format: :vtt | :verbose_json | :srt | :text | :json,
          prompt: String.t(),
          language: String.t(),
          stream_to: (... -> any()) | pid()
        ) ::
          {:ok,
           ExOpenAI.Components.CreateTranscriptionResponseVerboseJson.t()
           | ExOpenAI.Components.CreateTranscriptionResponseJson.t()}
          | {:error, any()}

however, doing this messes with LSP/ElixirSense, so we can no longer have autosuggestion for struct fields. Open to suggestions.

@dbrody When you have time, could you give #26 a go? I've never used File Search or Vector Store so I couldn't properly try (and write a test) for it yet

@dvcrn
Copy link
Owner

dvcrn commented Jul 13, 2024

Alright, I've figured the proper way to represent oneOf in responses out:

{:ok, ExOpenAI.Components.CreateTranscriptionResponseVerboseJson.t()}
| {:ok, ExOpenAI.Components.CreateTranscriptionResponseJson.t()}
| {:error, any()}

I've merged everything back into master. Please give it a go, and if all is good we can cut a release

@dvcrn
Copy link
Owner

dvcrn commented Jul 18, 2024

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.

4 participants