Skip to content

Docs/document hybrid params - #358

Merged
g-despot merged 11 commits into
weaviate:mainfrom
Shaurya2k06:docs/document-hybrid-params
Aug 19, 2026
Merged

Docs/document hybrid params#358
g-despot merged 11 commits into
weaviate:mainfrom
Shaurya2k06:docs/document-hybrid-params

Conversation

@Shaurya2k06

@Shaurya2k06 Shaurya2k06 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add <param> documentation to the 30 public query/generate methods that previously had only a <summary> (all Hybrid overloads on Query/Generate + typed clients, 4 Generate NearVector overloads, and 2 NearText extension overloads).
  • Follow the existing thin house style from QueryClient.NearVector / BM25 (The limit, The alpha, …); copy the boost line verbatim from the rest of the public API. Rich boost prose stays on Models/Boost.cs.
  • Add ci/check_xml_param_docs.py and 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.
  • Stacked on feat/boost-query-api so these methods are fully documented including boost once that lands. No behavior or public API surface changes.

Test plan

  • dotnet build src/Weaviate.Client/ → 0 errors
  • dotnet test --filter "FullyQualifiedName~Unit" → 877 passed / 0 failed / 2 skipped
  • python3 ci/check_xml_param_docs.py → passes
  • Generated Weaviate.Client.xml includes boost, alpha, filters, etc. on Hybrid / Generate Hybrid / NearText extension overloads

Fixes #357

@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 Secrets 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?

@Shaurya2k06

Copy link
Copy Markdown
Contributor Author

agree

@Shaurya2k06

Copy link
Copy Markdown
Contributor Author

Hey @g-despot , could I get a review on this? Happy to make changes if required.

Shaurya2k06 and others added 5 commits August 14, 2026 09:59
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>
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
g-despot force-pushed the docs/document-hybrid-params branch from 3f18bf9 to da85f56 Compare August 14, 2026 08:33
… 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.
@g-despot
g-despot deleted the branch weaviate:main August 18, 2026 11:22
@g-despot g-despot closed this Aug 18, 2026
@g-despot g-despot reopened this Aug 18, 2026
@g-despot
g-despot changed the base branch from feat/boost-query-api to main August 18, 2026 11:29
The param text restated the parameter name — "The alpha", "The bm 25
operator" — so it carried less than the signature already showed. Each
parameter now says what it does and what happens when it is left unset,
which for a nullable parameter is usually the half that matters.

alpha is corrected rather than matched to the Python client, which calls
it the weight of the BM25 score. Core weights the dense result set by
alpha and the sparse one by 1 - alpha, so 0.0 is pure keyword and 1.0 is
pure vector, and the server default is 0.75. The client also sets alpha
to 1.0 itself when no query text is given, which the text now mentions.

provider is corrected too: passing one when the prompt already carries a
provider throws rather than being ignored. The same wording is still
wrong in the files this change does not touch.

@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

main added a boost parameter to the query and generate methods (weaviate#355),
which lands in the same XML doc blocks this branch rewrote. Two files
conflicted, QueryClient.NearText and GenerateClient.NearVector, and both
are resolved by keeping both sides: main's boost parameter and the
branch's parameter descriptions.

Where main documented boost, its text is replaced by the description this
branch uses for the parameter, so the surface reads with one voice.
The XML param gate this branch adds requires a <param> tag for every
parameter of a documented public method. main documents boost on some of
the methods that took it in weaviate#355 and not on others, so the six Hybrid,
NearText and NearVector partials were left with 30 undocumented boost
parameters — the branch's own check would have failed on main's code.

Every boost parameter now carries the same description, taken from the
summary on Boost itself: it re-scores the candidate pool the search
fetches rather than excluding anything, nothing is boosted when it is
left unset, and a server older than 1.38 ignores it silently. That last
point is the one a caller cannot otherwise see, since the query still
succeeds and only the ranking is wrong.

106 tags across 24 files, replacing the 68 shorter ones main carried.
Boost is generally available. weaviate/docs commit da77b9e8 ("Promote the
Boost API note from preview to generally available") changed the shared
feature note from ":::caution Preview - added in v1.38" to ":::info Added
in v1.39", so every "Preview feature" claim in the C# boost docs is now
false.

Three places carried it:
  - the <param name="boost"> tag, 106 occurrences across 24 files
  - the <summary> on the Boost record itself
  - TestBoost.cs: the file summary and five RequireVersion skip messages

The version floor deliberately stays 1.38, not 1.39. The docs note says
"Added in v1.39" because that is when boost went GA, but the wire field
and this client's support for it are genuinely 1.38+: the integration
tests gate on >= 1.38.0 and the full boost surface was verified against a
live 1.38.4 server. Claiming 1.39 would understate what the client
supports. The silent-ignore warning is kept too - it is the fact a user
most needs, and GA status does not affect it.

The RequireVersion("1.38.0", ...) gates are unchanged; only the stale
"(Preview)" parenthetical is dropped from the skip messages, which still
explain why a test skipped on an older server.

Documentation only; no code or behaviour changes.
@g-despot

Copy link
Copy Markdown
Collaborator

Sorry for multiple commits and changes, this PR got caught by our Boost work merging into main. Closing it was accidental, deleting the base branch auto-closed it, and I've retargeted it to main and reopened it. I've also pushed three commits: the <param> descriptions were filled in with real text (including a correction to alpha, which we document as the vector weight, not BM25), and the boost parameter that main was missing is now documented.

Thanks a lot for the contribution 😄

@Shaurya2k06

Copy link
Copy Markdown
Contributor Author

Had a review, all good! Learnt a lot while contributing. I'd be happy to assist you with more features in the future 🚀

@g-despot
g-despot self-requested a review August 19, 2026 06:51
@g-despot
g-despot merged commit e439c83 into weaviate:main Aug 19, 2026
16 of 17 checks passed
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.

Hybrid overloads document none of their parameters (incl. boost)

3 participants