Docs/document hybrid params - #358
Open
Shaurya2k06 wants to merge 25 commits into
Open
Conversation
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Secrets | 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
|
agree |
Author
|
Hey @g-despot , could I get a review on this? Happy to make changes if required. |
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+.
The operator was already accepted on query.bm25, all four hybrid surfaces and aggregate.hybrid, but not on generate.bm25 — leaving AndCross unreachable from generative keyword search. Adds the parameter to the four GenerateClient/TypedGenerateClient overloads, in the same position the query client uses. It flows into the existing BuildBM25 path, so the server-version guard applies unchanged.
Diversity/MMR was wired into hybrid only. base_search.proto carries an optional Selection on NearVector, NearTextSearch, NearObject and each Near*Search message, and the server has supported it since 1.37.0 — hybrid diversity is the newer 1.38.6 addition, so the broader surface was in fact the older one. Threads diversitySelection through the 74 near* overloads across the query, generate and typed clients, and moves the hybrid mapping into a shared BuildSelection helper. No proto change was needed. Aggregate is deliberately excluded, matching the python client.
location has explicit presence in proto3, so assigning an empty string marked the field present and suppressed the server-side default. The Vertex path now sets it only for a non-empty value, and the Gemini path, where a Vertex region does not apply, leaves it unset entirely. Tests assert HasLocation rather than comparing against string.Empty, which could not distinguish "unset" from "explicitly empty" — both read back the same. Adds coverage for the Gemini path, which had none.
…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.
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.
…e-location feat: add endpoint setting for text2vec-openai/morph and location for generative-google
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.
… guard EnsureBM25OperatorSupported runs before the request ever leaves the client, but WeaviateFeatureNotSupportedException derives from WeaviateServerException. Switched to WeaviateVersionMismatchException (WeaviateClientException), which is what the client's other version gates throw via Internal/VersionGuard. The two previously sat under different bases, so a caller could not catch both with anything narrower than WeaviateException. That exception's only constructor takes (operation, requiredVersion, actualVersion), so the backport floors now travel in the operation argument, sourced from the existing AndCrossMinimumVersions ladder rather than a hand-written string. Tests assert the structured RequiredVersion and ActualVersion alongside the exception type. WeaviateFeatureNotSupportedException is left in place: ExceptionHelper still maps a genuine server-side gRPC Unimplemented to it, where the server base class is correct.
feat: add BM25Operator.AndCross for cross-property keyword AND (Weaviate 1.38.8+)
# Conflicts: # src/Weaviate.Client/PublicAPI.Unshipped.txt
feat: add diversity selection via MMR for hybrid and near* queries
…labs # Conflicts: # src/Weaviate.Client.Tests/Unit/TestVectorizers.cs # src/Weaviate.Client/PublicAPI.Unshipped.txt
Document every parameter on the 30 public methods that previously had only a <summary>, including boost, so IntelliSense matches the rest of the query/generate API. Signed-off-by: shaurya2k06 <shaurya2k06@gmail.com>
Add ci/check_xml_param_docs.py to fail when a public QueryClient, GenerateClient, or typed query/generate method has a <summary> but missing or incomplete <param> tags. Wire it into preflight so Hybrid-style gaps cannot regress silently. Signed-off-by: shaurya2k06 <shaurya2k06@gmail.com>
feat: add multi2vec-twelvelabs vectorizer; fix multimodal weighted configuration
PR weaviate#358 was written against weaviate#355 (feat/boost-query-api), so its <param> tags describe that branch's API rather than main's: - `boost` does not exist on main, so 30 `<param name="boost">` tags document a parameter that is not there (CS1572). They belong with weaviate#355, which is the PR that introduces the parameter. - `diversitySelection` was added to the same 30 methods by weaviate#366 after this PR was written, and was left undocumented (CS1573). Drop the boost tags and document diversitySelection instead, placing each tag at the position its parameter occupies in the signature (after `bm25Operator` in the Hybrid overloads, `distance` in GenerateClient.NearVector, `offset` in the QueryClient.NearText extensions). Also align two style details the PR introduced: `<param name="client">` reads "The client" (26 instances repo-wide, none of "The query client"), and the two QueryClient.NearText extension blocks take the trailing period their prose-style neighbours all use. The worklist was derived from the compiler, not by hand: `dotnet build --no-incremental` now reports 0 CS1572 and 0 CS1573, and the normalized warning set is byte-identical to origin/main (55 unique warnings on both).
… every PR The check added in dab0af0 was weaker than the compiler and wired into preflight unconditionally. Four defects, each now covered by a self-test in ci/test_check_xml_param_docs.py: - Generic methods were invisible. `(?P<name>\w+)\s*\(` cannot match `Hybrid<T>(`, so the gate silently skipped 4 of the very methods this PR documents (the generic extension overloads in Typed/TypedQueryClient.Hybrid and Typed/TypedGenerateClient.Hybrid). - Unsatisfiable false positive. Splitting the parameter list on every comma turned `Dictionary<string, object> filters` into a phantom parameter named `Dictionary<string`, which no <param> tag can ever satisfy. Latent today; it would have wedged CI permanently the first time such a parameter appeared. - Silent success on an empty scan. Zero scanned files printed OK and exited 0, so any path drift would have disabled the gate forever. The scan count is now reported and an empty scan (or a missing client root) fails loudly. - Nested parens defeated the `[^)]*` signature capture, dropping methods with `= default(CancellationToken)`, a tuple return type, or an attribute holding parens in a string. Declarations are now located with a literal-aware brace/paren scanner instead of one regex, since a regex cannot balance those constructs. Scanned declarations rise from 118 to 126 (+4 generic overloads, +4 public constructors) with none dropped; the checker stays restricted to the public surface, as its docstring always claimed. The preflight step is also guarded on `changed-files.any_changed` like the neighbouring formatting check, and moved ahead of Setup .NET / restore since it needs no .NET. preflight is a `needs:` of the 8-version test matrix, so before this an unrelated PR touching no C# would have been blocked by it.
…clean it Three follow-ups on the review of the previous two commits. Use the prose wording in the two prose-style blocks. The `diversitySelection` wording on main is block-sensitive, not global: terse blocks say "The diversity selection" (28 instances) and prose blocks say "Diversity selection to apply to the results." (50 instances), and every one of the 77 pre-existing instances matches its block. The two tags added to the QueryClient.NearText extension overloads sat in prose blocks, so they now take the prose wording; the other 28 this PR adds are in terse blocks and are unchanged. The convention now holds across all 107 instances with no exceptions. Run the self-test in CI. A test nobody runs is not a test, so the guarded preflight step now runs ci/test_check_xml_param_docs.py before the checker itself. Both are plain commands on separate lines, so the runner's `bash -e` propagates a failure: with a deliberately reverted generics fix the step exits 1 and never reaches the checker. Make both files type-clean. Pyright reported 13 errors in the new test, all stemming from monkey-patching module globals and from using the `ModuleSpec | None` returned by spec_from_file_location unguarded. Rather than suppressing them, `check_file` and `main` now take their roots as parameters defaulting to the module constants, so the test injects roots instead of patching them, and the spec and its loader are asserted non-None. Both files report 0 errors, and the checker's standalone behaviour is unchanged.
g-despot
force-pushed
the
docs/document-hybrid-params
branch
from
August 14, 2026 08:33
3f18bf9 to
da85f56
Compare
… place Two lint leftovers from the roots-injection refactor. `sys` became unused once the tests stopped writing to stderr directly, so remove it. The `_scanned` return from check_file was discarded; rather than dropping it, assert on it — the generic fixture holds exactly one declaration, so it now pins that the generic overload is counted rather than merely parsed. 18 self-tests pass; ruff and pyright both report clean.
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.
Summary
<param>documentation to the 30 public query/generate methods that previously had only a<summary>(all Hybrid overloads on Query/Generate + typed clients, 4 GenerateNearVectoroverloads, and 2 NearText extension overloads).QueryClient.NearVector/ BM25 (The limit,The alpha, …); copy theboostline verbatim from the rest of the public API. Rich boost prose stays onModels/Boost.cs.ci/check_xml_param_docs.pyand wire it into preflight so methods with a<summary>but missing/incomplete<param>tags fail CI on the Query/Generate (+ typed) surface — closes the CS1573/CS1591 blind spot that let this gap persist.feat/boost-query-apiso these methods are fully documented includingboostonce that lands. No behavior or public API surface changes.Test plan
dotnet build src/Weaviate.Client/→ 0 errorsdotnet test --filter "FullyQualifiedName~Unit"→ 877 passed / 0 failed / 2 skippedpython3 ci/check_xml_param_docs.py→ passesWeaviate.Client.xmlincludesboost,alpha,filters, etc. on Hybrid / Generate Hybrid / NearText extension overloadsFixes #357