From fd699d0e0791b24d39ee0a92f823bd8a763c5c28 Mon Sep 17 00:00:00 2001 From: David Snelling Date: Mon, 29 Jun 2026 16:40:02 -0700 Subject: [PATCH] fix: never serve a silent [] from find({connected}) on a cold-loaded graph MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a cold process start of a large brain (above the eager index-rebuild threshold), a native graph adjacency can report size()>0 — its membership set reloaded — while the source->target edges did NOT load, so find({connected}), neighbors() and related() returned [] for edges that are persisted on disk. A database returning empty for data that exists, based purely on warm/cold state, is a correctness bug; it hit a production deployment after every deploy. Refactor the cold-load guard into verifyGraphAdjacencyLive(), which gates its heal/throw decision on a GLOBAL known-edge sample (a persisted verb's source, which by definition has an outgoing edge) rather than the queried anchor: a stale adjacency is rebuilt from storage, an unrecoverable one throws GraphIndexNotReadyError instead of serving [], and a genuinely edgeless anchor still returns [] with no spurious rebuild. executeGraphSearch re-verifies before trusting an empty connected result and re-collects after a heal. Adds a 5-case integration test (self-heal / loud-throw / edgeless-no-false-positive / healthy / re-collect) plus updated unit coverage. --- src/brainy.ts | 98 +++++++--- .../integration/cold-graph-connected.test.ts | 170 ++++++++++++++++++ .../brainy/graph-adjacency-cold-load.test.ts | 16 +- 3 files changed, 251 insertions(+), 33 deletions(-) create mode 100644 tests/integration/cold-graph-connected.test.ts diff --git a/src/brainy.ts b/src/brainy.ts index 7975c42b..97a2e055 100644 --- a/src/brainy.ts +++ b/src/brainy.ts @@ -192,7 +192,8 @@ export class Brainy implements BrainyInterface { private _versions?: VersioningAPI private _vfs?: VirtualFileSystem private _vfsInitialized = false // Track VFS init completion separately - private _graphAdjacencyVerified = false // 7.33.2: one-time graph-adjacency cold-load consistency check + private _graphAdjacencyVerified = false // graph-adjacency cold-load consistency: verified-live this session + private _graphAdjacencyVerifying = false // re-entrancy guard: a verify (rebuild→reads) is in flight private _hub?: IntegrationHub // Integration Hub for external tools private _pendingMigrationRunner?: MigrationRunner // Deferred migration runner for large datasets private _aggregationIndex?: AggregationIndex // Incremental aggregation engine @@ -2488,7 +2489,7 @@ export class Brainy implements BrainyInterface { paramsOrId?: string | GetRelationsParams ): Promise[]> { await this.ensureInitialized() - await this.ensureGraphAdjacencyConsistent() + await this.verifyGraphAdjacencyLive() // Handle string ID shorthand: getRelations(id) -> getRelations({ from: id }) const params = typeof paramsOrId === 'string' @@ -8178,32 +8179,52 @@ export class Brainy implements BrainyInterface { * }) */ /** - * @description 7.33.2 — guard against a graph index that loaded its relationship - * COUNT (manifest) on a cold open but NOT its source→target adjacency, so graph - * traversals (`find({ connected })`, `neighbors`, `related`) would silently return - * `[]` despite persisted edges. Observed with a native graph provider whose - * cold-open SSTable load is swallowed on some storage adapters. Runs ONCE per brain - * (cached): if the index claims edges exist but a known persisted edge's source - * resolves to no neighbors, force a rebuild from storage; if even that can't read - * the edge, throw {@link GraphIndexNotReadyError} rather than serving `[]` as truth. - * O(1) (one verb read + one neighbor probe) and a no-op once the adjacency is live. + * @description Verify that the graph adjacency is actually LIVE before a graph read trusts + * its result. A native graph index can load its relationship COUNT (manifest) on a cold open + * of a LARGE brain (≥10k nouns, which skips the eager index rebuild) but NOT its + * source→target adjacency, so `getNeighbors()` returns `[]` for EVERY source even though + * edges are persisted — and `find({ connected })` / `neighbors()` / `related()` would serve + * that `[]` as if it were truth. + * + * The check is gated on a GLOBAL known-edge sample (a real persisted verb's `sourceId`, which + * by definition HAS an outgoing edge) — NOT on any queried anchor, because brainy cannot + * cheaply tell "adjacency unloaded" from "this node is genuinely edgeless" per-anchor + * (`getVerbsBySource`/`getVerbsByTarget` route through the same `isInitialized`-gated fast path + * that lies on cold open). If that known-edge source resolves to no neighbors, the adjacency + * did not load: rebuild from storage and re-probe; if even that fails, throw + * {@link GraphIndexNotReadyError} rather than returning `[]`. + * + * @returns `'live'` when the adjacency is already trustworthy (or there is genuinely nothing + * to verify), or `'rebuilt'` when a cold-unloaded adjacency was just healed from storage — + * in which case callers that observed an empty result must RE-RUN their collection. + * @throws {GraphIndexNotReadyError} when the index claims edges but cannot serve a known + * persisted edge even after a rebuild. */ - private async ensureGraphAdjacencyConsistent(): Promise { - if (this._graphAdjacencyVerified) return - // Set up-front so a re-entrant call (rebuild → reads) cannot loop. - this._graphAdjacencyVerified = true + private async verifyGraphAdjacencyLive(): Promise<'live' | 'rebuilt'> { + if (this._graphAdjacencyVerified) return 'live' + // Re-entrancy: rebuild() triggers reads (neighbors/getRelations) that call back into this + // guard. While a verify is in flight, short-circuit so we cannot recurse into rebuild(). + if (this._graphAdjacencyVerifying) return 'live' + this._graphAdjacencyVerifying = true try { const claimed = await this.graphIndex.size() - if (!claimed || claimed <= 0) return // no edges claimed — nothing to verify + if (!claimed || claimed <= 0) return 'live' // no edges claimed — nothing to verify const sample = await this.storage.getVerbs({ pagination: { limit: 1 } }) const verb = sample.items?.[0] - if (!verb || !verb.sourceId) return // no edges in storage — stale count, harmless + if (!verb || !verb.sourceId) { + // no edges in storage — stale count, harmless; don't re-probe on every read. + this._graphAdjacencyVerified = true + return 'live' + } const probe = await this.graphIndex.getNeighbors(verb.sourceId, { limit: 1 }) - if (probe.length > 0) return // adjacency is live — the common case + if (probe.length > 0) { + this._graphAdjacencyVerified = true + return 'live' // adjacency is live — the common case + } - // INCONSISTENT: the index reports edges but a persisted edge's source has none → + // INCONSISTENT: the index reports edges but a KNOWN persisted edge's source has none → // the adjacency did not load on open. Rebuild it from storage. if (!this.config.silent) { console.warn( @@ -8221,6 +8242,8 @@ export class Brainy implements BrainyInterface { `neighbors() and related() cannot be served reliably for this brain.` ) } + this._graphAdjacencyVerified = true + return 'rebuilt' } catch (err) { if (err instanceof GraphIndexNotReadyError) throw err // A transient probe/rebuild failure must not break the actual query NOR be @@ -8229,6 +8252,9 @@ export class Brainy implements BrainyInterface { if (!this.config.silent) { console.warn(`[Brainy] Graph adjacency consistency check skipped (transient): ${err}`) } + return 'live' + } finally { + this._graphAdjacencyVerifying = false } } @@ -8242,7 +8268,7 @@ export class Brainy implements BrainyInterface { } ): Promise { await this.ensureInitialized() - await this.ensureGraphAdjacencyConsistent() + await this.verifyGraphAdjacencyLive() const direction = options?.direction || 'both' const limit = options?.limit @@ -8798,13 +8824,35 @@ export class Brainy implements BrainyInterface { const connectedIds = new Set() - if (from) { - for (const id of await collect(from, toNeighborDir(direction))) connectedIds.add(id) + // Populate connectedIds from the from/to anchors. Extracted into a closure so it can be + // re-run VERBATIM after a cold-load heal (verdict 'rebuilt') without changing collection + // semantics — only re-executing the same traversal once the adjacency is actually loaded. + const populate = async (): Promise => { + if (from) { + for (const id of await collect(from, toNeighborDir(direction))) connectedIds.add(id) + } + if (to) { + const reverse: 'in' | 'out' | 'both' = direction === 'in' ? 'out' : direction === 'out' ? 'in' : 'both' + for (const id of await collect(to, toNeighborDir(reverse))) connectedIds.add(id) + } } - if (to) { - const reverse: 'in' | 'out' | 'both' = direction === 'in' ? 'out' : direction === 'out' ? 'in' : 'both' - for (const id of await collect(to, toNeighborDir(reverse))) connectedIds.add(id) + await populate() + + // Cold-load guard: an empty connected set is suspicious. The native adjacency can report + // size()>0 on a cold open yet have loaded NO source→target edges — so traversal silently + // returns []. Re-verify against a GLOBAL known-edge sample (NOT the queried anchor, which + // may be genuinely edgeless). If the adjacency was dead and a rebuild healed it, re-collect; + // if it stays dead, verifyGraphAdjacencyLive() throws GraphIndexNotReadyError. A genuinely + // edgeless anchor verifies 'live' and the empty result stands — no spurious rebuild/throw. + let rechecked = false + if (connectedIds.size === 0 && !rechecked) { + const verdict = await this.verifyGraphAdjacencyLive() + if (verdict === 'rebuilt') { + rechecked = true + connectedIds.clear() + await populate() + } } // Subtype filter (depth-1 only — see guard above). Intersect connectedIds with the set of diff --git a/tests/integration/cold-graph-connected.test.ts b/tests/integration/cold-graph-connected.test.ts new file mode 100644 index 00000000..1c3a0565 --- /dev/null +++ b/tests/integration/cold-graph-connected.test.ts @@ -0,0 +1,170 @@ +/** + * @module tests/integration/cold-graph-connected + * @description BRAINY-COLD-GRAPH-CONNECTED — regression coverage for the silent-empty + * graph-traversal bug. On the FIRST `find({ connected })` after a cold process start of a + * LARGE brain (≥10k nouns, which skips the eager index rebuild), a native graph adjacency can + * reload its relationship COUNT (so `size() > 0`) but NOT its source→target edges — so + * `getNeighbors()` returns `[]` for EVERY source and brainy would serve that `[]` as if the + * anchor were genuinely edgeless. + * + * The fix verifies any empty traversal against a GLOBAL known-edge sample (a real persisted + * verb's `sourceId`, which by definition HAS an outgoing edge) rather than the queried anchor: + * - a cold-unloaded adjacency is rebuilt from storage and the traversal re-runs ('rebuilt'); + * - if even a rebuild cannot serve a known edge, it throws {@link GraphIndexNotReadyError} + * instead of returning `[]`; + * - a genuinely edgeless anchor (while OTHER edges exist) verifies 'live' and the empty + * result stands — no spurious rebuild, no throw. + * + * These exercise REAL `find({ connected })` against an in-memory brain whose graph index is + * instrumented to simulate the empty-then-(maybe-)rebuilt cold load — only `getNeighbors`/ + * `rebuild` are wrapped; the underlying real adjacency (built by `relate()`) is unmasked once + * a rebuild "heals" it. + */ + +import { describe, it, expect, afterEach } from 'vitest' +import { Brainy } from '../../src/index.js' +import { NounType, VerbType } from '../../src/types/graphTypes.js' +import { GraphIndexNotReadyError } from '../../src/errors/brainyError.js' +import { createTestConfig } from '../helpers/test-factory.js' + +/** + * Build a real in-memory brain. Always seeds an UNRELATED edge (`E -> F`) so a GLOBAL known-edge + * sample exists for the verify probe even when the queried anchor is genuinely edgeless. When + * `anchorEdges` is set, the anchor links out to three target nouns via `Knows`. Ids are brainy- + * generated (7.x requires UUID-format ids) and returned for assertions. + */ +async function buildBrain( + opts: { anchorEdges: boolean } +): Promise<{ brain: any; anchorId: string; targetIds: string[] }> { + const brain: any = new Brainy(createTestConfig({ silent: true })) + await brain.init() + + // Unrelated edge — guarantees storage.getVerbs() always yields a known-edge source. + const eId = await brain.add({ data: 'node E', type: NounType.Person }) + const fId = await brain.add({ data: 'node F', type: NounType.Person }) + await brain.relate({ from: eId, to: fId, type: VerbType.Knows }) + + const anchorId = await brain.add({ data: 'anchor', type: NounType.Person }) + + const targetIds: string[] = [] + if (opts.anchorEdges) { + for (const label of ['B', 'C', 'D']) { + const tId = await brain.add({ data: `node ${label}`, type: NounType.Person }) + await brain.relate({ from: anchorId, to: tId, type: VerbType.Knows }) + targetIds.push(tId) + } + } + + // Fresh cold-start state: nothing verified yet. + brain._graphAdjacencyVerified = false + return { brain, anchorId, targetIds } +} + +/** + * Wrap the brain's real graph index so `getNeighbors` returns `[]` while `broken` (modelling the + * cold-unloaded adjacency) and delegates to the REAL index once a rebuild flips `broken` off. + * `rebuild` is counted; it heals only when `healsOnRebuild` is set. + */ +function instrumentGraphIndex( + brain: any, + opts: { broken: boolean; healsOnRebuild: boolean } +): { rebuildCalls: number } { + const gi = brain.graphIndex + const origGetNeighbors = gi.getNeighbors.bind(gi) + const state = { broken: opts.broken, rebuildCalls: 0 } + + gi.getNeighbors = async (id: string, optionsOrDirection?: any): Promise => + state.broken ? [] : origGetNeighbors(id, optionsOrDirection) + + gi.rebuild = async (): Promise => { + state.rebuildCalls++ + if (opts.healsOnRebuild) state.broken = false // unmask the real (already-populated) adjacency + } + + return state +} + +describe('BRAINY-COLD-GRAPH-CONNECTED — find({ connected }) never serves a silent []', () => { + let brains: any[] = [] + afterEach(async () => { + for (const b of brains) { + try { + await b.close() + } catch { + /* best-effort cleanup */ + } + } + brains = [] + }) + + it('(a) self-heals: cold-unloaded adjacency that rebuild() repopulates → correct N edges', async () => { + const { brain, anchorId, targetIds } = await buildBrain({ anchorEdges: true }) + brains.push(brain) + const state = instrumentGraphIndex(brain, { broken: true, healsOnRebuild: true }) + + const results = await brain.find({ connected: { from: anchorId, direction: 'out' }, limit: 10 }) + + expect(state.rebuildCalls).toBeGreaterThanOrEqual(1) // detected the empty adjacency + healed it + const ids = results.map((r: any) => r.id).sort() + expect(ids).toEqual(targetIds.sort()) // B, C, D — the real edges, served after the heal + }) + + it('(b) loud throw: cold-unloaded adjacency that rebuild() cannot repopulate → GraphIndexNotReadyError', async () => { + const { brain, anchorId } = await buildBrain({ anchorEdges: true }) + brains.push(brain) + instrumentGraphIndex(brain, { broken: true, healsOnRebuild: false }) // rebuild never heals + + await expect( + brain.find({ connected: { from: anchorId, direction: 'out' }, limit: 10 }) + ).rejects.toBeInstanceOf(GraphIndexNotReadyError) // NOT a silent [] + }) + + it('(c) edgeless anchor (other edges exist): returns [] with NO rebuild and NO throw', async () => { + // The anchor has no edges, but E -> F does — so the GLOBAL sample resolves and the empty stands. + const { brain, anchorId } = await buildBrain({ anchorEdges: false }) + brains.push(brain) + const state = instrumentGraphIndex(brain, { broken: false, healsOnRebuild: false }) + + const results = await brain.find({ connected: { from: anchorId, direction: 'out' }, limit: 10 }) + + expect(results).toEqual([]) // genuinely edgeless — empty is the truth + expect(state.rebuildCalls).toBe(0) // no spurious rebuild on a healthy adjacency + }) + + it('(d) healthy: edges present, adjacency live → correct results, no rebuild', async () => { + const { brain, anchorId, targetIds } = await buildBrain({ anchorEdges: true }) + brains.push(brain) + const state = instrumentGraphIndex(brain, { broken: false, healsOnRebuild: false }) + + const results = await brain.find({ connected: { from: anchorId, direction: 'out' }, limit: 10 }) + + expect(state.rebuildCalls).toBe(0) + const ids = results.map((r: any) => r.id).sort() + expect(ids).toEqual(targetIds.sort()) + }) + + it('(e) executeGraphSearch re-collect path: neighbors() verify hits a transient first, the empty-result guard then heals + re-collects', async () => { + // Force the FIRST verify (inside neighbors()) to fail transiently so it does NOT heal there; + // the empty connectedIds set then drives executeGraphSearch's own verify, which rebuilds and + // re-runs the from/to collection. This specifically exercises the 'rebuilt' → re-collect branch. + const { brain, anchorId, targetIds } = await buildBrain({ anchorEdges: true }) + brains.push(brain) + const state = instrumentGraphIndex(brain, { broken: true, healsOnRebuild: true }) + + const origGetVerbs = brain.storage.getVerbs.bind(brain.storage) + let firstVerbsCall = true + brain.storage.getVerbs = async (params?: any) => { + if (firstVerbsCall) { + firstVerbsCall = false + throw new Error('transient storage hiccup') + } + return origGetVerbs(params) + } + + const results = await brain.find({ connected: { from: anchorId, direction: 'out' }, limit: 10 }) + + expect(state.rebuildCalls).toBeGreaterThanOrEqual(1) + const ids = results.map((r: any) => r.id).sort() + expect(ids).toEqual(targetIds.sort()) // re-collected after the heal + }) +}) diff --git a/tests/unit/brainy/graph-adjacency-cold-load.test.ts b/tests/unit/brainy/graph-adjacency-cold-load.test.ts index 64dca651..06bede5c 100644 --- a/tests/unit/brainy/graph-adjacency-cold-load.test.ts +++ b/tests/unit/brainy/graph-adjacency-cold-load.test.ts @@ -52,7 +52,7 @@ describe('7.33.2 graph-adjacency cold-load consistency guard', () => { const calls = stubGraphIndex(brain, { size: 12, neighborsBeforeRebuild: ['n1'] }) brain.storage.getVerbs = async () => ({ items: [{ id: 'v1', sourceId: 's1', targetId: 't1' }], hasMore: false }) - await brain.ensureGraphAdjacencyConsistent() + await brain.verifyGraphAdjacencyLive() expect(calls.rebuild).toBe(0) await brain.close() }) @@ -62,11 +62,11 @@ describe('7.33.2 graph-adjacency cold-load consistency guard', () => { const calls = stubGraphIndex(brain, { size: 12, neighborsBeforeRebuild: [], neighborsAfterRebuild: ['n1'] }) brain.storage.getVerbs = async () => ({ items: [{ id: 'v1', sourceId: 's1', targetId: 't1' }], hasMore: false }) - await brain.ensureGraphAdjacencyConsistent() + await brain.verifyGraphAdjacencyLive() expect(calls.rebuild).toBe(1) // detected the empty adjacency + rebuilt it // Cached: a second call does not re-probe or re-rebuild. - await brain.ensureGraphAdjacencyConsistent() + await brain.verifyGraphAdjacencyLive() expect(calls.rebuild).toBe(1) await brain.close() }) @@ -76,7 +76,7 @@ describe('7.33.2 graph-adjacency cold-load consistency guard', () => { stubGraphIndex(brain, { size: 12, neighborsBeforeRebuild: [], neighborsAfterRebuild: [] }) // never loads brain.storage.getVerbs = async () => ({ items: [{ id: 'v1', sourceId: 's1', targetId: 't1' }], hasMore: false }) - await expect(brain.ensureGraphAdjacencyConsistent()).rejects.toBeInstanceOf(GraphIndexNotReadyError) + await expect(brain.verifyGraphAdjacencyLive()).rejects.toBeInstanceOf(GraphIndexNotReadyError) await brain.close() }) @@ -89,7 +89,7 @@ describe('7.33.2 graph-adjacency cold-load consistency guard', () => { return { items: [], hasMore: false } } - await brain.ensureGraphAdjacencyConsistent() + await brain.verifyGraphAdjacencyLive() expect(calls.rebuild).toBe(0) expect(probedStorage).toBe(false) await brain.close() @@ -100,7 +100,7 @@ describe('7.33.2 graph-adjacency cold-load consistency guard', () => { const calls = stubGraphIndex(brain, { size: 12, neighborsBeforeRebuild: [] }) brain.storage.getVerbs = async () => ({ items: [], hasMore: false }) - await brain.ensureGraphAdjacencyConsistent() + await brain.verifyGraphAdjacencyLive() expect(calls.rebuild).toBe(0) await brain.close() }) @@ -118,12 +118,12 @@ describe('7.33.2 graph-adjacency cold-load consistency guard', () => { } // First call: probe throws transiently → swallowed (no GraphIndexNotReadyError), flag reset. - await brain.ensureGraphAdjacencyConsistent() + await brain.verifyGraphAdjacencyLive() expect(calls.rebuild).toBe(0) expect(brain._graphAdjacencyVerified).toBe(false) // re-check allowed // Second call: storage works → detects the empty adjacency + rebuilds. - await brain.ensureGraphAdjacencyConsistent() + await brain.verifyGraphAdjacencyLive() expect(calls.rebuild).toBe(1) await brain.close() })