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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes Weaviate quantizer JSON field naming/shape mismatches so quantization settings are correctly sent to (and read back from) the server, instead of being silently dropped due to case/shape differences.
Changes:
- Renames quantizer JSON keys from snake_case to the server’s exact camelCase keys for RQ/BQ/SQ/PQ fields.
- Updates PQ’s encoder representation to match the server’s nested
encoder: {type, distribution}object. - Strengthens test coverage by updating JSON round-trip expectations and adding an integration round-trip assertion for
rq.rescoreLimit.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/io/weaviate/client6/v1/api/collections/quantizers/SQ.java | Switch SQ JSON keys to server-read camelCase names. |
| src/main/java/io/weaviate/client6/v1/api/collections/quantizers/RQ.java | Switch RQ rescoreLimit JSON key to server-read camelCase. |
| src/main/java/io/weaviate/client6/v1/api/collections/quantizers/BQ.java | Switch BQ rescoreLimit JSON key to server-read camelCase. |
| src/main/java/io/weaviate/client6/v1/api/collections/quantizers/PQ.java | Align PQ with server by nesting encoder settings and fixing key names. |
| src/test/java/io/weaviate/client6/v1/internal/json/JSONTest.java | Update serialization/deserialization golden JSON to match server spelling/shape. |
| src/it/java/io/weaviate/integration/CollectionsITest.java | Add IT that validates quantizer settings survive a server round trip. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
71
to
+76
| builder.enabled, | ||
| builder.centroids, | ||
| builder.segments, | ||
| builder.encoderType, | ||
| builder.encoderDistribution, | ||
| builder.encoderType == null && builder.encoderDistribution == null | ||
| ? null | ||
| : new Encoder(builder.encoderType, builder.encoderDistribution), |
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