fix(rerankers): send the Nvidia reranker base URL as "baseURL" - #610
Open
dudanogueira wants to merge 1 commit into
Open
fix(rerankers): send the Nvidia reranker base URL as "baseURL"#610dudanogueira wants to merge 1 commit into
dudanogueira wants to merge 1 commit into
Conversation
reranker-nvidia looks the key up verbatim, so the "baseUrl" spelling was
stored in the schema and then ignored: reranking silently went to the
default NVIDIA endpoint instead of the configured one.
Verified against Weaviate 1.39.0 with a stub reranker endpoint. Two
collections identical but for the key spelling:
baseURL -> the stub is called, the query returns the stub's scores
baseUrl -> no call to the stub, "connection to NVIDIA API failed
with status: 401" from the real endpoint
Every other module config in this client already uses "baseURL", which
matches the server: each module reads it through a case-sensitive map
lookup, and text2vec-weaviate goes as far as declaring the migration
{Name: "baseUrl", NewName: "baseURL"}.
The alternate keeps configs written by older versions of this client
readable, so upgrading turns a wrong value into a working one rather
than a missing one.
Closes #607
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmY5dAGWCccWDoqkKNC2JU
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 |
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.
Motivation
NvidiaRerankersends the base URL asbaseUrl. Thereranker-nvidiamodule readsbaseURL. The value is stored in the schema and then ignored, so reranking silently goes to the default NVIDIA endpoint instead of the configured one — no error, just the wrong endpoint.Note that #607 originally claimed the opposite — that 28 classes used the wrong key and
NvidiaRerankerwas the correct example. That was wrong; the issue has been rewritten with the evidence. The 28baseURLclasses are correct, and this is the only broken one.Why
baseURLis canonicalModules read the key through a plain case-sensitive map lookup (
usecases/modulecomponents/settings/class_settings_property_helper.go), and every module passes"baseURL"— includingreranker-nvidia:text2vec-weaviateeven declares an explicit migration away from the lowercase spelling:{Name: "baseUrl", NewName: "baseURL"}. And this repo's own history already normalised in that direction once —657865abmovedText2VecVoyageAiVectorizerfrombaseUrltobaseURL.Empirical confirmation
Not taken on faith from the source. Weaviate 1.39.0 with
reranker-nvidiaenabled and the base URL pointed at a stub reranker; two collections identical but for the key spelling:The 401 is the module falling back to the real endpoint — the configured URL never reached it.
Approach
@SerializedName(value = "baseURL", alternate = {"baseUrl"}).The
alternatematters for upgrades: collections created by earlier versions of this client have the URL stored underbaseUrl, and Gson drops unknown keys silently. Without it, upgrading would turn a value that was wrong-but-present into one that is missing. Gson only ever writesvalue, so new configs use the canonical key. The construct is already used in this repo (UserType,TenantStatus), though this is its first use on a record component.Testing
NvidiaRerankerTest— a legacybaseUrldocument still deserializes; serialization emitsbaseURLand neverbaseUrl.JSONTestrow forreranker-nvidia. It had none, which is how this slipped through — the modules with a base-url row all had the right key.Locally green: 386 unit tests.
What this does not cover
No integration test exercises a real provider module (the container enables only the dummy generative/reranker), so the end-to-end proof is the stub probe above rather than something CI re-runs. Wiring a stub-backed module into the IT suite would be a reasonable follow-up.
Separately worth reporting upstream, not fixed here:
text2vec-cohereandtext2vec-nvidiainject their default config under"baseUrl"(config.go) while their readers look for"baseURL". Harmless — the reader's own fallback is the same value — but it is what made the original report look correct.Closes #607
🤖 Generated with Claude Code
https://claude.ai/code/session_01WmY5dAGWCccWDoqkKNC2JU