diff --git a/integrationTests/server/vector-index-integrity.test.ts b/integrationTests/server/vector-index-integrity.test.ts new file mode 100644 index 0000000000..17e8dc7d25 --- /dev/null +++ b/integrationTests/server/vector-index-integrity.test.ts @@ -0,0 +1,577 @@ +/** + * HNSW vector-index data-integrity integration tests. + * + * Guards the six data-integrity fixes landed in commit 251e5b73 + * (fix(hnsw): six data-integrity fixes for HNSW vector index (5.1 GA)). + * + * Unlike the unit tests in unitTests/resources/vectorIndex.test.js (which run on + * a mock in-memory store), these tests exercise the FULL stack: schema-defined + * HNSW table, real RocksDB storage, real Table.search() query path. + * + * Tests: + * 1. Delete-entry-point survival — bulk-delete including the entry-point node + * leaves every surviving record reachable via vector search. + * Pre-fix: search() returned [] while records remained. + * + * 2. Update-churn reachability — repeatedly updating each record's vector + * (re-embed pattern) must not cause records to gradually lose inbound edges + * and vanish from search results. + * Pre-fix: reverse-edge sweep was too broad, accumulating asymmetry. + * + * 3. Threshold queries (le/lt) through the real query path — le boundary is + * inclusive, le with value 0 returns only exact-zero-distance matches. + * Pre-fix: le used strict < instead of <=. + * + * 4. Reindex over existing data — populate a table without the HNSW index, + * then alter the schema to add it so runIndexing backfills existing records; + * all records must be searchable after backfill. Then verify that post- + * backfill updates and deletes also work. + * + * Vector search is exercised via the HTTP QUERY method (RFC-draft, supported by + * Harper's REST layer) with a JSON body, which flows into Table.search() without + * the mapCondition stripping done by search_by_conditions. The body shape is: + * { sort: { attribute: , target: , distance: 'cosine' } } + * or for threshold queries: + * { conditions: [{ attribute: , comparator: 'le', value: , target: }] } + * + * Related PR: HarperFast/harper#1234 + * Related fixes: commit 251e5b73 (fix(hnsw): six data-integrity fixes) + */ +import { suite, test, before, after } from 'node:test'; +import { ok, strictEqual } from 'node:assert/strict'; +import { setTimeout as sleep } from 'node:timers/promises'; +import { startHarper, teardownHarper, type ContextWithHarper } from '@harperfast/integration-testing'; +// @ts-expect-error no type declarations on .mjs utils +import { createApiClient } from '../apiTests/utils/client.mjs'; +// @ts-expect-error no type declarations on .mjs utils +import { restartHttpWorkers } from '../apiTests/utils/lifecycle.mjs'; +import request from 'supertest'; + +// --------------------------------------------------------------------------- +// Schema helpers +// --------------------------------------------------------------------------- + +/** Schema WITH HNSW index on the embedding attribute. */ +function makeSchemaWithIndex(typeName: string, database: string): string { + return [ + `type ${typeName} @table(database: "${database}") @sealed @export {`, + '\tid: ID! @primaryKey', + '\ttag: String', + '\tembedding: [Float] @indexed(type: "HNSW", distance: "cosine")', + '}', + '', + ].join('\n'); +} + +/** Schema WITHOUT any index on the embedding attribute (pre-backfill state). */ +function makeSchemaWithoutIndex(typeName: string, database: string): string { + return [ + `type ${typeName} @table(database: "${database}") @sealed @export {`, + '\tid: ID! @primaryKey', + '\ttag: String', + '\tembedding: [Float]', + '}', + '', + ].join('\n'); +} + +// --------------------------------------------------------------------------- +// Vector helpers +// --------------------------------------------------------------------------- + +/** + * Deterministic unit vector seeded by an integer. + * Uses a simple LCG so tests are reproducible across runs. + */ +function seedVector(seed: number, dims: number = 8): number[] { + let s = (seed * 1664525 + 1013904223) >>> 0; + const rand = (): number => { + s = (s * 1664525 + 1013904223) >>> 0; + return s / 4294967296; + }; + const v: number[] = []; + let mag = 0; + for (let i = 0; i < dims; i++) { + const x = rand() * 2 - 1; + v.push(x); + mag += x * x; + } + const inv = 1 / (Math.sqrt(mag) || 1); + return v.map((x) => x * inv); +} + +// --------------------------------------------------------------------------- +// HTTP helpers +// --------------------------------------------------------------------------- + +/** + * Execute a vector sort search via the HTTP QUERY method. + * Returns an array of records sorted by ascending cosine distance. + * + * The HTTP QUERY method routes into Resource.static.query → Table.search(body), + * bypassing the operations-API mapCondition that strips `target` from conditions. + */ +async function vectorSearch( + httpURL: string, + headers: Record, + resourcePath: string, + target: number[], + opts: { limit?: number; select?: string[] } = {} +): Promise { + const body: any = { + sort: { attribute: 'embedding', target, distance: 'cosine' }, + }; + if (opts.limit !== undefined) body.limit = opts.limit; + if (opts.select !== undefined) body.select = opts.select; + return queryResource(httpURL, headers, resourcePath, body); +} + +/** + * Issue an HTTP QUERY request via fetch — supertest/superagent has no API for + * non-standard verbs, while undici's fetch passes custom method tokens through. + */ +async function queryResource( + httpURL: string, + headers: Record, + resourcePath: string, + body: any +): Promise { + const resp = await fetch(`${httpURL}${resourcePath}`, { + method: 'QUERY', + headers: { ...headers, 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }); + const text = await resp.text(); + if (resp.status !== 200) { + throw new Error(`QUERY ${resourcePath} returned ${resp.status}: ${text}`); + } + const data = JSON.parse(text); + return Array.isArray(data) ? data : []; +} + +/** + * Execute a threshold query (le/lt comparator) via the HTTP QUERY method. + * + * The `target` vector is carried on the condition object. Reaching Table.search() + * directly through the QUERY body preserves it; the operations-API + * search_by_conditions path would strip it via mapCondition. + */ +async function vectorThresholdSearch( + httpURL: string, + headers: Record, + resourcePath: string, + target: number[], + comparator: 'le' | 'lt', + value: number +): Promise { + const body = { + conditions: [{ attribute: 'embedding', comparator, value, target }], + }; + return queryResource(httpURL, headers, resourcePath, body); +} + +/** POST a record to the REST endpoint. */ +async function insertRecord( + httpURL: string, + headers: Record, + path: string, + record: any +): Promise { + const resp = await request(httpURL).post(path).set(headers).send(record); + ok([200, 201, 204].includes(resp.status), `POST ${path} returned ${resp.status}: ${JSON.stringify(resp.body)}`); +} + +/** PUT (full-record update) via REST. */ +async function updateRecord( + httpURL: string, + headers: Record, + path: string, + id: string | number, + record: any +): Promise { + const resp = await request(httpURL) + .put(`${path}${id}`) + .set(headers) + .send({ id, ...record }); + ok([200, 201, 204].includes(resp.status), `PUT ${path}${id} returned ${resp.status}: ${JSON.stringify(resp.body)}`); +} + +/** DELETE a record via REST. */ +async function deleteRecord( + httpURL: string, + headers: Record, + path: string, + id: string | number +): Promise { + const resp = await request(httpURL).delete(`${path}${id}`).set(headers); + ok([200, 204].includes(resp.status), `DELETE ${path}${id} returned ${resp.status}: ${JSON.stringify(resp.body)}`); +} + +/** + * Poll `predicate()` until it resolves true or `timeoutMs` elapses. + */ +async function waitFor(predicate: () => Promise, timeoutMs = 60_000, intervalMs = 500): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (await predicate()) return; + await sleep(intervalMs); + } + throw new Error(`waitFor timed out after ${timeoutMs}ms`); +} + +// --------------------------------------------------------------------------- +// Suite +// --------------------------------------------------------------------------- + +suite('HNSW vector-index data-integrity (integration)', (ctx: ContextWithHarper) => { + let client: any; + + before(async () => { + await startHarper(ctx, { config: {}, env: {} }); + client = createApiClient(ctx.harper); + }); + + after(async () => { + await teardownHarper(ctx); + }); + + // ── Test 1: Delete-entry-point survival ───────────────────────────────── + test('delete-entry-point: bulk-delete including entry point leaves survivors findable', async () => { + // Guards fix #2: entry-point replacement scan now passes the transaction to + // getRange and skips the node being deleted. + // Pre-fix: getRange ran outside the write-set → re-elected the deleted node + // as EP → dangling entry point → subsequent searches returned []. + const DB = 'vecinteg1'; + const TABLE = 'DelEPTable'; + const PROJECT = 'vecintegsuite1'; + const DIMS = 8; + const N = 50; + const SURVIVORS = 10; + const deleteCount = N - SURVIVORS; + + await client + .req() + .send({ operation: 'add_component', project: PROJECT }) + .expect((r: any) => { + ok( + JSON.stringify(r.body).includes('Successfully added project') || + JSON.stringify(r.body).includes('Project already exists'), + r.text + ); + }); + await client + .req() + .send({ + operation: 'set_component_file', + project: PROJECT, + file: 'schema.graphql', + payload: makeSchemaWithIndex(TABLE, DB), + }) + .expect(200); + + await restartHttpWorkers(client, `/${TABLE}/`); + + const httpURL = ctx.harper.httpURL; + const headers = client.headers; + const path = `/${TABLE}/`; + + // Insert N records — the first-inserted node becomes the initial entry point. + for (let i = 0; i < N; i++) { + await insertRecord(httpURL, headers, path, { id: `ep-${i}`, embedding: seedVector(i, DIMS) }); + } + + // Delete first (N − SURVIVORS) records. Very likely to include the EP + // (first-inserted node is elected EP on small graphs and rarely displaced). + for (let i = 0; i < deleteCount; i++) { + await deleteRecord(httpURL, headers, path, `ep-${i}`); + } + + // Every surviving record must be reachable. + const queryVec = seedVector(N + 1, DIMS); + const results = await vectorSearch(httpURL, headers, path, queryVec, { limit: SURVIVORS + 5 }); + const resultIds = new Set(results.map((r: any) => r.id)); + + for (let i = 0; i < deleteCount; i++) { + ok(!resultIds.has(`ep-${i}`), `deleted record ep-${i} must not appear in results`); + } + + let unreachable = 0; + for (let i = deleteCount; i < N; i++) { + if (!resultIds.has(`ep-${i}`)) unreachable++; + } + // Allow at most 1 HNSW-approximate miss on a 10-survivor graph. + ok( + unreachable <= 1, + `expected all ${SURVIVORS} survivors reachable; ${unreachable} missing. found: ${JSON.stringify([...resultIds])}` + ); + }); + + // ── Test 2: Update-churn reachability ──────────────────────────────────── + test('update-churn: repeated vector updates preserve full reachability', async () => { + // Guards fix #3: UPDATE path now removes reverse edge only at the exact level l + // where the old connection existed, not the full 0..l sweep (correct for DELETE). + // The broader sweep destroyed reverse edges that addConnection had just re-added, + // accumulating asymmetry with every re-embed round. + const DB = 'vecinteg2'; + const TABLE = 'ChurnTable'; + const PROJECT = 'vecintegsuite2'; + const DIMS = 8; + const N = 30; + const ROUNDS = 5; + + await client + .req() + .send({ operation: 'add_component', project: PROJECT }) + .expect((r: any) => { + ok( + JSON.stringify(r.body).includes('Successfully added project') || + JSON.stringify(r.body).includes('Project already exists'), + r.text + ); + }); + await client + .req() + .send({ + operation: 'set_component_file', + project: PROJECT, + file: 'schema.graphql', + payload: makeSchemaWithIndex(TABLE, DB), + }) + .expect(200); + + await restartHttpWorkers(client, `/${TABLE}/`); + + const httpURL = ctx.harper.httpURL; + const headers = client.headers; + const path = `/${TABLE}/`; + + for (let i = 0; i < N; i++) { + await insertRecord(httpURL, headers, path, { id: `churn-${i}`, embedding: seedVector(i, DIMS) }); + } + + // Churn: update every record's vector ROUNDS times with different seeds. + for (let round = 0; round < ROUNDS; round++) { + for (let i = 0; i < N; i++) { + await updateRecord(httpURL, headers, path, `churn-${i}`, { + embedding: seedVector(i * 100 + round + 1, DIMS), + tag: `r${round}`, + }); + } + } + + // Each record's final vector is seedVector(i*100 + ROUNDS, dims). + // Searching for that exact vector must return it in the top-5. + let misses = 0; + for (let i = 0; i < N; i++) { + const finalVec = seedVector(i * 100 + ROUNDS, DIMS); + const results = await vectorSearch(httpURL, headers, path, finalVec, { limit: 5 }); + if (!results.some((r: any) => r.id === `churn-${i}`)) misses++; + } + const allowed = Math.ceil(N * 0.1); + ok(misses <= allowed, `expected ≤${allowed} misses after ${ROUNDS} churn rounds; got ${misses}/${N}`); + }); + + // ── Test 3: Threshold queries (le/lt) ──────────────────────────────────── + test('threshold queries: le includes exact boundary; le(~0) returns exact-match only', async () => { + // Guards fix #6b: le comparator now uses <= (was <). + // Pre-fix: records at exactly the threshold distance were excluded by le. + const DB = 'vecinteg3'; + const TABLE = 'ThreshTable'; + const PROJECT = 'vecintegsuite3'; + + await client + .req() + .send({ operation: 'add_component', project: PROJECT }) + .expect((r: any) => { + ok( + JSON.stringify(r.body).includes('Successfully added project') || + JSON.stringify(r.body).includes('Project already exists'), + r.text + ); + }); + await client + .req() + .send({ + operation: 'set_component_file', + project: PROJECT, + file: 'schema.graphql', + payload: makeSchemaWithIndex(TABLE, DB), + }) + .expect(200); + + await restartHttpWorkers(client, `/${TABLE}/`); + + const httpURL = ctx.harper.httpURL; + const headers = client.headers; + const path = `/${TABLE}/`; + + // 2-D vectors at three well-separated cosine distances from [1,0]: + // [1,0] → distance 0 (exact; representable in float32, so exactly 0) + // [1/√2, 1/√2] → distance ≈0.293 (near) + // [0,1] → distance 1 (far) + // Vectors are STORED as float32, so a float64 prediction of the near distance + // differs from the server's computed value at ~1e-8. Boundary assertions must + // therefore use the server's own $distance values, not locally computed ones. + const INV_SQRT2 = 1 / Math.sqrt(2); + const target = [1, 0]; + + await insertRecord(httpURL, headers, path, { id: 'exact', embedding: [1, 0] }); + await insertRecord(httpURL, headers, path, { id: 'near', embedding: [INV_SQRT2, INV_SQRT2] }); + await insertRecord(httpURL, headers, path, { id: 'far', embedding: [0, 1] }); + + // Fetch the actual stored distances; these are the exact values the threshold + // filter compares against (same candidate-distance computation, no rerank on + // a non-quantized index). + const ranked = await vectorSearch(httpURL, headers, path, target, { select: ['id', '$distance'] }); + const distanceOf = (id: string) => { + const rec = ranked.find((r: any) => r.id === id); + ok(rec && typeof rec.$distance === 'number', `expected $distance for '${id}', got ${JSON.stringify(rec)}`); + return rec.$distance as number; + }; + const dExact = distanceOf('exact'); + const dNear = distanceOf('near'); + strictEqual(dExact, 0, `[1,0] is float32-representable; self-distance must be exactly 0, got ${dExact}`); + + // le(dNear) must include both "exact" (dist < boundary) and "near" + // (dist == boundary — this is the <= fix). + const leNear = await vectorThresholdSearch(httpURL, headers, path, target, 'le', dNear); + const leNearIds = new Set(leNear.map((r: any) => r.id)); + ok(leNearIds.has('exact'), `le(dNear) must include 'exact' (0 ≤ ${dNear})`); + ok(leNearIds.has('near'), `le(dNear) must include 'near' at exact boundary distance ${dNear}`); + ok(!leNearIds.has('far'), `le(dNear) must not include 'far' (distance 1 > ${dNear})`); + + // lt(dNear) must include "exact" but NOT "near" (strict). + const ltNear = await vectorThresholdSearch(httpURL, headers, path, target, 'lt', dNear); + const ltNearIds = new Set(ltNear.map((r: any) => r.id)); + ok(ltNearIds.has('exact'), `lt(dNear) must include 'exact' (0 < ${dNear})`); + ok(!ltNearIds.has('near'), `lt(dNear) must NOT include 'near' at boundary (strict less-than)`); + + // le(0) must return only the exact-match record. Pre-fix falsy-0 issue: a truthy + // guard (if (limit)) skipped the filter entirely for a threshold of 0, returning + // every record. dExact is exactly 0, so this exercises the real boundary. + const leZero = await vectorThresholdSearch(httpURL, headers, path, target, 'le', dExact); + const leZeroIds = new Set(leZero.map((r: any) => r.id)); + ok(leZeroIds.has('exact'), `le(0) must include the exact-match record`); + ok( + !leZeroIds.has('near') && !leZeroIds.has('far'), + `le(0) must return only the exact match; got ${JSON.stringify([...leZeroIds])}` + ); + }); + + // ── Test 4: Reindex over existing data ────────────────────────────────── + test('reindex backfill: adding HNSW index to populated table makes all records searchable', async () => { + // Guards fix #4 (backfill-resume idempotency) and fix #5 (lastIndexedKey reset + // on structural index change so runIndexing starts clean). + const DB = 'vecinteg4'; + const TABLE = 'ReindexTable'; + const PROJECT = 'vecintegsuite4'; + const DIMS = 8; + const N = 40; + + // Step 1: Deploy schema WITHOUT the HNSW index. + await client + .req() + .send({ operation: 'add_component', project: PROJECT }) + .expect((r: any) => { + ok( + JSON.stringify(r.body).includes('Successfully added project') || + JSON.stringify(r.body).includes('Project already exists'), + r.text + ); + }); + await client + .req() + .send({ + operation: 'set_component_file', + project: PROJECT, + file: 'schema.graphql', + payload: makeSchemaWithoutIndex(TABLE, DB), + }) + .expect(200); + + await restartHttpWorkers(client, `/${TABLE}/`); + + const httpURL = ctx.harper.httpURL; + const headers = client.headers; + const path = `/${TABLE}/`; + + // Step 2: Insert N records (plain [Float], no HNSW index yet). + for (let i = 0; i < N; i++) { + await insertRecord(httpURL, headers, path, { id: `reindex-${i}`, embedding: seedVector(i, DIMS) }); + } + + // Sanity-check: records exist. + const hashCheck = await client + .req() + .send({ + operation: 'search_by_hash', + database: DB, + table: TABLE, + hash_values: ['reindex-0'], + get_attributes: ['id'], + }) + .expect(200); + ok(Array.isArray(hashCheck.body) && hashCheck.body.length === 1, 'reindex-0 must exist before reindex'); + + // Step 3: Alter schema to ADD the HNSW index → triggers runIndexing backfill. + await client + .req() + .send({ + operation: 'set_component_file', + project: PROJECT, + file: 'schema.graphql', + payload: makeSchemaWithIndex(TABLE, DB), + }) + .expect(200); + + await restartHttpWorkers(client, `/${TABLE}/`); + + // Step 4: Poll until backfill completes (search returns results). + // During indexing the HNSW index has isIndexing=true and searches return 503. + await waitFor(async () => { + try { + const results = await vectorSearch(httpURL, headers, path, seedVector(0, DIMS), { limit: 5 }); + return results.length > 0; + } catch { + return false; + } + }, 60_000); + + // Step 5: All N pre-existing records must be reachable. + let misses = 0; + for (let i = 0; i < N; i++) { + const vec = seedVector(i, DIMS); + const results = await vectorSearch(httpURL, headers, path, vec, { limit: 5 }); + if (!results.some((r: any) => r.id === `reindex-${i}`)) misses++; + } + const allowed = Math.ceil(N * 0.1); + ok(misses <= allowed, `expected ≤${allowed} misses after backfill; got ${misses}/${N}`); + + // Step 6: Post-backfill mutations must work correctly. + for (let i = 0; i < 5; i++) { + await updateRecord(httpURL, headers, path, `reindex-${i}`, { + embedding: seedVector(i + 1000, DIMS), + tag: 'updated', + }); + } + for (let i = 5; i < 10; i++) { + await deleteRecord(httpURL, headers, path, `reindex-${i}`); + } + + // reindex-0's new vector (seed 1000) must be near the top; deleted records absent. + const updatedVec = seedVector(1000, DIMS); + const afterResults = await vectorSearch(httpURL, headers, path, updatedVec, { limit: 15 }); + const afterIds = new Set(afterResults.map((r: any) => r.id)); + + ok(afterIds.has('reindex-0'), 'updated reindex-0 must appear near its new vector'); + for (let i = 5; i < 10; i++) { + ok(!afterIds.has(`reindex-${i}`), `deleted reindex-${i} must not appear in search results`); + } + + // Note on interrupted-backfill-then-restart: + // This scenario is NOT covered here. A SIGKILL precisely during a 40-record + // backfill is non-deterministic (it completes in milliseconds). The backfill- + // resume idempotency fix (#4) is covered by unit tests in + // unitTests/resources/vectorIndex.test.js ('backfill resume idempotency'). + }); +}); diff --git a/resources/databases.ts b/resources/databases.ts index b66ed46e22..7f502c970c 100644 --- a/resources/databases.ts +++ b/resources/databases.ts @@ -1221,7 +1221,19 @@ export function table(tableDefinition: TableDefinition): Tabl break; } if (hasExistingData) { - attribute.lastIndexedKey = attributeDescriptor?.lastIndexedKey ?? undefined; + // When the index definition itself has structurally changed (different distance + // metric, M, quantization, etc.), any + // previous lastIndexedKey checkpoint is for a graph built under the old options — + // resuming from it would mix two incompatible graphs. Reset to undefined so + // runIndexing clears the dbi and starts from scratch. + // For pure crash-recovery (same options, different PID/restartNumber), preserve + // the checkpoint so the backfill resumes rather than restarts. + const indexOptionsChanged = + JSON.stringify(stripSearchOnly(attributeDescriptor?.indexed)) !== + JSON.stringify(stripSearchOnly(attribute.indexed)); + attribute.lastIndexedKey = indexOptionsChanged + ? undefined + : (attributeDescriptor?.lastIndexedKey ?? undefined); attribute.indexingPID = process.pid; delete attribute.indexingFailed; // clear failure flag for the new run dbi.isIndexing = true; diff --git a/resources/indexes/HierarchicalNavigableSmallWorld.ts b/resources/indexes/HierarchicalNavigableSmallWorld.ts index 79fdc3e16c..121a61f806 100644 --- a/resources/indexes/HierarchicalNavigableSmallWorld.ts +++ b/resources/indexes/HierarchicalNavigableSmallWorld.ts @@ -190,6 +190,19 @@ export class HierarchicalNavigableSmallWorld { } } index(primaryKey: Id, vector: number[], existingVector?: number[], options: any = {}) { + // Reject non-finite components before touching the graph. NaN in particular poisons + // bisectInsert (arr[mid].distance <= NaN is always false → returns 0, pinning the + // candidate to rank 1 of every future search). Infinity causes analogous ordering + // anomalies. embedHook.ts intentionally passes NaN through and expects HNSW to guard. + if (vector) { + for (let i = 0; i < vector.length; i++) { + if (!Number.isFinite(vector[i])) { + throw new ClientError( + `Vector for attribute "${String(primaryKey)}" contains non-finite component at index ${i}: ${vector[i]}. Ensure the embedding produces only finite values.` + ); + } + } + } // first get the node id for the primary key; we use internal node ids for better efficiency, // but we must use a safe key that won't collide with the node ids const safeKey = typeof primaryKey === 'number' ? [KEY_PREFIX, primaryKey] : primaryKey; @@ -228,7 +241,29 @@ export class HierarchicalNavigableSmallWorld { // If we are updating an existing entry, we need to update the entry point // if the new entry is closer to the entry point than the old one oldNode = { ...this.safeGetSync(nodeId, options) }; - } else oldNode = {} as Node; + } else { + // If this key already has a graph node — which happens + // when runIndexing re-feeds an already-indexed record after a crash/restart — load it + // and treat the call as an update rather than a fresh insert. Without this, the absent + // existingVector causes oldNode = {} → a new random level, connections overwritten, + // and old-connection cleanup skipped, leaving dangling reverse edges and wrong levels. + const storedNode = nodeId && vector ? this.safeGetSync(nodeId, options) : undefined; + if (storedNode && storedNode.level !== undefined) { + // Treat as an update: carry forward the stored graph state so cleanup and level + // assignment below use the real existing connections instead of starting fresh. + oldNode = { ...storedNode }; + // Reconstruct the existingVector from the stored node so distance computations + // in the cleanup pass use the right baseline (dequantize if int8). + if (!existingVector) { + existingVector = + storedNode.scale !== undefined + ? dequantizeInt8(storedNode.vector as Int8Array, storedNode.scale) + : (storedNode.vector as number[]); + } + } else { + oldNode = {} as Node; + } + } if (vector) { // Pre-compute 1/|vector| for cosine distance so searchLayer can skip sqrt per neighbor let invMag: number | undefined; @@ -423,15 +458,22 @@ export class HierarchicalNavigableSmallWorld { if (entryPointId !== undefined) break; } if (entryPointId === undefined) { - // scan through all nodes to find one with highest level + // Fallback scan: pass transaction so it sees the same write-set (not stale committed state), + // skip the node being deleted (it is typically the highest-level node and would be + // re-elected), and verify each candidate actually resolves before committing it. let highestLevel = -1; for (const { key, value } of this.indexStore.getRange({ start: 0, end: Infinity, + transaction: options.transaction, })) { + // skip the node being removed (safeKey mappings can't appear here: symbol-array + // and string keys sort outside the numeric 0..Infinity range) + if (key === nodeId) continue; + if (!value || value.level === undefined) continue; if (value.level > highestLevel) { entryPointId = key; - if (value.level === lastLevel) break; // if we found a node at the same level as the last entry point, we can stop + if (value.level === lastLevel) break; // found a node at the same level as the old entry point highestLevel = value.level; } } @@ -449,6 +491,9 @@ export class HierarchicalNavigableSmallWorld { } } this.indexStore.remove(nodeId, options); + // Remove the safeKey→nodeId mapping so the key count used by autoScaleEf stays accurate + // and a re-insert of this primary key gets a fresh node rather than the deleted node's id. + this.indexStore.remove(safeKey, options); } const needsReindexing = new Map(); // remove connections to this node that are no longer valid @@ -459,7 +504,14 @@ export class HierarchicalNavigableSmallWorld { // get and copy the neighbor node so we can modify it const neighborNode = updateNode(neighborId, this.safeGetSync(neighborId, options)); if (!neighborNode) continue; - for (let l2 = 0; l2 <= l; l2++) { + // On an UPDATE (vector != null), only remove the reverse edge at + // the exact level l where the old connection existed. Sweeping 0..l would destroy reverse + // edges at lower levels that were just re-added by addConnection or preserved by the + // splice logic above — causing asymmetry that accumulates with every re-embed. + // On DELETE (vector == null), the full 0..l sweep is correct: we want to remove every + // occurrence of nodeId from all levels of each neighbor. + const levelStart = vector ? l : 0; + for (let l2 = levelStart; l2 <= l; l2++) { // remove the connection to this node from the neighbor node neighborNode[l2] = neighborNode[l2]?.filter(({ id: nid }) => { return nid !== nodeId; @@ -491,8 +543,41 @@ export class HierarchicalNavigableSmallWorld { for (const [id, updatedNode] of updatedNodes) { this.indexStore.put(id, updatedNode, options); } - for (const [key, vector] of needsReindexing) { - this.index(key, vector, vector, options); + for (const [key, orphanVector] of needsReindexing) { + // If the orphan IS the current entry point, re-running + // index() from it would find no other nodes to connect to (the entry point search returns + // itself), leaving it permanently isolated. Elect a surviving neighbor as entry point first + // so the orphan can reconnect to the live graph. + const currentEP = this.indexStore.getSync(ENTRY_POINT, options); + const orphanNodeId = this.indexStore.getSync(typeof key === 'number' ? [KEY_PREFIX, key] : key, options); + if (currentEP !== undefined && currentEP === orphanNodeId) { + // The orphan's own connection lists are empty, so look for a surviving node to elect: + // first among the nodes updated in this pass, then a full scan as fallback. + let replacementEP: number | undefined; + for (const [candidateId, candidateNode] of updatedNodes) { + if (candidateId !== orphanNodeId && candidateNode[0]?.length > 0) { + replacementEP = candidateId; + break; + } + } + if (replacementEP === undefined) { + for (const { key: candidateKey, value: candidateNode } of this.indexStore.getRange({ + start: 0, + end: Infinity, + transaction: options.transaction, + })) { + if (candidateKey === orphanNodeId) continue; + if (candidateNode?.level !== undefined) { + replacementEP = candidateKey; + break; + } + } + } + if (replacementEP !== undefined) { + this.indexStore.put(ENTRY_POINT, replacementEP, options); + } + } + this.index(key, orphanVector, orphanVector, options); } this.checkSymmetry(nodeId, this.safeGetSync(nodeId, options), options); } @@ -669,10 +754,13 @@ export class HierarchicalNavigableSmallWorld { }, context: any ) { - let limit = 0; // zero is ignored, only used if set below + let limit: number | undefined; // only set for threshold comparators; 0 is a valid threshold (e.g. dotProduct) + let limitInclusive = false; // true for `le`, false for `lt` switch (comparator) { - case 'lt': case 'le': + limitInclusive = true; + // fallthrough + case 'lt': limit = value; // fallthrough case 'sort': @@ -716,7 +804,10 @@ export class HierarchicalNavigableSmallWorld { entryPointId = neighbor.id; } } - if (limit) results = results.filter((candidate) => candidate.distance < limit); + if (limit !== undefined) + results = results.filter((candidate) => + limitInclusive ? candidate.distance <= limit : candidate.distance < limit + ); return results.map((candidate) => ({ // we return the result as an entry so we can provide distance as metadata key: candidate.node.primaryKey, // return value diff --git a/resources/search.ts b/resources/search.ts index 5013ed27a8..457faa0fbc 100644 --- a/resources/search.ts +++ b/resources/search.ts @@ -414,8 +414,12 @@ export function searchByIndex( typeof attribute_name === 'string' ) { const rescored = (loaded as any[]).filter((e) => e !== SKIP && e && e.value); - for (const e of rescored) - e.distance = index.customIndex.exactDistance(searchCondition, e.value[attribute_name]); + for (const e of rescored) { + const d = index.customIndex.exactDistance(searchCondition, e.value[attribute_name]); + // Non-finite exact distances (NaN from a corrupt record vector, Infinity from a + // missing vector) sort last — consistent with the missing-vector sentinel in exactDistance. + e.distance = Number.isFinite(d) ? d : Infinity; + } // comparison-based (not subtraction) so Infinity sentinels for missing vectors // sort last without producing NaN (Infinity - Infinity). rescored.sort((a, b) => (a.distance === b.distance ? 0 : a.distance < b.distance ? -1 : 1)); diff --git a/unitTests/resources/vectorIndex.test.js b/unitTests/resources/vectorIndex.test.js index 4ff12d8d0a..ec5615c1ff 100644 --- a/unitTests/resources/vectorIndex.test.js +++ b/unitTests/resources/vectorIndex.test.js @@ -724,6 +724,331 @@ describeUnlessLmdb('HNSW int8 cold/frozen node reads (#1161)', () => { }); }); +// ─── Data-integrity fixes (5.1 GA) ────────────────────────────────────────── +describeUnlessLmdb('HNSW data-integrity fixes (5.1 GA)', () => { + // Minimal mock store used by several tests below. Supports put/get/remove with + // optional numeric-range scan (getRange), and a shared-buffer allocator. + function makeMockStore() { + const nodes = new Map(); + let ep; + return { + encoder: { useFloat32: false }, + getSync(key, _opts) { + if (key === Symbol.for('entryPoint')) return ep; + const k = typeof key === 'number' ? key : JSON.stringify(key); + return nodes.get(k); + }, + put(key, value, _opts) { + if (key === Symbol.for('entryPoint')) { + ep = value; + return; + } + const k = typeof key === 'number' ? key : JSON.stringify(key); + nodes.set(k, value); + }, + remove(key, _opts) { + if (key === Symbol.for('entryPoint')) { + ep = undefined; + return; + } + const k = typeof key === 'number' ? key : JSON.stringify(key); + nodes.delete(k); + }, + *getRange({ start = 0, end = Infinity } = {}) { + for (const [k, v] of nodes) { + if (typeof k === 'number' && k >= start && k <= end) yield { key: k, value: v }; + } + }, + getKeys({ transaction: _t } = {}) { + return []; + }, + getUserSharedBuffer(_name, buffer) { + return buffer; + }, + // For test assertions: iterate all numeric-key (node) entries + _nodes() { + return nodes.entries(); + }, + }; + } + + // ── non-finite vector guard ──────────────────────────────────────────── + describe('non-finite vector guard', () => { + it('throws ClientError for a vector containing NaN', () => { + const store = makeMockStore(); + const hnsw = new HierarchicalNavigableSmallWorld(store, {}); + // Insert a valid first node so the graph is not empty. + hnsw.index('a', [1, 0, 0], null, {}); + assert.throws( + () => hnsw.index('b', [0, NaN, 1], null, {}), + (err) => { + assert(err.message.includes('non-finite'), `expected "non-finite" in: ${err.message}`); + assert(err.message.includes('1'), `expected component index in: ${err.message}`); + return true; + } + ); + }); + + it('throws ClientError for a vector containing Infinity', () => { + const store = makeMockStore(); + const hnsw = new HierarchicalNavigableSmallWorld(store, {}); + hnsw.index('a', [1, 0, 0], null, {}); + assert.throws( + () => hnsw.index('b', [Infinity, 0, 0], null, {}), + (err) => { + assert(err.message.includes('non-finite'), `expected "non-finite" in: ${err.message}`); + return true; + } + ); + }); + + it('index remains searchable after a rejected NaN insert', () => { + const store = makeMockStore(); + const hnsw = new HierarchicalNavigableSmallWorld(store, { distance: 'euclidean' }); + hnsw.index('good', [1, 0, 0], null, {}); + // Swallow the expected error + try { + hnsw.index('bad', [NaN, 0, 0], null, {}); + } catch {} + // The good node must still be reachable + const results = hnsw.search( + { target: [1, 0, 0], comparator: 'sort', descending: false }, + { transaction: undefined } + ); + assert(Array.isArray(results), 'search must return an array'); + assert( + results.some((r) => r.key === 'good'), + 'valid node must be findable after rejected NaN insert' + ); + }); + }); + + // ── `le` boundary inclusion ────────────────────────────────────────────── + describe('le comparator includes the boundary distance', () => { + it('le returns records at exactly the threshold distance', () => { + const store = makeMockStore(); + // euclidean: distance([0], [d]) = d^2 — use 1D so we can predict exact distances + const hnsw = new HierarchicalNavigableSmallWorld(store, { distance: 'euclidean' }); + // Insert vectors at known euclidean-squared distances from [0]: 0.04, 0.09, 0.16, 0.25 + hnsw.index('d0.04', [0.2], null, {}); + hnsw.index('d0.09', [0.3], null, {}); + hnsw.index('d0.16', [0.4], null, {}); + hnsw.index('d0.25', [0.5], null, {}); + + const target = [0]; + // lt with value 0.09: should exclude the d0.09 record + const lt = hnsw.search({ target, comparator: 'lt', value: 0.09, descending: false }, { transaction: undefined }); + assert( + lt.every((r) => r.distance < 0.09), + 'lt should exclude exact boundary' + ); + assert(!lt.some((r) => r.key === 'd0.09'), 'lt must not include d0.09 (distance == threshold)'); + + // le with value 0.09: must include the d0.09 record + const le = hnsw.search({ target, comparator: 'le', value: 0.09, descending: false }, { transaction: undefined }); + assert( + le.some((r) => r.key === 'd0.09'), + 'le must include d0.09 (distance == threshold)' + ); + assert( + le.every((r) => r.distance <= 0.09), + 'le must not include records beyond threshold' + ); + }); + + it('le with a threshold of 0 filters to exact matches (0 is a valid threshold)', () => { + const store = makeMockStore(); + const hnsw = new HierarchicalNavigableSmallWorld(store, { distance: 'euclidean' }); + hnsw.index('exact', [0.2], null, {}); + hnsw.index('near', [0.3], null, {}); + hnsw.index('far', [0.9], null, {}); + + // le 0: only the exact-distance-0 record. A falsy-0 sentinel would skip the + // filter entirely and return all three. + const le0 = hnsw.search( + { target: [0.2], comparator: 'le', value: 0, descending: false }, + { transaction: undefined } + ); + assert.strictEqual(le0.length, 1, `le 0 must return only the exact match, got ${le0.length}`); + assert.strictEqual(le0[0].key, 'exact'); + + // lt 0: no distance can be negative for euclidean — must return nothing. + const lt0 = hnsw.search( + { target: [0.2], comparator: 'lt', value: 0, descending: false }, + { transaction: undefined } + ); + assert.strictEqual(lt0.length, 0, 'lt 0 must return no records'); + }); + }); + + // ── delete-entry-point, remaining records still findable ────────────────── + describe('delete-entry-point leaves remaining records findable', () => { + it('search returns remaining records after deleting the entry-point node', () => { + // Use the mock store so this test is self-contained and immune to DB state. + const store = makeMockStore(); + const hnsw = new HierarchicalNavigableSmallWorld(store, { distance: 'euclidean', optimizeRouting: 0 }); + + // Insert N records with spread-out vectors. + const N = 12; + for (let i = 0; i < N; i++) hnsw.index(String(i), [i, i * 0.5, i % 3], null, {}); + + // Identify the entry-point's primaryKey and delete it. + const epNodeId = store.getSync(Symbol.for('entryPoint')); + const epPrimaryKey = store.getSync(epNodeId)?.primaryKey ?? '0'; + hnsw.index(epPrimaryKey, null, null, {}); // deletion path (vector == null) + + // All remaining records must still be reachable via search. + const results = hnsw.search( + { target: [5, 2.5, 2], comparator: 'sort', descending: false }, + { transaction: undefined } + ); + assert(Array.isArray(results), 'search must return an array'); + assert(!results.some((r) => r.key === epPrimaryKey), 'deleted entry-point must not appear in results'); + // Most remaining nodes must be reachable. + assert( + results.length >= N - 3, + `expected at least ${N - 3} results after entry-point deletion, got ${results.length}` + ); + }); + + it('bulk-delete 50% including entry point still returns the remainder', () => { + const store = makeMockStore(); + const hnsw = new HierarchicalNavigableSmallWorld(store, { distance: 'euclidean', optimizeRouting: 0 }); + + const N = 14; + for (let i = 0; i < N; i++) hnsw.index(String(i), [i, i % 3], null, {}); + + // Collect the first half of node primary keys (including the entry point). + const epNodeId = store.getSync(Symbol.for('entryPoint')); + const deleteKeys = new Set(); + deleteKeys.add(store.getSync(epNodeId)?.primaryKey ?? '0'); + for (let i = 0; i < Math.floor(N / 2) - 1; i++) deleteKeys.add(String(i)); + + for (const pk of deleteKeys) hnsw.index(pk, null, null, {}); + + const results = hnsw.search( + { target: [10, 1], comparator: 'sort', descending: false }, + { transaction: undefined } + ); + // Deleted keys must not appear. + for (const pk of deleteKeys) { + assert(!results.some((r) => r.key === pk), `deleted key ${pk} must not appear in results`); + } + }); + }); + + // ── update-then-search reachability ─────────────────────────────────────── + describe('update-then-search reachability (sweep-levels fix)', () => { + it('repeatedly-updated record and its neighbors remain findable', () => { + const store = makeMockStore(); + const hnsw = new HierarchicalNavigableSmallWorld(store, { distance: 'euclidean', optimizeRouting: 0.5 }); + + // Build a small graph. + const N = 20; + for (let i = 0; i < N; i++) { + hnsw.index(String(i), [Math.cos(i * 0.5), Math.sin(i * 0.5), 0.1 * (i % 3)], null, {}); + } + // Update key '5' ten times — exercises the level-targeted reverse-edge sweep. + for (let round = 0; round < 10; round++) { + const newVec = [Math.cos(round * 0.7), Math.sin(round * 0.7), 0.2]; + // existingVector triggers the update path in index() + hnsw.index('5', newVec, [Math.cos((round - 1) * 0.7) || 1, 0.2, 0.2], {}); + } + // All N records must still be reachable via a broad search. + const results = hnsw.search( + { target: [1, 0, 0], comparator: 'sort', descending: false }, + { transaction: undefined } + ); + // HNSW is approximate; allow up to 2 misses on a small graph. + assert( + results.length >= N - 2, + `expected at least ${N - 2} results after repeated updates, got ${results.length}` + ); + }); + + it('symmetry check: fewer than 5 asymmetries after repeated updates', () => { + const store = makeMockStore(); + const hnsw = new HierarchicalNavigableSmallWorld(store, { distance: 'euclidean', optimizeRouting: 0.5 }); + + const N = 16; + for (let i = 0; i < N; i++) { + hnsw.index(String(i), [Math.cos(i * 0.5), Math.sin(i * 0.5)], null, {}); + } + for (let round = 0; round < 8; round++) { + const newVec = [Math.cos(round * 0.7), Math.sin(round * 0.7)]; + hnsw.index('3', newVec, [Math.cos((round - 1) * 0.7) || 1, 0.1], {}); + } + + // Check symmetry directly on the mock store. + let asymmetries = 0; + for (const [k, node] of store._nodes()) { + if (typeof k !== 'number' || node?.level === undefined) continue; + for (let l = 0; l <= node.level; l++) { + for (const { id: neighborId } of node[l] || []) { + const neighborNode = store.getSync(neighborId); + if (!neighborNode) continue; + const sym = (neighborNode[l] || []).find(({ id }) => id === k); + if (!sym) asymmetries++; + } + } + } + assert(asymmetries < 5, `expected < 5 asymmetries after repeated updates, got ${asymmetries}`); + }); + }); + + // ── backfill idempotency (re-feeding existing key) ──────────────────────── + describe('backfill resume idempotency', () => { + it('calling index() twice with the same key preserves symmetric connections', () => { + const store = makeMockStore(); + const hnsw = new HierarchicalNavigableSmallWorld(store, { distance: 'euclidean', optimizeRouting: 0 }); + + // First insert: establishes the node and its connections. + for (let i = 0; i < 8; i++) { + hnsw.index(String(i), [i, i * 0.5], null, {}); + } + + // Second call for key '0' with no existingVector (simulates a backfill re-feed). + // Should behave as an update, not a fresh insert. + hnsw.index('0', [0, 0], null, {}); + + // Verify symmetry: for every connection a→b, b→a must also exist at the same level. + let asymmetries = 0; + for (const [k, node] of store._nodes()) { + if (typeof k !== 'number' || node?.level === undefined) continue; + let l = 0; + while (node[l]) { + for (const { id: neighborId } of node[l]) { + const neighbor = store.getSync(neighborId); + if (!neighbor) continue; + const sym = (neighbor[l] || []).find(({ id }) => id === k); + if (!sym) asymmetries++; + } + l++; + } + } + assert(asymmetries < 3, `expected < 3 asymmetries after backfill re-feed, got ${asymmetries}`); + }); + + it('level is preserved when re-feeding an existing node without existingVector', () => { + const store = makeMockStore(); + const hnsw = new HierarchicalNavigableSmallWorld(store, { distance: 'euclidean', optimizeRouting: 0 }); + // Seed a few nodes so the graph is non-trivial. + for (let i = 0; i < 6; i++) hnsw.index(String(i), [i, 0], null, {}); + + // Capture the level of node '0' after first insert. + const safeKey0 = '0'; + const nodeId0 = store.getSync(safeKey0); + const levelBefore = store.getSync(nodeId0)?.level; + + // Re-feed with same vector, no existingVector (backfill scenario). + hnsw.index('0', [0, 0], null, {}); + + const levelAfter = store.getSync(nodeId0)?.level; + assert.equal(levelAfter, levelBefore, 'level must be preserved on backfill re-feed'); + }); + }); +}); + async function fromAsync(iterable) { let results = []; for await (let entry of iterable) {