test(chroma): assert the HNSW defaults rather than a threshold arithmetic - #2159
Merged
igorls merged 1 commit intoAug 11, 2026
Merged
Conversation
…etic test_hnsw_write_defaults_bound_index_rewrites divided a hypothetical corpus size by the configured sync_threshold and read the quotient as a count of full on-disk index rewrites. It measured no writes, no compactions, no persistence calls and no rewritten bytes, so it asserted arithmetic on a constant rather than a property of the implementation. The metadata tests already cover the defaults, and a write-amplification regression wants I/O measurement under the benchmark suite. Two neighbouring docstrings carried the same reasoning in prose and now state what holds: the values are chromadb's own documented defaults, so a collection this backend creates indexes on the same terms as one chromadb creates itself, and sync_threshold=2 reads as a value chromadb's own parameter validation rejects. The threshold relationship now reads batch_size <= sync_threshold, which is what chromadb permits. _hnsw_creation_metadata(options) reads hnsw_space directly, so get_collection's local copy of that resolution had no reader; removing it leaves the option's path to a created collection unchanged. tests/test_backends.py: 91 passed. Backend/palace selection: 593 passed, 16 skipped.
This was referenced Aug 5, 2026
igorls
approved these changes
Aug 11, 2026
igorls
left a comment
igorls
left a comment
Member
There was a problem hiding this comment.
Wave 3 minimal for 3.7.0: LGTM. Docs/tests/tiny fix only.
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
2 participants
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.
Follow-up to the review on #2107.
The HNSW default tests assert what a created collection carries:
batch_sizeandsync_thresholdland on fresh metadata at chromadb's own documented values, through bothChromaBackendcreation paths. The relationship they assert readsbatch_size <= sync_threshold— what chromadb permits — in both tests that check it.Their docstrings state the same ground: the values are chromadb's own defaults, so a collection this backend creates indexes on the same terms as one chromadb creates itself, and
sync_threshold=2reads as a value chromadb's own parameter validation rejects._hnsw_creation_metadata(options)is the single placehnsw_spaceresolves, soget_collection()holds no second copy of that resolution. The option's path to a created collection is unchanged.Write amplification belongs to the benchmark suite, where it can measure I/O.
tests/test_backends.py: 91 passed. Backend/palace selection: 593 passed, 16 skipped.