Skip to content

Document vectorize_collection_name as having no effect on multi2vec factories - #2128

Open
ckarnell wants to merge 1 commit into
weaviate:mainfrom
ckarnell:docs/multi2vec-vectorize-collection-name-no-effect
Open

Document vectorize_collection_name as having no effect on multi2vec factories#2128
ckarnell wants to merge 1 commit into
weaviate:mainfrom
ckarnell:docs/multi2vec-vectorize-collection-name-no-effect

Conversation

@ckarnell

Copy link
Copy Markdown

All eight multi2vec_* factories accept vectorize_collection_name and none of them uses it. Each one drops the argument before building its config object, so it never reaches the server. multi2vec_voyageai and multi2vec_nvidia already 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", passes False, 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. ValidateIndexState is reachable only from BaseClassSettings.Validate, and every multi2vec module implements its own Validate() calling ValidateMultiModal. The other, TextsWithTitleProperty in object_texts.go, is called by text2vec-google, ObjectVectorizer.Texts and batchtext. Six multi2vec modules construct an ObjectVectorizer and never call a method on it. Across the ten modules' 112 Go files, every occurrence of VectorizeClassName is a default constant, not a read.

Verified with the pinned ruff 0.14.7 (check and format both clean), and test/collection/test_vectorizer.py plus test/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, Dep029 in weaviate/warnings.py is 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.

…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.

@orca-security-eu orca-security-eu Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

@weaviate-git-bot

Copy link
Copy Markdown

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.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Forum?

@ckarnell

Copy link
Copy Markdown
Author

I agree with the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants