feat: Expose checkIntegrity() method to Python bindings#644
Open
jsbattig wants to merge 4 commits into
Open
Conversation
Add check_integrity() method to Python Index class that validates HNSW graph structure including: - Connection validity (no invalid neighbor IDs) - No self-loops - No duplicate connections - No orphan nodes (elements with no inbound connections) Returns dict with: valid, connections_checked, element_count, min_inbound, max_inbound, errors[] This enables CIDX to perform health checks on HNSW indexes.
…c HNSW orphan repair
Adds Index<float>::repairOrphans() (bindings.cpp), following the same
pattern as the existing checkIntegrity() Python-friendly wrapper. For each
zero-inbound ("orphan") node, forces a back-edge from its own existing
level-0 neighbors, evicting a neighbor's current weakest edge only when
that eviction cannot itself create a new orphan (inbound count > 1 guard).
When an orphan's own local neighborhood offers no viable anchor (a
fragile-sub-clique lockup), falls back to a distance-sorted scan of the
whole graph -- a pigeonhole argument guarantees a safe candidate exists
somewhere since total inbound edges vastly exceed the element count.
Bounded to at most cur_element_count + 1 passes (provable termination).
Idempotent and deterministic under single-threaded construction; also
repairs the exact-tie race regime under multi-threaded construction.
Story #1358 / spike #1330 (code-indexer docs/research/hnsw-temporal-orphans-1330.md).
Code review finding on Story #1358: the inbound-counting loop in repairOrphans() already bounds-checked neighbor ids before use (`if ((size_t)data[j] < n) ...`), but the try_connect lambda used for the anchor/victim connection logic did not apply the same guard: - get_linklist0(anchor) where `anchor` is read out of a link list -- an out-of-range id would index outside cur_element_count (a write beyond max_elements is a true out-of-bounds write). - inbound[cand] where `cand` is similarly sourced -- out-of-range std::vector::operator[] (undefined behavior) if that id is >= n. Unreachable via the near-tie/exact-tie regimes alone (they only ever produce zero-inbound nodes, never invalid-id connections), but S3's future fleet sweep will invoke repair_orphans() against arbitrary real production indexes, which could carry other corruption (torn-write scenarios). Verified both were real, reachable crashes prior to this fix by directly corrupting an on-disk index's link-list entries with out-of-range ids and calling repair_orphans(): the anchor-id case segfaulted, the candidate-id case aborted with a std::vector::operator[] out-of-range assertion. Rebuilt with the fix: both scenarios now return gracefully with no crash. Story #1358 / Epic #1333.
GitHub issue #1437 (LightspeedDMS/code-indexer): neither save_index() nor load_index() released the Python GIL during the native file read/write + graph (de)serialization, so every HNSW shard load blocked ALL Python threads in the server process for the whole native call. Over NFS with multi-hundred-MB/GB temporal shards this froze the code-indexer Web UI and MCP front door for seconds to tens of seconds per query. Fix: py::call_guard<py::gil_scoped_release>() on Index<float>::save_index and Index<float>::load_index (bindings.cpp), matching the existing gil_scoped_release pattern already used by add_items/knn_query in this file. Only Index<float> is touched; BFIndex<float>'s save_index/load_index are out of scope for this issue. Concurrency safety analysis (loadIndex's `delete appr_alg; appr_alg = new HierarchicalNSW(...)` reassignment): releasing the GIL for the whole call means two Python threads calling load_index() concurrently on the SAME Index object would race on this delete/reassign. Audited the actual consumer (code-indexer's HNSWIndexManager.load_index() and HNSWIndexCache.get_or_load()): every call site constructs a brand-new hnswlib.Index() Python object before calling load_index() on it, and the cache's per-key threading.Event single-flight sentinel guarantees only one thread ever calls the loader (and thus load_index()) for a given cache key at a time -- concurrent loads only ever happen for DIFFERENT keys, which are always DIFFERENT Index objects. No code path in the consumer repo calls load_index()/save_index() twice concurrently on the same Index instance, so a defensive mutex inside loadIndex/saveIndex was judged unnecessary and was not added, keeping the change minimal. TDD: tests/python/bindings_test_gil_release.py builds a real ~300MB on-disk index (500k x float32[128], M=8, ef_construction=40) and proves, via a concurrent recorder-thread timestamp-gap technique, that a background Python thread keeps making progress while save_index()/ load_index() run. Verified RED against the unfixed binding (recorder silent for 90.8% of load_index()'s duration, 96.2% of save_index()'s) and GREEN after the fix (gap ratio comfortably under the 0.5 threshold for both). Full existing bindings_test*.py suite re-run clean: 16/16 passing (14 pre-existing + 2 new), 92.9s, zero regressions.
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
1 participant
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.
Summary
Add
check_integrity()method to Python Index class that validates HNSW graph structure.Changes
check_integrity()method to the Python bindings inpython_bindings/bindings.cppReturn Value
Returns a dictionary with:
valid: Boolean indicating if graph passed all checksconnections_checked: Total number of connections validatedelement_count: Number of elements in the indexmin_inbound: Minimum inbound connections for any nodemax_inbound: Maximum inbound connections for any nodeerrors: List of error messages if any issues foundUse Case
This enables applications using hnswlib to perform health checks on HNSW indexes, detecting corruption or structural issues that could affect search quality.
Testing
The method has been tested with real HNSW indexes in the CIDX (Code Indexer) project.
🤖 Generated with Claude Code