feat(hnsw): versioned index nodes for VT-cached traversal + format migration - #1440
Merged
Conversation
…versal HNSW graph nodes were written via plain put() with no version, so the PrimaryRocksDatabase Verification-Table cache could never track them — every node visited during a search or insert was re-read and re-deserialized from RocksDB. At 768 dims a node decode (~8.6µs) dwarfs the cosine distance (~0.8µs), so deserialization dominated query and build time. For a FRESH custom-index object store (e.g. HNSW), initialise the index encoder as a versioned RocksDB store and add a self-versioning `autoVersion` encode mode: each value is prefixed with the record-encoder metadata header [8-byte BE float64 monotonic version][4-byte ACTION_32_BIT flags word]. The version is what the native VT extracts (first 8 bytes) to verify cache freshness; the empty flags word is required so decode consumes a metadata word instead of mis-reading the struct's first byte as flags. autoVersion never touches the shared timestampNextEncoding/metadataInNextEncoding globals (those belong to the enclosing primary write whose commit nests the index encode — harper#1307). Cache coherence rides the proven, race-safe native VT path: cold reads CAS-populate the slot from the value's version, warm reads verify via verifyVersion→FRESH (no decode), and transaction commit locks→settles the slot, invalidating stale cross-thread caches. Migration-safe: only enabled for an empty index. A pre-existing index holds un-prefixed values (incl. small-int id mappings a versioned decode would misread), so it stays in legacy mode (correct, uncached) until rebuilt. Set HNSW_NO_AUTOVERSION to disable. Measured (2000×768, 300 queries): search 26.1→9.9 ms/q (2.6x), p99 82→17 ms, build 23.3→9.7 s (2.4x). All 32 HNSW unit tests pass (recall, #1161 cold reads, #386 concurrent multi-worker). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…epend copy The autoVersion branch prepended the [version][flags] header into a freshly allocated buffer on every index-node encode — an extra allocation plus a full-payload memcpy per write, paid repeatedly during graph build as nodes are rewritten on neighbor reconnection. Switch to the same reserve-start mechanism the primary metadata path uses (the 2048|valueStart encode option reserves the header bytes at the front of the encode buffer; the header is written in place). The earlier belief that reserve-start was incompatible with the forced randomAccessStructure/typed- struct mode of custom-object indexes was a misdiagnosis: the real bug was the missing mandatory flags word. With the flags word present, reserve-start round- trips correctly — verified against the cold/frozen-read (#1161) and concurrent multi-worker (#386) suites (32/32 green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the per-open empty-guard that decided versioned-vs-legacy on every worker with a format (versioned|legacy) decided once at index create and persisted on the attribute descriptor (indexFormat). openIndex now resolves the format from the persisted descriptor via resolveIndexFormat, so every worker and reload agree on it; the empty-guard is only the initializer for the first open under this feature. table() persists the resolved format before the first node is written, closing the multi-worker race where a store that is non-empty mid-backfill was mis-read as legacy and opened with the wrong decoder. A legacy index upgrades to versioned only via an explicit reindex (a full rebuild from scratch), which flips the persisted format and re-arms the dbi encoder. The HNSW_NO_AUTOVERSION kill-switch now only blocks a NEW index from initializing versioned; an already-versioned store is still resolved versioned so its reads stay correct. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pgrade to RocksDB Cross-model review (Gemini + Codex) caught two blockers in the indexFormat migration: 1. The format resolved by the empty-guard initializer was only persisted when the schema otherwise changed. An index created before the indexFormat field existed (descriptor lacks it) over an empty store resolved 'versioned' but was never written to disk; once the store became non-empty a later load re-derived 'legacy' and opened the versioned nodes with the legacy decoder (silent corruption). Now persist the resolved format whenever it differs from the descriptor on disk (indexFormatNeedsPersist), before any node is written. 2. The legacy->versioned reindex-upgrade block was not guarded by 'rootStore instanceof RocksDatabase', unlike openIndex's arming. The versioned format is RocksDB-only (the Verification Table is RocksDB-specific), so on an LMDB-backed HNSW index it would stamp a misleading 'versioned' format. Added the guard for symmetry with openIndex. Adds a regression test for the persistence gap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kriszyp
requested review from
cb1kenobi,
dawsontoth and
kylebernhardy
and removed request for
kylebernhardy
June 22, 2026 13:22
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a self-versioning mode for custom-index object stores (such as HNSW) in RocksDB, enabling race-safe cache verification during graph traversal. It adds format resolution logic to persist whether an index is 'versioned' or 'legacy' to prevent decoder mismatches on reload, and supports upgrading legacy indexes to versioned during a full reindex. Comprehensive unit tests are also added to verify these migration paths. The feedback recommends hardening the environment variable check for HNSW_NO_AUTOVERSION to prevent truthy strings like 'false' or '0' from being misinterpreted as disabling the feature.
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.
This comment has been minimized.
This comment has been minimized.
PR #1440 review: - Gemini: the kill-switch used a bare process.env truthiness check, so HNSW_NO_AUTOVERSION="false"/"0" (truthy strings) would enable it — the opposite of intent. Centralize in hnswAutoVersionDisabled() which treats ""/"0"/"false"/unset as not-set, used at both call sites. Adds a test. - Claude: use plain node:assert (not node:assert/strict) per AGENTS.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kriszyp
marked this pull request as ready for review
June 22, 2026 14:13
dawsontoth
approved these changes
Jun 22, 2026
cb1kenobi
approved these changes
Jun 23, 2026
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
3 participants
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
Versions HNSW custom-index graph nodes so the record-caching Verification Table (VT) can track them, enabling cached, decode-free graph traversal — and adds a durable migration path for the storage format.
openIndexinitialises an HNSW object store as a versioned RocksDB store and theRecordEncodergains anautoVersionbranch that prepends the metadata header[8-byte BE float64 version][4-byte ACTION_32_BIT flags word]to each node. The flags word is mandatory — without it, decode mis-reads the struct's first byte as metadata flags. Measured (2000×768): search ~2.6×, build ~2.4×.'versioned'|'legacy') is decided once at index create and persisted on the attribute descriptor (indexFormatsibling field).resolveIndexFormatreads it back, so every worker and every reload agree — replacing the previous per-open empty-guard that re-derived the format on each open. The empty-guard is now only the first-time initializer.legacy → versionedhappens only via an explicit reindex (rebuild-from-scratch). Kill-switch:HNSW_NO_AUTOVERSION.Purpose
The VT cache is a large win for HNSW (deserialization dominates traversal), but it only caches entries with a non-null version — and HNSW nodes were un-versioned, so the cache was inert for them. Versioning the nodes makes the cache effective. The per-open empty-guard that the first cut used to decide the format was racy across workers (a store non-empty mid-backfill was mis-read as legacy → versioned data opened with the legacy decoder); persisting the decision closes that.
Where to focus
resolveIndexFormat+ the persist gate intable()(resources/databases.ts). The correctness invariant is: a versioned store is never opened with the legacy decoder (or vice-versa). The format is persisted before the first node is written (attributesDbi.putprecedesrunIndexing), so by the time a store is non-empty its format is already on disk.autoVersionencode branch inRecordEncoder.ts— the metadata-prefix composition with struct mode (the flags word being mandatory is the subtle part).legacy → versionedflips the persisted format and re-arms the encoder only on a full rebuild (lastIndexedKey === undefined), guarded to RocksDB; crash-recovery resumes keep their format.Cross-model review
Reviewed with Codex + Gemini + a Harper-domain pass. Two blockers were found and fixed in commit 4 (persistence gap for a pre-feature descriptor lacking
indexFormat; missing RocksDB guard on the reindex-upgrade block) — see commit history.Open items for reviewer attention (deliberate calls, not yet code):
f905284,20ec565) was armed versioned without persistingindexFormat, so on this final code it resolves aslegacy(unmarked non-empty → legacy) and reads its versioned nodes with the legacy decoder. This is a dev-branch artifact — the shipped feature always persistsindexFormatfrom first create — but reindex any HNSW index built on those intermediate commits. Not a production concern (the feature ships atomically).HNSW_NO_AUTOVERSIONblocks new indexes from initializing versioned but never downgrades a persisted-versioned store (downgrading would open versioned nodes with the legacy decoder until a rebuild). A true "force legacy" would need its own explicit path.indexFormatis node-local (computed per-node from each node's own store, likeindexingPID/lastIndexedKey) — it is not replicated, so a peer can't receive a foreign format that mismatches its physical store.Tests
New
unitTests/resources/vectorIndexFormat.test.js(5 cases: fresh→versioned+persist+arm; reload reads persisted not re-probed; kill-switch→legacy; legacy→versioned via reindex; persistence-gap regression). Existing HNSW / caching-rocks / index-canonical-options / index-restart-number suites pass.🤖 Generated by an LLM (Claude Opus 4.8). Base is
record-caching(#410) — this stacks on the VT caching work.