Beam the upper-layer descent so a local minimum cannot strand a search

[kriszyp]

The upper-layer descent walked with a beam of one. greedy_descend hill-climbed to the first node no neighbour improved on, then dropped a level. On a clustered corpus that local minimum can sit in the wrong basin, and layer-0 adjacency is intra-basin, so the layer-0 beam has no uphill edge with which to leave. The query's true nearest neighbour is then unreachable at any ef, which is why raising it never helped. That is what made concurrent_insert_search's misses == 0 assertion fail intermittently and blocked the v0.2.0 npm publish.

beam_descend runs the existing search_layer at width DESCENT_EF = 16 on each upper level instead, seeded by the level above's best, rolling the scratch epoch per level so a node present at several levels stays expandable at each. It is shared by the read and the write path, as the greedy descent was.

Calling search_layer per level would have meant allocating per level, so the layer search no longer allocates at all: its two heaps live in the reusable SearchScratch and it fills a caller-owned output buffer, which the descent reuses across levels and insert reuses across its per-level searches. That is what pays for the wider descent on the write side: before it, building 50k x 768-d cost 20% more than main; after it, seven paired 100k builds have medians of 92 s on main against 88 s here, and the least-loaded pair is 56 s against 61 s. Build throughput is within the noise of a shared box either way. A new test binary installs a counting global allocator and asserts per-query allocations do not grow with graph height — without it, 3 allocations per query on a 2-level graph against 5 on a 4-level one.

Concurrency was never the cause — it only shuffles the insertion order. Fixing that order with a seeded permutation reproduces the trap single-threaded, which is what the new regression test pins: seeds 57 and 240 lose 55 and 22 of their 8000 self-queries on the old descent, and none on the new one.

For the human reviewer

1. The beam width is 16, and it is a compile-time constant rather than a parameter. Kris ruled on this after seeing the sweep. Width 8 costs about 10% fewer visits per query (762 against 844 at ef 64), but one build in 1300 still loses 8 of its 8000 nodes; width 16 loses none in 1300. The alternative was exposing it on InsertParams and the search call — declined because adding a field to a public struct breaks struct-literal callers, a new search argument changes index.d.ts and every JS call site, and this is a correctness floor rather than a recall/latency dial (ef is the dial). Fully reversible: it is one constant. Saying no costs a rebuild and a re-run of the sweep.

2. The write path shares the widened descent, so this changes graph construction, not only queries. The step-6 planning review returned Framing-Verdict: better-alternative-exists and proposed read-side beam with greedy insertion retained, on the grounds that it repairs existing files and avoids the build cost. It asked for a causality matrix before coupling the two, so I ran one — read width and write width made independent, same seeds, same corpus, 200 builds per cell:

   build width	read width	self-query misses	builds with a miss
   1	1 (today)	125	17
   1	16 (read-only)	37	13
   16	1	245	20
   16	16 (this PR)	0	0

   Read-only widening cuts misses 3.4x and still leaves 37 across 13 builds, which is not enough to make the assertion hold. Build-only widening is worse than baseline, which is the same finding from the other side: a graph wired under one routing policy and queried under another is less navigable than one where they agree. insert seeds each level's search_layer from the descent's landing point, so a width-1 insert descent picks the new node's neighbours from the wrong basin and the node never acquires the in-edges a correctly-routed search arrives on. I overruled the framing verdict on that measurement. This is the entry to push back on if you want to. Reversing it later is a one-line change plus a rebuild, but it is not free: a plane file built under one policy keeps its topology.

3. Rollout: an existing plane file is improved on its next query, not repaired. Row two of that matrix is exactly the upgrade state — old graph, new reader — and it is 37 misses rather than 0. Full repair needs the nodes re-inserted. No format change and no version bump; this is traversal and construction only, so an old file opens and reads normally.

4. A downgrade to a pre-change binary reads a width-16-built plane with a width-1 descent, and nothing detects it. That is the worst cell of the matrix above: 245 lost self-queries per 200 builds, against 125 for the never-changed baseline. The on-disk layout is unchanged, so the format version still gates only layout while the incompatibility is graph shape. Fixing it properly means a header field recording the descent width a plane was built with, so an incompatible reader refuses the open — and this task explicitly rules out a format change. The exposure today is theoretical: this is v0.0.1, the v0.2.0 publish was blocked by the very test this fixes, and there are no field planes. It is filed as a finding rather than fixed here, but a rolling upgrade or rollback is the thing to think about before that changes.

