fix(hnsw): stop search() silently dropping stored points (#773)

[vidaunited]

Fixes #773.

VectorDB.search() intermittently omitted a stored row while db.get() still returned it and db.len() still counted it, and raising efSearch never recovered it. That last detail is the tell: the point had no in-edges to traverse to, so no amount of extra breadth could reach it.

Root cause is two defects in graph construction in the vendored patches/hnsw_rs. Both are required — fixing either alone still leaves indexes broken (ablation below).

1. Symmetric edges written on the wrong layer

reverse_update_neighborhood_simple (hnsw.rs:1245):

let l_n = n_to_add.point_ref.p_id.0 as usize;   // the NEW POINT's top level
q_point_neighbours[l_n].push(Arc::new(n_to_add));

The backlink was stored at the new point's top level rather than at l, the layer the forward edge was just built on. Consequences:

• a point at level >= 1 receives no layer-0 in-edges at all, so a layer-0 traversal can only reach it when it happens to be the entry point;
• edges are written above the neighbour's own level, so level-0 points end up carrying layer-1 neighbour lists.

Graph dump of a failing three-row index (search returned [0, 2], dropping 1):

origin_id=0 p_id=PointId(0,0) neighbours=[(0,[2]), (1,[1])]   <- level-0 point holding a layer-1 list
origin_id=2 p_id=PointId(0,1) neighbours=[(0,[0]), (1,[0])]
origin_id=1 p_id=PointId(1,0) neighbours=[(0,[0]), (1,[0])]   <- nothing in layer 0 points AT id 1

Node 1's edge to node 0 exists; the return edge landed on layer 1, so layer 0 can never arrive at node 1.

Fixed to min(l, q.level) — the layer the edge was built on, clamped so it is never written above either endpoint's own level. The clamp is needed because search_layer seeds its heap with the entry point unconditionally and can therefore hand back a q whose own level is below l.

2. search_layer short-circuited on an empty layer bucket

hnsw.rs:933:

if self.layer_indexed_points.points_by_layer.read()[layer as usize].is_empty() {
    return return_points;   // empty
}

points_by_layer buckets each point under its top level only (generate_new_point pushes to points_by_layer[p_id.0]), while a point of level L participates in the graph at every layer 0..=L. An empty bucket therefore does not mean an empty layer. Once every point in a small index had level >= 1, layer 0's bucket was empty while layer 0 still carried edges — and the bail-out left the point being inserted with no layer-0 neighbours whatsoever.

The traversal below that check is seeded from entry_point and bounded by neighbour lists, so it is well defined regardless of the bucket: an entry point with no neighbours at that layer simply yields itself. Check removed.

Measurements

20,000 independent three-row indexes, using the defaults the ruvector npm wrapper passes (m=32, efConstruction=200, k=64, efSearch=256):

build	short results
baseline	780/20000
fix 1 only	22/20000
fix 2 only	821/20000 (no improvement)
both	0/20000

Recall improves rather than regressing, and self-probe loss (search each stored vector with itself) goes to zero:

	points lost	recall@10
n=1000 uniform	1 -> 0	0.9930 -> 0.9940
n=1000 clustered	0 -> 0	0.9970 -> 0.9980
n=5000 uniform	12 -> 0	0.9445 -> 0.9565
n=5000 clustered	24 -> 0	0.8820 -> 0.9200

This also confirms the reporter's observation that clustered real embeddings fail at roughly twice the rate of near-orthogonal random vectors (24 vs 12).

End-to-end verification

Ran the reproduction script from #773 unchanged, against a locally built @ruvector/core swapped into node_modules. The control was built through the identical pipeline with only the hnsw.rs change reverted, so the clean result cannot be an artifact of a local build differing from the published one:

fix reverted:   short results: 7/200
fix applied:    short results: 0/1000

Tests

Adds crates/ruvector-core/tests/hnsw_issue_773_regression.rs:

• issue_773_three_row_index_returns_every_row — the exact shape from the issue, aggregated over 400 indexes
• issue_773_every_point_retrieves_itself — the detection probe from the issue over a 2,000-point index

Both were confirmed to fail against the unpatched vendored crate (9/400 short; 6/2000 unreachable), so they are not vacuous. Levels come from StdRng::from_entropy(), so each test aggregates enough independent indexes that passing with the defect present is ~1e-5.

cargo test -p ruvector-core --release: 494 passed, 0 failed, including the pre-existing hnsw_integration_test.

Note for reviewers

The vendored crate's own #[cfg(test)] modules do not compile — they use the rand 0.9 API (rand::rng(), Uniform::new(..).unwrap()) while the vendoring pins rand 0.8 for WASM compatibility. This is pre-existing and unrelated to this change (an unmodified patches/hnsw_rs/src/hnsw.rs produces the identical 25 errors under cargo test --lib), and it is why the regression test lives in ruvector-core rather than beside the code it guards.

Both defects most likely also exist upstream in jean-pierreBoth/hnswlib-rs, since patches/hnsw_rs was vendored for a rand downgrade rather than forked for behaviour. I have not checked their current main.

🤖 Generated with claude-flow

https://claude.ai/code/session_016Z5LMV7ZW9PFaG9fzvpuwu

[vidaunited]

Follow-up with measurements I owe this PR, from preparing the same fix against upstream hnswlib-rs (jean-pierreBoth/hnswlib-rs#38, issue #37). Two corrections and one caveat — none of them change the fix, but one of them qualifies the claim.

1. My original self-probe metric was too tight on dense data

The measurements in the PR description used k=10, ef=64 for the "is this point retrievable by its own vector" probe. On tightly clustered data at scale that asks "is this point in its own top 10", not "is this point reachable" — in one configuration it reported 479 orphans where widening to k=200, ef=1024 recovered 478 of them at rank 0. The tiny-index numbers (k=64 against 3 stored rows) and the end-to-end reproduction are unaffected, but the mid-size self-probe figures should be read as reachability-grade only at k=200, ef=1024.

2. Re-measured with a reachability-grade probe, the fix is broader than reported

Sequential insert, orphans = not returned by a search for its own vector at k=200, ef=1024:

configuration	before	after
n=3000, M=16, dim=128, uniform	15	0
n=3000, M=16, dim=768, clustered	37	0
n=3000, M=8, dim=64, uniform	102	0
n=10000, M=16, dim=128, uniform	116	0
n=10000, M=32, dim=128, clustered	43	0
n=20000, M=16, dim=128, uniform	306	0

3. Caveat: two dense-cluster configurations are not fixed, and one is worse

configuration	before	after
n=3000, M=6, dim=64, clustered	99	144
n=20000, M=16, dim=128, clustered	3241	3258

The second row matters for this repo specifically, because HnswConfig::default() uses m: 16 (the npm wrapper passes m: 32, which is clean in every configuration I measured). At 20k tightly clustered vectors that regime is ~16% orphaned before and after this change — so this PR should be read as "fixes the reported loss", not "search loss is now impossible".

I believe this is a distinct upstream defect that this change exposes rather than causes: reverse_update_neighborhood_simple shrinks an over-full neighbour list by dropping the farthest entry rather than applying the diversity heuristic from select_neighbours. Before the fix, backlinks were scattered to each point's top level, so layer-0 lists stayed short and rarely reached the 2 * max_nb_connection threshold; now they fill up and the naive shrink evicts the long-range edges, which fragments tightly clustered data. Reported upstream rather than patched here, since it is a real algorithmic change to the vendored crate.

Nothing above affects the two regression tests in this PR, the 494-test ruvector-core run, or the end-to-end result on the issue's own reproduction (7/200 short before, 0/1000 after).