diff --git a/src/hnsw/hnswIndex.ts b/src/hnsw/hnswIndex.ts index c5acd20f..b4948bf8 100644 --- a/src/hnsw/hnswIndex.ts +++ b/src/hnsw/hnswIndex.ts @@ -32,6 +32,14 @@ const DEFAULT_CONFIG: HNSWConfig = { */ export class JsHnswVectorIndex implements VectorIndexProvider { private nouns: Map = new Map() + /** + * Reverse adjacency: `target id → (level → set of node ids that link TO target)`. + * Lets `removeItem` find a node's in-neighbors in O(in-degree) instead of scanning + * the whole corpus (which made bulk delete O(N²)). Lazily built on first need and + * maintained incrementally on link/prune/remove; `null` means "stale — rebuild on + * next use" (set by every bulk path: cold-load restore + `clear()`). + */ + private incoming: Map>> | null = null private entryPointId: string | null = null private maxLevel = 0 // Track high-level nodes for O(1) entry point selection @@ -245,6 +253,9 @@ export class JsHnswVectorIndex implements VectorIndexProvider { hnswData: { level: number; connections: Record }, noun: HNSWNoun ): Promise { + // Bulk load sets connections directly (bypassing the link path that maintains + // the reverse index) — mark it stale so the next removeItem rebuilds it. + this.incoming = null const storageWithBlob = this.storage as unknown as { loadBinaryBlob?: (key: string) => Promise } @@ -489,12 +500,14 @@ export class JsHnswVectorIndex implements VectorIndexProvider { noun.connections.get(level)!.add(neighborId) + this.addIncoming(neighborId, level, id) // forward edge id → neighborId // Add reverse connection if (!neighbor.connections.has(level)) { neighbor.connections.set(level, new Set()) } neighbor.connections.get(level)!.add(id) + this.addIncoming(id, level, neighborId) // forward edge neighborId → id // Ensure neighbor doesn't have too many connections if (neighbor.connections.get(level)!.size > this.config.M) { @@ -841,6 +854,44 @@ export class JsHnswVectorIndex implements VectorIndexProvider { return [...nearestNouns].slice(0, k) } + /** + * Build (or return the cached) reverse-adjacency index. O(N + E) the first time + * after a bulk load/clear; O(1) thereafter while it stays live. + */ + private ensureIncoming(): Map>> { + if (this.incoming !== null) return this.incoming + const inc = new Map>>() + for (const [nodeId, node] of this.nouns) { + for (const [level, targets] of node.connections) { + for (const target of targets) { + let byLevel = inc.get(target) + if (!byLevel) { byLevel = new Map(); inc.set(target, byLevel) } + let set = byLevel.get(level) + if (!set) { set = new Set(); byLevel.set(level, set) } + set.add(nodeId) + } + } + } + this.incoming = inc + return inc + } + + /** Record a new forward edge `source → target` at `level` (no-op while the index is stale). */ + private addIncoming(target: string, level: number, source: string): void { + if (this.incoming === null) return + let byLevel = this.incoming.get(target) + if (!byLevel) { byLevel = new Map(); this.incoming.set(target, byLevel) } + let set = byLevel.get(level) + if (!set) { set = new Set(); byLevel.set(level, set) } + set.add(source) + } + + /** Record that the forward edge `source → target` at `level` is gone. */ + private removeIncoming(target: string, level: number, source: string): void { + if (this.incoming === null) return + this.incoming.get(target)?.get(level)?.delete(source) + } + /** * Remove an item from the index */ @@ -852,41 +903,38 @@ export class JsHnswVectorIndex implements VectorIndexProvider { const noun = this.nouns.get(id)! - // Remove connections to this noun from all neighbors - for (const [level, connections] of noun.connections.entries()) { - for (const neighborId of connections) { - - const neighbor = this.nouns.get(neighborId) - if (!neighbor) { - // Skip neighbors that don't exist (expected during rapid additions/deletions) - continue - } - if (neighbor.connections.has(level)) { - neighbor.connections.get(level)!.delete(id) - - // Prune connections after removing this noun to ensure consistency - await this.pruneConnections(neighbor, level) + // Reverse-adjacency lets us touch ONLY the nodes that actually reference `id` + // (its in-neighbors) rather than scanning the whole corpus — turning a delete + // from O(N) into O(in-degree) and a bulk delete from O(N²) into O(N·degree). + // Snapshot each referrer set because pruneConnections mutates the index. + const incoming = this.ensureIncoming() + const referrers = incoming.get(id) + if (referrers) { + for (const [level, refSet] of referrers) { + for (const refId of Array.from(refSet)) { + const ref = this.nouns.get(refId) + if (ref && ref.connections.has(level)) { + // Drop the forward edge ref → id, then re-prune ref so the graph stays + // navigable. (id's own reverse entry is dropped wholesale below, so we + // intentionally do not maintain incoming[id] inside this loop.) + ref.connections.get(level)!.delete(id) + await this.pruneConnections(ref, level) + } } } } - // Also check all other nouns for references to this noun and remove them - for (const [nounId, otherNoun] of this.nouns.entries()) { - if (nounId === id) continue // Skip the noun being removed - - - for (const [level, connections] of otherNoun.connections.entries()) { - if (connections.has(id)) { - connections.delete(id) - - // Prune connections after removing this reference - await this.pruneConnections(otherNoun, level) - } + // id's OUTGOING edges disappear with it → drop id from each out-neighbor's + // reverse set so no stale referrer survives. + for (const [level, targets] of noun.connections) { + for (const target of targets) { + this.removeIncoming(target, level, id) } } - // Remove the noun + // Remove the noun + its reverse-index entry. this.nouns.delete(id) + this.incoming?.delete(id) // If we removed the entry point, find a new one if (this.entryPointId === id) { @@ -966,6 +1014,7 @@ export class JsHnswVectorIndex implements VectorIndexProvider { */ public clear(): void { this.nouns.clear() + this.incoming = null // reverse index no longer reflects the (now empty) graph this.entryPointId = null this.maxLevel = 0 } @@ -1762,7 +1811,11 @@ export class JsHnswVectorIndex implements VectorIndexProvider { // Only proceed if we have valid neighbors if (distances.size === 0) { - // If no valid neighbors, clear connections at this level + // If no valid neighbors, clear connections at this level. Every current + // connection is dropped — unlink each from the reverse index. + if (this.incoming !== null) { + for (const dropped of connections) this.removeIncoming(dropped, level, noun.id) + } noun.connections.set(level, new Set()) return } @@ -1774,8 +1827,16 @@ export class JsHnswVectorIndex implements VectorIndexProvider { this.config.M ) - // Update connections with only valid neighbors - noun.connections.set(level, new Set(selectedNeighbors.keys())) + // Update connections with only valid neighbors. Pruning only ever drops + // edges (the selected set is a subset of the current connections), so unlink + // each dropped neighbor from the reverse index. + const kept = new Set(selectedNeighbors.keys()) + if (this.incoming !== null) { + for (const prev of connections) { + if (!kept.has(prev)) this.removeIncoming(prev, level, noun.id) + } + } + noun.connections.set(level, kept) } /** diff --git a/tests/unit/hnsw/remove-reverse-adjacency.test.ts b/tests/unit/hnsw/remove-reverse-adjacency.test.ts new file mode 100644 index 00000000..a3a3b7ce --- /dev/null +++ b/tests/unit/hnsw/remove-reverse-adjacency.test.ts @@ -0,0 +1,168 @@ +/** + * @module tests/unit/hnsw/remove-reverse-adjacency + * @description Correctness guard for the reverse-adjacency index that makes + * `removeItem` O(in-degree) instead of O(N) (and bulk delete O(N·degree) instead + * of O(N²)). The risk of a reverse index is staleness — a missed maintenance site + * leaves a dangling reference to a deleted node or a stale referrer. These tests + * exercise mixed add/delete workloads and assert three invariants: + * + * 1. No surviving node's connections reference a deleted id (no dangling edges). + * 2. The incrementally-maintained reverse index matches a fresh rebuild from the + * live adjacency — i.e. link/prune/remove maintenance is exactly consistent. + * 3. Search still returns the true nearest neighbours of the surviving set + * (verified exactly against a brute-force scan on a complete small graph). + */ +import { describe, it, expect, beforeEach } from 'vitest' +import { JsHnswVectorIndex } from '../../../src/hnsw/hnswIndex.js' +import { euclideanDistance } from '../../../src/utils/index.js' +import { MemoryStorage } from '../../../src/storage/adapters/memoryStorage.js' + +const DIM = 8 + +function seededRand(seed: number): () => number { + let s = seed >>> 0 + return () => { + s = (s + 0x6d2b79f5) | 0 + let t = Math.imul(s ^ (s >>> 15), 1 | s) + t = (t + Math.imul(t ^ (t >>> 7), 61 | t)) ^ t + return ((t ^ (t >>> 14)) >>> 0) / 4294967296 + } +} + +/** A deterministic vector pointing in a pseudo-random direction (well-connected graph). */ +function vec(idx: number): number[] { + const rand = seededRand(idx + 1) + return Array.from({ length: DIM }, () => rand() * 2 - 1) +} + +type Noun = { id: string; vector: number[]; connections: Map> } + +function nounsOf(index: JsHnswVectorIndex): Map { + return (index as unknown as { nouns: Map }).nouns +} + +/** Flatten a reverse index (or a freshly-derived one) to sorted `target|level|source` triples. */ +function triplesFromIncoming(inc: Map>>): string[] { + const out: string[] = [] + for (const [target, byLevel] of inc) { + for (const [level, sources] of byLevel) { + for (const source of sources) out.push(`${target}|${level}|${source}`) + } + } + return out.sort() +} + +/** Derive the ground-truth reverse index directly from the live forward adjacency. */ +function triplesFromAdjacency(nouns: Map): string[] { + const out: string[] = [] + for (const [nodeId, node] of nouns) { + for (const [level, targets] of node.connections) { + for (const target of targets) out.push(`${target}|${level}|${nodeId}`) + } + } + return out.sort() +} + +function assertNoDanglingRefs(nouns: Map): void { + for (const [nodeId, node] of nouns) { + for (const [level, targets] of node.connections) { + for (const target of targets) { + expect(nouns.has(target), `node ${nodeId} (L${level}) links to deleted ${target}`).toBe(true) + } + } + } +} + +function assertReverseIndexConsistent(index: JsHnswVectorIndex): void { + // ensureIncoming() returns the live (incrementally-maintained) index, building it + // only if it was invalidated. The triples it yields must equal the triples derived + // straight from the forward adjacency — empty leftover sets contribute nothing. + const live = (index as unknown as { ensureIncoming: () => Map>> }).ensureIncoming() + expect(triplesFromIncoming(live)).toEqual(triplesFromAdjacency(nounsOf(index))) +} + +function makeIndex(M: number): JsHnswVectorIndex { + return new JsHnswVectorIndex( + { M, efConstruction: 200, efSearch: 64, ml: 16 }, + euclideanDistance, + { useParallelization: false, storage: new MemoryStorage() } + ) +} + +describe('HNSW removeItem — reverse-adjacency correctness', () => { + it('leaves no dangling references and a consistent reverse index after interleaved add/delete', async () => { + const index = makeIndex(16) + const live = new Set() + + // Add 120, then interleave: delete every 3rd as we keep adding — exercises the + // incremental maintenance (link + prune on add, unlink on remove) continuously. + for (let i = 0; i < 120; i++) { + const id = `n-${i}` + await index.addItem({ id, vector: vec(i) }) + live.add(id) + if (i % 3 === 0 && i > 0) { + const victim = `n-${i - 1}` + await index.removeItem(victim) + live.delete(victim) + } + } + + // Now delete ~half of the survivors in one burst (the O(N²)-prone path). + const survivors = [...live] + for (let i = 0; i < survivors.length; i += 2) { + await index.removeItem(survivors[i]) + live.delete(survivors[i]) + } + + const nouns = nounsOf(index) + expect(nouns.size).toBe(live.size) + assertNoDanglingRefs(nouns) + assertReverseIndexConsistent(index) + }) + + it('removing the entry point repeatedly stays consistent', async () => { + const index = makeIndex(8) + for (let i = 0; i < 40; i++) await index.addItem({ id: `e-${i}`, vector: vec(i) }) + + // Delete whatever is currently the entry point, 20 times. + for (let k = 0; k < 20; k++) { + const ep = (index as unknown as { entryPointId: string | null }).entryPointId + if (!ep) break + await index.removeItem(ep) + assertNoDanglingRefs(nounsOf(index)) + } + assertReverseIndexConsistent(index) + }) + + it('search returns the true nearest neighbours of the surviving set (exact, complete graph)', async () => { + // M ≥ N-1 ⇒ the small graph is complete and never prunes, so HNSW recall is 100% + // and we can assert the survivors\' exact top-k against a brute-force scan. + const index = makeIndex(64) + const all: Array<{ id: string; v: number[] }> = [] + for (let i = 0; i < 24; i++) { + const v = vec(i + 1000) + all.push({ id: `s-${i}`, v }) + await index.addItem({ id: `s-${i}`, vector: v }) + } + + const deleted = new Set(['s-2', 's-5', 's-9', 's-14', 's-20']) + for (const id of deleted) await index.removeItem(id) + + const survivors = all.filter((x) => !deleted.has(x.id)) + const query = vec(7).map((x) => x * 0.5) // an arbitrary deterministic query point + const k = 5 + + const bruteTopK = survivors + .map((x) => ({ id: x.id, d: euclideanDistance(query, x.v) })) + .sort((a, b) => a.d - b.d) + .slice(0, k) + .map((x) => x.id) + + const got = (await index.search(query, k)).map(([id]) => id) + + expect(got).toEqual(bruteTopK) + // And none of the deleted ids can ever surface. + expect(got.some((id) => deleted.has(id))).toBe(false) + assertNoDanglingRefs(nounsOf(index)) + }) +})