From 96624f408cadcb824cdd2dabcf40aceb9a78fd8b Mon Sep 17 00:00:00 2001 From: David Snelling Date: Tue, 25 Aug 2026 10:09:45 -0700 Subject: [PATCH 1/3] feat(open-path): init never gates on the embedding model; open goes concurrent; slow opens narrate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A production restart storm measured 90,017ms for a single brain init vs 1,117ms quiet (~80x contention multiplier), traced to performInit() eagerly awaiting the process-global WASM embedding engine before the VFS root even existed. Every writer's open() queued on the one throttled model compile (90-140s on throttled CPUs). - VirtualFileSystem.doInitializeRoot() no longer embeds '/'. The root is system-tier plumbing nothing ever searches; when the default WASM engine is active it now gets an explicit all-zero placeholder vector (cosineDistance returns max distance for a zero vector, so it never ranks ahead of real content). deferEmbedding was considered and rejected: its landing path kicks the embed worker synchronously right after commit, which would still force the cold compile within milliseconds — just off the awaited path, not avoided. A registered native 'embeddings' provider (no cold-start cost, possibly a different dimension) still embeds the root for real, via the new Brainy.usesDefaultWasmEmbedder() seam. - performInit()'s eager-embedding step now only STARTS the WASM engine warm in the background instead of awaiting it inline. embed()/embeddingManager already serialize concurrent callers on one shared init promise, so the first real embed() converges correctly either way; a failed warm narrates loudly instead of surfacing as a silent latency spike or an unhandled rejection. eagerEmbeddings: false still means no warm at all. - FileSystemStorage.init() batches its ~8 independent bootstrap mkdirs (each creates its own full subtree via recursive:true, so none depend on the others existing) into one Promise.all. The restore-completion step and initializeCounts() stay strictly sequential — they have real order dependencies on rootDir and systemDir respectively. - performInit() now times five phases (storage init / generation-store open+fold / index init+gate / VFS bootstrap / embedding-warm-started) and logs one warning with the per-phase breakdown when total open exceeds 2000ms; silent otherwise. --- src/brainy.ts | 146 ++++++++++++++-- src/storage/adapters/fileSystemStorage.ts | 74 ++++---- src/types/brainy.types.ts | 33 ++-- src/vfs/VirtualFileSystem.ts | 47 ++++- tests/unit/brainy/open-path.test.ts | 199 ++++++++++++++++++++++ 5 files changed, 446 insertions(+), 53 deletions(-) create mode 100644 tests/unit/brainy/open-path.test.ts diff --git a/src/brainy.ts b/src/brainy.ts index c8912e26..9cd7dcb4 100644 --- a/src/brainy.ts +++ b/src/brainy.ts @@ -742,6 +742,15 @@ export class Brainy implements BrainyInterface { private _pendingEmbedIds = new Set() private _embedWorkerFlight: Promise | null = null + // OPEN-PATH FIX: the background embedding-engine warm kicked off (never + // awaited) by `performInit()` when `eagerEmbeddings` resolves true. Stored + // for observability only — `embed()`/`embeddingManager.embed()` already + // await the engine's OWN singleton init promise internally, so nothing + // needs to explicitly await this field for correctness. Never rejects on + // its own: a `.catch` narrates the failure and swallows it so a failed + // warm never surfaces as an unhandled rejection. + private _embeddingWarmPromise: Promise | null = null + /** The stored log-authority switch, read once at open (default: tree). */ private _logAuthority: LogAuthorityRecord = { authority: 'tree' } // A failed walk latches its error: retries within the cooldown rethrow it @@ -1079,6 +1088,23 @@ export class Brainy implements BrainyInterface { configureLogger({ level: LogLevel.DEBUG }) // Enable verbose logging } + // OPEN-PATH NARRATION: lightweight phase timing across the five + // named stretches of init — storage init / generation-store open+fold / + // index init+gate / VFS bootstrap / embedding-warm-started. Each + // `markPhase()` call records elapsed ms SINCE THE PREVIOUS checkpoint, + // so the buckets always sum to the pre-integration/warmOnOpen total. + // Silent under 2s; one `prodLog.warn` line naming every phase's ms + // above it, so the operator's next restart storm names its own slow + // phase instead of re-deriving it from a stack of raw timestamps. + const initStart = Date.now() + let lastPhaseCheckpoint = initStart + const phaseTimingsMs: Record = {} + const markPhase = (name: string): void => { + const now = Date.now() + phaseTimingsMs[name] = now - lastPhaseCheckpoint + lastPhaseCheckpoint = now + } + try { // Auto-detect and activate plugins BEFORE storage setup // so plugin-provided storage factories (e.g., filesystem override from cor) are available @@ -1150,6 +1176,12 @@ export class Brainy implements BrainyInterface { } } + // PHASE 1 of 5 — "storage init": plugin/legacy-layout bootstrap, + // storage adapter construction+init, the OS-limit check, and the + // writer-lock claim, all folded into one bucket (everything above this + // line since performInit started). + markPhase('storage-init') + // 8.0 generational MVCC: open the record layer BEFORE any index is // created or loaded. Crash recovery may rewrite canonical entity files // (restoring before-images of an uncommitted transaction), and every @@ -1224,6 +1256,12 @@ export class Brainy implements BrainyInterface { await this.createMigrationBackupIfNeeded() } + // PHASE 2 of 5 — "generation-store open+fold": GenerationStore + // construction+open (crash-recovery replay/rollback fold), the + // derived-family registration, the fact-scan seam, the entity-tree + // stamp check, the brain-format handshake, and the pre-upgrade backup. + markPhase('generation-store-open-fold') + // Provider: embeddings (reassign embedder if plugin provides one) const embeddingProvider = this.pluginRegistry.getProvider('embeddings') if (embeddingProvider) { @@ -1461,6 +1499,14 @@ export class Brainy implements BrainyInterface { // Check for pending data migrations await this.checkMigrations() + // PHASE 3 of 5 — "index init+gate": provider wiring (embeddings, + // cache, roaring, msgpack, sort:topK, distance), HNSW/metadata/graph + // index construction, the eager cold-load, id-resolver + connections- + // codec wiring, crash-recovery index rebuild, the replay-gap check, + // legacy VFS blob adoption, blob-history backfill, and the + // rebuildIndexesIfNeeded() gate + migration check. + markPhase('index-init-gate') + // Register shutdown hooks for graceful count flushing (once globally) if (!Brainy.shutdownHooksRegisteredGlobally) { this.registerShutdownHooks() @@ -1612,15 +1658,40 @@ export class Brainy implements BrainyInterface { } } - // Eager embedding initialization. + // PHASE 4 of 5 — "VFS bootstrap": shutdown-hook registration, blob + // storage init, the provider-summary log, flipping `initialized`, + // the migration-lock wait, VFS construction+init, flipping generation + // stamping active, the log-authority adopt/oracle check, and + // pending-embed crash recovery. + markPhase('vfs-bootstrap') + + // Eager embedding initialization — BACKGROUND WARM (open-path fix). // - // Adaptive default (8.0): the WASM embedding engine eagerly initializes + // Adaptive default (8.0): the WASM embedding engine eagerly WARMS // during init() WHENEVER it is the active embedder — i.e. no native // 'embeddings' provider has taken over — and the instance is a writer // (not reader-mode) outside of unit tests. The WASM module (≈93MB with - // the embedded model) takes 90-140s to compile on throttled CPUs; paying - // that during boot rather than on the first embed()-driven call is the - // right default for the overwhelmingly common single-process server. + // the embedded model) takes 90-140s to compile on throttled CPUs. + // + // Historically this AWAITED `embeddingManager.init()` INLINE, so every + // writer's open() blocked on the compile — N concurrent opens all + // queued on the ONE process-global singleton (an ~80x contention + // multiplier measured in a production restart storm: 90,017ms busy vs + // 1,117ms quiet). The engine only needs to be ready before the FIRST + // REAL embed() call, not before init() returns, so this now only + // STARTS the warm and moves on — init() never waits for it. + // + // No double-await needed for correctness: `this.embed()` (~line 15420) + // delegates to `this.embedder`, which for the default engine is + // `embeddingManager.getEmbeddingFunction()` → `embeddingManager.embed()` + // (src/embeddings/EmbeddingManager.ts). That method calls `await + // this.init()` FIRST, and `init()` itself serializes every concurrent + // caller onto ONE shared `globalInitPromise` — so the first real + // embed() automatically waits for whichever finishes first: this + // background warm (if still running) or a fresh init() (if the warm + // hasn't reached this code yet, e.g. `eagerEmbeddings: false`). + // Verified by reading both call sites; `_embeddingWarmPromise` below + // is stored for observability only, never re-awaited by embed(). // // Skipped automatically when: // - a native 'embeddings' provider is registered (it owns embeddings; @@ -1628,8 +1699,8 @@ export class Brainy implements BrainyInterface { // - reader-mode (readers don't embed — they query existing vectors), // - unit-test mode (tests must stay fast and use the mock embedder). // - // `eagerEmbeddings: false` is the explicit override to force lazy init - // (first-embed) even when this instance is the active embedder. + // `eagerEmbeddings: false` keeps meaning "no warm at all" — fully lazy, + // the first embed() call pays the full cost inline, same as before. const isUnitTestMode = isDeterministicEmbedMode() const eager = this.config.eagerEmbeddings ?? true if ( @@ -1638,9 +1709,45 @@ export class Brainy implements BrainyInterface { this.config.mode !== 'reader' && !isUnitTestMode ) { - console.log('Eager embedding initialization enabled...') - await embeddingManager.init() - console.log('Embedding engine ready') + const warmStart = Date.now() + console.log('Background embedding-engine warm started (init() does not wait for it)...') + this._embeddingWarmPromise = embeddingManager + .init() + .then(() => { + prodLog.info( + `[Brainy] background embedding-engine warm complete in ${Date.now() - warmStart}ms` + ) + }) + .catch((err) => { + // Loud, never silent: a warm that fails to compile must be + // heard NOW, not discovered as a mystery latency spike on + // whichever request happens to trigger the first real embed(). + // That first embed() call still retries init() itself (the + // singleton promise contract above) and surfaces its own typed + // error to its caller — this is the immediate, background echo. + prodLog.warn( + `[Brainy] background embedding-engine warm FAILED: ` + + `${(err as Error).message} — the first embed() call will retry ` + + `initialization and surface the error there` + ) + }) + } + + // PHASE 5 of 5 — "embedding-warm-started": just the synchronous cost + // of kicking off the background warm above (the warm's own compile + // time is NOT included — that's the whole point of backgrounding it). + markPhase('embedding-warm-started') + { + const totalOpenMs = Date.now() - initStart + if (totalOpenMs > 2000) { + const phaseList = Object.entries(phaseTimingsMs) + .map(([name, ms]) => `${name}=${ms}ms`) + .join(', ') + prodLog.warn( + `[Brainy] slow open: ${totalOpenMs}ms total (${phaseList}) — see the ` + + `phase breakdown above to find which one to investigate first` + ) + } } // Integration Hub initialization @@ -15695,6 +15802,25 @@ export class Brainy implements BrainyInterface { return embeddingManager.isInitialized() } + /** + * Whether the process-global WASM embedding engine (all-MiniLM-L6-v2, + * fixed 384-dim output, ≈93MB with the bundled model, 90-140s cold compile + * on throttled CPUs) is this instance's active embedder — `false` when a + * plugin has replaced it via the `'embeddings'` provider key. A native + * provider has no such cold-start cost and may use a different output + * dimension, so it is never worth avoiding. + * + * Used by init-path bootstrap writes (the VFS root — see + * `VirtualFileSystem.doInitializeRoot()`) to decide whether embedding a + * value during `init()` risks paying the WASM engine's cold compile. + * + * @returns true when the default WASM engine is active (no native + * `'embeddings'` provider registered). + */ + usesDefaultWasmEmbedder(): boolean { + return !this.pluginRegistry.hasProvider('embeddings') + } + /** * Setup embedder */ diff --git a/src/storage/adapters/fileSystemStorage.ts b/src/storage/adapters/fileSystemStorage.ts index 6d2d9b3c..965e97b9 100644 --- a/src/storage/adapters/fileSystemStorage.ts +++ b/src/storage/adapters/fileSystemStorage.ts @@ -239,38 +239,54 @@ export class FileSystemStorage extends BaseStorage { // Finish any restore interrupted by a crash (resume the staged swap, or // discard an uncommitted staging area) BEFORE counts/derived state load, - // so the rest of startup sees the completed store. + // so the rest of startup sees the completed store. ORDER-DEPENDENT: + // `swapStagedRestoreIn()` reads `fs.readdir(rootDir)` and then + // removes/renames rootDir's own TOP-LEVEL entries to place the staged + // copy — racing that against the directory-creation batch below (which + // also touches rootDir's children) could see a half-created directory + // mid-swap or a mkdir racing a concurrent rm/rename on the same path. + // Stays strictly sequential, never folded into the OPEN-PATH batch. await this.completeInterruptedRestore() - // Create the nouns directory if it doesn't exist - await this.ensureDirectoryExists(this.nounsDir) + // OPEN-PATH FIX: the remaining bootstrap directories are mutually + // independent — each is its own subtree under rootDir, and + // `fs.mkdir(dir, { recursive: true })` creates every intermediate + // segment of ITS OWN path in one call, so it never depends on any + // sibling here existing first. Nothing between here and + // `initializeCounts()` reads any of them, so batching collapses what + // was up to 8 sequential mkdir round-trips (each a real syscall+await) + // into one wave — this is what serialized an N-writer restart storm on + // filesystem I/O it never structurally needed. `initializeCounts()` + // right after DOES depend on `systemDir` (which the batch creates), so + // it stays outside, awaited only once every directory has landed. + await Promise.all([ + // Create the nouns directory if it doesn't exist + this.ensureDirectoryExists(this.nounsDir), + // Create the verbs directory if it doesn't exist + this.ensureDirectoryExists(this.verbsDir), + // Create the metadata directory if it doesn't exist + this.ensureDirectoryExists(this.metadataDir), + // Create the noun metadata directory if it doesn't exist + this.ensureDirectoryExists(this.nounMetadataDir), + // Create the verb metadata directory if it doesn't exist + this.ensureDirectoryExists(this.verbMetadataDir), + // Create both directories for backward compatibility + this.ensureDirectoryExists(this.systemDir), + // Only create legacy directory if it exists (don't create new legacy + // dirs) — a read-then-maybe-write, but on its own subtree, so it's + // still independent of every other entry in this batch. + (async () => { + if (await this.directoryExists(this.indexDir)) { + await this.ensureDirectoryExists(this.indexDir) + } + })(), + // Create the locks directory if it doesn't exist + this.ensureDirectoryExists(this.lockDir), + // Create the binary blobs directory if it doesn't exist + this.ensureDirectoryExists(this.blobsDir) + ]) - // Create the verbs directory if it doesn't exist - await this.ensureDirectoryExists(this.verbsDir) - - // Create the metadata directory if it doesn't exist - await this.ensureDirectoryExists(this.metadataDir) - - // Create the noun metadata directory if it doesn't exist - await this.ensureDirectoryExists(this.nounMetadataDir) - - // Create the verb metadata directory if it doesn't exist - await this.ensureDirectoryExists(this.verbMetadataDir) - - // Create both directories for backward compatibility - await this.ensureDirectoryExists(this.systemDir) - // Only create legacy directory if it exists (don't create new legacy dirs) - if (await this.directoryExists(this.indexDir)) { - await this.ensureDirectoryExists(this.indexDir) - } - - // Create the locks directory if it doesn't exist - await this.ensureDirectoryExists(this.lockDir) - - // Create the binary blobs directory if it doesn't exist - await this.ensureDirectoryExists(this.blobsDir) - - // Initialize count management + // Initialize count management — depends on systemDir, created above. this.countsFilePath = path.join(this.systemDir, 'counts.json') await this.initializeCounts() diff --git a/src/types/brainy.types.ts b/src/types/brainy.types.ts index 38bc4aa6..63356828 100644 --- a/src/types/brainy.types.ts +++ b/src/types/brainy.types.ts @@ -1983,25 +1983,32 @@ export interface BrainyConfig { reservedQueryMemory?: number // Memory reserved for queries in bytes (e.g., 1073741824 = 1GB) /** - * Controls when the WASM embedding engine is initialized. + * Controls whether `init()` starts a BACKGROUND warm of the WASM embedding + * engine. * - * **Adaptive default (8.0):** when omitted, the engine eagerly initializes - * during `init()` whenever the WASM embedder is the *active* one — i.e. no - * native `'embeddings'` provider is registered — and this instance is a - * writer (not `mode: 'reader'`) running outside unit tests. The WASM module - * (≈93MB with the embedded model) takes 90-140s to compile on throttled - * CPUs, so paying that during boot rather than on the first `embed()`-driven - * call is the right default for a single-process server. + * **Adaptive default (8.0, background since the open-path fix):** when + * omitted, `init()` STARTS a background warm of the engine whenever the + * WASM embedder is the *active* one — i.e. no native `'embeddings'` + * provider is registered — and this instance is a writer (not + * `mode: 'reader'`) running outside unit tests. The WASM module (≈93MB with + * the embedded model) takes 90-140s to compile on throttled CPUs — but + * `init()` never awaits that compile. It only starts it, so N concurrent + * opens no longer serialize on the one process-global engine singleton. + * The first `embed()` call then waits for whichever finishes first: the + * background warm (if still running) or its own fresh init (if the warm + * never started, e.g. `eagerEmbeddings: false`) — both paths converge on + * the SAME shared promise inside the engine singleton, so the vector is + * always correct; only the timing of who pays the wait differs. * * The adaptive path skips itself automatically when a native embeddings * provider owns embeddings, in reader-mode (readers query existing vectors * and never embed), and in unit-test mode (kept fast via the mock embedder). * - * - `true` — force eager init during `init()` (the adaptive default already - * does this for the active-embedder writer case; set it explicitly to be - * unambiguous). - * - `false` — explicit override to force lazy init (first `embed()` call) - * even when this instance is the active embedder. + * - `true` — force the background warm to start during `init()` (the + * adaptive default already does this for the active-embedder writer + * case; set it explicitly to be unambiguous). + * - `false` — no warm at all. Fully lazy: the first `embed()` call pays the + * full cold-compile cost inline, on whichever request triggers it. */ eagerEmbeddings?: boolean diff --git a/src/vfs/VirtualFileSystem.ts b/src/vfs/VirtualFileSystem.ts index ed272109..0a000396 100644 --- a/src/vfs/VirtualFileSystem.ts +++ b/src/vfs/VirtualFileSystem.ts @@ -89,6 +89,16 @@ export class VirtualFileSystem implements IVirtualFileSystem { // Uses deterministic UUID format for storage compatibility private static readonly VFS_ROOT_ID = '00000000-0000-0000-0000-000000000000' + // OPEN-PATH FIX: the dimension of the placeholder vector given to the VFS + // root when it is first created (see `doInitializeRoot`). Mirrors the + // built-in WASM embedding engine's fixed, hardcoded output size + // (all-MiniLM-L6-v2 — see src/embeddings/candle-wasm/src/lib.rs + // HIDDEN_SIZE and src/embeddings/EmbeddingManager.ts). Deliberately NOT + // derived from `brain.dimensions` — this constant only applies to the + // default-embedder branch, where the true output dimension is this fixed + // value by construction, never a moving target. + private static readonly VFS_ROOT_VECTOR_DIMENSIONS = 384 + /** * Construct a VFS bound to a Brainy instance. * @@ -260,6 +270,40 @@ export class VirtualFileSystem implements IVirtualFileSystem { try { console.log('VFS: Creating root directory (fixed ID: 00000000-0000-0000-0000-000000000000)') + // OPEN-PATH FIX: the VFS root is Brainy's own system-tier plumbing — it + // is hidden from find()/getNounCount()/stats() by default and nothing + // ever runs a semantic search against it — so it needs no REAL + // embedding. Historically this add() always called embed('/'), which + // meant every writer's FIRST-EVER open forced the process-global WASM + // engine to cold-compile its model (measured 90-140s on throttled + // CPUs) before the brain could even finish init(). This branch only + // runs once per store (the root already exists — with a real vector — + // in every previously-opened production store; reopening never re-adds + // it), so the fix applies only to brand-new stores. + // + // Chosen fix: an explicit all-zero vector, not `deferEmbedding: true`. + // `deferEmbedding` looked attractive (ack now, embed later) but its + // landing path (`kickEmbedWorker()`, called synchronously right after + // commit — see brainy.ts add()/update()) would still force the WASM + // engine to cold-compile within milliseconds of open, just off the + // awaited path instead of never paying it at all — worse than the + // explicit-vector path, which never touches the engine for this row. + // An all-zero vector is safe: `cosineDistance` (src/utils/distance.ts) + // explicitly returns the MAXIMUM distance whenever either operand's + // norm is zero, so the root can never rank ahead of real content in a + // similarity search, and HNSW indexes it like any other vector. + // + // Only the default WASM engine gets this treatment — its output + // dimension (384) is fixed and hardcoded, so the placeholder can never + // mis-pin `brain.dimensions` for it. A plugin-registered native + // 'embeddings' provider has no cold-compile cost AND may use a + // different dimension, so it keeps embedding the root for real (same + // as before this fix) rather than risk pinning the wrong dimension + // ahead of the caller's own first real embed. + const rootVector = this.brain.usesDefaultWasmEmbedder() + ? new Array(VirtualFileSystem.VFS_ROOT_VECTOR_DIMENSIONS).fill(0) + : undefined + await this.brain.add({ id: rootId, // Fixed ID - storage ensures uniqueness data: '/', @@ -271,7 +315,8 @@ export class VirtualFileSystem implements IVirtualFileSystem { // public AddParams.visibility union ('public' | 'internal') — this is the single // sanctioned internal setter, hence the cast. visibility: 'system' as 'public' | 'internal', - metadata: this.getRootMetadata() + metadata: this.getRootMetadata(), + ...(rootVector ? { vector: rootVector } : {}) }) return rootId diff --git a/tests/unit/brainy/open-path.test.ts b/tests/unit/brainy/open-path.test.ts new file mode 100644 index 00000000..3556d7a5 --- /dev/null +++ b/tests/unit/brainy/open-path.test.ts @@ -0,0 +1,199 @@ +/** + * OPEN-PATH tests: init() must never gate on the embedding model, the VFS + * root bootstrap must never touch the embedding engine, and a slow open + * must narrate its phases. + * + * Background: a production restart storm measured 90,017ms for a single + * brain init vs 1,117ms quiet — an ~80x contention multiplier — traced to + * every writer's init() eagerly awaiting the process-global WASM embedding + * engine before the VFS root even existed. See src/brainy.ts performInit() + * and src/vfs/VirtualFileSystem.ts doInitializeRoot(). + */ + +import { describe, it, expect, vi } from 'vitest' +import { Brainy } from '../../../src/brainy' +import { MemoryStorage } from '../../../src/storage/adapters/memoryStorage' +import { embeddingManager } from '../../../src/embeddings/EmbeddingManager' +import { createTestConfig } from '../../helpers/test-factory' + +/** + * The four signals `isDeterministicEmbedMode()` checks (see + * src/embeddings/deterministicEmbedMode.ts). The global unit-test setup + * (tests/setup-unit.ts) sets some of these for the whole file/run in some + * vitest configurations; other configurations leave them unset and run the + * real WASM engine instead. The background-warm tests below need the + * "not unit-test mode" branch of performInit() to actually execute, so they + * save/clear/restore all four explicitly — deterministic regardless of + * which config invoked this file, never relying on ambient state. + */ +function withRealEmbedderBranch(fn: () => Promise): Promise { + const savedEnvDeterministic = process.env.BRAINY_DETERMINISTIC_EMBEDDINGS + const savedEnvUnitTest = process.env.BRAINY_UNIT_TEST + const g = globalThis as Record + const savedGlobalDeterministic = g.__BRAINY_DETERMINISTIC_EMBED__ + const savedGlobalUnitTest = g.__BRAINY_UNIT_TEST__ + + delete process.env.BRAINY_DETERMINISTIC_EMBEDDINGS + delete process.env.BRAINY_UNIT_TEST + delete g.__BRAINY_DETERMINISTIC_EMBED__ + delete g.__BRAINY_UNIT_TEST__ + + const restore = () => { + if (savedEnvDeterministic !== undefined) process.env.BRAINY_DETERMINISTIC_EMBEDDINGS = savedEnvDeterministic + if (savedEnvUnitTest !== undefined) process.env.BRAINY_UNIT_TEST = savedEnvUnitTest + if (savedGlobalDeterministic !== undefined) g.__BRAINY_DETERMINISTIC_EMBED__ = savedGlobalDeterministic + if (savedGlobalUnitTest !== undefined) g.__BRAINY_UNIT_TEST__ = savedGlobalUnitTest + } + + return fn().finally(restore) +} + +/** + * A MemoryStorage whose init() takes an artificially long time — a + * controllable fake seam (not a wall-clock race) that reliably pushes + * performInit()'s "storage-init" phase (and therefore the total open time) + * past the 2000ms narration threshold, without touching the filesystem or + * relying on real contention. + */ +class SlowMemoryStorage extends MemoryStorage { + override async init(): Promise { + await new Promise((resolve) => setTimeout(resolve, 2200)) + await super.init() + } +} + +describe('OPEN-PATH: init() never gates on the embedding model', () => { + it('bootstrapping a fresh store never calls the embedding engine (VFS root add is engine-untouched)', async () => { + const embedSpy = vi.spyOn(embeddingManager, 'embed') + const brain = new Brainy(createTestConfig()) + try { + await brain.init() + + // The VFS root's add() must never have reached the embedding engine — + // it carries an explicit placeholder vector instead (see + // VirtualFileSystem.doInitializeRoot()). + expect(embedSpy).not.toHaveBeenCalled() + + // Sanity: the VFS is genuinely usable afterwards. + const files = await brain.vfs.readdir('/') + expect(files).toEqual([]) + } finally { + await brain.close() + embedSpy.mockRestore() + } + }) + + it('starts the embedding-engine warm in the BACKGROUND — init() resolves before the warm does', async () => { + await withRealEmbedderBranch(async () => { + const events: string[] = [] + let releaseWarm!: () => void + const warmGate = new Promise((resolve) => { + releaseWarm = resolve + }) + + const initSpy = vi.spyOn(embeddingManager, 'init').mockImplementation(async () => { + events.push('warm-start') + await warmGate + events.push('warm-resolve') + }) + + const brain = new Brainy(createTestConfig()) + try { + await brain.init() + events.push('init-resolved') + + // init() started the warm but returned WITHOUT waiting for it. + expect(initSpy).toHaveBeenCalledTimes(1) + expect(events).toEqual(['warm-start', 'init-resolved']) + + // Now let the fake warm finish and confirm it lands strictly after. + releaseWarm() + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(events).toEqual(['warm-start', 'init-resolved', 'warm-resolve']) + } finally { + await brain.close() + initSpy.mockRestore() + } + }) + }) + + it('narrates a background warm FAILURE loudly instead of losing it silently', async () => { + await withRealEmbedderBranch(async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const initSpy = vi + .spyOn(embeddingManager, 'init') + .mockRejectedValue(new Error('simulated cold-compile failure')) + + const brain = new Brainy(createTestConfig()) + try { + // init() itself must still resolve — a failed background warm is + // never fatal to open(). + await expect(brain.init()).resolves.toBeUndefined() + + // Give the background .catch() a microtask/macrotask to run. + await new Promise((resolve) => setTimeout(resolve, 0)) + + const failureLine = warnSpy.mock.calls + .map((args) => args.map(String).join(' ')) + .find((line) => line.includes('background embedding-engine warm FAILED')) + expect(failureLine).toBeDefined() + expect(failureLine).toContain('simulated cold-compile failure') + } finally { + await brain.close() + initSpy.mockRestore() + warnSpy.mockRestore() + } + }) + }) + + it('eagerEmbeddings: false starts no warm at all', async () => { + await withRealEmbedderBranch(async () => { + const initSpy = vi.spyOn(embeddingManager, 'init') + const brain = new Brainy({ ...createTestConfig(), eagerEmbeddings: false }) + try { + await brain.init() + expect(initSpy).not.toHaveBeenCalled() + } finally { + await brain.close() + initSpy.mockRestore() + } + }) + }) + + it('narrates a slow open with a per-phase ms breakdown once total time exceeds 2000ms', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const brain = new Brainy({ ...createTestConfig(), storage: new SlowMemoryStorage() }) + try { + await brain.init() + + const slowOpenLine = warnSpy.mock.calls + .map((args) => args.map(String).join(' ')) + .find((line) => line.includes('[Brainy] slow open:')) + + expect(slowOpenLine).toBeDefined() + expect(slowOpenLine).toContain('storage-init=') + expect(slowOpenLine).toContain('generation-store-open-fold=') + expect(slowOpenLine).toContain('index-init-gate=') + expect(slowOpenLine).toContain('vfs-bootstrap=') + expect(slowOpenLine).toContain('embedding-warm-started=') + } finally { + await brain.close() + warnSpy.mockRestore() + } + }, 20000) + + it('stays silent about phase timing when open is fast (under 2000ms)', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const brain = new Brainy(createTestConfig()) + try { + await brain.init() + const slowOpenLine = warnSpy.mock.calls + .map((args) => args.map(String).join(' ')) + .find((line) => line.includes('[Brainy] slow open:')) + expect(slowOpenLine).toBeUndefined() + } finally { + await brain.close() + warnSpy.mockRestore() + } + }) +}) From fc516da6eb38636a071ade83b5db459f5e0e2faa Mon Sep 17 00:00:00 2001 From: David Snelling Date: Tue, 25 Aug 2026 10:10:01 -0700 Subject: [PATCH 2/3] =?UTF-8?q?feat(vfs):=20implement=20readdir's=20recurs?= =?UTF-8?q?ive=20option=20=E2=80=94=20typed=20since=207.30,=20never=20read?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit vfs.readdir()'s ReaddirOptions.recursive was typed but silently ignored: a recursive request behaved identically to a non-recursive one. Implemented properly: recursive listing walks every descendant (files and directories, any depth) via the same graph-traversal + one-batch-fetch path getTreeStructure()/getDescendants() already use, and reports each entry as a path relative to the queried directory (Node's fs.readdir(dir, { recursive: true }) convention) — 'sub/file.txt', not just 'file.txt'. With withFileTypes: true, each VFSDirent.name carries that same relative path when recursive (matching the string-array form byte for byte); VFSDirent.path stays the absolute VFS path either way, so no information is lost. Filter/sort/pagination compose unchanged, now over the full recursive set. Non-recursive behavior (direct children, named by basename) is unchanged. --- src/vfs/VirtualFileSystem.ts | 39 ++++++++-- src/vfs/types.ts | 21 ++++- tests/unit/vfs-readdir-recursive.test.ts | 99 ++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 7 deletions(-) create mode 100644 tests/unit/vfs-readdir-recursive.test.ts diff --git a/src/vfs/VirtualFileSystem.ts b/src/vfs/VirtualFileSystem.ts index 0a000396..90018863 100644 --- a/src/vfs/VirtualFileSystem.ts +++ b/src/vfs/VirtualFileSystem.ts @@ -1274,7 +1274,20 @@ export class VirtualFileSystem implements IVirtualFileSystem { } /** - * Read directory contents + * @description List a directory's contents. Non-recursive (default) + * returns direct children only, named by basename. `recursive: true` + * lists every descendant at any depth (files and directories), each + * reported as a path RELATIVE TO THE QUERIED DIRECTORY — matching Node's + * `fs.readdir(dir, { recursive: true })` convention — e.g. `'sub'` and + * `'sub/file.txt'` for a nested file. With `withFileTypes: true`, each + * {@link VFSDirent}'s `name` carries that same value (relative when + * recursive, basename otherwise); `path` is always the absolute VFS path + * either way. + * @param path - The directory to list. + * @param options - `recursive`, `withFileTypes`, `filter`, `sort`, + * `offset`/`limit` (pagination applies AFTER filter/sort, over the full + * recursive set when `recursive: true`). + * @throws {VFSError} ENOTDIR when `path` is not a directory. */ async readdir(path: string, options?: ReaddirOptions): Promise { await this.ensureInitialized() @@ -1287,8 +1300,12 @@ export class VirtualFileSystem implements IVirtualFileSystem { throw new VFSError(VFSErrorCode.ENOTDIR, `Not a directory: ${path}`, path, 'readdir') } - // Get children - let children = await this.pathResolver.getChildren(entityId) + // Direct children, or every descendant at any depth. gatherDescendants() + // is the same graph-traversal + ONE-batch-fetch path getTreeStructure()/ + // getDescendants() already use — no per-directory storage round trips. + let children = options?.recursive + ? await this.gatherDescendants(entityId, Infinity) + : await this.pathResolver.getChildren(entityId) // Apply filters if (options?.filter) { @@ -1312,17 +1329,29 @@ export class VirtualFileSystem implements IVirtualFileSystem { // Directory access time updates caused 50-100ms GCS write on EVERY readdir // await this.updateAccessTime(entityId) // ← REMOVED + // The queried directory's own canonical (already-normalized) path — the + // base every recursive entry's relative name is computed against. Using + // the resolved entity's OWN path (rather than the raw `path` argument) + // means no separate normalization step is needed here. + const baseDir = entity.metadata.path + const relativeToBase = (childPath: string): string => { + const prefix = baseDir === '/' ? '/' : `${baseDir}/` + return childPath.startsWith(prefix) ? childPath.slice(prefix.length) : childPath + } + // Return appropriate format if (options?.withFileTypes) { return children.map(child => ({ - name: child.metadata.name, + name: options?.recursive ? relativeToBase(child.metadata.path) : child.metadata.name, path: child.metadata.path, type: child.metadata.vfsType, entityId: child.id } as VFSDirent)) } - return children.map(child => child.metadata.name) + return children.map(child => + options?.recursive ? relativeToBase(child.metadata.path) : child.metadata.name + ) } // ============= Metadata Operations ============= diff --git a/src/vfs/types.ts b/src/vfs/types.ts index 9188476b..17687dd3 100644 --- a/src/vfs/types.ts +++ b/src/vfs/types.ts @@ -133,8 +133,17 @@ export interface VFSStats { * Directory entry (for readdir) */ export interface VFSDirent { + /** + * The entry's basename (e.g. `'file.txt'`) when `readdir()` was called + * WITHOUT `recursive: true`. When `recursive: true` was set, this is + * instead the entry's path RELATIVE TO THE QUERIED DIRECTORY (e.g. + * `'sub/file.txt'` for a nested file) — the same value that would appear + * in the plain string-array form of a recursive `readdir()` call. `path` + * below always carries the absolute VFS path regardless of `recursive`, + * so nothing is lost either way. + */ name: string - path: string // Full path + path: string // Full (absolute) VFS path — always absolute, recursive or not type: 'file' | 'directory' | 'symlink' entityId: string // Underlying entity ID } @@ -240,7 +249,15 @@ export interface ReaddirOptions { withFileTypes?: boolean // Return Dirent objects // VFS-specific options - recursive?: boolean // Include subdirectories + /** + * List every descendant (files and directories, all depths), not just + * direct children. Entries are reported as paths RELATIVE TO THE QUERIED + * DIRECTORY (Node's `fs.readdir(dir, { recursive: true })` convention) — + * a string-array result contains e.g. `'sub/file.txt'`, and with + * `withFileTypes: true` each `VFSDirent.name` carries that same relative + * path (see {@link VFSDirent}). Default: `false` (direct children only). + */ + recursive?: boolean limit?: number // Max results offset?: number // Skip N results cursor?: string // Pagination cursor diff --git a/tests/unit/vfs-readdir-recursive.test.ts b/tests/unit/vfs-readdir-recursive.test.ts new file mode 100644 index 00000000..2ee8a775 --- /dev/null +++ b/tests/unit/vfs-readdir-recursive.test.ts @@ -0,0 +1,99 @@ +/** + * vfs.readdir()'s `recursive` option: typed since 7.30 but never read, so it + * silently behaved exactly like `recursive: false`. This pins the real, + * documented contract: a recursive listing returns every descendant (files + * AND directories, all depths) as paths RELATIVE TO THE QUERIED DIRECTORY — + * the same convention Node's `fs.readdir(dir, { recursive: true })` uses — + * for both the plain string-array form and the `withFileTypes` VFSDirent + * form (whose `name` carries that same relative path when recursive). + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import { Brainy } from '../../src/brainy.js' +import type { VFSDirent } from '../../src/vfs/types.js' + +describe('vfs.readdir() recursive option', () => { + let brain: Brainy + + beforeEach(async () => { + brain = new Brainy({ + requireSubtype: false, + storage: { type: 'memory' }, + silent: true + }) + await brain.init() + + // Build: + // /a/b.txt + // /a/sub/c.txt + // /a/sub/deeper/d.txt + // /a/sub2/ (empty directory) + await brain.vfs.writeFile('/a/b.txt', 'B') + await brain.vfs.writeFile('/a/sub/c.txt', 'C') + await brain.vfs.writeFile('/a/sub/deeper/d.txt', 'D') + await brain.vfs.mkdir('/a/sub2', { recursive: true }) + }) + + afterEach(async () => { + await brain.close() + }) + + it('non-recursive (default) still returns only direct children, by basename', async () => { + const entries = await brain.vfs.readdir('/a') as string[] + expect([...entries].sort()).toEqual(['b.txt', 'sub', 'sub2']) + }) + + it('recursive: true returns every descendant as a path relative to the queried directory', async () => { + const entries = await brain.vfs.readdir('/a', { recursive: true }) as string[] + expect([...entries].sort()).toEqual([ + 'b.txt', + 'sub', + 'sub/c.txt', + 'sub/deeper', + 'sub/deeper/d.txt', + 'sub2' + ]) + }) + + it('recursive: true at the root has no leading slash on relative entries', async () => { + const entries = await brain.vfs.readdir('/', { recursive: true }) as string[] + expect(entries).toContain('a') + expect(entries).toContain('a/b.txt') + expect(entries).toContain('a/sub/deeper/d.txt') + for (const entry of entries) { + expect(entry.startsWith('/')).toBe(false) + } + }) + + it('recursive + withFileTypes: VFSDirent.name is the relative path, .path stays absolute', async () => { + const entries = await brain.vfs.readdir('/a', { + recursive: true, + withFileTypes: true + }) as VFSDirent[] + + const byName = new Map(entries.map((e) => [e.name, e])) + + const nested = byName.get('sub/deeper/d.txt') + expect(nested).toBeDefined() + expect(nested!.path).toBe('/a/sub/deeper/d.txt') + expect(nested!.type).toBe('file') + + const nestedDir = byName.get('sub/deeper') + expect(nestedDir).toBeDefined() + expect(nestedDir!.path).toBe('/a/sub/deeper') + expect(nestedDir!.type).toBe('directory') + + // Non-recursive VFSDirent behavior is unchanged: name is the basename. + const direct = await brain.vfs.readdir('/a', { withFileTypes: true }) as VFSDirent[] + const directEntry = direct.find((e) => e.path === '/a/b.txt') + expect(directEntry?.name).toBe('b.txt') + }) + + it('recursive + filter composes: only files survive a type filter', async () => { + const entries = await brain.vfs.readdir('/a', { + recursive: true, + filter: { type: 'file' } + }) as string[] + expect([...entries].sort()).toEqual(['b.txt', 'sub/c.txt', 'sub/deeper/d.txt']) + }) +}) From 258e9042afb8dbc1e8ed2f91bf532d0692b0a995 Mon Sep 17 00:00:00 2001 From: David Snelling Date: Tue, 25 Aug 2026 10:10:19 -0700 Subject: [PATCH 3/3] fix(add): empty string is real data, not a missing field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validateAddParams() treated '' as falsy and rejected it with "Missing required field 'data'" — so a legitimate empty file's first write always failed. Only null/undefined data (with no vector either) is genuinely absent; '' is real content. Fixed the check, plus the identical bug in validateUpdateParams() (truncating a file to empty via overwrite hit the same falsy check) and in update()/transact()'s update planner, where a plain `Boolean(params.data)`/truthy check on the resolved vector would have silently skipped both the deferred-embed marker and the eager re-embed for an emptied value — a stale vector with no path to ever correct itself. Verified end-to-end: vfs.writeFile('/empty.txt', '') now succeeds, readFile() returns '', the file lists, and stat() reports size 0; the existing "should reject empty string as data" tests (unit + integration) asserted the old buggy behavior and are updated to assert the fixed contract instead. --- src/brainy.ts | 24 +++++++--- src/utils/paramValidation.ts | 19 +++++--- .../brainy-core.integration.test.ts | 12 +++-- tests/unit/brainy/add.test.ts | 19 +++++--- tests/unit/utils/paramValidation.test.ts | 45 ++++++++++++++++++- tests/vfs/vfs.unit.test.ts | 29 ++++++++++++ 6 files changed, 128 insertions(+), 20 deletions(-) diff --git a/src/brainy.ts b/src/brainy.ts index 9cd7dcb4..3791f768 100644 --- a/src/brainy.ts +++ b/src/brainy.ts @@ -3562,11 +3562,20 @@ export class Brainy implements BrainyInterface { // new `data`); otherwise new `data` re-embeds; otherwise the existing // vector is kept. Any vector change re-indexes HNSW below. let vector = existing.vector + // 'data' is a real new value whenever it's not null/undefined — an + // empty string ('') is legitimate content (e.g. truncating a file to + // empty via overwrite), matching validateUpdateParams's absent-vs-empty + // distinction. Using `Boolean(params.data)` here would treat '' as "no + // new data", silently skipping BOTH the deferred marker and the eager + // re-embed below — a stale vector left behind with no path to ever + // correct itself (a quiet loss, not the deferred-but-eventually- + // correct flicker the deferEmbedding contract promises). + const hasNewData = params.data !== undefined && params.data !== null // MT5 deferred re-embedding: the OLD vector keeps serving semantic // search — stale-but-present, never absent (the flicker law) — until // the background worker embeds the new data and swaps it atomically. const deferringEmbed = - params.deferEmbedding === true && Boolean(params.data) && !params.vector + params.deferEmbedding === true && hasNewData && !params.vector if (params.vector) { if (this.dimensions && params.vector.length !== this.dimensions) { throw new Error( @@ -3574,13 +3583,13 @@ export class Brainy implements BrainyInterface { ) } vector = params.vector - } else if (params.data && !deferringEmbed) { + } else if (hasNewData && !deferringEmbed) { vector = await this.embed(params.data) } // A deferred data change does NOT reindex now (the vector is unchanged; // the worker's atomic swap carries the real reindex later). const needsReindexing = Boolean( - (params.data && !deferringEmbed) || params.type || params.vector + (hasNewData && !deferringEmbed) || params.type || params.vector ) // Always update the noun with new metadata @@ -10501,6 +10510,11 @@ export class Brainy implements BrainyInterface { // Resolve the updated vector — mirror of update(): an explicit `vector` // always wins, new `data` re-embeds, otherwise the existing vector is // kept. Any vector change re-indexes HNSW below. + // 'data' is present whenever it's not null/undefined — '' is real + // content (see the identical hasNewData in update()); a plain truthy + // check would silently skip re-embedding an emptied value and leave a + // stale vector with no path to ever correct itself. + const hasNewData = params.data !== undefined && params.data !== null let vector = existing.vector if (params.vector) { if (this.dimensions && params.vector.length !== this.dimensions) { @@ -10509,10 +10523,10 @@ export class Brainy implements BrainyInterface { ) } vector = params.vector - } else if (params.data) { + } else if (hasNewData) { vector = await this.embed(params.data) } - const needsReindexing = Boolean(params.data || params.type || params.vector) + const needsReindexing = Boolean(hasNewData || params.type || params.vector) const newMetadata = params.merge !== false diff --git a/src/utils/paramValidation.ts b/src/utils/paramValidation.ts index b8036746..d43559dc 100644 --- a/src/utils/paramValidation.ts +++ b/src/utils/paramValidation.ts @@ -540,6 +540,11 @@ function rejectForgedSystemKeys(metadata: Record | undefined, s export function validateAddParams(params: AddParams): void { rejectForgedSystemKeys(params.metadata as Record | undefined, 'add()') + // 'data' is ABSENT only when null/undefined — an empty string ('') is real + // content (a legitimate empty file's first write) and must not be treated + // as missing. Falsy-but-present values (0, false, '') all count as present; + // only the true "nothing was given" case is absent. + const hasData = params.data !== undefined && params.data !== null // MT5 deferred embedding: an explicit vector has nothing to defer, and a // deferral without data has nothing to embed — both are caller bugs that // must refuse with the fix, never be silently reinterpreted. @@ -550,14 +555,14 @@ export function validateAddParams(params: AddParams): void { `the vector is already computed; drop one of the two.` ) } - if (!params.data) { + if (!hasData) { throw new Error( `add(): deferEmbedding requires 'data' (the content the background worker will embed).` ) } } // Universal truth: must have data or vector - if (!params.data && !params.vector) { + if (!hasData && !params.vector) { throw new Error( `Invalid add() parameters: Missing required field 'data'\n` + `\nReceived: ${JSON.stringify({ @@ -597,6 +602,10 @@ export function validateAddParams(params: AddParams): void { */ export function validateUpdateParams(params: UpdateParams): void { rejectForgedSystemKeys(params.metadata as Record | undefined, 'update()') + // Same absent-vs-empty distinction as validateAddParams: '' is a real new + // value (e.g. truncating a file to empty content via overwrite), only + // null/undefined means "no new data was given". + const hasData = params.data !== undefined && params.data !== null if ((params as UpdateParams & { deferEmbedding?: boolean }).deferEmbedding === true) { if (params.vector) { throw new Error( @@ -604,7 +613,7 @@ export function validateUpdateParams(params: UpdateParams): void { `the vector is already computed; drop one of the two.` ) } - if (!params.data) { + if (!hasData) { throw new Error( `update(): deferEmbedding requires new 'data' — without a data change there is nothing to re-embed.` ) @@ -614,10 +623,10 @@ export function validateUpdateParams(params: UpdateParams): void { if (!params.id) { throw new Error('id is required for update') } - + // Universal truth: must update something if ( - !params.data && + !hasData && !params.metadata && !params.type && !params.vector && diff --git a/tests/integration/brainy-core.integration.test.ts b/tests/integration/brainy-core.integration.test.ts index dd04703e..e5b01caf 100644 --- a/tests/integration/brainy-core.integration.test.ts +++ b/tests/integration/brainy-core.integration.test.ts @@ -337,12 +337,18 @@ describe('Brainy 3.0 Core (Integration Tests - Real AI)', () => { describe('Error Handling and Edge Cases', () => { it('should handle invalid inputs gracefully', async () => { - // Empty data is rejected with a clear validation error (8.0 requires a - // non-empty `data` or a `vector` — empty string carries no signal to embed). + // Empty string is REAL content (e.g. an empty file's first write), not + // a missing field — only null/undefined data (with no vector either) + // is rejected. See src/utils/paramValidation.ts validateAddParams(). await expect(brain.add({ data: '', type: 'document' - })).rejects.toThrow(/data/) + })).resolves.toBeDefined() + + // Missing BOTH data and vector is still the real "nothing to embed" error. + await expect(brain.add({ + type: 'document' + } as any)).rejects.toThrow(/data/) // Test with very long text — valid input, resolves to an id. const longText = 'Lorem ipsum '.repeat(10000) diff --git a/tests/unit/brainy/add.test.ts b/tests/unit/brainy/add.test.ts index 10690e2f..7203690c 100644 --- a/tests/unit/brainy/add.test.ts +++ b/tests/unit/brainy/add.test.ts @@ -335,15 +335,24 @@ describe('Brainy.add()', () => { }) describe('edge cases', () => { - it('should reject empty string as data', async () => { - // Arrange + it('should accept an empty string as real (empty) data', async () => { + // Arrange — '' is legitimate content (e.g. an empty file's first + // write), not a missing field. Only null/undefined data (with no + // vector either) is "missing" — see the separate + // 'data and vector are both missing' test above. const params = createAddParams({ data: '', type: 'thing' }) - - // Act & Assert - Empty string is not valid data - await expect(brain.add(params)).rejects.toThrow('Invalid add() parameters: Missing required field \'data\'') + + // Act + const id = await brain.add(params) + + // Assert — stored and readable back as empty, not rejected + expect(id).toBeDefined() + const entity = await brain.get(id) + expect(entity).not.toBeNull() + expect(entity!.data).toBe('') }) it('should handle very long text content', async () => { diff --git a/tests/unit/utils/paramValidation.test.ts b/tests/unit/utils/paramValidation.test.ts index 7e5212b8..805dd40d 100644 --- a/tests/unit/utils/paramValidation.test.ts +++ b/tests/unit/utils/paramValidation.test.ts @@ -149,7 +149,33 @@ describe('Zero-Config Parameter Validation', () => { type: NounType.Document } as AddParams)).toThrow('Invalid add() parameters: Missing required field \'data\'') }) - + + it('should accept an empty string as real data — only null/undefined is "missing"', () => { + // A legitimate empty file's first write: '' is content, not absence. + expect(() => validateAddParams({ + data: '', + type: NounType.Document + })).not.toThrow() + + // null/undefined (with no vector) is still the genuine missing-field case. + expect(() => validateAddParams({ + data: null as any, + type: NounType.Document + })).toThrow('Invalid add() parameters: Missing required field \'data\'') + expect(() => validateAddParams({ + data: undefined, + type: NounType.Document + })).toThrow('Invalid add() parameters: Missing required field \'data\'') + }) + + it('deferEmbedding accepts empty-string data (real content, not absence)', () => { + expect(() => validateAddParams({ + data: '', + type: NounType.Document, + deferEmbedding: true + } as AddParams)).not.toThrow() + }) + it('should validate NounType', () => { expect(() => validateAddParams({ data: 'test', @@ -190,7 +216,22 @@ describe('Zero-Config Parameter Validation', () => { id: 'test-id' })).toThrow('must specify at least one field to update') }) - + + it('empty-string data counts as a real field to update (truncating content)', () => { + expect(() => validateUpdateParams({ + id: 'test-id', + data: '' + })).not.toThrow() + }) + + it('deferEmbedding accepts empty-string data on update', () => { + expect(() => validateUpdateParams({ + id: 'test-id', + data: '', + deferEmbedding: true + } as UpdateParams)).not.toThrow() + }) + it('should validate NounType if changing', () => { expect(() => validateUpdateParams({ id: 'test-id', diff --git a/tests/vfs/vfs.unit.test.ts b/tests/vfs/vfs.unit.test.ts index 5ea79377..4b4ba8d2 100644 --- a/tests/vfs/vfs.unit.test.ts +++ b/tests/vfs/vfs.unit.test.ts @@ -53,6 +53,35 @@ describe('VirtualFileSystem - Production Tests', () => { expect(exists).toBe(true) }) + it('should write and read an empty (0-byte) file end-to-end', async () => { + // Pin: validateAddParams() used to treat '' as a missing 'data' field + // (falsy check), so a legitimate empty file's FIRST write threw + // "Missing required field 'data'". '' is real content, not an absent + // field — only null/undefined is absent. + const path = '/empty.txt' + + await vfs.writeFile(path, '') + + const result = await vfs.readFile(path) + expect(result.toString()).toBe('') + + const exists = await vfs.exists(path) + expect(exists).toBe(true) + + const stats = await vfs.stat(path) + expect(stats.size).toBe(0) + expect(stats.isFile()).toBe(true) + + // The file lists like any other. + const entries = await vfs.readdir('/') as string[] + expect(entries).toContain('empty.txt') + + // Overwriting it back to empty (truncate) must also succeed. + await vfs.writeFile(path, 'not empty anymore') + await vfs.writeFile(path, '') + expect((await vfs.readFile(path)).toString()).toBe('') + }) + it('should handle binary files', async () => { const binaryData = Buffer.from([0x00, 0x01, 0x02, 0xFF]) const path = '/binary.dat'