From 3a3aa43b3a0f610ded453343d664ad02a1eb8bb1 Mon Sep 17 00:00:00 2001 From: David Snelling Date: Fri, 19 Jun 2026 11:45:13 -0700 Subject: [PATCH] fix(8.0): VFS path-cache instance-scoping + verb totalCount page-cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent restart/multi-instance correctness bugs. VFS path cache (A.3): PathResolver keyed the PROCESS-GLOBAL path cache on `vfs:path:` with no instance scope. Multiple Brainys per process (a supported pattern) over DIFFERENT storage collided — instance A's `/x → id_A` satisfied instance B's `stat('/x')` against unrelated storage → stale id → "Entity not found" (surfaced by metadata-only-comprehensive's VFS stat() once another VFS test had seeded the cache). Scope every `vfs:path:` key by a monotonic per-process instance token (the VFS root id is a fixed sentinel, so it can't disambiguate; the global cache is itself process-scoped so the token suffices). Scoped the invalidate/prefix-delete paths too, so a branch-switch / clear / fork on one brain no longer wipes another brain's cache. Cross-instance sharing was only an optimization and was the collision itself; each instance keeps its own local pathCache. Verb totalCount (verb mirror of b2005ff): getVerbsWithPagination returned `collectedVerbs.length` (the peeked page size, bounded by offset+limit+1) as totalCount, so getVerbs({limit:1}).totalCount read 1/2 for any non-empty brain on the unfiltered path. Now returns the authoritative O(1) totalVerbCount (isNew-gated, visibility-filtered, rehydrated on init); filtered scans keep the collected length. Regression: tests/unit/storage/getVerbs-totalCount.test.ts (warm + cold reopen, mirrors the noun test). metadata-only-comprehensive + vfs-api-wiring integration green; 1471 unit green. --- src/storage/baseStorage.ts | 15 ++- src/vfs/PathResolver.ts | 46 ++++++++- .../unit/storage/getVerbs-totalCount.test.ts | 94 +++++++++++++++++++ 3 files changed, 149 insertions(+), 6 deletions(-) create mode 100644 tests/unit/storage/getVerbs-totalCount.test.ts diff --git a/src/storage/baseStorage.ts b/src/storage/baseStorage.ts index c8001e05..88fb17e6 100644 --- a/src/storage/baseStorage.ts +++ b/src/storage/baseStorage.ts @@ -1789,9 +1789,22 @@ export abstract class BaseStorage extends BaseStorageAdapter { const paginatedVerbs = collectedVerbs.slice(offset, offset + limit) const hasMore = collectedVerbs.length > targetCount + // totalCount must be the TRUE dataset total, not this peeked page. The verb + // type-scan early-terminates at `peekCount = offset + limit + 1`, so + // `collectedVerbs.length` only ever reaches the page size + 1 — returning it + // made `getVerbs({ pagination: { limit: 1 } }).totalCount` read 1 (or 2) for + // any non-empty brain. This is the verb mirror of the noun fix (b2005ff): for + // the unfiltered scan the authoritative total is the O(1) `totalVerbCount` + // counter (isNew-gated, visibility-filtered, rehydrated on init); `Math.max` + // guards a stale counter from under-reporting. A filtered scan has no cheap + // exact total, so it keeps the collected length (a lower bound). + const totalCount = filter + ? collectedVerbs.length + : Math.max(this.totalVerbCount, collectedVerbs.length) + return { items: paginatedVerbs, - totalCount: collectedVerbs.length, // Accurate count of collected results + totalCount, hasMore, nextCursor: hasMore && paginatedVerbs.length > 0 ? paginatedVerbs[paginatedVerbs.length - 1].id diff --git a/src/vfs/PathResolver.ts b/src/vfs/PathResolver.ts index 6fb97c68..6babd922 100644 --- a/src/vfs/PathResolver.ts +++ b/src/vfs/PathResolver.ts @@ -20,12 +20,29 @@ interface PathCacheEntry { hits: number // Track hot paths } +/** + * Per-process sequence used to scope the PROCESS-GLOBAL path cache to a single + * PathResolver instance. The unified path cache (`getGlobalCache()`) is shared + * across every Brainy in the process; multiple instances over DIFFERENT storage + * are a supported pattern, and the VFS root id is a fixed sentinel, so a path key + * with no instance scope let instance A's `/x → id_A` satisfy instance B's + * `stat('/x')` against unrelated storage → stale id → "Entity not found". A + * monotonic per-process token isolates each instance (the in-memory global cache + * is itself process-scoped, so this is sufficient and survives restarts trivially + * — the cache is empty then anyway). Cross-instance sharing was only an + * optimization and is the very collision this removes; each instance keeps its + * own local pathCache. + */ +let pathResolverScopeSeq = 0 + /** * High-performance path resolver with intelligent caching */ export class PathResolver { private brain: Brainy private rootEntityId: string + /** Stable per-instance prefix scoping this resolver's global-cache entries. */ + private readonly cacheScope: string // Multi-layer cache system private pathCache: Map @@ -54,6 +71,9 @@ export class PathResolver { }) { this.brain = brain this.rootEntityId = rootEntityId + // Scope global-cache keys to this instance so two Brainys in one process + // (over different storage) can't read each other's path→id mappings. + this.cacheScope = `${rootEntityId}#${++pathResolverScopeSeq}` // Initialize caches this.pathCache = new Map() @@ -86,7 +106,7 @@ export class PathResolver { return this.rootEntityId } - const cacheKey = `vfs:path:${normalizedPath}` + const cacheKey = this.globalPathKey(normalizedPath) // L1: UnifiedCache (global LRU cache, <1ms, works for ALL adapters) if (options?.cache !== false) { @@ -148,6 +168,21 @@ export class PathResolver { return entityId } + /** + * Build this instance's process-global cache key for a normalized path. + * Scoped by {@link cacheScope} so instances over different storage never collide. + */ + private globalPathKey(normalizedPath: string): string { + return `vfs:path:${this.cacheScope}:${normalizedPath}` + } + + /** + * This instance's global-cache key prefix (for scoped prefix deletes). + */ + private globalPathPrefix(): string { + return `vfs:path:${this.cacheScope}:` + } + /** * Full path resolution by traversing the graph */ @@ -366,8 +401,9 @@ export class PathResolver { this.parentCache.clear() this.hotPaths.clear() - // Clear all VFS entries from UnifiedCache - getGlobalCache().deleteByPrefix('vfs:path:') + // Clear THIS instance's VFS entries from UnifiedCache (scoped so a branch + // switch / clear / fork on one brain can't wipe another brain's cache). + getGlobalCache().deleteByPrefix(this.globalPathPrefix()) // Reset statistics (optional but helpful for debugging) this.cacheHits = 0 @@ -400,7 +436,7 @@ export class PathResolver { // CRITICAL FIX: Also invalidate UnifiedCache (global LRU cache) // This was missing before, causing stale entity IDs to be returned after delete - const cacheKey = `vfs:path:${normalizedPath}` + const cacheKey = this.globalPathKey(normalizedPath) getGlobalCache().delete(cacheKey) if (recursive) { @@ -417,7 +453,7 @@ export class PathResolver { } // CRITICAL FIX: Also invalidate UnifiedCache entries with this prefix - const globalCachePrefix = `vfs:path:${prefix}` + const globalCachePrefix = this.globalPathKey(prefix) getGlobalCache().deleteByPrefix(globalCachePrefix) } } diff --git a/tests/unit/storage/getVerbs-totalCount.test.ts b/tests/unit/storage/getVerbs-totalCount.test.ts new file mode 100644 index 00000000..325a3824 --- /dev/null +++ b/tests/unit/storage/getVerbs-totalCount.test.ts @@ -0,0 +1,94 @@ +/** + * @module tests/unit/storage/getVerbs-totalCount + * @description Verb-side mirror of getNouns-totalCount (the b2005ff fix). The + * verb type-scan pagination (`BaseStorage.getVerbsWithPagination`) early-terminates + * at `offset + limit + 1` (it peeks one extra item to compute `hasMore`), then + * returned `collectedVerbs.length` as `totalCount` — i.e. the PAGE size, never the + * dataset total. So `getVerbs({ pagination: { limit: 1 } }).totalCount` reported 1 + * (or 2) for any non-empty brain on the unfiltered path. `totalCount` must be the + * authoritative O(1) verb counter (`totalVerbCount`, isNew-gated + + * visibility-filtered, rehydrated from `counts.json` on init). + */ +import { describe, it, expect } from 'vitest' +import { mkdtempSync, rmSync } from 'fs' +import { tmpdir } from 'os' +import { join } from 'path' +import { FileSystemStorage } from '../../../src/storage/adapters/fileSystemStorage.js' +import type { VerbMetadata, HNSWVerb } from '../../../src/coreTypes.js' +import { VerbType } from '../../../src/types/graphTypes.js' + +const DIM = 8 + +/** A deterministic non-zero vector so the verb record is well-formed. */ +function vec(seed: number): number[] { + return Array.from({ length: DIM }, (_, i) => ((seed + i) % 7) / 7 - 0.5) +} + +/** Storage shards by UUID, so ids must be 32 hex chars. */ +function uuid(i: number, salt = 0): string { + return (i + salt).toString(16).padStart(32, '0') +} + +/** + * Seed `n` unique verbs. Metadata is saved BEFORE the verb record: it is where + * the O(1) verb counter is incremented (isNew-gated), mirroring the real + * relate() path. + */ +async function seed(storage: FileSystemStorage, n: number): Promise { + for (let i = 0; i < n; i++) { + const id = uuid(i, 0x1000) + const sourceId = uuid(i, 0x4000) + const targetId = uuid(i, 0x8000) + await storage.saveVerbMetadata(id, { + verb: VerbType.RelatedTo, + sourceId, + targetId, + createdAt: Date.now(), + updatedAt: Date.now() + } as VerbMetadata) + await storage.saveVerb({ + id, + vector: vec(i), + connections: new Map(), + verb: VerbType.RelatedTo, + sourceId, + targetId + } as HNSWVerb) + } +} + +describe('FileSystemStorage.getVerbs totalCount (verb mirror of b2005ff)', () => { + it('reports the TRUE total, not the page size, for a 1-item page', async () => { + const dir = mkdtempSync(join(tmpdir(), 'brainy-verbtotal-')) + try { + const storage = new FileSystemStorage(dir) + await storage.init() + await seed(storage, 25) + + const onePage = await storage.getVerbs({ pagination: { limit: 1 } }) + expect(onePage.items.length).toBe(1) + // Before the fix this was the peeked page size (limit + 1 == 2). + expect(onePage.totalCount).toBe(25) + expect(onePage.hasMore).toBe(true) + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }) + + it('still reports the true total after a cold reopen (counts rehydrate)', async () => { + const dir = mkdtempSync(join(tmpdir(), 'brainy-verbtotal-reopen-')) + try { + const first = new FileSystemStorage(dir) + await first.init() + await seed(first, 25) + await (first as unknown as { persistCounts(): Promise }).persistCounts() + + const reopened = new FileSystemStorage(dir) + await reopened.init() + const page = await reopened.getVerbs({ pagination: { limit: 1 } }) + expect(page.totalCount).toBe(25) + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }) +})