5. A per-level visit cap was tried and removed. The planning review called the tied-distance case a blocker: a zero query gives every stored vector cosine distance exactly 1.0, so "it can visit the entire reachable upper graph and grow BinaryHeap". Measured, it does not — the descent visits 608 nodes on a 50k graph whose upper component is about 3300, because search_layer pushes a neighbour only on d < worst and < is strict, so once the result set fills, tied candidates are never pushed and the candidate heap drains after at most ef expansions per level. The same measurement shows a cap cannot be sized safely. The worst ordinary query over 3000 random queries visits 788 nodes at level 1 on a 50k graph and 1044 on a 500k graph, so the obvious ef * UPPER_CAP = 1024 ceiling has 1.3x headroom at 50k and is already exceeded at 500k. It would clip real searches, silently degrading the recall this change exists to restore. What ships instead is a characterization test that fails if that < is ever relaxed to <=. The residual, stated plainly: a beam's worst case per level is O(level population), the same class as the layer-0 beam that search already runs with visit_budget = u64::MAX.

6. Filtered and predicated searches now count their visit budget from where the descent left off. The budget is documented as the layer-0 cap, but it was measured against a counter the descent had already advanced, so a wider descent would have silently eaten into it. Both paths have a test that fails without the rebase, returning the descent's landing point alone instead of ten hits. It is a semantic change for hosts already tuned against the old meaning: filterExpansion and visitBudget now buy layer-0 visits rather than total visits, so a filtered query's worst case is the descent plus the budget, and only the second term is host-bounded.

7. Where to look hardest: concurrent throughput, because the box I measured on is shared. Fourteen alternating base/head runs of the benchmark's concurrent pass — 8 searcher threads plus a background writer at 100k x 768-d — put aggregate throughput at 8200-9000 QPS on main and 7100-7800 here at comparable load, which is the per-query visit increase and nothing more; medians over all fourteen are within 3%, mostly because machine load dominates. One head run of seven collapsed to 1168 QPS unexplained, and one base run of seven collapsed to 1584, so I cannot attribute either. If you want to satisfy yourself on a quiet machine, the mechanism to rule out is add_reverse_edge: its upper-level branch runs prune_with_coverage inside update_upper_level's seqlock write lock, while the layer-0 branch directly above it deliberately runs the same prune outside the lock and applies it with a compare-and-set. A wider descent reads many more upper entries per query, so it collides with that critical section more often. That lock discipline is pre-existing and untouched here; it is filed as a finding rather than changed under a deadline.

8. greedy_descend is renamed, not kept as an alias. The crate ships as an npm native addon; its rlib has no consumer outside this repository's own tests, bins and the ignored sweep, and it is not published to crates.io. The planning review asked for the removal to be treated explicitly as a Rust API break — this is that acknowledgement.

9. One defect this change introduced, and the review caught it. Filling a caller-owned buffer looked like it needed reserving up front, and ef reaches that reservation from an unvalidated u32 at the N-API boundary — the previous collect() sized to the results actually found. A query carrying ef: 4294967295, a config typo or an API that hands efSearch to its caller, asks for 34 GB, and handle_alloc_error answers by aborting the process, uncatchable from JS and taking every in-flight query with it. Linux overcommit usually hides this; the Windows CI runner and strict-overcommit hosts do not. The buffer is filled by one extend off an exact-size drain, which already reserves once for the results actually found, so nothing is sized by ef at all now — verified by the allocation bound above, which still holds with no reservation.

Verification

End-to-end route: the existing concurrent integration test, a deterministic version of it, and the packaged N-API smoke. Everything below was executed on this branch; main figures come from the same commands against 95c8076.

The release blocker itself. The documented oversubscribed repro, 8 processes pinned to 2 cores:

cargo test --release --test concurrent --no-run
BIN=$(ls -t target/release/deps/concurrent-* | grep -v '\.d$' | head -1)
for round in $(seq 1 60); do for j in $(seq 1 8); do taskset -c 0,1 $BIN concurrent_insert_search --exact >/tmp/r.$round.$j.log 2>&1 & done; wait; done
grep -l 'panicked' /tmp/r.*.log | wc -l

0 failures over 480 runs, against 6 over 400 on main (1.5%).

The new tests fail on base. Reverting src/ and re-running: the descent regression panics with seed 57: 55 of 8000 nodes are their own true nearest neighbor but unreachable from the entry point; both budget tests return the descent's landing point alone instead of ten hits; the allocation test reports 3 allocations per query on a 2-level graph against 5 on a 4-level one. The descent regression also fails if DESCENT_EF is lowered — 8 misses at width 8, 20 at width 4 — so the width is pinned, not just the beam.

Deterministic width sweep (descent_width_sweep, ignored by default), one 8000-node build per seed, self-querying every node at ef 256:

descent width	builds	self-query misses	builds with a miss
1 (main)	200	125	17
4	200	20	6
8	1300	8	1
16 (this PR)	1300	0	0
24	700	0	0

Build throughput, seven paired 100k x 768-d builds, sorted (seconds): main 55.6 / 71.6 / 76.3 / 91.7 / 114.3 / 116.4 / 172.0; here 61.4 / 80.1 / 82.7 / 87.9 / 90.3 / 179.1 / 182.7. Medians differ by 4% in this branch's favour, the least-loaded pair by 10% against it. Before the allocation work the same comparison at 50k was a clear 20% regression (11.8 s against 14.2 s).

Suites. cargo test --release — 49 tests green (21 lib, 1 allocation, 4 concurrent including 1 ignored, 24 reopen), against CI's 20-minute cap on three runners. The added tests are the dominant cost of the suite; they are also the only guards for a defect that DESIGN.md records as not reproducing on any smaller corpus. node build.mjs && node smoke.mjs — green, which is what exercises search, searchSync and searchWithPredicate through the packaged addon rather than the crate.

Cost. Visit counts are deterministic, so they are the load-independent measure:

ef	visits/query on main	visits/query here	recall@10 on main	recall@10 here
16	570	722	0.844	0.903
32	664	802	0.945	0.989
64	721	844	0.983	1.000
128	771	884	1.000	1.000
512	1342	1467	1.000	1.000

Recall improves below ef 128 and is unchanged above. The descent is fixed work, so the visit tax is largest where ef is smallest.

Latency. The box these ran on is shared with other agents at load average 11-25, so single-shot timings swing by 3x in both directions and are not usable. These are the best of seven repetitions per point, which is the estimate least polluted by other tenants, and the plane files are built once and reused across all reps:

ef	best p50 on main	best p50 here
16	0.20 ms	0.31 ms
32	0.20 ms	0.31 ms
64	0.24 ms	0.33 ms
128	0.25 ms	0.31 ms
512	0.68 ms	0.72 ms

The descent is fixed work, so the cost is a roughly flat +0.06 to +0.11 ms: about half again on a 0.20 ms query at ef 16, and 6% at ef 512.

Out of cache, 200 000 x 768-d (269 MB of slots, one upper level more than 50k, paired back to back):

	ef	p50	visits/query	recall@10
main	16	0.22 ms	615	0.675
here	16	0.34 ms	978	0.899
main	64	0.74 ms	868	0.909
here	64	0.45 ms	1117	0.989
main	512	1.61 ms	1639	1.000
here	512	1.77 ms	1823	1.000

Recall gains grow with the graph: at 50k the beam buys 0.844 -> 0.903 at ef 16, at 200k it buys 0.675 -> 0.899. A taller hierarchy gives a width-1 descent more places to strand.

Concurrent, 100 000 x 768-d, 8 searcher threads plus a background writer, fourteen alternating base/head runs with machine load recorded per run. Aggregate QPS, sorted:

	runs
main	1584, 3507, 4114, 6880, 7743, 8232, 8959
here	1168, 4290, 6595, 6662, 6704, 7142, 7818

Medians are within 3%. At the lowest observed load the comparison is 8200-9000 QPS on main against 7100-7800 here, which is the per-query visit increase and nothing beyond it. One run on each side collapsed unexplained and neither could be attributed on a shared box.

Complexity: complicated

_{Review-Coverage: authored=claude; ran=gemini,codex; declined=cursor-grok,cursor-composer,domain; rounds=7 @ 8a5eb71}

_{Human-Review-Need: 3 @ 8a5eb71}

[kriszyp]

Reviewers picked from the expertise matrix rather than affinity: every line these files touch was written by the PR author, so the affinity ranking returns no candidates. @dawsontoth is Responsible Expert for vector search and @kylebernhardy is listed competent in it.

The two entries most worth a ruling are the beam width staying a compile-time constant, and the write path sharing the widened descent — the second overrules a better-alternative-exists planning verdict on a measured causality matrix, which is in the description.

— Claude Fable 5.1