-
Notifications
You must be signed in to change notification settings - Fork 159
chore: adds field embeddings validation for quantization "none" and warn when vectorSearch is not configured correctly #717
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 19171504420Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances vector search configuration validation and improves tool metadata clarity. The main purpose is to ensure proper vector search setup and provide better user feedback when configurations are incorrect.
Key changes:
- Adds validation for embeddings when quantization is set to "none"
- Introduces warnings when vector search feature and embeddings provider configurations are mismatched
- Updates warning message formatting for better readability
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/common/search/vectorSearchEmbeddingsManager.ts | Implements validation logic for "none" quantization to ensure embeddings are proper numeric arrays with correct dimensions |
| src/common/config.ts | Adds configuration validation warnings for vector search/embeddings provider mismatches and updates warning message formatting |
| src/tools/mongodb/create/createIndex.ts | Makes vectorSearch reference conditional in tool description based on feature availability |
| tests/unit/common/search/vectorSearchEmbeddingsManager.test.ts | Extends test coverage to validate "none" quantization handling across multiple scenarios |
| tests/unit/common/config.test.ts | Updates test expectations to match new warning message format |
| tests/integration/tools/mongodb/create/createIndex.test.ts | Adds metadata validation and updates expected description text |
| tests/integration/tools/mongodb/create/insertMany.test.ts | Adds metadata validation test for insert-many tool |
| ({ | ||
| pipeline: z | ||
| .array(vectorSearchEnabled ? z.union([AnyAggregateStage, VectorSearchStage]) : AnyAggregateStage) | ||
| .describe(pipelineDescription), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pipeline description here is static and includes references to vector search - should we modify it so that we don't include them if the feature is disabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's certainly an option but I think that the description applies in a generic sense to the pipeline and I don't think it would be confusing for the LLMs. Or do you feel otherwise?
Proposed changes
Proposes the following:
vectorSearchonly when the feature is enabled. Additionally adds additional tests for the metadata - 198b36enone- 5cd2e51Checklist