Open fzliu opened 1 month ago
@fzliu thanks for your contribution. Please take a look at the CI reports and fix them. The rest looks okay to me.
After analyzing the pull request, here are my findings:
Overall Feedback:
The addition of the Voyage AI embeddings is a significant enhancement to the project, providing new functionality for embedding models. The code is generally well-structured, but there are areas where improvements can be made, particularly regarding error handling and code clarity.
Score: 85/100
Labels: Enhancement, Tests
Code Suggestions:
File: libs/kotaemon/kotaemon/embeddings/voyageai.py
api_key
is validated before using it to create the client.+ self._client = _import_voyageai().Client(api_key=self.api_key)
self._client = _import_voyageai().Client(api_key=self.api_key)
if not self.api_key:
raise ValueError("API key must be provided for VoyageAIEmbeddings.")
self._client = _import_voyageai().Client(api_key=self.api_key)
File: libs/kotaemon/kotaemon/embeddings/voyageai.py
+ embeddings = self._client.embed(texts, model=self.model_name).embeddings
embeddings = self._client.embed(texts, model=self.model_name).embeddings
try:
embeddings = self._client.embed(texts, model=self.model_name).embeddings
except Exception as e:
raise RuntimeError(f"Failed to retrieve embeddings: {e}")
File: libs/kotaemon/tests/test_embedding_models.py
VoyageAIEmbeddings
checks for the correct output structure.+ assert_embedding_result(output)
assert_embedding_result(output)
assert isinstance(output, list) and all(isinstance(doc, DocumentWithEmbedding) for doc in output)
The addition of Voyage AI embeddings is a great enhancement! Here are some suggestions:
api_key
before using it to create the client in VoyageAIEmbeddings
.VoyageAIEmbeddings
checks for the correct output structure.
Description
Adds embeddings from Voyage AI.
Type of change
Checklist