fix(backends): keep post-deletion dim-None HNSW segments instead of quarantining (#1710) - #1723
Conversation
…uarantining (MemPalace#1710) _missing_dimensionality_appears_recoverable required total_elements_added == len(id_to_label) before keeping a dim-None segment, but total_elements_added is a monotonic add-counter while id_to_label holds only the live elements. After any delete the counts diverge, so every post-deletion segment was quarantined; once the WAL is pruned that destroys the only copy of those vectors. Relax the check to >= and keep the bijection and size checks as the integrity backstop.
There was a problem hiding this comment.
Code Review
This pull request fixes an issue where post-deletion segments with missing dimensionality were incorrectly quarantined. It relaxes the check in _missing_dimensionality_appears_recoverable to allow total elements to be greater than or equal to label_count, since total_elements_added is monotonic and can exceed the live label count after deletions. A corresponding unit test has been added to verify this behavior. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
What does this PR do?
quarantine_invalid_hnsw_metadatarenames an HNSW segment to*.corrupt-*when itsindex_metadata.picklehasdimensionality: Nonetogether with labels, unless_missing_dimensionality_appears_recoverablejudges the pickle safe to keep. developalready keeps such a segment when its bookkeeping is internally consistent, but the helper
required
total_elements_added == len(id_to_label), and that equality only holds for asegment that has never had a delete.
total_elements_addedis a monotonic counter of every element ever added, whileid_to_labelandlabel_to_idtrack only the live elements. After any deletiontotal_elements_addedexceeds the live label count, so the helper classified the segmentas unrecoverable and quarantined it. When the WAL had already been pruned (as it is after
the flush that writes the dim-None pickle), that segment was the only copy of the vectors,
so a later cold start rebuilds an empty index: the rows still count, but vector search
returns nothing for them. That is the silent erosion QuiqueMH reported in #1710 (forensics:
total_elements_added: 4826against4792labels).The change relaxes the counter check from
==to>=(one operator,!=to<). Thelabel-map size check and the bijection check are untouched, so inconsistent label maps are
still quarantined, and an impossible
total_elements_added < len(id_to_label)is stillrejected.
This closes the remaining reproducible case in #1710; the
total == labelsvariant isalready kept by the helper on develop.
How to test
New regression test:
It writes a dim-None pickle with
total_elements_added=5over two live labels (threedeleted) and asserts the segment is kept rather than quarantined. It fails on develop and
passes with this change.
Checked end to end against real ChromaDB 1.5.7, 1.5.8 and 1.5.9: a collection that is
built, then has rows deleted and rows added, persists a pickle with
dimensionality=Noneand
total_elements_added > len(id_to_label). Before this change the guard quarantinesthat segment and a later cold start finds it gone (count still reports the rows, but vector
search returns nothing); after it the segment is kept and a fresh process opens and queries
it with no error and no SIGSEGV on any of the three versions.
Full suite (
python -m pytest tests/ --ignore=tests/benchmarks): 2431 passed, 5 skipped.Checklist
python -m pytest tests/ -v)ruff check .)