From 67e5fc8779cd82bd142f747f1152dbdfe867c849 Mon Sep 17 00:00:00 2001 From: David Snelling Date: Thu, 11 Jun 2026 10:35:08 -0700 Subject: [PATCH] fix: remap reserved fields from update() metadata patches to their canonical location MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit add({metadata: {confidence: 0.8}}) lifts reserved fields out of the metadata bag to their canonical top-level entity fields — teaching consumers that the metadata bag is a valid write path. update({metadata: {confidence: 0.33}}) then silently dropped the same shape: the patch value survived the metadata merge but was clobbered one expression later by the preserve-existing spread. No error, no warning, nothing written. A production consumer's confidence- evolution writes no-oped for weeks before being caught by reading values back. Fix: update() now normalizes the metadata patch before any enforcement or persistence logic runs, mirroring add()'s lift exactly: - confidence, weight, subtype — remapped to the top-level param unless the caller also passed that param explicitly (top-level wins). The remapped subtype flows through subtype-pairing enforcement like a top-level one. - noun, data, createdAt, updatedAt, service, createdBy, _rev — system-managed or owned by a dedicated param; dropped from the patch with a one-shot warning naming the correct write path (silent drop was the only wrong behavior here). Five regression tests pin the contract, including the production repro verbatim (add with metadata.confidence → top-level update → metadata-patch update → read-back) and the both-paths-supplied precedence case. 1475/1475 unit suite passing. --- src/brainy.ts | 71 ++++++++++- .../update-reserved-metadata-remap.test.ts | 117 ++++++++++++++++++ 2 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 tests/unit/brainy/update-reserved-metadata-remap.test.ts diff --git a/src/brainy.ts b/src/brainy.ts index 1c188c96..a8c6e8e4 100644 --- a/src/brainy.ts +++ b/src/brainy.ts @@ -32,7 +32,7 @@ import { CommitBuilder } from './storage/cow/CommitObject.js' import { BlobStorage } from './storage/cow/BlobStorage.js' import { NULL_HASH } from './storage/cow/constants.js' import { createPipeline } from './streaming/pipeline.js' -import { configureLogger, LogLevel } from './utils/logger.js' +import { configureLogger, LogLevel, prodLog } from './utils/logger.js' import { setGlobalCache } from './utils/unifiedCache.js' import type { UnifiedCache } from './utils/unifiedCache.js' import { rankIndicesByScore, reorderByIndices } from './utils/resultRanking.js' @@ -1527,6 +1527,64 @@ export class Brainy implements BrainyInterface { * }) * ``` */ + /** + * @description Normalize an update's metadata patch with respect to + * Brainy-reserved fields. `add()` lifts reserved fields out of `metadata` + * to their canonical top-level location; this applies the same contract to + * `update()` so the two write paths behave identically: + * - `confidence`, `weight`, `subtype` — remapped to the top-level param + * unless the caller also passed that param explicitly (top-level wins). + * - `noun`, `data`, `createdAt`, `updatedAt`, `service`, `createdBy`, + * `_rev` — system-managed or owned by a dedicated param; dropped from the + * patch with a one-shot warning that names the correct write path. + * @param params - The caller's update params (not mutated). + * @returns Params with reserved fields normalized out of `metadata`. + */ + private remapReservedMetadataFields(params: UpdateParams): UpdateParams { + if (!params.metadata || typeof params.metadata !== 'object') return params + + const { + noun, subtype, createdAt, updatedAt, confidence, weight, service, + data, createdBy, _rev, ...customMetadata + } = params.metadata as Record + + const hasReserved = + noun !== undefined || subtype !== undefined || createdAt !== undefined || + updatedAt !== undefined || confidence !== undefined || weight !== undefined || + service !== undefined || data !== undefined || createdBy !== undefined || + _rev !== undefined + if (!hasReserved) return params + + const warnDropped = (field: string, rightPath: string) => { + if (Brainy.warnedReservedFields.has(field)) return + Brainy.warnedReservedFields.add(field) + prodLog.warn( + `[brainy] update(): '${field}' is a reserved field and cannot be set ` + + `through a metadata patch — use ${rightPath}. The value was ignored. ` + + `(This warning is shown once per field per process.)` + ) + } + + if (noun !== undefined) warnDropped('noun', "the top-level 'type' param") + if (data !== undefined) warnDropped('data', "the top-level 'data' param") + if (createdAt !== undefined) warnDropped('createdAt', 'nothing — creation time is immutable') + if (updatedAt !== undefined) warnDropped('updatedAt', 'nothing — set automatically on every update') + if (service !== undefined) warnDropped('service', 'nothing — fixed at add() time') + if (createdBy !== undefined) warnDropped('createdBy', 'nothing — fixed at add() time') + if (_rev !== undefined) warnDropped('_rev', "the 'ifRev' param for optimistic concurrency") + + return { + ...params, + metadata: customMetadata as UpdateParams['metadata'], + ...(params.confidence === undefined && typeof confidence === 'number' && { confidence }), + ...(params.weight === undefined && typeof weight === 'number' && { weight }), + ...(params.subtype === undefined && typeof subtype === 'string' && { subtype }) + } + } + + /** One-shot registry for reserved-field warnings (per process). */ + private static warnedReservedFields = new Set() + async update(params: UpdateParams): Promise { this.assertWritable('update') await this.ensureInitialized() @@ -1534,6 +1592,17 @@ export class Brainy implements BrainyInterface { // Zero-config validation (static import for performance) validateUpdateParams(params) + // Reserved fields arriving via the metadata patch are remapped to their + // canonical top-level location, mirroring add()'s lift behavior. Before + // this fix the patch value survived the merge but was then clobbered by + // the preserve-existing spreads below — a silent no-op that consumers + // could only detect by reading values back. User-mutable reserved fields + // (confidence, weight, subtype) remap unless the same field was also + // passed top-level (top-level wins). System-managed fields (createdAt, + // updatedAt, _rev, noun, data, service, createdBy) cannot be set through + // a metadata patch and are dropped with a warning naming the right path. + params = this.remapReservedMetadataFields(params) + // Tracked-field vocabulary enforcement (Layer 2). Same as add() — the // metadata bag carries fields registered via trackField(), and subtype is // a tracked top-level candidate. diff --git a/tests/unit/brainy/update-reserved-metadata-remap.test.ts b/tests/unit/brainy/update-reserved-metadata-remap.test.ts new file mode 100644 index 00000000..122ed393 --- /dev/null +++ b/tests/unit/brainy/update-reserved-metadata-remap.test.ts @@ -0,0 +1,117 @@ +/** + * @module brainy/update-reserved-metadata-remap.test + * @description Regression tests for the reserved-field metadata-patch trap + * (7.31.6). `add({metadata: {confidence}})` lifts reserved fields to their + * canonical top-level location, but pre-7.31.6 `update({metadata: + * {confidence}})` silently dropped the same shape — the patch value survived + * the merge and was then clobbered by the preserve-existing spread. A + * production consumer's confidence-evolution writes no-oped for weeks before + * being caught by reading values back. + * + * Contract under test: update() remaps user-mutable reserved fields + * (confidence, weight, subtype) from the metadata patch to top-level + * (top-level param wins when both are present), and drops system-managed + * fields (createdAt, _rev, noun, data, ...) from patches with a warning. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import { Brainy } from '../../../src/index.js' + +describe('update() reserved-field metadata-patch remap (7.31.6)', () => { + let brain: Brainy + + beforeEach(async () => { + brain = new Brainy({ storage: { type: 'memory' } }) + await brain.init() + }) + + afterEach(async () => { + await brain.close() + }) + + it('remaps metadata.confidence to the top-level field (the production repro)', async () => { + const id = await brain.add({ + type: 'concept', + subtype: 'general', + data: 'x', + metadata: { confidence: 0.8 } + }) + + // Top-level write works (always did) + await brain.update({ id, confidence: 0.42 }) + let entity = await brain.get(id) + expect(entity?.confidence).toBe(0.42) + + // Metadata-patch write — silently dropped pre-7.31.6, remapped now + await brain.update({ id, metadata: { confidence: 0.33 } }) + entity = await brain.get(id) + expect(entity?.confidence).toBe(0.33) + // The reserved key must not linger inside the metadata bag + expect((entity?.metadata as any)?.confidence).toBeUndefined() + }) + + it('remaps metadata.weight and metadata.subtype the same way', async () => { + const id = await brain.add({ + type: 'concept', + subtype: 'general', + data: 'y', + metadata: {} + }) + + await brain.update({ id, metadata: { weight: 0.7, subtype: 'specialized' } }) + const entity = await brain.get(id) + expect(entity?.weight).toBe(0.7) + expect(entity?.subtype).toBe('specialized') + expect((entity?.metadata as any)?.weight).toBeUndefined() + expect((entity?.metadata as any)?.subtype).toBeUndefined() + }) + + it('top-level param wins when both top-level and metadata-patch carry the field', async () => { + const id = await brain.add({ + type: 'concept', + subtype: 'general', + data: 'z', + metadata: { confidence: 0.5 } + }) + + await brain.update({ id, confidence: 0.9, metadata: { confidence: 0.1 } }) + const entity = await brain.get(id) + expect(entity?.confidence).toBe(0.9) + }) + + it('drops system-managed fields from patches without corrupting the entity', async () => { + const id = await brain.add({ + type: 'concept', + subtype: 'general', + data: 'w', + metadata: { keep: 'me' } + }) + const before = await brain.get(id) + + await brain.update({ + id, + metadata: { createdAt: 1, _rev: 999, noun: 'organization', other: 'applied' } + }) + const after = await brain.get(id) + + expect(after?.createdAt).toBe(before?.createdAt) // immutable + expect(after?.type).toBe('concept') // noun patch ignored + expect((after?.metadata as any)?.other).toBe('applied') // custom fields still merge + expect((after?.metadata as any)?.keep).toBe('me') + expect((after?.metadata as any)?._rev_).toBeUndefined() + }) + + it('custom (non-reserved) metadata patches are unaffected by the remap', async () => { + const id = await brain.add({ + type: 'concept', + subtype: 'general', + data: 'v', + metadata: { status: 'draft' } + }) + + await brain.update({ id, metadata: { status: 'reviewed', rating: 4.5 } }) + const entity = await brain.get(id) + expect((entity?.metadata as any)?.status).toBe('reviewed') + expect((entity?.metadata as any)?.rating).toBe(4.5) + }) +})