fix(add): empty string is real data, not a missing field

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.
This commit is contained in:
David Snelling 2026-08-25 10:10:19 -07:00
parent fc516da6eb
commit 258e9042af
6 changed files with 128 additions and 20 deletions

View file

@ -3562,11 +3562,20 @@ export class Brainy<T = any> implements BrainyInterface<T> {
// new `data`); otherwise new `data` re-embeds; otherwise the existing // new `data`); otherwise new `data` re-embeds; otherwise the existing
// vector is kept. Any vector change re-indexes HNSW below. // vector is kept. Any vector change re-indexes HNSW below.
let vector = existing.vector 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 // MT5 deferred re-embedding: the OLD vector keeps serving semantic
// search — stale-but-present, never absent (the flicker law) — until // search — stale-but-present, never absent (the flicker law) — until
// the background worker embeds the new data and swaps it atomically. // the background worker embeds the new data and swaps it atomically.
const deferringEmbed = const deferringEmbed =
params.deferEmbedding === true && Boolean(params.data) && !params.vector params.deferEmbedding === true && hasNewData && !params.vector
if (params.vector) { if (params.vector) {
if (this.dimensions && params.vector.length !== this.dimensions) { if (this.dimensions && params.vector.length !== this.dimensions) {
throw new Error( throw new Error(
@ -3574,13 +3583,13 @@ export class Brainy<T = any> implements BrainyInterface<T> {
) )
} }
vector = params.vector vector = params.vector
} else if (params.data && !deferringEmbed) { } else if (hasNewData && !deferringEmbed) {
vector = await this.embed(params.data) vector = await this.embed(params.data)
} }
// A deferred data change does NOT reindex now (the vector is unchanged; // A deferred data change does NOT reindex now (the vector is unchanged;
// the worker's atomic swap carries the real reindex later). // the worker's atomic swap carries the real reindex later).
const needsReindexing = Boolean( const needsReindexing = Boolean(
(params.data && !deferringEmbed) || params.type || params.vector (hasNewData && !deferringEmbed) || params.type || params.vector
) )
// Always update the noun with new metadata // Always update the noun with new metadata
@ -10501,6 +10510,11 @@ export class Brainy<T = any> implements BrainyInterface<T> {
// Resolve the updated vector — mirror of update(): an explicit `vector` // Resolve the updated vector — mirror of update(): an explicit `vector`
// always wins, new `data` re-embeds, otherwise the existing vector is // always wins, new `data` re-embeds, otherwise the existing vector is
// kept. Any vector change re-indexes HNSW below. // 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 let vector = existing.vector
if (params.vector) { if (params.vector) {
if (this.dimensions && params.vector.length !== this.dimensions) { if (this.dimensions && params.vector.length !== this.dimensions) {
@ -10509,10 +10523,10 @@ export class Brainy<T = any> implements BrainyInterface<T> {
) )
} }
vector = params.vector vector = params.vector
} else if (params.data) { } else if (hasNewData) {
vector = await this.embed(params.data) 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 = const newMetadata =
params.merge !== false params.merge !== false

View file

@ -540,6 +540,11 @@ function rejectForgedSystemKeys(metadata: Record<string, unknown> | undefined, s
export function validateAddParams(params: AddParams): void { export function validateAddParams(params: AddParams): void {
rejectForgedSystemKeys(params.metadata as Record<string, unknown> | undefined, 'add()') rejectForgedSystemKeys(params.metadata as Record<string, unknown> | 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 // MT5 deferred embedding: an explicit vector has nothing to defer, and a
// deferral without data has nothing to embed — both are caller bugs that // deferral without data has nothing to embed — both are caller bugs that
// must refuse with the fix, never be silently reinterpreted. // 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.` `the vector is already computed; drop one of the two.`
) )
} }
if (!params.data) { if (!hasData) {
throw new Error( throw new Error(
`add(): deferEmbedding requires 'data' (the content the background worker will embed).` `add(): deferEmbedding requires 'data' (the content the background worker will embed).`
) )
} }
} }
// Universal truth: must have data or vector // Universal truth: must have data or vector
if (!params.data && !params.vector) { if (!hasData && !params.vector) {
throw new Error( throw new Error(
`Invalid add() parameters: Missing required field 'data'\n` + `Invalid add() parameters: Missing required field 'data'\n` +
`\nReceived: ${JSON.stringify({ `\nReceived: ${JSON.stringify({
@ -597,6 +602,10 @@ export function validateAddParams(params: AddParams): void {
*/ */
export function validateUpdateParams(params: UpdateParams): void { export function validateUpdateParams(params: UpdateParams): void {
rejectForgedSystemKeys(params.metadata as Record<string, unknown> | undefined, 'update()') rejectForgedSystemKeys(params.metadata as Record<string, unknown> | 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 as UpdateParams & { deferEmbedding?: boolean }).deferEmbedding === true) {
if (params.vector) { if (params.vector) {
throw new Error( throw new Error(
@ -604,7 +613,7 @@ export function validateUpdateParams(params: UpdateParams): void {
`the vector is already computed; drop one of the two.` `the vector is already computed; drop one of the two.`
) )
} }
if (!params.data) { if (!hasData) {
throw new Error( throw new Error(
`update(): deferEmbedding requires new 'data' — without a data change there is nothing to re-embed.` `update(): deferEmbedding requires new 'data' — without a data change there is nothing to re-embed.`
) )
@ -617,7 +626,7 @@ export function validateUpdateParams(params: UpdateParams): void {
// Universal truth: must update something // Universal truth: must update something
if ( if (
!params.data && !hasData &&
!params.metadata && !params.metadata &&
!params.type && !params.type &&
!params.vector && !params.vector &&

View file

@ -337,12 +337,18 @@ describe('Brainy 3.0 Core (Integration Tests - Real AI)', () => {
describe('Error Handling and Edge Cases', () => { describe('Error Handling and Edge Cases', () => {
it('should handle invalid inputs gracefully', async () => { it('should handle invalid inputs gracefully', async () => {
// Empty data is rejected with a clear validation error (8.0 requires a // Empty string is REAL content (e.g. an empty file's first write), not
// non-empty `data` or a `vector` — empty string carries no signal to embed). // a missing field — only null/undefined data (with no vector either)
// is rejected. See src/utils/paramValidation.ts validateAddParams().
await expect(brain.add({ await expect(brain.add({
data: '', data: '',
type: 'document' 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. // Test with very long text — valid input, resolves to an id.
const longText = 'Lorem ipsum '.repeat(10000) const longText = 'Lorem ipsum '.repeat(10000)

View file

@ -335,15 +335,24 @@ describe('Brainy.add()', () => {
}) })
describe('edge cases', () => { describe('edge cases', () => {
it('should reject empty string as data', async () => { it('should accept an empty string as real (empty) data', async () => {
// Arrange // 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({ const params = createAddParams({
data: '', data: '',
type: 'thing' type: 'thing'
}) })
// Act & Assert - Empty string is not valid data // Act
await expect(brain.add(params)).rejects.toThrow('Invalid add() parameters: Missing required field \'data\'') 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 () => { it('should handle very long text content', async () => {

View file

@ -150,6 +150,32 @@ describe('Zero-Config Parameter Validation', () => {
} as AddParams)).toThrow('Invalid add() parameters: Missing required field \'data\'') } 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', () => { it('should validate NounType', () => {
expect(() => validateAddParams({ expect(() => validateAddParams({
data: 'test', data: 'test',
@ -191,6 +217,21 @@ describe('Zero-Config Parameter Validation', () => {
})).toThrow('must specify at least one field to update') })).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', () => { it('should validate NounType if changing', () => {
expect(() => validateUpdateParams({ expect(() => validateUpdateParams({
id: 'test-id', id: 'test-id',

View file

@ -53,6 +53,35 @@ describe('VirtualFileSystem - Production Tests', () => {
expect(exists).toBe(true) 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 () => { it('should handle binary files', async () => {
const binaryData = Buffer.from([0x00, 0x01, 0x02, 0xFF]) const binaryData = Buffer.from([0x00, 0x01, 0x02, 0xFF])
const path = '/binary.dat' const path = '/binary.dat'