cohere[patch]: update embeddings and rerank #5928
Conversation
dosubot
bot
added
the
size:L
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
dosubot
bot
added
the
auto:improvement
![dosubot[bot]](https://avatars.githubusercontent.com/in/324583?s=60&v=4){kind=small}
| @@ -27,7 +27,7 @@ const docs = [ | |||
|
|
|||
There was a problem hiding this comment.
Hey team, I've flagged a change in the cohereRerank instantiation that accesses an environment variable via process.env. Please review this change to ensure it aligns with our environment variable handling practices.
| @@ -28,7 +28,7 @@ const docs = [ | |||
| const cohereRerank = new CohereRerank({ | |||
There was a problem hiding this comment.
Hey team, just a heads up that I've flagged a change in the cohereRerank instantiation that sets the apiKey property using an environment variable. This is for your review to ensure it aligns with our best practices for handling environment variables.
| @@ -20,6 +20,7 @@ const documents = [ | |||
| test("CohereRerank can indeed rerank documents with compressDocuments method", async () => { | |||
There was a problem hiding this comment.
Hey there! I've reviewed the code and noticed that the addition in this PR references an environment variable via process.env. I've flagged this for your review to ensure it aligns with the project's requirements. Let me know if you have any questions!
| @@ -70,8 +75,16 @@ export class CohereEmbeddings | |||
| token: apiKey, | |||
| }); | |||
| this.model = fieldsWithDefaults?.model ?? this.model; | |||
|
|
|||
| if (!this.model) { | |||
There was a problem hiding this comment.
Would prefer to just make this a required param and ship a new minor version release along with your tool calling stuff
| this.batchSize = fieldsWithDefaults?.batchSize ?? this.batchSize; | ||
| this.inputType = fieldsWithDefaults?.inputType; | ||
| this.embeddingTypes = | ||
| fieldsWithDefaults?.embeddingTypes ?? this.embeddingTypes; |
There was a problem hiding this comment.
Do we need to remove inputType from CohereEmbeddingsParams as well?
|
Thanks, see comments! Let's gear up for a minor version release along with your other PR. |
jacoblee93
changed the title
jacoblee93
added
lgtm
This PR makes the model parameter mandatory for CohereEmbeddings class and the CohereRerank class.
Along with that, adds a embed method for the CohereEmbeddings class, and some minor improvements and docs updates.