Conversation
reVrost
left a comment
reVrost
left a comment
There was a problem hiding this comment.
Thanks @nitschmann , just a few small comments
embeddings.go
Outdated
| ) (response EmbeddingsResponse, err error) { | ||
| if !isSupportingModel(embeddingsSuffix, request.Model) { | ||
| // Keep behavior consistent with chat/completions: let the server return a | ||
| // proper API error until we implement local model validation. |
There was a problem hiding this comment.
Are we potentially missing a return err here? otherwise we could remove this if block if it is not currently implemented
There was a problem hiding this comment.
I'll remove this for now, the API always return an proper error if anyhow an invalid model is being used :)
embeddings.go
Outdated
| withBody(request), | ||
| ) | ||
| if err != nil { | ||
| return |
There was a problem hiding this comment.
Better if we could be explicit with our returns here and return an empty embedding response and err here
e.g
return EmbeddingResponse{}, err
Similarly on line 123/124, preferably avoid Named Return Values
There was a problem hiding this comment.
Agree. Let me adjust this.
|
Made some changes @reVrost and ensuring now that GET v1/embeddings/models API endpoint is supported as well. |
reVrost
left a comment
reVrost
left a comment
There was a problem hiding this comment.
LGTM, thank you nice work @nitschmann! 🎉
2 participants
Adds first-class support for the OpenRouter embeddings API to
go-openrouter, plus a small example underexamples/embeddings.This implements the feature requested in #37 and follows the official embeddings API spec from OpenRouter.AI docs: https://openrouter.ai/docs/api/reference/embeddings