fix(hnsw): skip unvectored rows on rebuild; refuse empty vectors in the index
A canonical row persisted with vector: [] (a system row, a deferred embed not yet landed, or any other legitimately-unvectored record) is a normal, enumerable row -- but rebuild()'s storage walk had no guard against it. storage.getVectorIndexData() derives its answer from the row's own record, so it returns non-null for any existing noun whether or not that noun was ever actually indexed -- rebuild() admitted such rows into the live graph with a length-0 vector. A vector-less node could become the entry point (or occupy any graph position); the next real insert then ran a distance calculation against it and blew up with a dimension mismatch. Fix at two layers in src/hnsw/hnswIndex.ts: - rebuild() now skips any row whose vector.length === 0 before it ever becomes a graph node (one summary count line, never per-row spam), and restores the pinned dimension from the first real vector it loads -- previously the pin stayed null across a restart, since addItem/updateItem are the only sites that set it and rebuild() never goes through either. - addItem/updateItem now refuse a length-0 vector with a typed EmptyVectorIndexError instead of ever pinning dimension to 0 or storing a vector-less node, so no future fill/rebuild/load path can poison the index silently. getVectorSafe's lazy-load "not found" check also missed that an empty array is truthy -- tightened to catch it. IndexOperations.ts's ReplaceInVectorIndexOperation rollback paths now skip re-adding an oldVector of length 0 (never a legal index member) instead of attempting an illegal empty re-insert on rollback. biography.test.ts's final ledger-exactness assertion assumed every noun the lane creates is vectored, including the VFS root counted in vfsBaselineNouns -- but the root is deliberately persisted unvectored. Corrected the expected formula to exclude it. Adds tests/integration/index-skips-unvectored.test.ts pinning: rebuild() indexes only vectored rows with the dimension pinned correctly; clear() then real adds never trip a dimension mismatch; addItem/updateItem refuse a length-0 vector; and a crash/repair cycle stays dimension-consistent.
This commit is contained in:
parent
fd6b4ce4ff
commit
8fc553b126
4 changed files with 371 additions and 8 deletions
|
|
@ -64,6 +64,34 @@ export class HnswFlushError extends Error {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @description Thrown by {@link JsHnswVectorIndex.addItem} / {@link
|
||||
* JsHnswVectorIndex.updateItem} when handed a length-0 vector. A length-0
|
||||
* vector is the sanctioned "unvectored" shape for a canonical noun record
|
||||
* (class-J: a VFS-system row, a deferred embed not yet landed, or any other
|
||||
* legitimately-vector-less row) — but it is NEVER a legal INDEX insert. The
|
||||
* index itself has no concept of "unvectored"; deciding that a row is
|
||||
* unvectored and therefore skippable is the FILL/REBUILD/LOAD consumer's job
|
||||
* (see {@link JsHnswVectorIndex.rebuild}), done BEFORE ever calling addItem.
|
||||
* A length-0 vector reaching this point is a caller bug: silently accepting
|
||||
* it would pin `this.dimension = 0` on an empty index (poisoning every real
|
||||
* insert thereafter with a dimension mismatch) or store a vector-less node
|
||||
* that a distance calculation can never safely compare against. Loud errors,
|
||||
* never quiet losses — this throws instead of either.
|
||||
*/
|
||||
export class EmptyVectorIndexError extends Error {
|
||||
constructor(public readonly id: string, operation: 'addItem' | 'updateItem') {
|
||||
super(
|
||||
`${operation}(${id}): refusing to index a length-0 vector — a length-0 vector is the ` +
|
||||
`sanctioned "unvectored" shape for a canonical row, but it is never a legal index ` +
|
||||
`insert. Callers that fill/rebuild/load the index must skip vector.length === 0 rows ` +
|
||||
`themselves (unvectored = nothing to index, not an error at that layer); reaching ` +
|
||||
`here with one is a caller bug.`
|
||||
)
|
||||
this.name = 'EmptyVectorIndexError'
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Implements {@link VectorIndexProvider}: the vector-index surface Brainy calls
|
||||
* on whatever the `'vector'` factory returns (its own `JsHnswVectorIndex`, or a native
|
||||
|
|
@ -580,6 +608,15 @@ export class JsHnswVectorIndex implements VectorIndexProvider {
|
|||
throw new Error('Vector is undefined or null')
|
||||
}
|
||||
|
||||
// THE INDEX REFUSES A LENGTH-0 VECTOR (see EmptyVectorIndexError's JSDoc):
|
||||
// an empty vector is the sanctioned "unvectored" shape at the canonical
|
||||
// layer, never a legal index member. Refusing here — loudly, before the
|
||||
// dimension pin below — means no future fill/rebuild/load path can ever
|
||||
// poison `this.dimension` to 0 or park a vector-less node in the graph.
|
||||
if (vector.length === 0) {
|
||||
throw new EmptyVectorIndexError(id, 'addItem')
|
||||
}
|
||||
|
||||
// Set dimension on first insert
|
||||
if (this.dimension === null) {
|
||||
this.dimension = vector.length
|
||||
|
|
@ -954,6 +991,13 @@ export class JsHnswVectorIndex implements VectorIndexProvider {
|
|||
return
|
||||
}
|
||||
|
||||
// Same refusal as addItem (see EmptyVectorIndexError's JSDoc) — an
|
||||
// in-place relink must never rewrite an already-indexed node down to the
|
||||
// unvectored shape or poison the pinned dimension.
|
||||
if (vector.length === 0) {
|
||||
throw new EmptyVectorIndexError(id, 'updateItem')
|
||||
}
|
||||
|
||||
if (this.dimension === null) {
|
||||
this.dimension = vector.length
|
||||
} else if (vector.length !== this.dimension) {
|
||||
|
|
@ -1555,7 +1599,15 @@ export class JsHnswVectorIndex implements VectorIndexProvider {
|
|||
}
|
||||
|
||||
const loaded = await this.storage.getNounVector(noun.id)
|
||||
if (!loaded) {
|
||||
// `loaded` is a length-0 array (not null/undefined) for a canonical row
|
||||
// that is legitimately unvectored — `![]` is FALSE (an empty array is
|
||||
// truthy), so the bare `!loaded` check below would silently accept it
|
||||
// as "found" and hand a dimension-0 vector to a distance calculation.
|
||||
// A node only reaches this lazy-load path because it is a MEMBER of
|
||||
// the live index (rebuild() now refuses to admit unvectored rows — see
|
||||
// its JSDoc), so an empty vector here is never legitimate: treat it
|
||||
// exactly like "not found", loudly.
|
||||
if (!loaded || loaded.length === 0) {
|
||||
throw new Error(`Vector not found for noun ${noun.id}`)
|
||||
}
|
||||
|
||||
|
|
@ -1765,9 +1817,42 @@ export class JsHnswVectorIndex implements VectorIndexProvider {
|
|||
|
||||
totalCount = result.totalCount || result.items.length
|
||||
|
||||
// UNVECTORED ROWS ARE NOT AN INDEX MEMBER (the class-J law): a canonical
|
||||
// noun whose vector leg is `[]` (a VFS-root-style system row, a
|
||||
// deferred embed not yet landed, or a best-effort fallback for an
|
||||
// unreadable vector leg) is a normal, enumerable, countable row — it
|
||||
// is simply not indexed. `storage.getVectorIndexData()` derives its
|
||||
// {level, connections} answer straight from the noun's OWN record, so
|
||||
// it returns non-null for every existing noun regardless of whether
|
||||
// that noun ever actually reached `addItem()` — it cannot be used to
|
||||
// decide indexability. `nounData.vector.length === 0` is the one
|
||||
// truthful signal (mirrors the `noun.vector.length > 0` guards in
|
||||
// {@link getVectorSafe} / {@link getVectorSync}): skip here, counted
|
||||
// once in a summary line, never per-row spam.
|
||||
let skippedUnvectored = 0
|
||||
|
||||
// Process all nouns at once
|
||||
for (const nounData of result.items) {
|
||||
try {
|
||||
if (!nounData.vector || nounData.vector.length === 0) {
|
||||
skippedUnvectored++
|
||||
continue
|
||||
}
|
||||
|
||||
// Restore the pinned dimension from the first real vector this
|
||||
// rebuild loads. `addItem`/`updateItem` only pin `this.dimension`
|
||||
// on a LIVE insert — a fresh rebuild from storage never goes
|
||||
// through either, so without this the pin stays `null` across a
|
||||
// restart. A `null` pin means the very next insert (correct OR
|
||||
// wrong length) silently BECOMES the new pin instead of being
|
||||
// checked against the store's real dimension — the wrong-length
|
||||
// case then fails much later and less clearly, inside a distance
|
||||
// calculation against an already-loaded node, instead of here,
|
||||
// immediately, with a named expected-vs-got mismatch.
|
||||
if (this.dimension === null) {
|
||||
this.dimension = nounData.vector.length
|
||||
}
|
||||
|
||||
// Load HNSW graph data for this entity
|
||||
const hnswData = await this.storage.getVectorIndexData(nounData.id)
|
||||
|
||||
|
|
@ -1815,7 +1900,10 @@ export class JsHnswVectorIndex implements VectorIndexProvider {
|
|||
options.onProgress(loadedCount, totalCount)
|
||||
}
|
||||
|
||||
prodLog.info(`HNSW: Loaded ${loadedCount.toLocaleString()} nodes (${storageType})`)
|
||||
prodLog.info(
|
||||
`HNSW: Loaded ${loadedCount.toLocaleString()} nodes (${storageType})` +
|
||||
(skippedUnvectored > 0 ? ` — ${skippedUnvectored.toLocaleString()} unvectored row(s) skipped` : '')
|
||||
)
|
||||
}
|
||||
|
||||
// Step 5: CRITICAL - Recover entry point if missing)
|
||||
|
|
|
|||
|
|
@ -324,8 +324,17 @@ export class ReplaceInVectorIndexOperation implements Operation {
|
|||
|
||||
return async () => {
|
||||
// Restore the declared before-state in place (see class JSDoc for
|
||||
// the item-did-not-exist posture).
|
||||
await index.updateItem!({ id: this.id, vector: this.oldVector }, generation)
|
||||
// the item-did-not-exist posture). A length-0 oldVector means the row
|
||||
// was never actually indexed before this op ran (a length-0 vector is
|
||||
// never a legal index member — see EmptyVectorIndexError) — there is
|
||||
// no in-place "restore to empty" for the provider to perform, so
|
||||
// rollback removes the row instead, leaving the same "not indexed"
|
||||
// state the row was in before execute().
|
||||
if (this.oldVector.length > 0) {
|
||||
await index.updateItem!({ id: this.id, vector: this.oldVector }, generation)
|
||||
} else {
|
||||
await this.index.removeItem(this.id, generation)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -336,9 +345,14 @@ export class ReplaceInVectorIndexOperation implements Operation {
|
|||
|
||||
return async () => {
|
||||
// updateItem-style restore via the same adjacent pair, back to the
|
||||
// declared before-state.
|
||||
// declared before-state. Same length-0 carve-out as the updateItem
|
||||
// path above: an empty oldVector was never a legal index member, so
|
||||
// rollback just leaves the row removed rather than attempting an
|
||||
// illegal empty re-add.
|
||||
await this.index.removeItem(this.id, generation)
|
||||
await this.index.addItem({ id: this.id, vector: this.oldVector }, generation)
|
||||
if (this.oldVector.length > 0) {
|
||||
await this.index.addItem({ id: this.id, vector: this.oldVector }, generation)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue