feat(go/plugins): Generic OpenAI plugin #2440
Conversation
feat(openai plugin): Initial openai plugin
feat(openai plugin): Add embedding support
| initted bool | ||
| client *openaiGo.Client | ||
| Opts []option.RequestOption | ||
| Provider string |
There was a problem hiding this comment.
Some comments here would be good. What is provider?
There was a problem hiding this comment.
Hmm, don't see anything here.
There was a problem hiding this comment.
I added a small comment here: https://github.com/kekoawong/genkit/blob/efc737682364ddb300b99259ac0b0a7bab7dae47/go/plugins/compat_oai/compat_oai.go#L52 I can add more info
There was a problem hiding this comment.
We'd like comments on each field of OpenAICompatible. As a user, how do I know what you put for Provider? Comment explaining what it is and examples of what goes in there is helpful without looking up documentation.
There was a problem hiding this comment.
| }, | ||
| { | ||
| name: "float and int fields", | ||
| config: &openai.ChatCompletionNewParams{ |
There was a problem hiding this comment.
We should try not to depend directly on the config type defined elsewhere (like in 3P SDKs) so that the SDKs are implementation details and we could in theory swap out to another or even call the REST API directly. We sometimes implement configuration via native Genkit features so if we have it both natively and in the config, it will be confusing as to what happens when you combine them.
There was a problem hiding this comment.
check new implementation here: https://github.com/kekoawong/genkit/pull/13/files#r2048072574
| t.Log("genkit initialized") | ||
|
|
||
| // Initialize the OpenAI plugin | ||
| apiKeyOption := option.WithAPIKey(apiKey) |
There was a problem hiding this comment.
Which other options do we (want to) support? Maybe APIKey should be a top level field in OpenAI like it is in GoogleAI? Better to err on the side of being simple and complete rather than supporting everything that the SDK supports.
feat(go/openai plugin): support additional OpenAI models
| ) | ||
|
|
||
| type OpenAI struct { | ||
| Opts []option.RequestOption |
There was a problem hiding this comment.
This Opts field is never used, in other words, the current implementation does not handle the passed options.
For example, even if the API key is passed via the option like this, the OpenAI client is reading the environment variable instead:
apiKeyOption := option.WithAPIKey("sk-xxxxx")
oai := &openai.OpenAI{
Opts: []option.RequestOption{apiKeyOption},
}There was a problem hiding this comment.
+1, we might want to just bring out APIKey as a top level field in the OpenAI struct.
There was a problem hiding this comment.
fixed in here: kekoawong#11
| t.Logf("streaming response: %+v", finalOutput) | ||
| }) | ||
|
|
||
| t.Run("tool usage with basic completion", func(t *testing.T) { |
There was a problem hiding this comment.
This function calling test is flaky depending on the model. It seems to be related to math.Pow(), and I'll give it a try to fix it.
There was a problem hiding this comment.
I believe this is fixed already
There was a problem hiding this comment.
Sorry, I haven't done this yet. What I’ve found so far is that the math.Pow() function returns a value between 11 and 13, depending on how the model generates code and handles floating-point values internally. Since all tests are currently passing and the issue isn’t critical, I’d leave it as is for now and revise it if it comes up again.
| t.Logf("tool usage with basic completion response: %+v", out) | ||
| }) | ||
|
|
||
| t.Run("tool usage with streaming", func(t *testing.T) { |
There was a problem hiding this comment.
It's flaky, just like the function calling test with basic completion.
There was a problem hiding this comment.
I believe this is fixed already
There was a problem hiding this comment.
Same comment: #2440 (comment)
…ls-and-versions chore(go/openai plugin): add OpenAI model details and versions
chore(refactor): added minor refactors before merging
| } | ||
|
|
||
| // DefineModel defines a model in the registry | ||
| func (o *OpenAICompatible) DefineModel(g *genkit.Genkit, name, provider string, info ai.ModelInfo) (ai.Model, error) { |
There was a problem hiding this comment.
| func (o *OpenAICompatible) DefineModel(g *genkit.Genkit, name, provider string, info ai.ModelInfo) (ai.Model, error) { | |
| func (o *OpenAICompatible) DefineModel(g *genkit.Genkit, provider, name string, info ai.ModelInfo) (ai.Model, error) { |
| } | ||
|
|
||
| // DefineEmbedder defines an embedder with a given name. | ||
| func (o *OpenAICompatible) DefineEmbedder(g *genkit.Genkit, name string, provider string) (ai.Embedder, error) { |
There was a problem hiding this comment.
| func (o *OpenAICompatible) DefineEmbedder(g *genkit.Genkit, name string, provider string) (ai.Embedder, error) { | |
| func (o *OpenAICompatible) DefineEmbedder(g *genkit.Genkit, provider, name string) (ai.Embedder, error) { |
feat(go): add Anthropic models
feat(go): add OpenAI GPT-4.1 family models
Adding support for an openai go plugin, along with additional generic support for any providers compatible with the openai (from feature request #2204). Collaboration between @xavidop @yukinagae @kekoawong.
Checklist: