feat: add multi2vec-twelvelabs vectorizer; fix multimodal weighted configuration - #367
Conversation
Adds the Vectorizer.Multi2VecTwelveLabs config record and matching VectorizerFactory.Multi2VecTwelveLabs overloads (string arrays and WeightedFields), mirroring the existing multi2vec siblings. Fields per the server module: baseURL, model, imageFields, textFields, vectorizeClassName, weights. Requires Weaviate 1.38.9+ / 1.39.0+.
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 |
Summary - Weaviate C# Client CoverageSummary
CoverageWeaviate.Client - 49.3%
Weaviate.Client.Analyzers - 0%
Weaviate.Client.VectorData - 50.3%
|
There was a problem hiding this comment.
Pull request overview
Adds TwelveLabs multi-modal vectorizer configuration support.
Changes:
- Adds the
Multi2VecTwelveLabsmodel and factory overloads. - Tracks the new public API.
- Adds serialization and deserialization tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
PublicAPI.Unshipped.txt |
Tracks new public symbols. |
Models/Vectorizer.cs |
Defines TwelveLabs configuration. |
Configure/VectorizerFactory.cs |
Adds configuration factory overloads. |
Unit/TestVectorizers.cs |
Tests JSON mapping and optional fields. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…factories VectorizerWeights was assigned by every weighted multi2vec factory but never reached the wire: the property is internal and System.Text.Json skips non-public members, so all ten weighted overloads silently dropped their weights. Adds [JsonInclude] with WhenWritingNull, and returns null from FromWeightedFields when no modality carries weights, so the key is omitted rather than emitted as an empty object. Making weights serialize exposed a transposition in Multi2VecGoogle and Multi2VecGoogleGemini, which passed videoFields and audioFields into the audioFields and depthFields parameters. Inert while weights were dropped; live it labels video weights as audio, parks audio weights under a modality the module does not have, and can fail collection creation with a weights count mismatch. All ten call sites now use named arguments. Tests assert the full weights object with a distinct value per modality — the previous substring-presence assertions passed despite the transposition — and cover Bind and VoyageAI, which had no weighted coverage.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
src/Weaviate.Client/Configure/VectorizerFactory.cs:264
- This comment is inaccurate: the positional call shown before this change already matched
FromWeightedFields's image/text/audio/depth/imu/thermal/video order, so it did not route Bind weights to neighboring modalities. Keep the named arguments, but describe them as a safeguard rather than a required correctness fix.
// Named arguments are mandatory here: FromWeightedFields declares seven optional
// modalities in the order image, text, audio, depth, imu, thermal, video, so a
// positional call would silently file each modality's weights under its neighbour.
An empty WeightedFields or string[] converted to [] and was sent as e.g. "textFields": [], which the server rejects — so an image-only weighted config failed collection creation even though its empty weight array was already correctly omitted. Multi2VecBind was worst affected: all seven modalities are required parameters, so callers must pass empty for the ones they don't use, making any real subset unusable. Normalises empty to null at the 61 modality assignment sites across all ten weighted factories and every string[] overload, matching the guard already applied to the weights themselves — a modality's names and its weight array now drop out by one rule. Write path only; deserialization of server responses is untouched. Also corrects the named-argument comments. Only the two Google factories were actually transposed; Bind and VoyageAI were already correct, so their named arguments are a safeguard against future reordering rather than a fix.
| /// <summary> | ||
| /// Gets or sets the value of the vectorize collection name | ||
| /// </summary> | ||
| [JsonPropertyName("vectorizeClassName")] | ||
| public bool? VectorizeCollectionName { get; set; } = null; |
There was a problem hiding this comment.
issue: IIRC this setting is "undesirable". While present in existing vectorizers, I think we shouldn't provide it for the ones we're adding now.
IIDRC (if I don't remember correctly 😄) then feel free to ignore this
There was a problem hiding this comment.
Right, fixed here but will later also do a global fix for the other modules
No multi2vec module reads the setting. In modules/multi2vec-twelvelabs the only references are a default registration and the DefaultVectorizeClassName constant; nothing consumes it. The python client documents it as "Deprecated, has no effect" across all eight of its multi2vec_* factories and omits it from multi2vec_twelvelabs entirely. C# was worse than python here: python drops the value, C# sent it, so the setting round-tripped through the stored schema and read as though it had taken effect. Removed from the record and both factory overloads. The API is unreleased — PublicAPI.Shipped.txt has no TwelveLabs entries — so the Unshipped lines are deleted rather than marked *REMOVED*. The other nine multi2vec records keep theirs; removing those is a breaking change and a separate decision.
…labs # Conflicts: # src/Weaviate.Client.Tests/Unit/TestVectorizers.cs # src/Weaviate.Client/PublicAPI.Unshipped.txt
Brings in #359 (module endpoint/location), #365 (BM25Operator.AndCross + searchOperator on generate.bm25), #366 (diversitySelection across hybrid and the near* family), and #367 (multimodal vectorizer weights). All of the .cs overload files auto-merged: boost is inserted after `rerank` and diversitySelection after `bm25Operator`/before `autoLimit`, so the two parameter additions never collided. Verified structurally in both directions (merged-minus-boost == origin/main, merged-minus-diversitySelection/ searchOperator == the branch) rather than trusting the auto-merge. The only textual conflict was PublicAPI.Unshipped.txt, where both sides emit a *REMOVED* line plus a replacement signature for the same 102 overloads. Resolved to a deduplicated union (102 identical *REMOVED* lines collapsed), then adjudicated by the public API analyzer: 204 single-parameter signatures were stale (RS0017) and 102 combined signatures were missing (RS0016). After applying those, the RS0016/RS0017 sets are identical to both parents'. Parameter counts after the merge match each parent exactly: boost 111 declarations (as on the branch), diversitySelection 105 and searchOperator 10 (as on main).
Adds
Multi2VecTwelveLabsvectorizer configuration (modulemulti2vec-twelvelabs, Weaviate 1.39.0, backported to 1.38.9) with the matchingConfigurefactory overloads. Wire keys verified against the server module schema:baseURL,model,imageFields,textFields,vectorizeClassName.Review then surfaced three defects in the multimodal weighted-config path, all wider than twelvelabs — the feature had never worked, so nothing behind it had been exercised:
VectorizerWeightsis assigned by every weighted multi2vec factory, but the property isinternaland System.Text.Json skips non-public members — so all ten weighted overloads silently discarded their weights. Fixed with[JsonInclude]+WhenWritingNull;FromWeightedFieldsreturns null when no modality carries weights, so the key is omitted rather than emitted as{}.Multi2VecGoogleandMulti2VecGoogleGeminipassedvideoFields, audioFieldsinto theaudioFields, depthFieldsparameters. Inert while weights were dropped; live it labels video weights as audio and can fail collection creation with a count mismatch. All ten call sites now use named arguments. Only these two were transposed — Bind and VoyageAI were already correct.[]. An emptyWeightedFieldsorstring[]emitted e.g."textFields": [], and the server rejects a present-but-empty modality, so an image-only weighted config failed collection creation.Multi2VecBindwas worst hit: all seven modalities are required parameters, so any real subset was unusable. Empty now normalises to omitted at all 61 modality assignment sites, matching the guard already applied to the weights themselves. Write path only.Verified end to end against Weaviate 1.39.0 at each step: weights round-trip in field order, a deliberate count mismatch is rejected, and the image-only config that previously returned
must contain at least one text field name in textFieldsnow succeeds with notextFieldskey stored. Tests assert full JSON objects with a distinct value per modality — the previous substring-presence assertions passed despite the transposition — and each fix has a test proven to fail without it.Note: none of the end-to-end verification is committed as an integration test, because the CI compose enables no multi2vec module. Adding
multi2vec-cohere(no inference container, no API key needed at schema-create time) would close that gap in a follow-up.