soulcraftlabs/open-brainy
Archived
2
0
Fork 0
Code Issues 1 Pull requests Projects Releases 345 Packages Wiki Activity

fix(metadata): the legacy sparse range path orders values, or refuses — never ranks by hash

Browse source 
`getIdsForRange` routes two ways. The column store compares RAW values and is
correct. The legacy sparse chunk index — the pre-7.20.0 fallback, still read for
workspaces that have not been rebuilt — compared normalizeValue() output, and
normalizeValue carries an escape hatch that destroys order on purpose: a string
over 100 characters becomes a short hash so it can serve as a filesystem-safe
key. Ordering hashes ranks rows by digest.

Two shapes, both silent:

(a) A LONG BOUND against ordinary values. `{ gte: <a 120-character string> }`
    collapsed the BOUND to `__HASH_…`, whose leading underscores sort below
    every letter — so a bound that should have excluded everything matched the
    entire field instead. Measured on the fixture here: 3 of 3 rows returned
    where 0 is correct. This shape reaches a caller who never stored a long
    value at all.

(b) LONG VALUES in the index. The field was persisted hashed, so its order is
    not recoverable from this index. The old code compared the digests anyway
    and returned a subset chosen by hash — 1 of 3 rows, the wrong one.

Bounds are now normalized WITHOUT the hash escape hatch, so a long bound stays
comparable and (a) is simply fixed. Where the persisted KEY is a hash the order
does not exist to be computed, and the query throws a typed
BrainyError('INVALID_QUERY') naming the field and the cure. The refusal is
checked before chunk SELECTION as well as during the scan: selection orders the
bounds against each chunk's zone map, and its failure mode is an empty answer —
the quietest wrong answer of all. Equality on a hashed field is untouched; only
ordering is refused.

KNOWN, NAMED DIVERGENCE, recorded in the doc comment rather than papered over:
the persisted keys are also lower-cased and trimmed, so this path's string
ranges are case-INSENSITIVE where the column store's are not. The raw values are
not in the index to compare — that is a property of the bytes a pre-7.20.0
engine wrote, and it ends when the column store adopts the field.

The pin builds a genuine legacy index through the same ChunkManager /
SparseIndex doors that engine wrote through, into a field the column store does
not serve. The chunk write path was removed in 11be039, so that is the only way
to build the shape this read path exists for.
This commit is contained in:
David Snelling 2026-09-02 13:37:08 -07:00
parent 5e720d17ae
commit a7eb7f5222
2 changed files with 366 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

118
src/utils/metadataIndex.ts
View file

