Document vectorize_collection_name as having no effect on multi2vec factories - #2128
Open
ckarnell wants to merge 1 commit into
Open
Conversation
…actories The parameter is accepted by all eight multi2vec_* factories and passed to none of them: every factory drops it before building its config object, so it never reaches the server. multi2vec_voyageai and multi2vec_nvidia already document it as deprecated. This applies the same wording to the other six. Server side, no multi2vec module reads the setting. The reader lives on BaseClassSettings, and its two production consumers (ValidateIndexState, via BaseClassSettings.Validate, and object_texts.go's TextsWithTitleProperty) sit on paths the multi2vec modules never enter: they implement their own Validate() that calls ValidateMultiModal, and the six that hold an ObjectVectorizer never call a method on it.
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Infrastructure as Code | View in Orca | ||
| SAST | View in Orca | ||
| Secrets | View in Orca | ||
| Vulnerabilities | View in Orca |
|
To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge. |
Author
|
I agree with the CLA. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
All eight
multi2vec_*factories acceptvectorize_collection_nameand none of them uses it. Each one drops the argument before building its config object, so it never reaches the server.multi2vec_voyageaiandmulti2vec_nvidiaalready document it as "Deprecated, has no effect." This applies that same wording to the other six (clip, bind, cohere, palm, google, jinaai). Docstrings only, no signature or behaviour change.Right now a caller reads "Whether to vectorize the collection name. Defaults to
True", passesFalse, and gets neither a change nor a warning.Server side, no multi2vec module reads the setting either, checked against weaviate/weaviate at e5bee97. The reader is
BaseClassSettings.VectorizeClassName(), which the multi2vec modules do inherit. Its two production consumers sit on paths those modules never enter.ValidateIndexStateis reachable only fromBaseClassSettings.Validate, and every multi2vec module implements its ownValidate()callingValidateMultiModal. The other,TextsWithTitlePropertyinobject_texts.go, is called by text2vec-google,ObjectVectorizer.Textsand batchtext. Six multi2vec modules construct anObjectVectorizerand never call a method on it. Across the ten modules' 112 Go files, every occurrence ofVectorizeClassNameis a default constant, not a read.Verified with the pinned ruff 0.14.7 (check and format both clean), and
test/collection/test_vectorizer.pyplustest/collection/test_config.py, 193 passed. No running server was used, so the server half is source-level only.Low severity. Nothing breaks, it just misleads.
If you would rather warn at runtime than only document it,
Dep029inweaviate/warnings.pyis the existing pattern, though it needs the default to become a sentinel so an explicit argument can be told from the default. Happy to do that instead.Context and the full trace are in #2106, which was opened from our small studio's account (@toolshedlabs-hash). I run that studio, and I'm filing the fix from my personal account.