fix: remap reserved fields from update() metadata patches to their canonical location
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.
This commit is contained in:
parent
e5ec658fab
commit
67e5fc8779
2 changed files with 187 additions and 1 deletions
|
|
@ -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<T = any> implements BrainyInterface<T> {
|
|||
* })
|
||||
* ```
|
||||
*/
|
||||
/**
|
||||
* @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<T>): UpdateParams<T> {
|
||||
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<string, unknown>
|
||||
|
||||
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<T>['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<string>()
|
||||
|
||||
async update(params: UpdateParams<T>): Promise<void> {
|
||||
this.assertWritable('update')
|
||||
await this.ensureInitialized()
|
||||
|
|
@ -1534,6 +1592,17 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
// 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.
|
||||
|
|
|
|||
117
tests/unit/brainy/update-reserved-metadata-remap.test.ts
Normal file
117
tests/unit/brainy/update-reserved-metadata-remap.test.ts
Normal file
|
|
@ -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)
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue