Skip to content

Fix flaky HNSW vector index unit test - #808

Merged
kriszyp merged 2 commits into
mainfrom
kris/fix-hnsw-flaky-tests
May 27, 2026
Merged

Fix flaky HNSW vector index unit test#808
kriszyp merged 2 commits into
mainfrom
kris/fix-hnsw-flaky-tests

Conversation

@kriszyp

@kriszyp kriszyp commented May 26, 2026

Copy link
Copy Markdown
Member

Summary

Fixes the flaky HierarchicalNavigableSmallWorld indexing > can delete and update and search with vector index with one dimension test seen on unrelated PRs (e.g. run 26469662007).

Purpose

Two independent fragilities surfaced intermittently. HNSW level assignment uses Math.random(), so each run produces a different graph topology and the failure mode varied across runs (~15% local repro rate):

  1. Test asserted exact recall. verifySearch used assert.deepEqual on HNSW top‑5 vs brute‑force top‑5. HNSW is approximate; under the test's stress sequence (200 inserts → delete 100 → upsert all 200 with many near‑duplicate modular‑arithmetic vectors), recall@5 can dip below 100%. Replaced with a per‑rank distance tolerance (HNSW distance must be within 0.05 of brute‑force best at that rank).
  2. validateConnectivity crashed on stale neighbor refs. HNSW deletes only scrub the deleted node's own connection list, and asymmetric edges can survive (this is logged as asymmetry detected warnings). When BFS traversed a stale neighbor ID, getSync returned undefined and the BFS threw TypeError: Cannot read properties of undefined (reading 'level'). It also inflated visited.size by adding stale IDs before confirming the node existed, which broke isFullyConnected. BFS now uses safeGetSync, skips missing nodes, and tracks queue membership separately so visited reflects real nodes only.

Where to focus review

  • Tolerance value (0.05) in unitTests/resources/vectorIndex.test.js: chose this empirically — comfortably covers observed approximation gaps (largest delta seen was ~0.016 cosine) without masking real regressions. Open to a tighter value if reviewers prefer.
  • Underlying HNSW characteristic, not a bug per se: asymmetric edges + delete‑only‑scrubs‑own‑list means stale refs are a known tradeoff (full symmetry on delete would require an inverted index). This PR makes the debug utility robust to that; it doesn't change deletion semantics. Filing a follow‑up may be worth considering if we want strong invariants here.
  • Integration tests: skipped test:integration:all since no integration tests exercise HNSW (grep confirms zero HNSW/vectorIndex refs under integrationTests/). test:unit:resources (536 tests) all pass.

Verification

  • Before fix: 5–7 failures per 30 local mocha runs.
  • After fix: 0 failures over 50 consecutive runs.
  • Cross‑model review: Codex (no concerns) + Gemini 3.1 Pro (one suggestion incorporated — added assert(results.length >= 5) bounds check).

🤖 Generated with Claude Opus 4.7

Two independent fragilities surfaced intermittently in
HierarchicalNavigableSmallWorld indexing > can delete and update and
search with vector index with one dimension (HNSW uses Math.random()
for level assignment, so the failure mode varies across runs):

- verifySearch asserted exact vector equality between HNSW top-5 and
  brute-force top-5. HNSW is approximate; under heavy delete/update
  churn with many near-duplicate vectors, recall@5 can drop below
  100%. Replaced with a per-rank distance tolerance check (HNSW
  distance must be within 0.05 of brute-force best at that rank).

- validateConnectivity crashed (TypeError reading 'level' on
  undefined) when BFS traversed stale neighbor IDs left over from
  deletes — HNSW only scrubs the deleted node's own connection
  list, and asymmetric edges can survive. It also inflated
  visited.size by recording stale neighbor IDs before confirming
  the underlying node existed, breaking isFullyConnected. BFS now
  uses safeGetSync, skips missing nodes, and tracks queue
  membership in a separate set so visited reflects real nodes only.

Verified stable: 50 consecutive runs pass; all 536 resources tests
still pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@kriszyp
kriszyp requested a review from dawsontoth May 26, 2026 23:46
@claude

claude Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

Pre-existing Prettier drift on main from 46109e6 was breaking the
Format Check job on every new PR. Pure whitespace reformat, no
behavior change.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@kriszyp
kriszyp marked this pull request as ready for review May 27, 2026 00:55
@kriszyp
kriszyp requested a review from cb1kenobi May 27, 2026 21:00
@kriszyp
kriszyp merged commit b1a8698 into main May 27, 2026
38 checks passed
@kriszyp
kriszyp deleted the kris/fix-hnsw-flaky-tests branch May 27, 2026 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants