diff --git a/src/brainy.ts b/src/brainy.ts index 2f6a5293..b90194d1 100644 --- a/src/brainy.ts +++ b/src/brainy.ts @@ -1708,95 +1708,162 @@ export class Brainy implements BrainyInterface { return entity } - /** One-shot registry for reserved-field drop warnings (per process, per method+field). */ + /** One-shot registry for reserved-field warnings (per process, per method+field). */ private static warnedReservedFields = new Set() /** - * @description Warn once per process (per method+field) that a reserved - * field arrived inside a metadata bag and was dropped, naming the correct - * write path. Only dropped (system-managed) fields warn — user-settable - * fields are remapped to their dedicated param and honored silently. + * @description Resolve the human-readable "correct write path" guidance for a + * reserved field on a given write method. Single source of truth shared by the + * `'throw'` (Error message) and `'warn'` (one-shot warning) paths so the two + * never drift. The trio `confidence` / `weight` / `subtype` and the + * add()/relate()-time fields `service` / `createdBy` / `visibility` map to a + * dedicated param; everything else is system-managed. * @param method - The public write method the bag arrived through. - * @param field - The reserved field name that was dropped. - * @param rightPath - Human guidance naming the correct write path. + * @param field - The reserved field name found in the metadata bag. + * @returns Guidance naming the correct way to set the field. */ - private warnDroppedReservedField(method: string, field: string, rightPath: string): void { - const key = `${method}:${field}` - if (Brainy.warnedReservedFields.has(key)) return - Brainy.warnedReservedFields.add(key) - prodLog.warn( - `[brainy] ${method}(): '${field}' is a reserved field and cannot be set ` + - `through the metadata bag — use ${rightPath}. The value was ignored. ` + - `(This warning is shown once per field per process.)` - ) + private reservedWritePath( + method: 'add' | 'update' | 'relate' | 'updateRelation', + field: string + ): string { + const typeParam = "the top-level 'type' param" + switch (field) { + case 'noun': + case 'verb': + return typeParam + case 'data': + return "the top-level 'data' param" + case 'confidence': + return "the 'confidence' param" + case 'weight': + return "the 'weight' param" + case 'subtype': + return "the 'subtype' param" + case 'visibility': + return "the 'visibility' param ('public' | 'internal')" + case 'service': + return method === 'add' + ? "the 'service' param of add()" + : method === 'relate' + ? "the 'service' param of relate()" + : 'nothing — service is fixed at create time' + case 'createdBy': + return method === 'add' + ? "the 'createdBy' param of add()" + : 'nothing — createdBy is system-managed' + case 'createdAt': + return 'nothing — creation time is set automatically' + case 'updatedAt': + return 'nothing — set automatically on every write' + case '_rev': + return method === 'update' + ? "the 'ifRev' param for optimistic concurrency" + : 'nothing — revisions are system-managed' + default: + return 'a dedicated top-level param' + } } /** - * @description Emit the one-shot drop warnings for system-managed entity - * fields found in a metadata bag. Shared by the `add` and `update` remaps - * (live calls and their `transact()` mirrors). - * @param method - `'add'` or `'update'` (the contract is identical for the - * matching `transact()` operation). - * @param reserved - The reserved half of the split metadata bag. + * @description Enforce {@link BrainyConfig.reservedFieldPolicy} for reserved + * fields found inside a metadata bag. Called by every write-path remap once + * the bag has been split and at least one reserved key is present. + * + * - `'throw'` (default): throw a clear Error naming every offending key and + * its correct write path. The caller never reaches the remap. + * - `'warn'`: emit a ONE-SHOT (per method+field, per process) warning for + * EVERY reserved key found — both the user-mutable fields that are about to + * be remapped and the system-managed fields that are about to be dropped — + * then fall through to the legacy remap. + * - `'remap'`: silent legacy remap, no warning. + * + * @param method - The public write method the bag arrived through. + * @param reserved - The reserved half of the split metadata bag (non-empty). + * @param reservedListName - `'RESERVED_ENTITY_FIELDS'` or + * `'RESERVED_RELATION_FIELDS'` — named in the thrown Error for discoverability. + * @returns `true` when the caller should proceed with the legacy remap + * (`'warn'` / `'remap'`); `'throw'` never returns (it throws first). + * @throws {Error} When the policy is `'throw'` and any reserved key is present. */ - private warnDroppedReservedEntityFields( - method: 'add' | 'update', - reserved: Partial> - ): void { - if (reserved.noun !== undefined) { - this.warnDroppedReservedField(method, 'noun', "the top-level 'type' param") - } - if (reserved.data !== undefined) { - this.warnDroppedReservedField(method, 'data', "the top-level 'data' param") - } - if (reserved.createdAt !== undefined) { - this.warnDroppedReservedField( - method, - 'createdAt', - method === 'add' - ? 'nothing — creation time is set automatically' - : 'nothing — creation time is immutable' + private enforceReservedPolicy( + method: 'add' | 'update' | 'relate' | 'updateRelation', + reserved: Partial>, + reservedListName: 'RESERVED_ENTITY_FIELDS' | 'RESERVED_RELATION_FIELDS' + ): boolean { + const policy = this.config.reservedFieldPolicy ?? 'throw' + const keys = Object.keys(reserved) + if (keys.length === 0) return true + + if (policy === 'throw') { + const detail = keys + .map((k) => { + const path = this.reservedWritePath(method, k) + // System-managed fields resolve to a "nothing — …" sentinel; phrase + // those as "is system-managed" rather than "pass it as the nothing". + return path.startsWith('nothing') + ? `metadata.${k} is a reserved field (${path.replace(/^nothing\s*—\s*/, '')}) and cannot be set through ${method}()` + : `metadata.${k} is a reserved field — pass it as ${path} to ${method}()` + }) + .join('; ') + throw new Error( + `${detail} (reserved: see ${reservedListName}). ` + + `Set reservedFieldPolicy:'remap' to opt into legacy remapping, ` + + `or reservedFieldPolicy:'warn' to remap with a warning.` ) } - if (reserved.updatedAt !== undefined) { - this.warnDroppedReservedField(method, 'updatedAt', 'nothing — set automatically on every write') - } - if (reserved._rev !== undefined) { - this.warnDroppedReservedField( - method, - '_rev', - method === 'update' - ? "the 'ifRev' param for optimistic concurrency" - : 'nothing — revisions are system-managed' - ) - } - // 'system' visibility is Brainy-only — a user bag cannot set it. (A 'public'/'internal' - // value IS user-settable and is silently remapped to the top-level param below.) - if (reserved.visibility === 'system') { - this.warnDroppedReservedField(method, 'visibility', "the 'visibility' param ('public' | 'internal')") - } - if (method === 'update') { - if (reserved.service !== undefined) { - this.warnDroppedReservedField(method, 'service', 'nothing — fixed at add() time') - } - if (reserved.createdBy !== undefined) { - this.warnDroppedReservedField(method, 'createdBy', 'nothing — fixed at add() time') + + if (policy === 'warn') { + // One-shot warning for EVERY reserved key (today only system-managed ones + // warn — this closes that gap so user-mutable remaps are visible too). + for (const k of keys) { + this.warnReservedRemapped(method, k, this.reservedWritePath(method, k)) } } + + // 'warn' and 'remap' both fall through to the legacy remap. + return true + } + + /** + * @description One-shot (per method+field, per process) warning that a + * reserved field arrived inside a metadata bag under the `'warn'` policy. The + * wording is neutral on "remapped vs dropped" — `reservedWritePath()` already + * tells the caller where the value goes (a dedicated param, or "nothing"). + * @param method - The public write method the bag arrived through. + * @param field - The reserved field name found in the bag. + * @param rightPath - Guidance naming the correct write path. + */ + private warnReservedRemapped(method: string, field: string, rightPath: string): void { + const key = `${method}:${field}` + if (Brainy.warnedReservedFields.has(key)) return + Brainy.warnedReservedFields.add(key) + // System-managed fields resolve to a "nothing — …" sentinel; phrase the + // guidance so it reads cleanly in both the remapped and dropped cases. + const guidance = rightPath.startsWith('nothing') + ? `it is ${rightPath.replace(/^nothing\s*—\s*/, '')} and was dropped` + : `set it via ${rightPath} instead` + prodLog.warn( + `[brainy] ${method}(): '${field}' is a reserved field and was found inside the ` + + `metadata bag — ${guidance}. (Legacy remap applied because ` + + `reservedFieldPolicy is 'warn'. This warning is shown once per field per process.)` + ) } /** * @description Normalize an `add()` params object with respect to * Brainy-reserved fields arriving inside `metadata` (untyped callers only — * the compile-time guard on `AddParams.metadata` stops TypeScript callers). - * Fields with a dedicated `add()` param (`confidence`, `weight`, `subtype`, - * `service`, `createdBy`) are remapped to that param unless the caller also - * passed it explicitly (top-level wins); system-managed fields (`noun`, - * `data`, `createdAt`, `updatedAt`, `_rev`) are dropped with a one-shot - * warning naming the correct write path. The remapped `subtype` flows + * Governed by {@link BrainyConfig.reservedFieldPolicy} (default `'throw'`): + * `'throw'` rejects the write naming the offending key(s); `'warn'`/`'remap'` + * fall through to the legacy remap, where fields with a dedicated `add()` + * param (`confidence`, `weight`, `subtype`, `visibility`, `service`, + * `createdBy`) are remapped to that param unless the caller also passed it + * explicitly (top-level wins) and system-managed fields (`noun`, `data`, + * `createdAt`, `updatedAt`, `_rev`) are dropped. A remapped `subtype` flows * through subtype-pairing enforcement exactly like a top-level one. * @param params - The caller's add params (not mutated). * @returns Params with reserved fields normalized out of `metadata`. + * @throws {Error} When `reservedFieldPolicy` is `'throw'` and the bag carries a reserved key. */ private remapReservedAddMetadata(params: AddParams): AddParams { const bag = params.metadata as Record | undefined @@ -1804,7 +1871,9 @@ export class Brainy implements BrainyInterface { const { reserved, custom } = splitNounMetadataRecord(bag) if (Object.keys(reserved).length === 0) return params - this.warnDroppedReservedEntityFields('add', reserved) + // Policy gate: 'throw' (default) throws here; 'warn' warns once per key then + // remaps; 'remap' silently remaps. (Throw never returns.) + this.enforceReservedPolicy('add', reserved, 'RESERVED_ENTITY_FIELDS') const createdBy = reserved.createdBy as { augmentation?: unknown; version?: unknown } | undefined const createdByValid = @@ -1841,13 +1910,15 @@ export class Brainy implements BrainyInterface { * `update({metadata:{confidence}})` silently dropped it (the patch value * survived the merge and was then clobbered by the preserve-existing * spread; a production consumer's confidence-evolution writes no-oped until - * read back). User-mutable fields (`confidence`, `weight`, `subtype`) - * remap to their dedicated param unless the caller also passed it - * (top-level wins); everything else (`noun`, `data`, `createdAt`, - * `updatedAt`, `service`, `createdBy`, `_rev`) is system-managed or fixed - * at `add()` time and is dropped with a one-shot warning. + * read back). Governed by {@link BrainyConfig.reservedFieldPolicy} (default + * `'throw'`): `'throw'` rejects the write; `'warn'`/`'remap'` remap + * user-mutable fields (`confidence`, `weight`, `subtype`) to their dedicated + * param unless the caller also passed it (top-level wins) and drop everything + * else (`noun`, `data`, `createdAt`, `updatedAt`, `service`, `createdBy`, + * `_rev`) as system-managed or fixed at `add()` time. * @param params - The caller's update params (not mutated). * @returns Params with reserved fields normalized out of `metadata`. + * @throws {Error} When `reservedFieldPolicy` is `'throw'` and the bag carries a reserved key. */ private remapReservedUpdateMetadata(params: UpdateParams): UpdateParams { const bag = params.metadata as Record | undefined @@ -1855,7 +1926,9 @@ export class Brainy implements BrainyInterface { const { reserved, custom } = splitNounMetadataRecord(bag) if (Object.keys(reserved).length === 0) return params - this.warnDroppedReservedEntityFields('update', reserved) + // Policy gate: 'throw' (default) throws; 'warn' warns once per key then + // remaps; 'remap' silently remaps. + this.enforceReservedPolicy('update', reserved, 'RESERVED_ENTITY_FIELDS') return { ...params, @@ -1869,61 +1942,18 @@ export class Brainy implements BrainyInterface { } } - /** - * @description Emit the one-shot drop warnings for system-managed - * relationship fields found in a metadata bag. Shared by the `relate` and - * `updateRelation` remaps (live calls and the `transact()` relate mirror). - * @param method - `'relate'` or `'updateRelation'`. - * @param reserved - The reserved half of the split metadata bag. - */ - private warnDroppedReservedRelationFields( - method: 'relate' | 'updateRelation', - reserved: Partial> - ): void { - if (reserved.verb !== undefined) { - this.warnDroppedReservedField(method, 'verb', "the top-level 'type' param") - } - if (reserved.data !== undefined) { - this.warnDroppedReservedField(method, 'data', "the top-level 'data' param") - } - if (reserved.createdAt !== undefined) { - this.warnDroppedReservedField( - method, - 'createdAt', - method === 'relate' - ? 'nothing — creation time is set automatically' - : 'nothing — creation time is immutable' - ) - } - if (reserved.updatedAt !== undefined) { - this.warnDroppedReservedField(method, 'updatedAt', 'nothing — set automatically on every write') - } - if (reserved.createdBy !== undefined) { - this.warnDroppedReservedField(method, 'createdBy', 'nothing — system-managed') - } - if (reserved._rev !== undefined) { - this.warnDroppedReservedField(method, '_rev', 'nothing — system-managed') - } - // 'system' visibility is Brainy-only — a user bag cannot set it. (A 'public'/'internal' - // value IS user-settable and is silently remapped to the top-level param below.) - if (reserved.visibility === 'system') { - this.warnDroppedReservedField(method, 'visibility', "the 'visibility' param ('public' | 'internal')") - } - if (method === 'updateRelation' && reserved.service !== undefined) { - this.warnDroppedReservedField(method, 'service', 'nothing — fixed at relate() time') - } - } - /** * @description Normalize a `relate()` params object with respect to * Brainy-reserved fields arriving inside `metadata` — the relationship - * mirror of {@link remapReservedAddMetadata}. Fields with a dedicated - * `relate()` param (`confidence`, `weight`, `subtype`, `service`) remap to - * that param (top-level wins); system-managed fields (`verb`, `data`, - * `createdAt`, `updatedAt`, `createdBy`, `_rev`) are dropped with a - * one-shot warning. + * mirror of {@link remapReservedAddMetadata}. Governed by + * {@link BrainyConfig.reservedFieldPolicy} (default `'throw'`): `'throw'` + * rejects the write; `'warn'`/`'remap'` remap fields with a dedicated + * `relate()` param (`confidence`, `weight`, `subtype`, `visibility`, + * `service`) to that param (top-level wins) and drop system-managed fields + * (`verb`, `data`, `createdAt`, `updatedAt`, `createdBy`, `_rev`). * @param params - The caller's relate params (not mutated). * @returns Params with reserved fields normalized out of `metadata`. + * @throws {Error} When `reservedFieldPolicy` is `'throw'` and the bag carries a reserved key. */ private remapReservedRelateMetadata(params: RelateParams): RelateParams { const bag = params.metadata as Record | undefined @@ -1931,7 +1961,9 @@ export class Brainy implements BrainyInterface { const { reserved, custom } = splitVerbMetadataRecord(bag) if (Object.keys(reserved).length === 0) return params - this.warnDroppedReservedRelationFields('relate', reserved) + // Policy gate: 'throw' (default) throws; 'warn' warns once per key then + // remaps; 'remap' silently remaps. + this.enforceReservedPolicy('relate', reserved, 'RESERVED_RELATION_FIELDS') return { ...params, @@ -1954,12 +1986,14 @@ export class Brainy implements BrainyInterface { /** * @description Normalize an `updateRelation()` params object with respect * to Brainy-reserved fields arriving inside the metadata patch — the - * relationship mirror of {@link remapReservedUpdateMetadata}. User-mutable - * fields (`confidence`, `weight`, `subtype`) remap to their dedicated - * param (top-level wins); everything else is dropped with a one-shot - * warning. + * relationship mirror of {@link remapReservedUpdateMetadata}. Governed by + * {@link BrainyConfig.reservedFieldPolicy} (default `'throw'`): `'throw'` + * rejects the write; `'warn'`/`'remap'` remap user-mutable fields + * (`confidence`, `weight`, `subtype`, `visibility`) to their dedicated param + * (top-level wins) and drop everything else. * @param params - The caller's update-relation params (not mutated). * @returns Params with reserved fields normalized out of `metadata`. + * @throws {Error} When `reservedFieldPolicy` is `'throw'` and the bag carries a reserved key. */ private remapReservedUpdateRelationMetadata( params: UpdateRelationParams @@ -1969,7 +2003,9 @@ export class Brainy implements BrainyInterface { const { reserved, custom } = splitVerbMetadataRecord(bag) if (Object.keys(reserved).length === 0) return params - this.warnDroppedReservedRelationFields('updateRelation', reserved) + // Policy gate: 'throw' (default) throws; 'warn' warns once per key then + // remaps; 'remap' silently remaps. + this.enforceReservedPolicy('updateRelation', reserved, 'RESERVED_RELATION_FIELDS') return { ...params, @@ -11229,7 +11265,12 @@ export class Brainy implements BrainyInterface { requireSubtype: config?.requireSubtype ?? true, // Multi-process safety mode: config?.mode ?? 'writer', - force: config?.force ?? false + force: config?.force ?? false, + // Reserved-field-in-metadata-bag policy (8.0 — no silent failures). + // Default 'throw': an untyped caller that smuggles a reserved key past + // the compile guard gets a loud Error naming the correct write path. + // 'warn' = remap + one-shot warning per key; 'remap' = legacy silent remap. + reservedFieldPolicy: config?.reservedFieldPolicy ?? 'throw' } } diff --git a/src/import/ImportCoordinator.ts b/src/import/ImportCoordinator.ts index c2eaaef5..21d09410 100644 --- a/src/import/ImportCoordinator.ts +++ b/src/import/ImportCoordinator.ts @@ -23,6 +23,7 @@ import { SmartYAMLImporter } from '../importers/SmartYAMLImporter.js' import { SmartDOCXImporter } from '../importers/SmartDOCXImporter.js' import { VFSStructureGenerator } from '../importers/VFSStructureGenerator.js' import { NounType, VerbType } from '../types/graphTypes.js' +import { splitNounMetadataRecord, splitVerbMetadataRecord } from '../types/reservedFields.js' import { v4 as uuidv4 } from '../universal/uuid.js' import * as fs from 'fs' import * as path from 'path' @@ -834,7 +835,11 @@ export class ImportCoordinator { type: 'media', description: '', confidence: 1.0, - metadata: { subtype: 'image' } + // `subtype` is a reserved field — keep it top-level on the extractor + // entity (read into the `subtype` param at add() time), never inside + // the metadata bag that gets spread into add({ metadata }). + subtype: 'image', + metadata: {} }, relatedEntities: [], relationships: [] @@ -843,7 +848,8 @@ export class ImportCoordinator { id: imageId, name: imageName, type: 'media', - metadata: { subtype: 'image' } + subtype: 'image', + metadata: {} }], relationships: [], metadata: {}, @@ -862,6 +868,38 @@ export class ImportCoordinator { } } + /** + * Strip Brainy-reserved entity keys out of an extractor-supplied metadata bag. + * + * Extractors (and consumer `customMetadata`) can carry reserved keys + * (`confidence`, `subtype`, `weight`, …) inside `metadata`. Brainy 8.0's + * default `reservedFieldPolicy` is `'throw'`, so spreading such a bag into + * `add({ metadata })` would reject the whole import. The import pipeline owns + * the correct write path: user-mutable reserved values are passed as dedicated + * `AddParams` params (see the call sites), so here we simply drop the reserved + * half of the bag and keep only the custom fields that belong in `metadata`. + * + * @param bag - The extractor/consumer metadata bag (may be undefined). + * @returns The custom-only metadata (reserved keys removed). + */ + private stripReservedFromBag(bag: Record | undefined | null): Record { + if (!bag || typeof bag !== 'object') return {} + return splitNounMetadataRecord(bag).custom + } + + /** + * Relationship mirror of {@link stripReservedFromBag} — strips reserved verb + * keys (`verb`, `confidence`, `weight`, `subtype`, …) out of an edge metadata + * bag so it carries only custom fields. Reserved values that have a dedicated + * `RelateParams` param are passed there by the call site instead. + * @param bag - The extractor/consumer edge metadata bag (may be undefined). + * @returns The custom-only edge metadata (reserved keys removed). + */ + private stripReservedFromRelationBag(bag: Record | undefined | null): Record { + if (!bag || typeof bag !== 'object') return {} + return splitVerbMetadataRecord(bag).custom + } + /** * Create entities and relationships in knowledge graph * Added sourceInfo parameter for document entity creation @@ -977,7 +1015,7 @@ export class ImportCoordinator { importedAt: trackingContext.importedAt, importFormat: trackingContext.importFormat, importSource: trackingContext.importSource, - ...trackingContext.customMetadata + ...this.stripReservedFromBag(trackingContext.customMetadata) }) } }) @@ -1005,10 +1043,14 @@ export class ImportCoordinator { data: entity.description || entity.name, type: entity.type, subtype: entity.subtype ?? options.defaultSubtype ?? 'imported', + // `confidence` is a reserved field — pass it as the dedicated param, + // never inside the metadata bag (8.0 reservedFieldPolicy defaults to 'throw'). + confidence: entity.confidence, metadata: { - ...entity.metadata, + // Extractor/consumer bags may smuggle reserved keys — strip them so + // the bag carries only custom fields. + ...this.stripReservedFromBag(entity.metadata), name: entity.name, - confidence: entity.confidence, vfsPath: vfsFile?.path, importedFrom: 'import-coordinator', imports: [importSource], @@ -1020,7 +1062,7 @@ export class ImportCoordinator { importSource: trackingContext.importSource, sourceRow: row.rowNumber, sourceSheet: row.sheet, - ...trackingContext.customMetadata + ...this.stripReservedFromBag(trackingContext.customMetadata) }) } } @@ -1101,7 +1143,7 @@ export class ImportCoordinator { importIds: [trackingContext.importId], projectId: trackingContext.projectId, importFormat: trackingContext.importFormat, - ...trackingContext.customMetadata + ...this.stripReservedFromRelationBag(trackingContext.customMetadata) }) } } @@ -1132,10 +1174,12 @@ export class ImportCoordinator { data: entity.description || entity.name, type: entity.type, subtype: entity.subtype ?? options.defaultSubtype ?? 'imported', + // `confidence` is a reserved field — dedicated param, not metadata. + confidence: entity.confidence, metadata: { - ...entity.metadata, + // Strip any reserved keys an extractor smuggled into the bag. + ...this.stripReservedFromBag(entity.metadata), name: entity.name, - confidence: entity.confidence, vfsPath: vfsFile?.path, importedFrom: 'import-coordinator', // Import tracking metadata @@ -1148,7 +1192,7 @@ export class ImportCoordinator { importSource: trackingContext.importSource, sourceRow: row.rowNumber, sourceSheet: row.sheet, - ...trackingContext.customMetadata + ...this.stripReservedFromBag(trackingContext.customMetadata) }) } }) @@ -1188,7 +1232,7 @@ export class ImportCoordinator { importIds: [trackingContext.importId], projectId: trackingContext.projectId, importFormat: trackingContext.importFormat, - ...trackingContext.customMetadata + ...this.stripReservedFromRelationBag(trackingContext.customMetadata) }) } }) @@ -1243,7 +1287,7 @@ export class ImportCoordinator { projectId: trackingContext.projectId, importedAt: trackingContext.importedAt, importFormat: trackingContext.importFormat, - ...trackingContext.customMetadata + ...this.stripReservedFromBag(trackingContext.customMetadata) }) } }) @@ -1273,7 +1317,7 @@ export class ImportCoordinator { projectId: trackingContext.projectId, importedAt: trackingContext.importedAt, importFormat: trackingContext.importFormat, - ...trackingContext.customMetadata + ...this.stripReservedFromRelationBag(trackingContext.customMetadata) }) } }) @@ -1371,8 +1415,12 @@ export class ImportCoordinator { from: rel.from, to: rel.to, type: verbType, // Enhanced type + // confidence/weight are reserved — carry them as dedicated params, + // never inside the bag (they were collected top-level on each rel). + ...(typeof (rel as any).confidence === 'number' && { confidence: (rel as any).confidence }), + ...(typeof (rel as any).weight === 'number' && { weight: (rel as any).weight }), metadata: { - ...(rel.metadata || {}), + ...this.stripReservedFromRelationBag(rel.metadata), relationshipType: 'semantic', // Distinguish from VFS/provenance inferredType: verbType !== rel.type, // Track if type was enhanced originalType: rel.type diff --git a/src/importers/SmartImportOrchestrator.ts b/src/importers/SmartImportOrchestrator.ts index d9157852..2084250d 100644 --- a/src/importers/SmartImportOrchestrator.ts +++ b/src/importers/SmartImportOrchestrator.ts @@ -12,6 +12,7 @@ import { Brainy } from '../brainy.js' import { VirtualFileSystem } from '../vfs/VirtualFileSystem.js' import { NounType, VerbType } from '../types/graphTypes.js' +import { splitNounMetadataRecord } from '../types/reservedFields.js' import { SmartExcelImporter, SmartExcelOptions, SmartExcelResult } from './SmartExcelImporter.js' import { SmartPDFImporter, SmartPDFOptions, SmartPDFResult } from './SmartPDFImporter.js' import { SmartCSVImporter, SmartCSVOptions, SmartCSVResult } from './SmartCSVImporter.js' @@ -201,7 +202,9 @@ export class SmartImportOrchestrator { .subtype ?? options.defaultSubtype ?? 'imported', confidence: extracted.entity.confidence, // reserved field — dedicated param, not metadata metadata: { - ...extracted.entity.metadata, + // Strip reserved keys an extractor may have smuggled into the bag + // (8.0 reservedFieldPolicy defaults to 'throw'). + ...splitNounMetadataRecord(extracted.entity.metadata).custom, name: extracted.entity.name, importedFrom: 'smart-import' } @@ -245,7 +248,7 @@ export class SmartImportOrchestrator { } // Collect all relationship parameters - const relationshipParams: Array<{from: string; to: string; type: VerbType; subtype?: string; metadata?: any}> = [] + const relationshipParams: Array<{from: string; to: string; type: VerbType; subtype?: string; confidence?: number; metadata?: any}> = [] for (const extracted of result.extraction.rows) { for (const rel of extracted.relationships) { @@ -288,8 +291,8 @@ export class SmartImportOrchestrator { subtype: (rel as typeof rel & { subtype?: string }).subtype ?? options.defaultSubtype ?? 'imported', + confidence: rel.confidence, // reserved field — dedicated param, not metadata metadata: { - confidence: rel.confidence, evidence: rel.evidence } }) @@ -615,7 +618,7 @@ export class SmartImportOrchestrator { (extracted.entity as typeof extracted.entity & { subtype?: string }) .subtype ?? options.defaultSubtype ?? 'imported', confidence: extracted.entity.confidence, // reserved field — dedicated param, not metadata - metadata: { ...extracted.entity.metadata, name: extracted.entity.name, importedFrom: 'smart-import' } + metadata: { ...splitNounMetadataRecord(extracted.entity.metadata).custom, name: extracted.entity.name, importedFrom: 'smart-import' } }) result.entityIds.push(entityId) result.stats.entitiesCreated++ diff --git a/src/neural/neuralImport.ts b/src/neural/neuralImport.ts index 8a2ba843..c8d19eb5 100644 --- a/src/neural/neuralImport.ts +++ b/src/neural/neuralImport.ts @@ -7,6 +7,7 @@ import { Brainy } from '../brainy.js' import { NounType, VerbType } from '../types/graphTypes.js' +import { splitNounMetadataRecord, splitVerbMetadataRecord } from '../types/reservedFields.js' import * as fs from '../universal/fs.js' import * as path from '../universal/path.js' // @ts-ignore @@ -802,9 +803,12 @@ export class NeuralImport { data: this.extractMainText(entity.originalData), type: entity.nounType as NounType, subtype: entity.subtype ?? options.defaultSubtype ?? 'extracted', + // `confidence` is a reserved field — dedicated param, not metadata + // (8.0 reservedFieldPolicy defaults to 'throw'). + confidence: entity.confidence, metadata: { - ...entity.originalData, - confidence: entity.confidence, + // Strip any reserved keys the source data smuggled into the bag. + ...splitNounMetadataRecord(entity.originalData).custom, id: entity.suggestedId } }) @@ -821,7 +825,8 @@ export class NeuralImport { confidence: relationship.confidence, // reserved field — dedicated param, not metadata metadata: { context: relationship.context, - ...relationship.metadata + // Strip any reserved keys smuggled into the edge metadata bag. + ...splitVerbMetadataRecord(relationship.metadata).custom } }) } diff --git a/src/types/brainy.types.ts b/src/types/brainy.types.ts index 61e6a86a..5df12e11 100644 --- a/src/types/brainy.types.ts +++ b/src/types/brainy.types.ts @@ -1496,6 +1496,33 @@ export interface BrainyConfig { * (PID liveness + heartbeat) cannot prove it. Logs a warning regardless. */ force?: boolean + + /** + * How write paths react when an untyped (JavaScript) caller smuggles a + * Brainy-reserved field (`RESERVED_ENTITY_FIELDS` / `RESERVED_RELATION_FIELDS` + * — `confidence`, `weight`, `subtype`, `visibility`, `service`, `createdBy`, + * `noun`/`verb`, `data`, `createdAt`, `updatedAt`, `_rev`) **inside the + * `metadata` bag** of `add()` / `update()` / `relate()` / `updateRelation()` + * (and their `transact()` / `with()` mirrors). TypeScript callers can't write + * these shapes at all — the compile-time guard on the metadata param types + * (`NoReservedEntityKeys` / `NoReservedRelationKeys`) rejects a literal + * reserved key — so this policy only governs untyped callers that slip one + * past the compiler. + * + * - `'throw'` (**default, 8.0**): a reserved key in the bag throws a clear + * `Error` naming the offending key(s) and the correct write path. No silent + * remap, no data loss, no surprise. This is the 8.0 "no silent failures" + * contract. + * - `'warn'`: legacy remapping with a loud, one-shot (per key, per process) + * warning for EVERY reserved key found — user-mutable fields are remapped to + * their dedicated top-level param (top-level wins when both are supplied), + * system-managed fields are dropped. Use while migrating untyped call sites. + * - `'remap'`: the pre-8.0 silent remapping, no warning. Last-resort + * compatibility hatch for code that intentionally relies on the bag path. + * + * @default 'throw' + */ + reservedFieldPolicy?: 'throw' | 'warn' | 'remap' } // ============= Neural API Types ============= diff --git a/tests/api/performance-benchmarks.test.ts b/tests/api/performance-benchmarks.test.ts index d26281d3..e83cebf2 100644 --- a/tests/api/performance-benchmarks.test.ts +++ b/tests/api/performance-benchmarks.test.ts @@ -201,7 +201,8 @@ describe('Performance Benchmarks - SLA Validation', () => { const start = performance.now() await brainy.update({ id, - metadata: { version: 2, updatedAt: Date.now() } + // `updatedAt` is system-managed — Brainy sets it on every write. + metadata: { version: 2 } }) const latency = performance.now() - start benchmark.recordOperation(latency) diff --git a/tests/helpers/test-factory.ts b/tests/helpers/test-factory.ts index 40bbe325..62dd53e6 100644 --- a/tests/helpers/test-factory.ts +++ b/tests/helpers/test-factory.ts @@ -39,17 +39,21 @@ export function generateTestVector(dimension = 384): Vector { return vector.map(val => val / magnitude) } -// Generate realistic metadata +// Generate realistic CUSTOM metadata. +// +// NOTE: this deliberately contains only custom (consumer-owned) fields. Reserved +// Brainy fields (`createdAt`, `updatedAt`, `confidence`, `weight`, `subtype`, +// `visibility`, `service`, `createdBy`, `data`, `_rev`) must NOT live in the +// metadata bag — Brainy 8.0's default `reservedFieldPolicy: 'throw'` rejects any +// reserved key passed through `metadata`. (Reserved values belong in their +// dedicated top-level params, set by the add/relate factories below.) export function generateTestMetadata(overrides: any = {}): any { return { name: `Test Item ${generateTestId()}`, description: 'Test description', tags: ['test', 'automated'], - createdAt: Date.now(), - updatedAt: Date.now(), version: 1, source: 'test-factory', - confidence: 0.95, ...overrides, } } diff --git a/tests/integration/batchImportWithRelations.test.ts b/tests/integration/batchImportWithRelations.test.ts index 1594db88..7fe8e511 100644 --- a/tests/integration/batchImportWithRelations.test.ts +++ b/tests/integration/batchImportWithRelations.test.ts @@ -61,10 +61,12 @@ describe('Batch Import with Immediate Relations (v5.7.3 Fix)', () => { const entityParams = Array(372).fill(null).map((_, i) => ({ data: `Extracted Entity ${i}`, type: NounType.Thing, + // `confidence` is a reserved field — pass it as the dedicated param, not + // inside the metadata bag (8.0 reservedFieldPolicy defaults to 'throw'). + confidence: 0.9, metadata: { extractedFrom: 'TfT~Sapient Species.pdf', - entityNumber: i, - confidence: 0.9 + entityNumber: i } })) diff --git a/tests/integration/remaining-apis.test.ts b/tests/integration/remaining-apis.test.ts index 5514bf17..12d60983 100644 --- a/tests/integration/remaining-apis.test.ts +++ b/tests/integration/remaining-apis.test.ts @@ -61,7 +61,9 @@ describe('Remaining APIs Comprehensive Test', () => { await brain.updateMany({ items: ids.successful.map(id => ({ id, - metadata: { status: 'published', updatedAt: Date.now() } + // `updatedAt` is system-managed (set automatically on every write) — + // only the custom `status` field belongs in the metadata bag. + metadata: { status: 'published' } })) }) diff --git a/tests/unit/brainy/reserved-field-policy.test.ts b/tests/unit/brainy/reserved-field-policy.test.ts new file mode 100644 index 00000000..c5f37af4 --- /dev/null +++ b/tests/unit/brainy/reserved-field-policy.test.ts @@ -0,0 +1,251 @@ +/** + * @module tests/unit/brainy/reserved-field-policy + * @description The 8.0 `reservedFieldPolicy` matrix — what happens when an + * untyped (JavaScript) caller smuggles a Brainy-reserved field INSIDE the + * `metadata` bag of a write call, past the compile-time guard. + * + * 8.0 is a clean break with no silent failures. The decided contract: + * - `'throw'` (DEFAULT): a reserved key in the bag throws a clear Error naming + * the offending key(s) and the correct write path. No remap, no data loss. + * - `'warn'`: legacy remap PLUS a one-shot (per method+field, per process) + * warning for EVERY reserved key found. + * - `'remap'`: the pre-8.0 silent remap, no warning. + * + * The deep correctness of the remap itself (top-level precedence, system-managed + * drops, transact()/with() mirrors, read-side splitting) lives in + * tests/unit/brainy/update-reserved-metadata-remap.test.ts (which now runs under + * `reservedFieldPolicy: 'remap'`). This file pins the POLICY SELECTION and the + * throw/warn behaviors. + * + * Compile-time callers can't write these shapes at all (see + * tests/unit/types/reserved-metadata-keys.test-d.ts); the `as object` widenings + * below simulate untyped callers. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import { Brainy } from '../../../src/index.js' +import { NounType, VerbType } from '../../../src/types/graphTypes.js' +import { createTestConfig } from '../../helpers/test-factory.js' +import { prodLog } from '../../../src/utils/logger.js' + +describe('reservedFieldPolicy', () => { + describe("default policy is 'throw'", () => { + let brain: Brainy + + beforeEach(async () => { + // No reservedFieldPolicy override → resolves to 'throw'. + brain = new Brainy(createTestConfig()) + await brain.init() + }) + + afterEach(async () => { + await brain.close() + }) + + it('add() throws naming the offending key and the correct write path', async () => { + await expect( + brain.add({ + type: NounType.Concept, + subtype: 'general', + data: 'x', + metadata: { confidence: 0.8 } as object + }) + ).rejects.toThrow(/metadata\.confidence is a reserved field/) + + // The error names the right param and the reserved list for discoverability. + await expect( + brain.add({ + type: NounType.Concept, + subtype: 'general', + data: 'x', + metadata: { confidence: 0.8 } as object + }) + ).rejects.toThrow(/'confidence' param.*RESERVED_ENTITY_FIELDS/s) + }) + + it('add() lists EVERY offending key when several are present', async () => { + const err = await brain + .add({ + type: NounType.Person, + data: 'multi', + metadata: { confidence: 0.5, weight: 0.6, subtype: 'employee' } as object + }) + .catch((e) => e as Error) + expect(err).toBeInstanceOf(Error) + expect(err.message).toMatch(/confidence/) + expect(err.message).toMatch(/weight/) + expect(err.message).toMatch(/subtype/) + }) + + it('update() throws on a reserved key in the patch', async () => { + const id = await brain.add({ type: NounType.Concept, subtype: 'general', data: 'y' }) + await expect( + brain.update({ id, metadata: { confidence: 0.3 } as object }) + ).rejects.toThrow(/metadata\.confidence is a reserved field/) + }) + + it('relate() throws on a reserved key in the bag', async () => { + const a = await brain.add({ type: NounType.Person, subtype: 'employee', data: 'A' }) + const b = await brain.add({ type: NounType.Person, subtype: 'employee', data: 'B' }) + await expect( + brain.relate({ + from: a, + to: b, + type: VerbType.RelatedTo, + subtype: 'colleague', + metadata: { confidence: 0.4 } as object + }) + ).rejects.toThrow(/metadata\.confidence is a reserved field.*RESERVED_RELATION_FIELDS/s) + }) + + it('updateRelation() throws on a reserved key in the patch', async () => { + const a = await brain.add({ type: NounType.Person, subtype: 'employee', data: 'A' }) + const b = await brain.add({ type: NounType.Person, subtype: 'employee', data: 'B' }) + const relId = await brain.relate({ + from: a, + to: b, + type: VerbType.ReportsTo, + subtype: 'direct' + }) + await expect( + brain.updateRelation({ id: relId, metadata: { weight: 0.2 } as object }) + ).rejects.toThrow(/metadata\.weight is a reserved field/) + }) + + it('transact() add op throws on a reserved key in the bag', async () => { + await expect( + brain.transact([ + { + op: 'add', + type: NounType.Concept, + subtype: 'general', + data: 'tx', + metadata: { confidence: 0.7 } as object + } + ]) + ).rejects.toThrow(/metadata\.confidence is a reserved field/) + }) + + it('a custom (non-reserved) key in the bag does NOT throw', async () => { + const id = await brain.add({ + type: NounType.Concept, + subtype: 'general', + data: 'ok', + metadata: { status: 'draft', rating: 4 } + }) + const entity = await brain.get(id) + expect(entity?.metadata).toEqual({ status: 'draft', rating: 4 }) + }) + }) + + describe("'remap' policy remaps silently (no warning)", () => { + let brain: Brainy + let warnSpy: ReturnType + + beforeEach(async () => { + warnSpy = vi.spyOn(prodLog, 'warn').mockImplementation(() => {}) + brain = new Brainy(createTestConfig({ reservedFieldPolicy: 'remap' })) + await brain.init() + }) + + afterEach(async () => { + await brain.close() + warnSpy.mockRestore() + }) + + it('lifts user-mutable reserved fields to top-level without warning', async () => { + const id = await brain.add({ + type: NounType.Person, + data: 'remap lift', + metadata: { confidence: 0.8, weight: 0.6, subtype: 'employee', dept: 'eng' } as object + }) + const entity = await brain.get(id) + expect(entity?.confidence).toBe(0.8) + expect(entity?.weight).toBe(0.6) + expect(entity?.subtype).toBe('employee') + expect(entity?.metadata).toEqual({ dept: 'eng' }) + // 'remap' is silent about reserved fields (unrelated storage logs may fire, + // so assert specifically that no reserved-field warning was emitted). + const reservedWarned = warnSpy.mock.calls.some((c) => + String(c[0]).includes('reserved field') + ) + expect(reservedWarned).toBe(false) + }) + + it('preserves _originalId on natural-key ids through the remap path', async () => { + // A speculative view applies the same normalization and maps a natural-key + // id to a stable UUID, preserving the caller's original string. + const base = await brain.now() + const speculative = await base.with([ + { + op: 'add', + id: 'remap-spec-entity', + type: NounType.Concept, + subtype: 'general', + data: 'spec', + metadata: { confidence: 0.65, custom: 'spec' } as object + } + ]) + const entity = await speculative.get('remap-spec-entity') + expect(entity?.confidence).toBe(0.65) + expect(entity?.metadata).toEqual({ custom: 'spec', _originalId: 'remap-spec-entity' }) + await speculative.release() + await base.release() + }) + }) + + describe("'warn' policy remaps AND warns once per key", () => { + let brain: Brainy + let warnSpy: ReturnType + + beforeEach(async () => { + warnSpy = vi.spyOn(prodLog, 'warn').mockImplementation(() => {}) + brain = new Brainy(createTestConfig({ reservedFieldPolicy: 'warn' })) + await brain.init() + }) + + afterEach(async () => { + await brain.close() + warnSpy.mockRestore() + }) + + it('remaps the value (same as remap) and emits a warning naming the field', async () => { + // Use a method+field combo unique to this test so the per-process one-shot + // registry has not already consumed it. + const id = await brain.add({ + type: NounType.Person, + data: 'warn lift', + // weight is user-mutable → remapped; this is the only 'warn'-policy + // add({ weight }) in the suite, so the one-shot warning fires here. + metadata: { weight: 0.42, dept: 'eng' } as object + }) + const entity = await brain.get(id) + // Value is honored (remap still happens under 'warn'). + expect(entity?.weight).toBe(0.42) + expect(entity?.metadata).toEqual({ dept: 'eng' }) + // And a warning was emitted naming the reserved field. + expect(warnSpy).toHaveBeenCalled() + const warned = warnSpy.mock.calls.some((c) => + String(c[0]).includes("'weight'") + ) + expect(warned).toBe(true) + }) + + it('warns for system-managed keys too (closes the historical gap)', async () => { + // Pre-8.0 only system-managed fields warned; 'warn' warns for every key. + // 'createdBy' (system-managed on update) is unique to this test. + const id = await brain.add({ type: NounType.Concept, subtype: 'general', data: 'sys' }) + warnSpy.mockClear() + await brain.update({ id, metadata: { createdBy: 'nope', keep: 'me' } as object }) + const entity = await brain.get(id) + // System-managed key dropped; custom field merged. + expect((entity?.metadata as Record)?.createdBy).toBeUndefined() + expect((entity?.metadata as Record)?.keep).toBe('me') + // A warning was emitted for the dropped system-managed key. + const warned = warnSpy.mock.calls.some((c) => + String(c[0]).includes("'createdBy'") + ) + expect(warned).toBe(true) + }) + }) +}) diff --git a/tests/unit/brainy/update-reserved-metadata-remap.test.ts b/tests/unit/brainy/update-reserved-metadata-remap.test.ts index 8e1a97c3..31713f99 100644 --- a/tests/unit/brainy/update-reserved-metadata-remap.test.ts +++ b/tests/unit/brainy/update-reserved-metadata-remap.test.ts @@ -10,13 +10,19 @@ * consumer's confidence-evolution writes no-oped for weeks before being * caught by reading values back. * - * 8.0 contract under test (every write path, entities AND relationships): + * These tests pin the LEGACY REMAP behavior, which in 8.0 is opt-in via + * `reservedFieldPolicy: 'remap'` (the default is `'throw'` — see the policy + * matrix in tests/unit/brainy/reserved-field-policy.test.ts). The brain in + * every test below is constructed with `reservedFieldPolicy: 'remap'` so these + * deep correctness assertions about the remap path stay exercised. + * + * Remap contract under test (every write path, entities AND relationships): * - user-mutable reserved fields (`confidence`, `weight`, `subtype` — plus * `service`/`createdBy` at add()/relate() time) remap from the metadata * bag to their dedicated top-level param, with top-level winning when both * are present; * - system-managed reserved fields (`createdAt`, `_rev`, `noun`/`verb`, - * `data`, …) are dropped from the bag (one-shot warning); + * `data`, …) are dropped from the bag; * - the same normalization applies to `transact()` operations and `with()` * speculative views; * - reads NEVER echo a reserved field inside `metadata`. @@ -32,11 +38,12 @@ import { Brainy } from '../../../src/index.js' import { NounType, VerbType } from '../../../src/types/graphTypes.js' import { createTestConfig } from '../../helpers/test-factory.js' -describe('reserved-field metadata remap (8.0 contract)', () => { +describe('reserved-field metadata remap (8.0 legacy remap path)', () => { let brain: Brainy beforeEach(async () => { - brain = new Brainy(createTestConfig()) + // The remap path is opt-in in 8.0 (default policy is 'throw'). + brain = new Brainy(createTestConfig({ reservedFieldPolicy: 'remap' })) await brain.init() }) diff --git a/tests/unit/brainy/visibility.test.ts b/tests/unit/brainy/visibility.test.ts index 8bd37ebd..a5a02422 100644 --- a/tests/unit/brainy/visibility.test.ts +++ b/tests/unit/brainy/visibility.test.ts @@ -198,36 +198,60 @@ describe('visibility (8.0 reserved field)', () => { expect(entity?.visibility).toBeUndefined() }) - it('an untyped caller passing visibility inside metadata is normalized (lifted to top-level)', async () => { + it('an untyped caller passing visibility inside metadata is normalized under reservedFieldPolicy:"remap" (lifted to top-level)', async () => { // Simulate a JavaScript caller smuggling the reserved key past the compile-time guard. - const id = await brain.add({ - type: NounType.Concept, - data: 'y', - metadata: { visibility: 'internal', tag: 't' } as object - }) - const entity = await brain.get(id) - // Lifted to the top-level field… - expect(entity?.visibility).toBe('internal') - // …and stripped from the metadata bag. - expect((entity?.metadata as Record)?.visibility).toBeUndefined() - expect((entity?.metadata as Record)?.tag).toBe('t') - // It is excluded from the default count, exactly like a top-level internal write. - expect(await brain.getNounCount()).toBe(0) + // The legacy remap behavior is now opt-in (8.0 default is 'throw'). + const remapBrain = new Brainy(createTestConfig({ reservedFieldPolicy: 'remap' })) + await remapBrain.init() + try { + const id = await remapBrain.add({ + type: NounType.Concept, + data: 'y', + metadata: { visibility: 'internal', tag: 't' } as object + }) + const entity = await remapBrain.get(id) + // Lifted to the top-level field… + expect(entity?.visibility).toBe('internal') + // …and stripped from the metadata bag. + expect((entity?.metadata as Record)?.visibility).toBeUndefined() + expect((entity?.metadata as Record)?.tag).toBe('t') + // It is excluded from the default count, exactly like a top-level internal write. + expect(await remapBrain.getNounCount()).toBe(0) + } finally { + await remapBrain.close() + } }) - it('a "system" value smuggled through metadata is dropped, not honored', async () => { + it('a "system" value smuggled through metadata is dropped under reservedFieldPolicy:"remap", not honored', async () => { // 'system' is Brainy-only; an untyped caller must not be able to set it. - const id = await brain.add({ - type: NounType.Concept, - data: 'z', - metadata: { visibility: 'system' } as object - }) - const entity = await brain.get(id) - // The smuggled 'system' was dropped → entity stays public (counted, visible). - expect(entity?.visibility).toBeUndefined() - expect(await brain.getNounCount()).toBe(1) - const found = await brain.find({ type: NounType.Concept, limit: 10 }) - expect(found.map((r) => r.id)).toContain(id) + const remapBrain = new Brainy(createTestConfig({ reservedFieldPolicy: 'remap' })) + await remapBrain.init() + try { + const id = await remapBrain.add({ + type: NounType.Concept, + data: 'z', + metadata: { visibility: 'system' } as object + }) + const entity = await remapBrain.get(id) + // The smuggled 'system' was dropped → entity stays public (counted, visible). + expect(entity?.visibility).toBeUndefined() + expect(await remapBrain.getNounCount()).toBe(1) + const found = await remapBrain.find({ type: NounType.Concept, limit: 10 }) + expect(found.map((r) => r.id)).toContain(id) + } finally { + await remapBrain.close() + } + }) + + it('an untyped caller passing visibility inside metadata throws under the default policy', async () => { + // 8.0 default: no silent remap — a reserved key in the bag is a loud error. + await expect( + brain.add({ + type: NounType.Concept, + data: 'throws', + metadata: { visibility: 'internal', tag: 't' } as object + }) + ).rejects.toThrow(/visibility.*reserved field/) }) }) })