fix(metadata): the indexable-array bound is a named law with a refusal, not a silent skip
An array-valued metadata field indexes one posting per element, so the index has
always carried a ceiling. It was 10, and it was applied by a bare `continue`
deep inside field extraction:
if (Array.isArray(value) && value.length > 10) continue
A row whose `tags` array held ELEVEN entries therefore had that field skipped
entirely — no posting, no error, no warning. The row then failed to match every
filtered search on `tags`, including a query for a tag it demonstrably held, and
the caller had no way to tell that from "no row matches". Eleven tags is not an
exotic shape; the eleventh tag made the row invisible. Measured on the pin here:
the where-clause returns [] on the base for all eleven values.
The ceiling is not the defect. The silence was.
THE LAW. MAX_INDEXED_ARRAY_LENGTH = 64, hardcoded (the zero-config law: no
knob), sitting far above every legitimate multi-value field — tags, authors,
categories, labels, participants — and far below any real embedding width, so
the two populations do not overlap and nobody has to tune it. Arrays of scalars
index in full up to the bound. Above it the WRITE IS REFUSED by name:
MetadataArrayTooLargeError carries the field (its full dotted address), the
length and the bound, and names the three cures. It fires at all four write
doors — add, update, relate, updateRelation — beside the existing forged-system-
key rejection, and walks nested bags because a nested field indexes under its
dotted address exactly like a top-level one.
THE ONE PLACE THE BOUND STILL SKIPS is a row already on disk, written by an
older engine under the old rule and read back by a rebuild, a catch-up fold or a
remove. extractIndexableFields serves all three, so refusing there would make an
existing store un-rebuildable — the row is admitted and the skipped field is
NARRATED with the field, the length and the bound. Never silent, either way.
tests/integration/metadata-vector-exclusion.test.ts carried the old law as a
green assertion ("should skip indexing large arrays (>10 elements)"). It is
rewritten to the new one, plus a case proving a 64-element array indexes in full
and its eleventh element is searchable. The original bug that suite exists for —
per-dimension numeric field explosion — is still asserted on both paths.
This commit is contained in:
parent
a7eb7f5222
commit
0d5ab6077d
6 changed files with 436 additions and 25 deletions
|
|
@ -405,3 +405,68 @@ export class MigrationInProgressError extends BrainyError {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* THE INDEXABLE-ARRAY BOUND. An array-valued metadata field indexes one posting
|
||||
* per element, so an unbounded array is an unbounded write — a 384-float
|
||||
* embedding parked in the metadata bag would mint 384 postings for one row.
|
||||
* The bound exists to keep that out of the index.
|
||||
*
|
||||
* 64 is hardcoded on purpose (the zero-config law: no knob). It sits far above
|
||||
* every legitimate multi-value field the engine has seen — tags, authors,
|
||||
* categories, labels, participant lists — and far below any real embedding
|
||||
* width, so the two populations do not overlap and no caller has to tune it.
|
||||
*
|
||||
* It replaces a limit of 10 that was applied SILENTLY: a row whose `tags` array
|
||||
* held eleven entries had that field skipped entirely and dropped out of every
|
||||
* filtered search on it, with no error, no warning and no way to tell the
|
||||
* difference from "no row matches". A rule this consequential is a law with a
|
||||
* name and a refusal, not a `continue`.
|
||||
*/
|
||||
export const MAX_INDEXED_ARRAY_LENGTH = 64
|
||||
|
||||
/**
|
||||
* A metadata field carries an array longer than {@link MAX_INDEXED_ARRAY_LENGTH}.
|
||||
*
|
||||
* Thrown at the WRITE door (`add` / `update` / `relate` / `updateRelation`), so
|
||||
* the caller learns at the moment of writing that the field will not be
|
||||
* searchable — rather than discovering it later as rows that quietly fail to
|
||||
* match. Carries the field, its length and the bound so a handler can report
|
||||
* or repair without parsing the message.
|
||||
*
|
||||
* The cure is one of: store the long array outside the indexed bag (`data`
|
||||
* carries arbitrary content and is not indexed element-wise); pass an embedding
|
||||
* as the first-class `vector` parameter, which is where a vector belongs; or
|
||||
* shorten the field to the values that are actually queried.
|
||||
*/
|
||||
export class MetadataArrayTooLargeError extends BrainyError {
|
||||
/** The metadata field whose array is too long (its full dotted address). */
|
||||
public readonly field: string
|
||||
/** How many elements that array holds. */
|
||||
public readonly length: number
|
||||
/** The bound it exceeded — {@link MAX_INDEXED_ARRAY_LENGTH}. */
|
||||
public readonly limit: number
|
||||
|
||||
constructor(site: string, field: string, length: number, limit: number) {
|
||||
super(
|
||||
`${site}: metadata field '${field}' holds ${length} array elements, ` +
|
||||
`over the ${limit}-element indexing bound. An array field indexes one ` +
|
||||
`posting per element, so an unbounded array is an unbounded write. ` +
|
||||
`This write is refused rather than indexed partially or skipped silently ` +
|
||||
`— a skipped field drops the row out of every filtered search on '${field}' ` +
|
||||
`with no way to tell that from "nothing matched". ` +
|
||||
`Cures: put the long array in 'data' (stored, not indexed element-wise); ` +
|
||||
`pass an embedding as the first-class 'vector' parameter; or keep only ` +
|
||||
`the values you actually query in '${field}'.`,
|
||||
'VALIDATION',
|
||||
false
|
||||
)
|
||||
this.name = 'MetadataArrayTooLargeError'
|
||||
this.field = field
|
||||
this.length = length
|
||||
this.limit = limit
|
||||
if (Error.captureStackTrace) {
|
||||
Error.captureStackTrace(this, MetadataArrayTooLargeError)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -203,7 +203,7 @@ export { EntityNotFoundError, RelationNotFoundError } from './errors/notFound.js
|
|||
|
||||
// Base error + typed migration-lock error — thrown by any data-plane call while a
|
||||
// brain runs its one-time 7.x→8.0 upgrade; catch to answer HTTP 503 + Retry-After.
|
||||
export { BrainyError, MigrationInProgressError, GraphIndexNotReadyError, MetadataIndexNotReadyError, VectorIndexNotReadyError, ProtectedArtifactError, DerivedArtifactMissingError } from './errors/brainyError.js'
|
||||
export { BrainyError, MigrationInProgressError, GraphIndexNotReadyError, MetadataIndexNotReadyError, VectorIndexNotReadyError, ProtectedArtifactError, DerivedArtifactMissingError, MetadataArrayTooLargeError, MAX_INDEXED_ARRAY_LENGTH } from './errors/brainyError.js'
|
||||
export type { BrainyErrorType } from './errors/brainyError.js'
|
||||
|
||||
// ============= 8.0 Db API — generational MVCC =============
|
||||
|
|
|
|||
|
|
@ -40,7 +40,7 @@ import {
|
|||
import { EntityIdMapper } from './entityIdMapper.js'
|
||||
import { RoaringBitmap32, roaringLibraryInitialize } from './roaring/index.js'
|
||||
import { FieldTypeInference, FieldType } from './fieldTypeInference.js'
|
||||
import { BrainyError } from '../errors/brainyError.js'
|
||||
import { BrainyError, MAX_INDEXED_ARRAY_LENGTH } from '../errors/brainyError.js'
|
||||
|
||||
/**
|
||||
* Fields whose values are stored in the sparse index as BUCKETED values
|
||||
|
|
@ -289,8 +289,10 @@ export class MetadataIndexManager implements MetadataIndexProvider {
|
|||
// No name-based exclude/allow lists — the field-addressing law: every
|
||||
// user field indexes, whatever its name ('content', 'data', 'id',
|
||||
// 'vector', … included). Bulk payloads are kept out by uniform value-
|
||||
// SHAPE rules in extractIndexableFields (arrays >10 never become
|
||||
// posting scalars; >100-char values index hashed), never by name.
|
||||
// SHAPE rules in extractIndexableFields (arrays longer than
|
||||
// MAX_INDEXED_ARRAY_LENGTH never become posting scalars, and the write
|
||||
// door refuses them by name; >100-char values index hashed), never by
|
||||
// field name.
|
||||
}
|
||||
|
||||
// Initialize metadata cache with similar config to search cache
|
||||
|
|
@ -1387,9 +1389,10 @@ export class MetadataIndexManager implements MetadataIndexProvider {
|
|||
* 'content', 'vector' in a bag are ordinary user fields)
|
||||
* - Record-frame plumbing (vector, connections, level, data, _rev, id)
|
||||
* never indexes — that is namespace routing, not a name carve-out
|
||||
* - Value-SHAPE rules apply uniformly to all names: arrays >10 never
|
||||
* become posting scalars; purely numeric key names (array indices)
|
||||
* skip; >100-char values index hashed (normalizeValue)
|
||||
* - Value-SHAPE rules apply uniformly to all names: arrays longer than
|
||||
* MAX_INDEXED_ARRAY_LENGTH never become posting scalars (and say so —
|
||||
* the write door refuses them outright); purely numeric key names
|
||||
* (array indices) skip; >100-char values index hashed (normalizeValue)
|
||||
*/
|
||||
private extractIndexableFields(data: any): Array<{ field: string, value: any }> {
|
||||
const fields: Array<{ field: string, value: any }> = []
|
||||
|
|
@ -1451,13 +1454,37 @@ export class MetadataIndexManager implements MetadataIndexProvider {
|
|||
// This catches vectors stored as objects: {0: 0.1, 1: 0.2, ...}
|
||||
if (/^\d+$/.test(key)) continue
|
||||
|
||||
// Skip large arrays (> 10 elements) - likely vectors or bulk data
|
||||
if (Array.isArray(value) && value.length > 10) continue
|
||||
// THE INDEXABLE-ARRAY BOUND ({@link MAX_INDEXED_ARRAY_LENGTH}). An
|
||||
// array field mints one posting per element, so the index has always
|
||||
// carried a ceiling — it was 10, and it was applied by this bare
|
||||
// `continue`: an eleven-element `tags` array had its whole field
|
||||
// skipped and the row dropped out of every filtered search on it, with
|
||||
// no error, no warning, and nothing to distinguish that from "no row
|
||||
// matches". The ceiling is not the defect; the silence was.
|
||||
//
|
||||
// The write door refuses this shape by name now
|
||||
// (`MetadataArrayTooLargeError`, thrown from paramValidation's
|
||||
// `rejectOversizeIndexArrays`), so a live add/update never reaches
|
||||
// here over the bound. Reaching it means the row is ALREADY on disk —
|
||||
// written by an older engine under the old rule — and this is a
|
||||
// rebuild, a catch-up fold or a remove reading it back. Refusing there
|
||||
// would make an existing store un-rebuildable, so the row is admitted
|
||||
// and the skipped field is NARRATED instead. Never silent, either way.
|
||||
if (Array.isArray(value) && value.length > MAX_INDEXED_ARRAY_LENGTH) {
|
||||
prodLog.warn(
|
||||
`[brainy] metadata field '${fullKey}' holds ${value.length} array elements, ` +
|
||||
`over the ${MAX_INDEXED_ARRAY_LENGTH}-element indexing bound — the field is ` +
|
||||
`NOT indexed for this row, so it will not match a where-clause on '${fullKey}'. ` +
|
||||
`This row predates the bound (the write door refuses this shape now). ` +
|
||||
`Move the long array into 'data', or pass an embedding as the 'vector' parameter.`
|
||||
)
|
||||
continue
|
||||
}
|
||||
|
||||
if (value && typeof value === 'object' && !Array.isArray(value)) {
|
||||
// Recurse into nested objects (but not arrays), keeping the frame
|
||||
extract(value, fullKey, frame)
|
||||
} else if (Array.isArray(value) && value.length <= 10) {
|
||||
} else if (Array.isArray(value)) {
|
||||
// Small arrays: index as multi-value field (all with same field name)
|
||||
// Example: tags: ["javascript", "node"] → field="tags", value="javascript" + field="tags", value="node"
|
||||
for (const item of value) {
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ import { findCallerLocation } from './callerLocation.js'
|
|||
import * as os from 'node:os'
|
||||
import * as fs from 'node:fs'
|
||||
import { parseFieldAddress, UnsupportedFindOptionError } from '../db/fieldAddressing.js'
|
||||
import { MAX_INDEXED_ARRAY_LENGTH, MetadataArrayTooLargeError } from '../errors/brainyError.js'
|
||||
|
||||
const getSystemMemory = (): number => {
|
||||
if (os) {
|
||||
|
|
@ -538,8 +539,53 @@ function rejectForgedSystemKeys(metadata: Record<string, unknown> | undefined, s
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* THE INDEXABLE-ARRAY BOUND, enforced at the write door.
|
||||
*
|
||||
* An array-valued metadata field indexes one posting per element, so the index
|
||||
* has always carried a ceiling. It used to be 10, and it was applied by a bare
|
||||
* `continue` deep inside field extraction: a row whose `tags` array held eleven
|
||||
* entries had that field skipped entirely and dropped out of every filtered
|
||||
* search on it — no error, no warning, and no way for the caller to tell the
|
||||
* difference from "no row matches". Silence is the defect; the ceiling is not.
|
||||
*
|
||||
* The bound is now {@link MAX_INDEXED_ARRAY_LENGTH}, high enough that every
|
||||
* legitimate multi-value field clears it, and it REFUSES here instead of
|
||||
* dropping data downstream. Refusing at the write door is what makes it
|
||||
* actionable: the caller learns at the moment of writing, with the field, the
|
||||
* length and the bound in hand.
|
||||
*
|
||||
* Scope is the caller's own metadata bag — the values that become postings.
|
||||
* Nested bags are walked, because a nested field indexes under its dotted
|
||||
* address exactly like a top-level one. Arrays of OBJECTS are not walked: the
|
||||
* index only ever makes postings from an array's scalar elements.
|
||||
*
|
||||
* @param metadata - The caller's metadata bag (undefined is fine).
|
||||
* @param site - The write door's name, for the message ('add()', 'update()', …).
|
||||
* @throws {MetadataArrayTooLargeError} Naming the field, its length and the bound.
|
||||
*/
|
||||
function rejectOversizeIndexArrays(metadata: Record<string, unknown> | undefined, site: string): void {
|
||||
if (!metadata) return
|
||||
|
||||
const walk = (bag: Record<string, unknown>, prefix: string): void => {
|
||||
for (const [key, value] of Object.entries(bag)) {
|
||||
const address = prefix ? `${prefix}.${key}` : key
|
||||
if (Array.isArray(value)) {
|
||||
if (value.length > MAX_INDEXED_ARRAY_LENGTH) {
|
||||
throw new MetadataArrayTooLargeError(site, address, value.length, MAX_INDEXED_ARRAY_LENGTH)
|
||||
}
|
||||
} else if (value && typeof value === 'object') {
|
||||
walk(value as Record<string, unknown>, address)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
walk(metadata, '')
|
||||
}
|
||||
|
||||
export function validateAddParams(params: AddParams): void {
|
||||
rejectForgedSystemKeys(params.metadata as Record<string, unknown> | undefined, 'add()')
|
||||
rejectOversizeIndexArrays(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;
|
||||
|
|
@ -608,6 +654,7 @@ export function validateAddParams(params: AddParams): void {
|
|||
*/
|
||||
export function validateUpdateParams(params: UpdateParams): void {
|
||||
rejectForgedSystemKeys(params.metadata as Record<string, unknown> | undefined, 'update()')
|
||||
rejectOversizeIndexArrays(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".
|
||||
|
|
@ -682,6 +729,7 @@ export function validateUpdateParams(params: UpdateParams): void {
|
|||
*/
|
||||
export function validateRelateParams(params: RelateParams): void {
|
||||
rejectForgedSystemKeys(params.metadata as Record<string, unknown> | undefined, 'relate()')
|
||||
rejectOversizeIndexArrays(params.metadata as Record<string, unknown> | undefined, 'relate()')
|
||||
// 8.0 verb-id contract (L.7): verb ids are UUIDs, generated by brainy.
|
||||
// RelateParams has no `id` field — an untyped caller passing one would
|
||||
// previously have it silently ignored (a generated UUID was used instead).
|
||||
|
|
@ -731,6 +779,7 @@ export function validateRelateParams(params: RelateParams): void {
|
|||
*/
|
||||
export function validateUpdateRelationParams(params: UpdateRelationParams): void {
|
||||
rejectForgedSystemKeys(params.metadata as Record<string, unknown> | undefined, 'updateRelation()')
|
||||
rejectOversizeIndexArrays(params.metadata as Record<string, unknown> | undefined, 'updateRelation()')
|
||||
if (!params.id) {
|
||||
throw new Error('id is required for updateRelation')
|
||||
}
|
||||
|
|
|
|||
Reference in a new issue