fix(quantizers): use the field names the server actually reads - #612
Open
dudanogueira wants to merge 1 commit into
Open
fix(quantizers): use the field names the server actually reads#612dudanogueira wants to merge 1 commit into
dudanogueira wants to merge 1 commit into
Conversation
Weaviate parses quantizer settings out of the config map by exact key,
and returns them under the same names. The client used snake_case for
half of them, so rescoreLimit, trainingLimit and bitCompression were
dropped on write and came back null on read -- on every index type. A
user who set a rescore limit never set one.
rescore_limit -> rescoreLimit (BQ, SQ, RQ)
training_limit -> trainingLimit (PQ, SQ)
bit_compression -> bitCompression (PQ)
PQ's encoder needed a shape change rather than a rename: the server
nests it as encoder: {type, distribution} while the client had two flat
components. PQ.encoderType() and PQ.encoderDistribution() are kept as
derived accessors and the builder is unchanged, so only the canonical
constructor differs.
No alternate names: the snake_case spellings were never valid on the
wire, so no stored config uses them.
Closes #611
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
Weaviate parses quantizer settings out of the config map by exact key, with no case folding and no snake_case fallback:
and returns them under the same names (
RescoreLimit int \json:"rescoreLimit"``). The client used snake_case for half of them, so those settings were silently dropped on write and null on read, on hnsw and flat alike. Setting a rescore limit did nothing and reported nothing.Found while investigating #606, which is unfixable in practice without this: locating a nested
rqunder a dynamic index still yields a nullrescoreLimit.Approach
rescore_limitrescoreLimittraining_limittrainingLimitbit_compressionbitCompressionencoder_type+encoder_distributionencoder: {type, distribution}PQ's encoder needed a shape change rather than a rename — the server models it as a nested object (
pq_config.go:Encoder PQEncoder \json:"encoder"`, read viaencoderFromMap/encoderDistributionFromMap), while the client had two flat components. The record now carries anEncodercomponent;PQ.encoderType()andPQ.encoderDistribution()` are kept as derived accessors and the builder API is untouched, so the flat, convenient way of setting them still works.No
alternatenames, deliberately: unlike #607, the snake_case spellings were never valid on the wire, so there is no stored config spelled that way to stay compatible with.enabled,bits,segments,centroidsandcachealready matched and are unchanged.Key areas for review
PQ.java— theEncoderrecord and the builder→record mapping, which collapses tonullwhen neither encoder field was set so an emptyencoder: {}is never emittedencoderType()/encoderDistribution()as derived accessors is the right call, or whether callers should be pushed toencoder()Testing
JSONTest— the PQ/SQ/RQ/BQ rows now expect the server's spelling. Each row asserts both directions, so they cover read as well as write.CollectionsITest.test_quantizerSettingsRoundTrip— creates a collection withrq.rescoreLimit(42).bits(8), reads the config back from a real server and asserts both values survive. This is the assertion that was missing: the existing quantizer IT only checked_kind(), which matched even while every setting was being discarded.Verified the new test fails without the fix:
expected:<42> but was:<null>.Locally green: 382 unit tests, and
CollectionsITestagainst a 1.39.0 container (16 run, 0 failures).Breaking changes
PQ's canonical constructor changes —encoderType, encoderDistributionbecome a singleencodercomponent. Accessors and the builder are unchanged, so only code callingnew PQ(...)positionally or destructuring it in a record pattern is affected.Behaviourally, collections created by older clients have no quantizer settings stored at all (they were dropped), so there is nothing to migrate — this only starts sending values that previously went nowhere. Worth knowing that a collection created after this change will actually apply a rescore limit where before it silently used the server default.
Closes #611
🤖 Generated with Claude Code
https://claude.ai/code/session_01WmY5dAGWCccWDoqkKNC2JU