diff --git a/src/errors/brainyError.ts b/src/errors/brainyError.ts index 2b366cb6..f627ba00 100644 --- a/src/errors/brainyError.ts +++ b/src/errors/brainyError.ts @@ -10,6 +10,7 @@ export type BrainyErrorType = | 'NOT_FOUND' | 'RETRY_EXHAUSTED' | 'VALIDATION' + | 'INVALID_QUERY' | 'FIELD_NOT_INDEXED' | 'GRAPH_INDEX_NOT_READY' | 'METADATA_INDEX_NOT_READY' diff --git a/src/utils/metadataFilter.ts b/src/utils/metadataFilter.ts index ed0bb1c1..b763dcd5 100644 --- a/src/utils/metadataFilter.ts +++ b/src/utils/metadataFilter.ts @@ -5,6 +5,7 @@ */ import { SearchResult, HNSWNoun, HNSWNounWithMetadata } from '../coreTypes.js' +import { BrainyError } from '../errors/brainyError.js' /** * Brainy Field Operators (BFO) - Our own field query system @@ -30,6 +31,7 @@ export interface BrainyFieldOperators { // Array/Set operators oneOf?: any[] + in?: any[] // documented alias for oneOf noneOf?: any[] contains?: any excludes?: any @@ -70,6 +72,65 @@ export interface MetadataFilterOptions { } } +/** + * Value-level operators (the keys inside `{ field: { : operand } }`) — the + * complete documented set, kept in lockstep with the `matchesQuery` switch below + * and the metadata-index path. `in` is the documented alias for `oneOf`. + */ +const VALUE_OPERATORS = new Set([ + 'equals', 'eq', 'notEquals', 'ne', + 'greaterThan', 'gt', 'greaterThanOrEqual', 'gte', + 'lessThan', 'lt', 'lessThanOrEqual', 'lte', + 'between', 'oneOf', 'in', 'noneOf', + 'contains', 'excludes', 'hasAll', 'length', + 'exists', 'missing', 'matches', 'startsWith', 'endsWith' +]) + +/** Filter-level logical operators (siblings of field names). */ +const LOGICAL_OPERATORS = new Set(['allOf', 'anyOf', 'not']) + +/** + * Validate a `where` filter's operators up front, throwing a typed + * `BrainyError('INVALID_QUERY')` on the first unrecognized operator — so a typo + * like `{ subtype: { notIn: [...] } }` fails LOUD instead of silently matching + * nothing (the invisible-degrade class). Called by `find()` before either the + * index path or the in-memory matcher runs, so the throw fires even when the + * result set is empty (the index path would otherwise return `[]` without ever + * invoking the matcher). Nested fields use dot notation (`{ 'a.b': v }`), so a + * field's object value carries operators, never sub-field names. + * + * @param filter - the user-supplied `where` clause (validated raw, before any + * internal field injection like visibility or `type`→`noun`). + * @throws BrainyError('INVALID_QUERY') naming the bad operator + the valid set. + */ +export function validateWhereFilter(filter: unknown): void { + if (!filter || typeof filter !== 'object' || Array.isArray(filter)) return + for (const [key, value] of Object.entries(filter as Record)) { + if (LOGICAL_OPERATORS.has(key)) { + if (key === 'not') { + validateWhereFilter(value) + } else if (Array.isArray(value)) { + for (const sub of value) validateWhereFilter(sub) + } + continue + } + // A field key. Its value is a scalar/array (equality) — nothing to validate — + // or an object of value-operators, every key of which must be recognized. + if (value && typeof value === 'object' && !Array.isArray(value)) { + for (const op of Object.keys(value as Record)) { + if (!VALUE_OPERATORS.has(op)) { + throw new BrainyError( + `Unknown filter operator "${op}" on field "${key}". Valid operators: ` + + `${[...VALUE_OPERATORS].sort().join(', ')}. For nested fields use dot ` + + `notation, e.g. { '${key}.subfield': value }.`, + 'INVALID_QUERY' + ) + } + } + } + } +} + /** * Check if a value matches a query with operators */ @@ -103,6 +164,7 @@ function matchesQuery(value: any, query: any): boolean { if (typeof value !== 'number' || typeof operand !== 'number' || !(value > operand)) return false break case 'gte': + case 'greaterThanOrEqual': if (typeof value !== 'number' || typeof operand !== 'number' || !(value >= operand)) return false break case 'lessThan': @@ -110,6 +172,7 @@ function matchesQuery(value: any, query: any): boolean { if (typeof value !== 'number' || typeof operand !== 'number' || !(value < operand)) return false break case 'lte': + case 'lessThanOrEqual': if (typeof value !== 'number' || typeof operand !== 'number' || !(value <= operand)) return false break case 'between': @@ -119,6 +182,7 @@ function matchesQuery(value: any, query: any): boolean { // Array/Set operators case 'oneOf': + case 'in': // documented alias for oneOf if (!Array.isArray(operand) || !operand.includes(value)) return false break case 'noneOf': @@ -161,22 +225,26 @@ function matchesQuery(value: any, query: any): boolean { break default: - // Unknown operator, treat as field name - if (!matchesFieldQuery(value, op, operand)) return false + // Unknown operator. The old behavior treated any unknown key as a + // nested-object field name (an UNDOCUMENTED fallback — dot notation + // `{ 'a.b': v }` is the supported nested form), which silently swallowed + // operator typos: `{ x: { notIn: [...] } }` matched nothing instead of + // erroring. Fail loud. find() validates the whole where clause up front + // (validateWhereFilter), so a typo throws even on an empty result set; + // this is the belt-and-suspenders for any matcher caller that bypasses + // that path. + throw new BrainyError( + `Unknown filter operator "${op}". Valid operators: ` + + `${[...VALUE_OPERATORS].sort().join(', ')}. For nested fields use dot ` + + `notation, e.g. { 'address.city': 'NYC' }.`, + 'INVALID_QUERY' + ) } } return true } -/** - * Check if a field matches a query - */ -function matchesFieldQuery(obj: any, field: string, query: any): boolean { - const value = getNestedValue(obj, field) - return matchesQuery(value, query) -} - /** * Get nested value from object using dot notation */ diff --git a/src/utils/metadataIndex.ts b/src/utils/metadataIndex.ts index 830fbfa6..fd325319 100644 --- a/src/utils/metadataIndex.ts +++ b/src/utils/metadataIndex.ts @@ -1166,8 +1166,17 @@ export class MetadataIndexManager implements MetadataIndexProvider { private extractIndexableFields(data: any): Array<{ field: string, value: any }> { const fields: Array<{ field: string, value: any }> = [] - // Fields that should NEVER be indexed (vectors, embeddings, large arrays, HNSW internals) - const NEVER_INDEX = new Set(['vector', 'embedding', 'embeddings', 'connections', 'level', 'id']) + // Fields that should NEVER be indexed: bulk structural payloads that would + // blow up the index (the 384-dim vector, embeddings, the adjacency list). + // These are also caught by the array-size guard below, but naming them is + // belt-and-suspenders. NOTE: `level` was previously here (an HNSW node's + // layer) but it never actually reaches this path — every caller passes a + // metadata bag or Entity record, neither of which carries the node's + // `level` — so its only effect was to silently drop a legitimate USER + // metadata field named `level` (log level, skill level, access level…), + // making `where: { level: … }` return nothing. Removed. (`id` stays: it is + // the reserved entity-identity field, resolved specially by find().) + const NEVER_INDEX = new Set(['vector', 'embedding', 'embeddings', 'connections', 'id']) const extract = (obj: any, prefix = ''): void => { for (const [key, value] of Object.entries(obj)) { @@ -1278,7 +1287,10 @@ export class MetadataIndexManager implements MetadataIndexProvider { return data.map(d => this.extractTextContent(d)).filter(Boolean).join(' ') } if (typeof data === 'object') { - const skipKeys = new Set(['vector', 'embedding', 'embeddings', 'connections', 'level', 'id']) + // Mirror of NEVER_INDEX for the text-extraction path: bulk structural + // payloads only. `level` removed for the same reason (it silently dropped + // a real user field from hybrid text search too). + const skipKeys = new Set(['vector', 'embedding', 'embeddings', 'connections', 'id']) const texts: string[] = [] for (const [key, value] of Object.entries(data)) { // Skip internal fields and numeric keys (array indices) diff --git a/tests/unit/where-operator-validation.test.ts b/tests/unit/where-operator-validation.test.ts new file mode 100644 index 00000000..34dc1d7e --- /dev/null +++ b/tests/unit/where-operator-validation.test.ts @@ -0,0 +1,82 @@ +/** + * Where-clause operator correctness (8.0.12) — the three query-layer fixes: + * + * 1. Unknown operators fail LOUD. A typo like `{ role: { notIn: [...] } }` or + * any gibberish operator now throws BrainyError('INVALID_QUERY') instead of + * silently matching nothing (the invisible-degrade class). Validated up + * front in find(), so it throws even when the result set is empty. + * 2. Documented operators the in-memory matcher was missing now work: `in` + * (alias for oneOf) and the long-form comparison aliases + * `greaterThanOrEqual` / `lessThanOrEqual`. + * 3. A user metadata field named `level` is indexable + queryable — it was + * silently dropped by an over-broad HNSW-internal skip-list, so + * `where: { level: ... }` returned nothing. + */ +import { describe, it, expect, afterEach } from 'vitest' +import { Brainy, NounType, BrainyError } from '../../src/index.js' + +const brains: any[] = [] +async function makeBrain(): Promise { + const brain: any = new Brainy({ storage: { type: 'memory' }, requireSubtype: false, silent: true, plugins: [] }) + await brain.init() + brains.push(brain) + return brain +} +afterEach(async () => { + for (const b of brains.splice(0)) await b.close().catch(() => {}) +}) + +const V = () => Array.from({ length: 384 }, () => Math.random()) + +describe('Where-operator validation + documented-operator coverage (8.0.12)', () => { + it('unknown operators throw INVALID_QUERY instead of silently returning [] (item 3)', async () => { + const brain = await makeBrain() + for (const role of ['staff', 'customer', 'vendor']) { + await brain.add({ data: role, type: NounType.Person, metadata: { role }, vector: V() }) + } + // notIn is not an operator (use ne / not+in). It must THROW, not match nothing. + await expect(brain.find({ type: NounType.Person, where: { role: { notIn: ['staff'] } } })) + .rejects.toBeInstanceOf(BrainyError) + await expect(brain.find({ type: NounType.Person, where: { role: { notIn: ['staff'] } } })) + .rejects.toMatchObject({ type: 'INVALID_QUERY' }) + // Gibberish likewise. + await expect(brain.find({ where: { role: { blorp: 1 } } })).rejects.toMatchObject({ type: 'INVALID_QUERY' }) + // Nested inside a logical operator is still validated. + await expect(brain.find({ where: { not: { role: { nope: 1 } } } })).rejects.toMatchObject({ type: 'INVALID_QUERY' }) + await expect(brain.find({ where: { anyOf: [{ role: { xx: 1 } }] } })).rejects.toMatchObject({ type: 'INVALID_QUERY' }) + }) + + it('the documented `in` alias and long-form comparison aliases work (item 2)', async () => { + const brain = await makeBrain() + for (let i = 0; i < 9; i++) { + await brain.add({ data: 'n' + i, type: NounType.Person, metadata: { role: ['staff', 'customer', 'vendor'][i % 3], score: i }, vector: V() }) + } + await brain.flush() + // `in` === oneOf + expect((await brain.find({ type: NounType.Person, where: { role: { in: ['customer', 'vendor'] } } })).length).toBe(6) + expect((await brain.find({ type: NounType.Person, where: { not: { role: { in: ['staff'] } } } })).length).toBe(6) + // long-form comparison aliases (were silently unhandled by the in-memory matcher) + expect((await brain.find({ type: NounType.Person, where: { score: { greaterThanOrEqual: 6 } } })).length).toBe(3) // 6,7,8 + expect((await brain.find({ type: NounType.Person, where: { score: { lessThanOrEqual: 2 } } })).length).toBe(3) // 0,1,2 + }) + + it('a user metadata field named "level" is indexable + queryable (item: HNSW field-name collision)', async () => { + const brain = await makeBrain() + for (let i = 0; i < 9; i++) { + await brain.add({ data: 'entity ' + i, type: NounType.Person, metadata: { level: i }, vector: V() }) + } + await brain.flush() + expect((await brain.find({ type: NounType.Person, where: { level: 4 } })).length).toBe(1) // exact + expect((await brain.find({ type: NounType.Person, where: { level: { gt: 4 } } })).length).toBe(4) // 5..8 + expect((await brain.find({ type: NounType.Person, where: { level: { exists: true } } })).length).toBe(9) + }) + + it('valid operators are unaffected — control', async () => { + const brain = await makeBrain() + for (const s of ['a', 'b', 'c']) await brain.add({ data: s, type: NounType.Concept, metadata: { tag: s }, vector: V() }) + await brain.flush() + expect((await brain.find({ type: NounType.Concept, where: { tag: { oneOf: ['a', 'b'] } } })).length).toBe(2) + expect((await brain.find({ type: NounType.Concept, where: { tag: 'c' } })).length).toBe(1) + expect((await brain.find({ type: NounType.Concept, where: { tag: { exists: true } } })).length).toBe(3) + }) +})