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
*/