@ -961,9 +961,41 @@ export class MetadataIndexManager implements MetadataIndexProvider {
}
/**
* Get IDs for a range using chunked sparse index with zone maps and roaring bitmaps
* Now fully lazy-loaded via UnifiedCache (no local sparseIndices Map)
* Normalize min/max for timestamp bucketing before comparison
* Get IDs for a range using the legacy chunked sparse index (zone maps +
* roaring bitmaps). Lazy-loaded via UnifiedCache.
*
* ORDER IS NOT A KEY. This path compares NORMALIZED values, and
* {@link normalizeValue} carries an escape hatch that is order-destroying by
* design: a string over 100 characters is replaced by {@link hashValue}'s
* digest so it can be used as a filesystem-safe key. Feeding that digest to
* an ORDERING comparison — which is what a `gte` / `lt` / `between` does —
* ranks rows by hash. The result is not empty and not an error: it is a
* confidently ordered wrong answer, and it disagrees with the column-store
* path (`getIdsForRange` above), which compares raw values and is correct.
*
* Two changes hold the line here:
*
* 1. THE BOUNDS ARE NEVER HASHED. They are normalized with `allowHash =
* false`, so a long bound stays comparable instead of collapsing to a
* digest. This alone fixes the common shape — a long bound queried
* against ordinary short values, where the digest sorts below every
* letter and `gte` therefore matched the entire store.
*
* 2. A HASHED KEY IS REFUSED, NEVER GUESSED. The persisted keys are whatever
* the pre-7.20.0 writer normalized them to, so a field whose values ran
* long is stored hashed and its order is simply not recoverable from this
* index. Rather than compare digests, the query throws a typed
* `BrainyError('INVALID_QUERY')` naming the field, the bound and the cure.
* Loud beats wrong.
*
* KNOWN, NAMED DIVERGENCE. The persisted keys are also lower-cased and
* trimmed by `normalizeValue`, so this path's string ranges are
* CASE-INSENSITIVE where the column store's are not. That is a property of
* the bytes a pre-7.20.0 engine wrote, not of the comparison: the raw values
* are not in the index to compare. The bounds are normalized into the same
* case-folded space so the comparison is at least self-consistent, and the
* divergence disappears with the field itself once the column store adopts
* it. See the module note on `getIdsFromChunks` for the path's lifetime.
*/
private async getIdsFromChunksForRange(
field: string,
@ -979,9 +1011,27 @@ export class MetadataIndexManager implements MetadataIndexProvider {
}
// Normalize min/max for consistent comparison with indexed values
// (indexed values are bucketed for timestamps, so we must bucket the query bounds too)
const normalizedMin = min !== undefined ? this.normalizeValue(min, field) : undefined
const normalizedMax = max !== undefined ? this.normalizeValue(max, field) : undefined
// (indexed values are bucketed for timestamps, so we must bucket the query
// bounds too) — but NEVER through the hash escape hatch, which would make
// the bound incomparable. See the doc comment above.
const normalizedMin = min !== undefined ? this.normalizeValue(min, field, false) : undefined
const normalizedMax = max !== undefined ? this.normalizeValue(max, field, false) : undefined
// REFUSE BEFORE SELECTING. Chunk selection itself orders values: it tests
// the bounds against each chunk's zone-map min/max. If those are hashes the
// selection is already meaningless — and its failure mode is an EMPTY
// answer (no chunk appears to overlap), which is the quietest wrong answer
// of all. So the key space is checked here, before a single chunk is
// chosen, and again per key below for a chunk whose zone map happens to
// read clean.
for (const chunkId of sparseIndex.getAllChunkIds()) {
const zoneMap = sparseIndex.getChunk(chunkId)?.zoneMap
for (const bound of [zoneMap?.min, zoneMap?.max]) {
if (typeof bound === 'string' && MetadataIndexManager.isHashedValue(bound)) {
throw MetadataIndexManager.rangeOverHashedIndex(field)
}
}
}
// Find candidate chunks using zone maps
const candidateChunkIds = sparseIndex.findChunksForRange(normalizedMin, normalizedMax)
@ -996,6 +1046,13 @@ export class MetadataIndexManager implements MetadataIndexProvider {
const chunk = await this.chunkManager.loadChunk(field, chunkId)
if (chunk) {
for (const [value, bitmap] of chunk.entries) {
// A hashed key carries no order. Refuse the range rather than rank by
// digest — the whole answer is unsound, so failing on the first one
// is the honest outcome.
if (MetadataIndexManager.isHashedValue(value)) {
throw MetadataIndexManager.rangeOverHashedIndex(field)
}
// Check if value is in range using numeric-aware comparison
// (normalizeValue converts numbers to strings, so we must compare numerically)
let inRange = true
@ -1024,6 +1081,25 @@ export class MetadataIndexManager implements MetadataIndexProvider {
return this.idMapper.intsIterableToUuids(allIntIds)
}
/**
* The refusal a range query gets when the legacy sparse index holds hashed
* keys for the field. Names the field and the cure; never a wrong answer.
*/
private static rangeOverHashedIndex(field: string): BrainyError {
return new BrainyError(
`Range query on field "${field}" cannot be served by the legacy sparse index: ` +
`its values were persisted as hashes (values over 100 characters are stored ` +
`hashed to stay within filesystem name limits), and a hash carries no order — ` +
`comparing them would return a confidently ordered wrong answer. ` +
`Equality (\`where: { ${field}: value }\`) still works on this index. ` +
`To range over this field, let the column store adopt it: run ` +
`brain.repairIndex({ rebuild: ['metadata'] }), which rebuilds the field into ` +
`the column store, where ranges compare raw values.`,
'INVALID_QUERY',
false
)
}
/**
* Get roaring bitmap for a field-value pair without converting to UUIDs
* This is used for fast multi-field intersection queries using hardware-accelerated bitmap AND
@ -1191,8 +1267,17 @@ export class MetadataIndexManager implements MetadataIndexProvider {
* value-based detection (DuckDB-inspired). Analyzes actual data values, not names.
*
* NO FALLBACKS - Pure value-based detection only.
*
* @param value - The value to normalize.
* @param field - Optional field name (drives the per-field statistics strategy).
* @param allowHash - Whether the >100-character escape hatch may fire. TRUE
* everywhere a normalized value is used as a KEY (equality postings, chunk
* entries, filenames) — that is what the hash exists for. FALSE on the
* ORDER-comparing path: a hash is deliberately order-destroying, so a
* bound that hashes can only be compared as nonsense. See
* {@link isHashedValue} and `getIdsFromChunksForRange`.
*/
private normalizeValue(value: any, field?: string): string {
private normalizeValue(value: any, field?: string, allowHash: boolean = true): string {
if (value === null || value === undefined) return '__NULL__'
if (typeof value === 'boolean') return value ? '__TRUE__' : '__FALSE__'
@ -1250,21 +1335,34 @@ export class MetadataIndexManager implements MetadataIndexProvider {
// Default normalization
if (typeof value === 'number') return value.toString()
if (Array.isArray(value)) {
const joined = value.map(v => this.normalizeValue(v, field)).join(',')
const joined = value.map(v => this.normalizeValue(v, field, allowHash)).join(',')
// Hash very long array values to avoid filesystem limits
if (joined.length > 100) {
if (allowHash && joined.length > 100) {
return this.hashValue(joined)
}
return joined
}
const stringValue = String(value).toLowerCase().trim()
// Hash very long string values to avoid filesystem limits
if (stringValue.length > 100) {
if (allowHash && stringValue.length > 100) {
return this.hashValue(stringValue)
}
return stringValue
}
/**
* Is this normalized value a HASH rather than the value itself?
*
* {@link hashValue} is an escape hatch for filesystem name limits, and it is
* deliberately order-destroying: two values whose hashes compare one way
* routinely compare the other way themselves. Anything that ORDERS normalized
* values has to know when it is holding one, because comparing hashes yields
* a confident, wrong answer rather than an error.
*/
private static isHashedValue(normalized: string): boolean {
return normalized.startsWith('__HASH_')
}
/**
* Create a short hash for long values to avoid filesystem filename limits
*/

258
tests/unit/utils/metadataIndex-sparse-range-collation.test.ts Normal file
View file

@ -0,0 +1,258 @@
/**
* @module tests/unit/utils/metadataIndex-sparse-range-collation
* @description RANGE QUERIES ON THE LEGACY SPARSE INDEX — order, or a refusal.
* Never a confidently ordered wrong answer.
*
* THE TWO RANGE PATHS. `getIdsForRange` routes a `gte` / `lt` / `between` two
* ways. The column store compares RAW values and is correct. The legacy sparse
* chunk index — the pre-7.20.0 fallback, still read for workspaces that have
* not been rebuilt — compared `normalizeValue()` output, and `normalizeValue`
* carries an escape hatch that destroys order on purpose: a string over 100
* characters is replaced by a short hash so it can serve as a filesystem-safe
* key. Ordering hashes ranks rows by digest.
*
* THE DEFECT, IN TWO SHAPES.
*
* (a) A LONG BOUND against ordinary values. `where: { title: { gte: <a
* 120-character string> } }` collapsed the BOUND to `__HASH_…`, whose
* leading underscores sort below every letter — so a bound that should
* have excluded everything matched the entire field instead. This is the
* shape that reaches a caller who never stored a long value at all.
*
* (b) LONG VALUES in the index. A field whose values ran long was persisted
* hashed, so its order is not recoverable from this index at all. The old
* code compared the digests anyway and returned a subset chosen by hash.
*
* THE LAW. Bounds are normalized WITHOUT the hash escape hatch, so a long
* bound stays comparable — (a) is simply fixed. Where the persisted KEY is a
* hash, the order does not exist to be computed, and the query throws a typed
* `BrainyError('INVALID_QUERY')` naming the field and the cure — (b) is
* refused by name. Loud beats wrong.
*
* THE FIXTURE is a genuine legacy index: it is written through the same
* `ChunkManager` / `SparseIndex` doors a pre-7.20.0 engine wrote through, with
* keys normalized exactly as that engine normalized them, into a field the
* column store does not serve. The chunk WRITE path was removed in 11be039, so
* this is the only way the shape the read path exists for can be built.
*
* NOT CLAIMED HERE. The persisted keys are also lower-cased and trimmed by
* `normalizeValue`, so this path's string ranges are case-INSENSITIVE where
* the column store's are not. The raw values are not in the index to compare —
* that divergence is a property of the bytes on disk and it ends when the
* column store adopts the field. It is named in `getIdsFromChunksForRange`'s
* doc comment rather than papered over.
*/
import { describe, it, expect, beforeEach } from 'vitest'
import { Brainy } from '../../../src/brainy'
import { NounType } from '../../../src/types/graphTypes'
import { SparseIndex, ChunkManager } from '../../../src/utils/metadataIndexChunking'
import { BrainyError } from '../../../src/errors/brainyError'
/** The field the legacy index covers — deliberately never given to a row, so
* the column store never learns it and the sparse fallback is the only path. */
const FIELD = 'legacyTitle'
/**
* Write a legacy sparse index for `field` exactly as a pre-7.20.0 engine did:
* one chunk, keys normalized through the index's own `normalizeValue`, ids as
* roaring bitmaps, a zone map and a bloom filter over the chunk.
*
* @param brain - The live brain whose metadata index gains the legacy field.
* @param field - Field name to index.
* @param valueToIds - Raw value → the entity ids that carried it.
*/
async function writeLegacySparseIndex(
brain: any,
field: string,
valueToIds: Array<[string, string[]]>
): Promise<void> {
const index = brain.metadataIndex
const chunkManager: ChunkManager = index.chunkManager
const sparseIndex = new SparseIndex(field)
// The keys a pre-7.20.0 writer persisted: normalizeValue output, hash escape
// hatch and all. This is what makes the fixture the real shape.
const chunk = await chunkManager.createChunk(field)
for (const [value, ids] of valueToIds) {
const key = index.normalizeValue(value, field)
for (const id of ids) await chunkManager.addToChunk(chunk, key, id)
}
await chunkManager.saveChunk(chunk)
sparseIndex.registerChunk(
{
chunkId: chunk.chunkId,
field,
valueCount: chunk.entries.size,
idCount: Array.from(chunk.entries.values()).reduce((s: number, b: any) => s + b.size, 0),
zoneMap: (chunkManager as any).calculateZoneMap(chunk),
lastUpdated: Date.now(),
splitThreshold: 80,
mergeThreshold: 20
},
chunkManager.createBloomFilter(chunk)
)
await index.saveSparseIndex(field, sparseIndex)
}
/** A deterministic string of `n` characters starting with `lead`. */
function longString(lead: string, n: number): string {
return lead + 'x'.repeat(n - lead.length)
}
describe('legacy sparse index: range queries order values, or refuse', () => {
let brain: Brainy<any>
let index: any
let ids: string[]
beforeEach(async () => {
brain = new Brainy({ requireSubtype: false, storage: { type: 'memory' } })
await brain.init()
index = (brain as any).metadataIndex
// Rows exist (so the id mapper can resolve them) but carry NO `legacyTitle`
// — the column store must not serve the field the pins query.
ids = []
for (let i = 0; i < 3; i++) {
const id = `row-${i}`
await brain.add({ id, data: `row ${i}`, type: NounType.Thing, metadata: { lane: 'a' }, vector: [] })
ids.push(id)
}
expect(index.columnStore.hasField(FIELD)).toBe(false)
})
describe('(a) a long BOUND against ordinary short values', () => {
// 'apple' < 'mango' < 'zebra', and every bound below is compared against
// these three raw keys.
beforeEach(async () => {
await writeLegacySparseIndex(brain, FIELD, [
['apple', [ids[0]]],
['mango', [ids[1]]],
['zebra', [ids[2]]]
])
})
it('the fixture: the values are stored raw, the long bound is what hashes', () => {
expect(index.normalizeValue('apple', FIELD)).toBe('apple')
// The bound is what the old code collapsed — and a digest sorts below
// every letter, which is exactly why `gte` matched everything.
const bound = longString('zzz', 120)
expect(index.normalizeValue(bound, FIELD)).toMatch(/^__HASH_/)
expect(index.normalizeValue(bound, FIELD) < 'apple').toBe(true)
})
it('gte a bound above every value matches NOTHING (it used to match all)', async () => {
const bound = longString('zzz', 120)
const matched = await index.getIdsForRange(FIELD, bound, undefined, true, true)
expect(matched).toEqual([])
})
it('lte a bound above every value matches EVERY value', async () => {
const bound = longString('zzz', 120)
const matched = await index.getIdsForRange(FIELD, undefined, bound, true, true)
expect(matched).toHaveLength(3)
})
it('gte a long bound below every value matches every value', async () => {
const bound = longString('aaa', 120)
const matched = await index.getIdsForRange(FIELD, bound, undefined, true, true)
expect(matched).toHaveLength(3)
})
it('a long bound orders BETWEEN the values, not below all of them', async () => {
// 'mmm…' sits between 'mango' and 'zebra'.
const bound = longString('mmm', 120)
const matched = await index.getIdsForRange(FIELD, bound, undefined, true, true)
expect(matched).toHaveLength(1)
})
it('short bounds are unchanged — the ordinary case still orders correctly', async () => {
expect(await index.getIdsForRange(FIELD, 'b', undefined, true, true)).toHaveLength(2)
expect(await index.getIdsForRange(FIELD, undefined, 'n', true, true)).toHaveLength(2)
expect(await index.getIdsForRange(FIELD, 'b', 'n', true, true)).toHaveLength(1)
// Strict bounds stay strict.
expect(await index.getIdsForRange(FIELD, 'mango', undefined, false, true)).toHaveLength(1)
expect(await index.getIdsForRange(FIELD, 'mango', undefined, true, true)).toHaveLength(2)
})
})
describe('(b) long VALUES — the index holds hashes, so the range is refused', () => {
beforeEach(async () => {
await writeLegacySparseIndex(brain, FIELD, [
[longString('alpha', 140), [ids[0]]],
[longString('mike', 140), [ids[1]]],
[longString('zulu', 140), [ids[2]]]
])
})
it('the fixture: the persisted keys really are hashes', async () => {
const chunk = await index.chunkManager.loadChunk(FIELD, 0)
const keys = Array.from(chunk.entries.keys()) as string[]
expect(keys).toHaveLength(3)
for (const k of keys) expect(k).toMatch(/^__HASH_/)
// And their digest order is NOT their value order — the wrong answer the
// old code returned was wrong, not merely arbitrary.
const digestOrder = [...keys].sort()
const valueOrder = [
index.normalizeValue(longString('alpha', 140), FIELD),
index.normalizeValue(longString('mike', 140), FIELD),
index.normalizeValue(longString('zulu', 140), FIELD)
]
expect(digestOrder).not.toEqual(valueOrder)
})
it('a range over the hashed field throws a typed refusal naming the field', async () => {
await expect(
index.getIdsForRange(FIELD, longString('mike', 140), undefined, true, true)
).rejects.toThrow(BrainyError)
const err = await index
.getIdsForRange(FIELD, longString('mike', 140), undefined, true, true)
.catch((e: any) => e)
expect(err).toBeInstanceOf(BrainyError)
expect(err.type).toBe('INVALID_QUERY')
expect(err.message).toContain(FIELD)
expect(err.message).toContain('hash')
// The cure is named, not left to the caller to guess.
expect(err.message).toContain('repairIndex')
})
it('every range shape refuses — gte, lte and between alike', async () => {
const lo = longString('alpha', 140)
const hi = longString('zulu', 140)
for (const [min, max] of [
[lo, undefined],
[undefined, hi],
[lo, hi]
] as Array<[any, any]>) {
const err = await index.getIdsForRange(FIELD, min, max, true, true).catch((e: any) => e)
expect(err).toBeInstanceOf(BrainyError)
expect(err.type).toBe('INVALID_QUERY')
}
})
it('EQUALITY still works on the hashed field — only ordering is refused', async () => {
const matched = await index.getIds(FIELD, longString('mike', 140))
expect(matched).toHaveLength(1)
})
})
describe('numeric ranges on the legacy path are untouched', () => {
beforeEach(async () => {
await writeLegacySparseIndex(brain, FIELD, [
['5', [ids[0]]],
['50', [ids[1]]],
['500', [ids[2]]]
])
})
it('numbers still compare numerically, not lexicographically', async () => {
// The whole point of compareNormalizedValues: "50" < "500" numerically
// even though "500" < "50" would hold as strings by prefix.
expect(await index.getIdsForRange(FIELD, 10, undefined, true, true)).toHaveLength(2)
expect(await index.getIdsForRange(FIELD, undefined, 100, true, true)).toHaveLength(2)
expect(await index.getIdsForRange(FIELD, 10, 100, true, true)).toHaveLength(1)
})
})
})