Require essential dependencies in vector store builder constructors #1983
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit refactors the builder pattern implementation across all VectorStore implementations to make the EmbeddingModel a required constructor parameter rather than an optional builder method. Key changes include: - Move embeddingModel from being a builder method to a required constructor parameter - Make embeddingModel final in AbstractVectorStoreBuilder - Remove redundant validate() methods since EmbeddingModel validation now happens in constructor - Update all VectorStore builder instantiations to pass EmbeddingModel in builder creation - Add @nullable annotations to appropriate methods in VectorStore interface This change improves the API design by: 1. Enforcing that EmbeddingModel is provided at builder creation time 2. Removing the possibility of forgotten EmbeddingModel configuration 3. Simplifying the builder implementation by moving validation to construction 4. Making the dependency on EmbeddingModel more explicit in the API Breaking Changes: - VectorStore builders must now be created with an EmbeddingModel parameter - The embeddingModel() builder method has been removed from all implementations
- Replace Document.getContent() with getText() across all vector store implementations - Fix incorrect package declarations in package-info.java files - Make builder constructors private and implement proper builder patterns - Add @nullable annotations for better null safety - Simplify PineconeVectorStore builder API by requiring essential parameters in factory method - Make static Map fields final - Clean up code and improve JavaDoc documentation The changes focus on making the vector store APIs more consistent, type-safe, and maintainable while following best practices for builder patterns and null safety.
|
merged in 1a6e79c |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit refactors the builder pattern implementation across all VectorStore implementations to make the EmbeddingModel a required constructor parameter rather than an optional builder method. Key changes include:
This change improves the API design by:
Breaking Changes: