From 258e9042afb8dbc1e8ed2f91bf532d0692b0a995 Mon Sep 17 00:00:00 2001 From: David Snelling Date: Tue, 25 Aug 2026 10:10:19 -0700 Subject: [PATCH] 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'