Skip to content

fix(collections): nest the quantizer inside a dynamic index - #613

Open
dudanogueira wants to merge 2 commits into
mainfrom
fix/dynamic-index-quantization
Open

fix(collections): nest the quantizer inside a dynamic index#613
dudanogueira wants to merge 2 commits into
mainfrom
fix/dynamic-index-quantization

Conversation

@dudanogueira

@dudanogueira dudanogueira commented Aug 25, 2026

Copy link
Copy Markdown

Contains #612. The two commits are separable but not independent: the nested rq this PR finds still yields a null rescoreLimit without the field-name fix, so the integration test needs both. Review commit-by-commit — fix(quantizers) is #612 unchanged, fix(collections) is what this PR adds. If #612 merges first, this collapses to the second commit on its own.

Motivation

A dynamic index is {distance, threshold, hnsw: {...}, flat: {...}} and each sub-index carries its own quantizer (entities/vectorindex/dynamic/config.go: HnswUC hnsw.UserConfig, FlatUC flat.UserConfig). The client put the quantizer beside hnsw/flat and scanned for it there on the way back.

Both directions were broken, not just the read side the issue reports:

  • ReadVectorConfig.read scanned only the top level of vectorIndexConfig, so hnsw.rq was never found and quantization() came back null. This is what v6: dynamic index quantization is never parsed into VectorConfig #606 reports, verified against a live 1.38.0 collection.
  • WriteVectorConfig.write appended the quantizer as a sibling of hnsw/flat, where the server's dynamic parser never looks. A dynamic + quantized collection created through this client was silently unquantized.

The write half makes the read half partly self-consistent: a config created by this client really did have no quantizer to read back.

Approach

VectorConfig has a single quantization() slot, so:

  • read from hnsw when it has a quantizer, flat otherwise;
  • write always to hnsw — also the only sub-index that accepts every quantizer type, flat being limited to bq.

Both sides go through QuantizerJson.host(...), which UpdateCollectionRequest also uses: the skipDefaultQuantization flag it has to preserve on update was being written at the same wrong level.

Documented limitation: giving hnsw and flat different quantizers is not expressible. That needs a quantization component on Hnsw and Flat with VectorConfig.quantization() becoming a derived view — a breaking change across 33 vectorizer records, worth doing only if someone actually needs it.

Also on Dynamic, both called out in #606:

  • distance is now read and written; the server keeps one at the dynamic level and it was dropped in both directions.
  • the response parser no longer assumes threshold is present — a dropped index has neither, and getAsLong() on a missing key would NPE.

Key areas for review

  • QuantizerJson.host — the create flag distinguishes "make room for it" (write) from "find it or find nothing" (read). The read path returning an empty object rather than null is what keeps the scan loop unchanged.
  • The hnsw-wins choice when both sub-indexes carry a quantizer. It is arbitrary but has to be something while there is one slot.
  • UpdateCollectionRequest — worth confirming the flag now lands where the server reads it for dynamic indexes.

Testing

  • DynamicQuantizationTest — 6 read cases the round-trip rows cannot cover, since this client always writes to hnsw: quantizer on hnsw, on flat only, on both (hnsw wins), absent, disabled, and distance at the dynamic level.
  • JSONTest — a dynamic row carrying rq and distance, asserted in both directions.
  • CollectionsITest.test_dynamicIndexQuantizationRoundTrip — creates a dynamic index with rq against a real server and reads it back. Needs ASYNC_INDEXING=true, which the shared container is not (async indexing makes freshly inserted vectors searchable only eventually, and the other suites rely on immediate searchability), so it gets its own container via a new Weaviate.Builder.enableAsyncIndexing.

Verified the integration test fails without the fix: [quantizer nested under hnsw] Expecting actual not to be null.

Locally green: 390 unit tests, and CollectionsITest against a 1.39.0 container (17 run, 0 failures).

Conflict note

enableAsyncIndexing is also added by #598, which is still open. Whichever merges second will want that hunk dropped — same method, same body.

Breaking changes

Dynamic's canonical constructor gains a distance component. The builder and accessors are additive; only positional construction is affected.

Behaviourally: a dynamic index created after this change actually applies its quantizer, where before it was accepted and ignored. Existing collections are unaffected — they have no quantizer stored to migrate.

Closes #606

🤖 Generated with Claude Code

https://claude.ai/code/session_01WmY5dAGWCccWDoqkKNC2JU

dudanogueira and others added 2 commits August 25, 2026 16:01
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
A dynamic index is {distance, threshold, hnsw: {...}, flat: {...}} and
each sub-index carries its own quantizer, but the client put the
quantizer beside them and looked for it there on the way back. So a
dynamic index created through this client was silently unquantized, and
one quantized by other means read as quantization() == null.

VectorConfig has a single quantization slot, so a dynamic index is read
from hnsw when it has a quantizer and from flat otherwise, and always
written to hnsw -- the only sub-index accepting every quantizer type,
flat being limited to bq. Giving hnsw and flat different quantizers
stays inexpressible; that needs per-index slots on Hnsw and Flat.

The same nesting applies to the skipDefaultQuantization flag that the
update request has to preserve, so both call the shared QuantizerJson.

Also on Dynamic: "distance" is read and written (the server keeps one at
the dynamic level), and the response parser no longer assumes
"threshold" is present -- a dropped index has neither.

Closes #606

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmY5dAGWCccWDoqkKNC2JU

@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

@dudanogueira
dudanogueira changed the base branch from fix/quantizer-field-names to main August 25, 2026 19:16
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.

v6: dynamic index quantization is never parsed into VectorConfig

1 participant