diff --git a/RELEASES.md b/RELEASES.md index 3086332a..bd3efcdd 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -11,6 +11,103 @@ Collective. The SDK wraps it — most products never call Brainy directly. Read --- +## v7.22.0 — 2026-05-15 + +**Affected products:** All. Fixes a silent-data-loss class affecting `find()` and +`brain.stats()`, plus Cortex-compatibility regression from 7.21.0. Recommended +upgrade for everyone on 7.20.x or 7.21.x. + +### Two correctness fixes + +#### 1. `find({ where })` no longer silently returns `[]` (BR-FIND-WHERE-ZERO) + +The 7.20.0 column-store refactor deleted the legacy sparse-index write path +but left `getStats()` and `getIds()` reading from sparse indices. New +workspaces had no sparse-index files, so: +- `brain.stats().entityCount` was `0` regardless of actual entity count +- `find({ where: { ... } })` returned `[]` regardless of actual matches +- `rebuildIndexesIfNeeded()` saw `0` entries → fired the `[Brainy] CRITICAL ... + Second rebuild result: 0 entries` log → application saw no indexed data + +7.22.0 makes the ColumnStore the single source of truth post-7.20.0: +- `MetadataIndex.getStats()` reads from `ColumnStore.getIndexedFields()` and + `idMapper.size`. Entity count is now accurate across writer → close → + reader-open cycles. +- `MetadataIndex.getIdsFromChunks()` throws `BrainyError(FIELD_NOT_INDEXED)` + when neither column store nor legacy sparse index has the field. +- `MetadataIndex.getIdsForFilter()` catches per-clause, logs + `[brainy] find() where-clause referenced unindexed field(s): ...`, returns + `[]`. Production `find()` is now consistent with `brain.explain()`'s + `path: 'none'` diagnostic. + +Separately, `BaseStorage.getNounType()` was hardcoded to return `'thing'` +since a type-cache removal in commit `42ae5be`. Every noun save attributed +the entity to `'thing'` regardless of its actual type, poisoning +`_system/type-statistics.json` and `brain.stats().entitiesByType`. 7.22.0 +restores type-correct attribution: +- New `nounTypeByIdCache: Map` on `BaseStorage`, populated + in `saveNounMetadata_internal` and consumed in `saveNoun_internal`. +- New `flushCounts()` override that persists `nounCountsByType` / + `verbCountsByType` alongside the per-entity counter. Readers opening the + same directory after a writer flush now see correct per-type counts. + +**Self-heal at init.** If `loadTypeStatistics()` detects the poisoned-state +signature (all nouns attributed to `'thing'` with multiple non-`thing` +entities on disk), it auto-runs `rebuildTypeCounts()` once and rewrites the +file. A `[BaseStorage] Detected poisoned type-statistics.json signature ...` +warning is logged. Operators don't need to take any action — the first 7.22 +open of a directory poisoned by 7.20.0/7.21.0 fixes it. + +**`brainy inspect repair`** can also be used to manually trigger +`rebuildTypeCounts()`. It's now public on `BaseStorage`. + +#### 2. Cortex 2.2.x no longer crashes 7.21.0 boot (BR-DEFENSIVE-INTERFACE) + +7.21.0 added new storage-adapter methods (`supportsMultiProcessLocking`, +`acquireWriterLock`, etc.) and called them unconditionally. Cortex 2.2.0's +mmap storage adapter bundles a pre-7.21 `BaseStorage` and doesn't define +them, so Venue's 7.21.0 upgrade attempt threw +`TypeError: this.storage.supportsMultiProcessLocking is not a function` +at boot. + +7.22.0 introduces `hasStorageMethod(name)` on Brainy and guards every new- +method call site. Older adapters degrade with a one-line warning at init: +``` +[brainy] Storage adapter `CortexMmapStorage` predates the 7.21 +multi-process methods. Writer locking and the flush-request RPC are +disabled for this directory. Upgrade the plugin (e.g. +`@soulcraft/cortex` ≥2.3.0) for full enforcement. +``` + +### Dead-code cleanup + +Removed `MetadataIndexManager.dirtyChunks`, `dirtySparseIndices`, and the +`flushDirtyMetadata()` no-op machinery. These accumulators stopped being +populated when the sparse-index write path was deleted in 7.20.0; the +remaining 6 callers wasted async hops on an empty Map iteration. + +### Upgrade notes + +- **Behaviour change:** `find({ where: { unindexedField: ... } })` previously + returned `[]` silently. It now logs a one-line warning and still returns + `[]`. The diagnostic message names the field — use + `brain.explain({ where: { ... } })` for full diagnosis. No code change + needed for callers that already handle empty results. +- **Behaviour change (good):** `brain.stats()` and `brain.health()` now + report accurate per-type counts. If you previously relied on the buggy + `entitiesByType.thing` over-count, update consumers. +- **No API breaks.** Pure additive on the public surface. + +### Cortex consumers + +Cortex 2.2.x continues to work against 7.22.0 thanks to the defensive +guards, but for full multi-process protection on Cortex-backed stores and +to make sure Cortex's native MetadataIndex picks up the find-where-zero +fix, Cortex 2.3.0 is recommended. Tracked as `CX-MULTIPROC-METHOD` in the +internal platform handoff. + +--- + ## v7.21.0 — 2026-05-15 **Affected products:** All consumers using filesystem storage (Venue, Workshop dev, diff --git a/src/brainy.ts b/src/brainy.ts index f2d690db..84668012 100644 --- a/src/brainy.ts +++ b/src/brainy.ts @@ -302,6 +302,20 @@ export class Brainy implements BrainyInterface { return !this.operationalMode.canWrite } + /** + * Whether the active storage adapter implements a given optional method. + * + * Brainy ships new storage-adapter capabilities over time (multi-process + * locking, cross-process flush RPC, etc.). Older plugins (notably + * `@soulcraft/cortex` ≤2.2.0) bundle their own `BaseStorage` from an + * earlier Brainy version that doesn't have those methods. Rather than crash + * at runtime, every call site that uses an optional storage method goes + * through this helper and degrades gracefully when the method is absent. + */ + private hasStorageMethod(name: string): boolean { + return !!this.storage && typeof (this.storage as unknown as Record)[name] === 'function' + } + /** * Throw if this instance cannot perform writes. Called at the top of every * mutation method. The check is cheap (a property lookup + boolean test) and @@ -385,24 +399,41 @@ export class Brainy implements BrainyInterface { // Acquire the writer lock for filesystem (and other locking-capable) backends. // Skipped in reader mode and on backends that don't support multi-process locking. // Throws if another live writer holds the directory (unless force: true). - if (this.config.mode !== 'reader' && this.storage.supportsMultiProcessLocking()) { - await this.storage.acquireWriterLock({ force: this.config.force }) - // Watch for cross-process flush requests so inspectors can see fresh - // state on demand. Reader instances don't run a watcher (nothing to flush). - this.storage.startFlushRequestWatcher(async () => { - if (this.initialized) { - await this.flush() + // + // Defensive call: older storage adapters (e.g. `@soulcraft/cortex@2.2.0` + // and earlier) bundle a pre-7.21 `BaseStorage` that doesn't define these + // methods. We feature-detect each one rather than fail boot. + if (this.config.mode !== 'reader') { + const canLock = this.hasStorageMethod('supportsMultiProcessLocking') && + (this.storage as any).supportsMultiProcessLocking() + if (canLock && this.hasStorageMethod('acquireWriterLock')) { + await (this.storage as any).acquireWriterLock({ force: this.config.force }) + if (this.hasStorageMethod('startFlushRequestWatcher')) { + (this.storage as any).startFlushRequestWatcher(async () => { + if (this.initialized) { + await this.flush() + } + }) + } + } else if (!this.config.silent) { + // Older adapter OR a backend that doesn't enforce locking (cloud / memory). + // Surface this so operators know the multi-process protections aren't active. + const backendName = (this.storage.constructor as any).name || 'storage' + if (backendName === 'MemoryStorage') { + // Memory is single-process by construction — no warning needed. + } else if (!this.hasStorageMethod('supportsMultiProcessLocking')) { + console.warn( + `[brainy] Storage adapter \`${backendName}\` predates the 7.21 ` + + `multi-process methods. Writer locking and the flush-request RPC ` + + `are disabled for this directory. Upgrade the plugin (e.g. ` + + `\`@soulcraft/cortex\` ≥2.3.0) for full enforcement.` + ) + } else { + console.warn( + `[brainy] Multi-process writer protection is not enforced on ${backendName}. ` + + `See docs/concepts/multi-process.md for the model.` + ) } - }) - } else if (this.config.mode !== 'reader' && !this.config.silent) { - // Cloud / memory backends: multi-process safety not enforced. One-time - // warning so operators know what they're getting. - const backendName = (this.storage.constructor as any).name || 'storage' - if (backendName !== 'MemoryStorage') { - console.warn( - `[brainy] Multi-process writer protection is not enforced on ${backendName}. ` + - `See docs/concepts/multi-process.md for the model.` - ) } } diff --git a/src/errors/brainyError.ts b/src/errors/brainyError.ts index e49d1b1c..350e98ba 100644 --- a/src/errors/brainyError.ts +++ b/src/errors/brainyError.ts @@ -3,7 +3,14 @@ * Provides better error classification and handling */ -export type BrainyErrorType = 'TIMEOUT' | 'NETWORK' | 'STORAGE' | 'NOT_FOUND' | 'RETRY_EXHAUSTED' | 'VALIDATION' +export type BrainyErrorType = + | 'TIMEOUT' + | 'NETWORK' + | 'STORAGE' + | 'NOT_FOUND' + | 'RETRY_EXHAUSTED' + | 'VALIDATION' + | 'FIELD_NOT_INDEXED' /** * Custom error class for Brainy operations @@ -99,6 +106,25 @@ export class BrainyError extends Error { ) } + /** + * Create a "field is not indexed" error. Thrown by metadata-index reads + * when a `where` clause names a field that has neither a column-store + * entry nor a sparse-index entry. Callers in `find()` evaluation catch + * this, translate the offending clause to an empty result, and log so + * the silent-empty behavior is replaced with a loud one. Use + * `brain.explain({ where: {...} })` to discover this before running. + */ + static fieldNotIndexed(field: string): BrainyError { + return new BrainyError( + `Field "${field}" is not indexed. find()/where will not match any entities. ` + + `Likely causes: (1) the writer registered the field in memory but has not flushed; ` + + `(2) the field name is mistyped; (3) no entity has ever held this field. ` + + `Run brain.explain({ where: { ${field}: ... } }) for the diagnostic.`, + 'FIELD_NOT_INDEXED', + false + ) + } + /** * Create a validation error */ diff --git a/src/indexes/columnStore/ColumnStore.ts b/src/indexes/columnStore/ColumnStore.ts index 2a8496fd..b8e755c6 100644 --- a/src/indexes/columnStore/ColumnStore.ts +++ b/src/indexes/columnStore/ColumnStore.ts @@ -309,6 +309,44 @@ export class ColumnStore implements ColumnStoreProvider { return (manifest !== undefined && !manifest.isEmpty()) || (buffer !== undefined && buffer.size > 0) } + /** + * List every field that has at least one persisted segment or buffered + * write. Used by `MetadataIndexManager.getStats()` and by + * `brainy inspect fields` so callers see the same field set the column + * store will actually serve queries from. + */ + getIndexedFields(): string[] { + const fields = new Set() + for (const [field, manifest] of this.manifests) { + if (!manifest.isEmpty()) fields.add(field) + } + for (const [field, buffer] of this.tailBuffers) { + if (buffer.size > 0) fields.add(field) + } + return Array.from(fields).sort() + } + + /** + * Approximate segment + tail-buffer sizes per indexed field. Returns + * `segmentCount` (number of persisted L0+ segments) and `tailSize` (entries + * buffered but not yet flushed) per field. Suitable for stats / health + * surfaces. For exact distinct-value cardinality use + * `getFilterValues(field).length` per field on demand. + */ + getFieldSizeSummary(): Array<{ field: string; segmentCount: number; tailSize: number }> { + const summary: Array<{ field: string; segmentCount: number; tailSize: number }> = [] + for (const field of this.getIndexedFields()) { + const manifest = this.manifests.get(field) + const buffer = this.tailBuffers.get(field) + const segmentCount = manifest && !manifest.isEmpty() + ? manifest.getAllSegments().length + : 0 + const tailSize = buffer ? buffer.size : 0 + summary.push({ field, segmentCount, tailSize }) + } + return summary + } + /** * Flush all tail buffers to L0 segments and save manifests. * No-op if init() hasn't been called (no storage to write to). diff --git a/src/storage/baseStorage.ts b/src/storage/baseStorage.ts index bf607f07..b3b225d2 100644 --- a/src/storage/baseStorage.ts +++ b/src/storage/baseStorage.ts @@ -223,6 +223,21 @@ export abstract class BaseStorage extends BaseStorageAdapter { // Built into all storage adapters for billion-scale efficiency protected nounCountsByType = new Uint32Array(NOUN_TYPE_COUNT) // 168 bytes (Stage 3: 42 types) protected verbCountsByType = new Uint32Array(VERB_TYPE_COUNT) // 508 bytes (Stage 3: 127 types) + + /** + * In-memory map from noun id → its `noun` (NounType) value, populated when + * `saveNounMetadata_internal()` runs. `saveNoun_internal()` consults this + * to attribute the entity to the correct slot in `nounCountsByType` so the + * persisted `_system/type-statistics.json` is honest. + * + * History: a previous cache was removed in commit `42ae5be` and replaced + * with a hardcoded `return 'thing'` from `getNounType()`, which silently + * poisoned the on-disk type statistics. This cache restores the correct + * behavior. Memory footprint is one Map entry per live noun id (typically + * tens of bytes each); the writer process keeps it for the duration of + * its lifetime and prunes on delete. + */ + protected nounTypeByIdCache = new Map() // Total: 676 bytes (99.2% reduction vs Map-based tracking) // Type caches REMOVED - ID-first paths eliminate need for type lookups! @@ -2172,6 +2187,15 @@ export abstract class BaseStorage extends BaseStorageAdapter { // Save the metadata (COW-aware - writes to branch-specific path) await this.writeObjectToBranch(path, metadata) + // Record the id→type mapping so `saveNoun_internal()` (which receives only + // an HNSWNoun and has no metadata access in its signature) can attribute + // the entity to the right slot in `nounCountsByType`. This is what makes + // `_system/type-statistics.json` honest. Updates the cache even for + // existing entities so a type change via `update()` is reflected. + if (metadata.noun) { + this.nounTypeByIdCache.set(id, metadata.noun as NounType) + } + // CRITICAL FIX: Increment count for new entities // This runs AFTER metadata is saved, guaranteeing type information is available // Uses synchronous increment since storage operations are already serialized @@ -2560,6 +2584,19 @@ export abstract class BaseStorage extends BaseStorageAdapter { // Direct O(1) delete with ID-first path const path = getNounMetadataPath(id) await this.deleteObjectFromBranch(path) + + // Prune the id→type cache so a future re-add of the same id (e.g. churn + // during tests) doesn't see a stale type. Lookup the prior type and + // decrement `nounCountsByType` so type-statistics stay honest across + // deletes; symmetric with the increment in `saveNounMetadata_internal()`. + const priorType = this.nounTypeByIdCache.get(id) + if (priorType) { + this.nounTypeByIdCache.delete(id) + const idx = TypeUtils.getNounIndex(priorType) + if (this.nounCountsByType[idx] > 0) { + this.nounCountsByType[idx]-- + } + } } /** @@ -2660,6 +2697,11 @@ export abstract class BaseStorage extends BaseStorageAdapter { /** * Load type statistics from storage * Rebuilds type counts if needed (called during init) + * + * Auto-detects the 7.20.0–7.21.0 poisoned-statistics signature: every + * noun attributed to `'thing'` because the old `getNounType()` was hardcoded. + * If detected, runs `rebuildTypeCounts()` once to rewrite the file with + * correct per-type counts derived from on-disk metadata. Logged loudly. */ protected async loadTypeStatistics(): Promise { try { @@ -2673,12 +2715,69 @@ export abstract class BaseStorage extends BaseStorageAdapter { if (stats.verbCounts && stats.verbCounts.length === VERB_TYPE_COUNT) { this.verbCountsByType = new Uint32Array(stats.verbCounts) } + + if (await this.detectPoisonedTypeStatistics()) { + prodLog.warn( + '[BaseStorage] Detected poisoned type-statistics.json signature ' + + '(all nouns attributed to \'thing\' — symptom of pre-7.22 getNounType ' + + 'hardcode). Rebuilding counts from on-disk metadata.' + ) + await this.rebuildTypeCounts() + } } } catch (error) { // No existing type statistics, starting fresh } } + /** + * Detect the 7.20.0–7.21.0 poisoned-statistics signature. + * + * The defect: the old `getNounType()` returned hardcoded `'thing'`, so + * `_system/type-statistics.json` ended up with `nounCounts[thing] === N` + * and every other index `=== 0`, regardless of the actual mix on disk. + * + * Heuristic: nounCounts has at least 2 non-thing entities on disk (so the + * file *should* show multiple non-zero buckets) but only the `thing` + * bucket is non-zero. We bound the on-disk check with `limit: 3` so this + * never costs more than a couple of cheap reads. + * + * Returns false (no self-heal needed) for genuinely thing-only stores or + * empty stores. + */ + private async detectPoisonedTypeStatistics(): Promise { + const thingIdx = TypeUtils.getNounIndex('thing' as NounType) + if (thingIdx < 0) return false + + let nonZeroBuckets = 0 + let thingCount = 0 + for (let i = 0; i < this.nounCountsByType.length; i++) { + if (this.nounCountsByType[i] > 0) { + nonZeroBuckets++ + if (i === thingIdx) thingCount = this.nounCountsByType[i] + } + } + + // Only one bucket populated, and it's 'thing' with ≥2 entities — suspicious. + if (nonZeroBuckets !== 1 || thingCount < 2) return false + + // Cross-check: sample a few metadata files. If any non-'thing' types + // show up, we're confirmed poisoned. If only 'thing' types appear in the + // sample, this is a genuine thing-only store and we leave the file alone. + try { + const sample = await this.getNouns({ pagination: { offset: 0, limit: 3 } }) + for (const noun of sample.items) { + const type = await this.getNounTypeFromStorageAsync(noun.id) + if (type && type !== ('thing' as NounType)) { + return true + } + } + } catch { + // If we can't read the metadata, don't trigger a rebuild — leave state alone. + } + return false + } + /** * Save type statistics to storage * Periodically called when counts are updated @@ -2693,6 +2792,24 @@ export abstract class BaseStorage extends BaseStorageAdapter { await this.writeObjectToPath(`${SYSTEM_DIR}/type-statistics.json`, stats) } + /** + * Persist both counter systems atomically when an explicit flush is + * requested. `super.flushCounts()` writes `entityCounts` (the Map in + * `BaseStorageAdapter`); we additionally write `nounCountsByType` / + * `verbCountsByType` so a reader opening the same directory sees the + * exact same counts the writer holds in memory. + * + * Before this override the Uint32Array counters were only persisted + * on a heuristic schedule inside `saveNoun_internal` (first-of-type or + * every-100th), which left readers seeing stale counts after a clean + * writer flush — the same failure mode that BR-FIND-WHERE-ZERO surfaced + * via `brain.stats()`. + */ + public async flushCounts(): Promise { + await super.flushCounts() + await this.saveTypeStatistics() + } + /** * Get noun counts by type (O(1) access to type statistics) * Exposed for MetadataIndexManager to use as single source of truth @@ -2712,11 +2829,19 @@ export abstract class BaseStorage extends BaseStorageAdapter { } /** - * Rebuild type counts from actual storage - * Called when statistics are missing or inconsistent - * Ensures verbCountsByType is always accurate for reliable pagination + * Rebuild type counts from actual storage. Scans every shard, reads each + * entity's metadata, and reconstructs `nounCountsByType` / `verbCountsByType` + * from ground truth. Persists the corrected counts to `type-statistics.json`. + * + * Public so it can be triggered by `brainy inspect repair` and by the + * `brain.health()` reconciliation path. Called automatically by + * `loadTypeStatistics()` when the persisted state matches the poisoned + * 7.20.0–7.21.0 signature (all entities attributed to `'thing'`). + * + * For very large stores this is O(N) where N is total entities; intended + * for diagnostic / repair use, not on every init. */ - protected async rebuildTypeCounts(): Promise { + public async rebuildTypeCounts(): Promise { prodLog.info('[BaseStorage] Rebuilding type counts from storage...') // Rebuild by scanning shards (0x00-0xFF) and reading metadata @@ -2788,16 +2913,63 @@ export abstract class BaseStorage extends BaseStorageAdapter { } /** - * Get noun type (type no longer needed for paths!) - * With ID-first paths, this is only used for internal statistics tracking. - * The actual type is stored in metadata and indexed by MetadataIndexManager. + * Resolve the `NounType` for a noun, used to attribute the entity to the + * correct slot in `nounCountsByType` (which backs `_system/type-statistics.json` + * and `brain.stats().entitiesByType`). + * + * Lookup order: + * 1. `nounTypeByIdCache`, populated by `saveNounMetadata_internal()` + * whenever a noun's metadata is written. This is the common path — + * add() saves metadata before the HNSW noun, so by the time we get + * here the cache is warm. + * 2. `'thing'` as a final fallback when metadata genuinely doesn't exist + * (a noun added without metadata — irregular but tolerated for back- + * compat). Logged so a real bug doesn't go unnoticed. + * + * Note this is intentionally synchronous because `saveNoun_internal()` and + * its callers are not async-friendly at this layer. For cases where only + * an id is known after a process restart and the cache is cold, + * `getNounTypeFromStorageAsync()` is available for callers that can await. */ protected getNounType(noun: HNSWNoun): NounType { - // Type cache removed - default to 'thing' for statistics - // The real type is in metadata, accessible via getNounMetadata(id) + const cached = this.nounTypeByIdCache.get(noun.id) + if (cached) return cached + // Cache miss on a noun we're about to write means metadata was not saved + // first. Brainy.add() always saves metadata before HNSW, so this only + // fires in unusual code paths (raw `storage.saveNoun()` without metadata). + prodLog.warn( + `[BaseStorage] getNounType: no type cached for noun ${noun.id}. ` + + `Type-statistics will attribute this entity to 'thing'. ` + + `Call saveNounMetadata() before saveNoun() to avoid stat drift.` + ) return 'thing' } + /** + * Async variant of `getNounType()` that consults disk if the in-memory + * cache is cold (e.g. after a process restart with deferred work). Used by + * `rebuildTypeCounts()` and other callers that can await an IO. Returns + * `null` if metadata genuinely doesn't exist; callers decide whether to + * fall back to 'thing' or skip the entity. + */ + protected async getNounTypeFromStorageAsync(id: string): Promise { + const cached = this.nounTypeByIdCache.get(id) + if (cached) return cached + try { + const metadataPath = getNounMetadataPath(id) + const metadata = await this.readWithInheritance(metadataPath) + if (metadata && (metadata as NounMetadata).noun) { + const type = (metadata as NounMetadata).noun as NounType + // Warm the cache so subsequent sync calls hit fast. + this.nounTypeByIdCache.set(id, type) + return type + } + } catch { + // Storage error — treat as unknown. + } + return null + } + /** * Get verb type from verb object * Verb type is a required field in HNSWVerb diff --git a/src/utils/metadataIndex.ts b/src/utils/metadataIndex.ts index 76bcdbc7..33c9476c 100644 --- a/src/utils/metadataIndex.ts +++ b/src/utils/metadataIndex.ts @@ -28,6 +28,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' /** * Fields whose values are stored in the sparse index as BUCKETED values @@ -150,11 +151,10 @@ export class MetadataIndexManager { private chunkManager: ChunkManager private chunkingStrategy: AdaptiveChunkingStrategy - // Deferred write tracking: accumulate chunk/sparse index writes during add/remove - // operations and flush them in a single concurrent batch at the end. - // This eliminates per-field sequential writes that cause cloud storage rate limiting. - private dirtyChunks = new Map() // "field:chunkId" -> ChunkData - private dirtySparseIndices = new Map() // field -> SparseIndex + // (Removed in 7.22.0) `dirtyChunks` and `dirtySparseIndices` Maps — + // never populated since the sparse-index write path was deleted in 7.20.0 + // (commit 11be039). The associated `flushDirtyMetadata()` no-op was also + // removed. Column store is the single source of truth for indexed writes. // Roaring Bitmap Support // EntityIdMapper for UUID ↔ integer conversion @@ -728,46 +728,31 @@ export class MetadataIndexManager { this.unifiedCache.set(unifiedKey, sparseIndex, 'metadata', size, 200) } - /** - * Flush all deferred chunk and sparse index writes accumulated during add/remove operations. - * Writes are deduplicated (same chunk/field written once even if updated multiple times) - * and executed concurrently via Promise.all for maximum throughput. - */ - private async flushDirtyMetadata(): Promise { - if (this.dirtyChunks.size === 0 && this.dirtySparseIndices.size === 0) { - return - } - - const promises: Promise[] = [] - - // Save all dirty chunks (deduplicated — same chunk written once even if updated multiple times) - for (const [_key, chunk] of this.dirtyChunks) { - promises.push(this.chunkManager.saveChunk(chunk)) - } - - // Save all dirty sparse indices (deduplicated — same field's index written once) - for (const [field, sparseIndex] of this.dirtySparseIndices) { - promises.push(this.saveSparseIndex(field, sparseIndex)) - } - - // Execute all writes concurrently - await Promise.all(promises) - - this.dirtyChunks.clear() - this.dirtySparseIndices.clear() - } - - // splitChunkDeferred — DELETED. Only called from addToChunkedIndex (also deleted). + // flushDirtyMetadata — DELETED in 7.22.0. The dirtyChunks / dirtySparseIndices + // accumulators it drained were never populated after the 7.20.0 column-store + // refactor (commit 11be039). Column store flush happens in flush() directly. /** - * Get IDs for a value using chunked sparse index with roaring bitmaps - * Now fully lazy-loaded via UnifiedCache (no local sparseIndices Map) + * Get IDs for a value using the legacy chunked sparse index. + * + * **This path is only for pre-7.20.0 workspaces** still being migrated to + * the column store. The write path for sparse indices was removed in + * commit `11be039` — new workspaces never get them. + * + * If neither the column store nor a sparse index covers the field, the + * function throws `BrainyError(FIELD_NOT_INDEXED)`. Returning `[]` for a + * genuinely unindexed field was the bug class `BR-FIND-WHERE-ZERO` + * tracked — a silent empty result indistinguishable from "the data + * really isn't there." */ private async getIdsFromChunks(field: string, value: any): Promise { // Load sparse index via UnifiedCache (lazy loading) const sparseIndex = await this.loadSparseIndex(field) if (!sparseIndex) { - return [] // No chunked index exists yet + // No column store match (we'd have returned in getIds()) AND no legacy + // sparse index for this field — the field is genuinely not indexed. + // Throw so find()-evaluation can log and translate to []. + throw BrainyError.fieldNotIndexed(field) } // Find candidate chunks using zone maps and bloom filters @@ -1323,7 +1308,19 @@ export class MetadataIndexManager { const wordIdSets: Map[] = [] for (const word of queryWords) { const wordHash = this.hashWord(word) - const ids = await this.getIds('__words__', wordHash) + let ids: string[] + try { + ids = await this.getIds('__words__', wordHash) + } catch (err) { + // `__words__` is not yet indexed (e.g. no text content has been + // added). Treat as no matches and continue — text search against + // an empty workspace should return [], not throw. + if (err instanceof BrainyError && err.type === 'FIELD_NOT_INDEXED') { + ids = [] + } else { + throw err + } + } const idSet = new Map() for (const id of ids) { idSet.set(id, 1) @@ -1585,6 +1582,20 @@ export class MetadataIndexManager { } } + /** + * Resolve a `where: { field: value }` clause to entity UUIDs. + * + * Lookup order: + * 1. **Column store** — the post-7.20.0 single source of truth. Fast. + * 2. **Legacy sparse index** — only consulted for pre-7.20.0 workspaces + * that haven't been migrated. Returns `[]` if no sparse data either. + * + * Throws `BrainyError(FIELD_NOT_INDEXED)` if the field has no entries in + * either store. Callers in find()-evaluation catch this and translate to + * an empty result with a logged warning. The throw aligns the production + * `find()` path with the `brain.explain()` diagnostic, so the silent- + * empty bug class (BR-FIND-WHERE-ZERO) is no longer possible. + */ async getIds(field: string, value: any): Promise { // Track exact query for field statistics if (this.fieldStats.has(field)) { @@ -1600,7 +1611,9 @@ export class MetadataIndexManager { return this.idMapper.intsIterableToUuids(bitmap) } - // Fallback: sparse index scan (legacy path during migration) + // Fallback: sparse index scan (legacy path during migration). If neither + // store has the field, throw FIELD_NOT_INDEXED so the caller knows it's a + // genuine "no such index" rather than "the value isn't there". return await this.getIdsFromChunks(field, value) } @@ -1780,13 +1793,23 @@ export class MetadataIndexManager { // Process field filters with range support const idSets: string[][] = [] - + // Capture field-not-indexed warnings so we log once per find() call, + // not once per AND-clause inside it. + const unindexedFields: string[] = [] + for (const [field, condition] of Object.entries(filter)) { // Skip logical operators if (field === 'allOf' || field === 'anyOf' || field === 'not') continue - + let fieldResults: string[] = [] - + + try { + // The block below evaluates one field clause. If `getIds()` throws + // FIELD_NOT_INDEXED (no column-store and no legacy sparse index for + // this field), we treat the clause as matching zero entities. This + // makes the production `find()` path consistent with the + // `brain.explain()` diagnostic: an unindexed field returns no + // results AND logs a warning, instead of silently returning []. if (condition && typeof condition === 'object' && !Array.isArray(condition)) { // Handle Brainy Field Operators (canonical operators defined) // See docs/api/README.md for complete operator reference @@ -1925,16 +1948,38 @@ export class MetadataIndexManager { // Direct value match (shorthand for 'eq' operator) fieldResults = await this.getIds(field, condition) } - + } catch (err) { + if (err instanceof BrainyError && err.type === 'FIELD_NOT_INDEXED') { + unindexedFields.push(field) + fieldResults = [] + } else { + throw err + } + } + if (fieldResults.length > 0) { idSets.push(fieldResults) } else { // If any field has no matches, intersection will be empty + if (unindexedFields.length > 0) { + prodLog.warn( + `[brainy] find() where-clause referenced unindexed field(s): ` + + `${unindexedFields.join(', ')}. Returning []. Use ` + + `brain.explain({ where: {...} }) for diagnostics.` + ) + } return [] } } - + if (idSets.length === 0) return [] + if (unindexedFields.length > 0) { + prodLog.warn( + `[brainy] find() where-clause referenced unindexed field(s) ` + + `${unindexedFields.join(', ')}; their clauses contributed no rows. ` + + `Use brain.explain({ where: {...} }) for diagnostics.` + ) + } if (idSets.length === 1) return idSets[0] // Set-based intersection O(n) — start with smallest set for optimal perf @@ -2212,21 +2257,38 @@ export class MetadataIndexManager { // Handle regular field filters const criteria = this.convertFilterToCriteria(filter) const idSets: string[][] = [] - + const unindexedFields: string[] = [] + for (const { field, values } of criteria) { const unionIds = new Set() - for (const value of values) { - const ids = await this.getIds(field, value) - ids.forEach(id => unionIds.add(id)) + try { + for (const value of values) { + const ids = await this.getIds(field, value) + ids.forEach(id => unionIds.add(id)) + } + } catch (err) { + if (err instanceof BrainyError && err.type === 'FIELD_NOT_INDEXED') { + unindexedFields.push(field) + // Treat as no matches and continue — same semantics as the main + // getIdsForFilter() path. + } else { + throw err + } } idSets.push(Array.from(unionIds)) } - + + if (unindexedFields.length > 0) { + prodLog.warn( + `[brainy] find() where-clause referenced unindexed field(s) ` + + `${unindexedFields.join(', ')}; their clauses contributed no rows.` + ) + } if (idSets.length === 0) return [] if (idSets.length === 1) return idSets[0] - + // Intersection of all field criteria (implicit $and) - return idSets.reduce((intersection, currentSet) => + return idSets.reduce((intersection, currentSet) => intersection.filter(id => currentSet.includes(id)) ) } @@ -2244,9 +2306,6 @@ export class MetadataIndexManager { * NOTE: Sparse indices are flushed immediately in add/remove operations */ async flush(): Promise { - // Flush any deferred chunk/sparse writes first - await this.flushDirtyMetadata() - // Always save field registry — even with no dirty fields. This tiny file // (list of field names) is the critical link that init() needs to discover // persisted indices. Without it, the index appears empty after restart. @@ -2776,54 +2835,70 @@ export class MetadataIndexManager { } /** - * Get index statistics with enhanced counting information - * Sparse indices now lazy-loaded via UnifiedCache - * Note: This method may load sparse indices to calculate stats + * Get index statistics. + * + * Source-of-truth precedence (post-7.20.0 column-store-first architecture): + * 1. **EntityIdMapper** — `idMapper.size` is the canonical entity count. + * Every indexed entity gets a UUID→int mapping; nothing else is + * consistent across instances. + * 2. **ColumnStore** — `getIndexedFields()` is the canonical list of + * indexed fields. `getFieldSizeSummary()` provides segment / tail + * bookkeeping per field. + * 3. **Legacy sparse-index registry** — only for pre-7.20.0 workspaces + * whose data hasn't been migrated. `getPersistedFieldList()` may know + * fields the column store doesn't yet, so we union them in. + * + * Prior implementation read from `this.fieldIndexes` + lazy-loaded sparse + * indices, which silently returned `0` entries for any workspace written + * after sparse-index writes were deleted in commit `11be039`. That defect + * is what `BR-FIND-WHERE-ZERO` tracked. */ async getStats(): Promise { + const entityCount = this.idMapper.size + + // Field set: union of column-store fields and any legacy sparse-index + // fields registered on disk. Exclude the `__words__` text index by + // convention (it's not a metadata field in the public sense). const fields = new Set() - let totalEntries = 0 - let totalIds = 0 - - // Collect stats from metadata field indexes only (excludes __words__ keyword index) - for (const field of this.fieldIndexes.keys()) { - if (field === '__words__') continue // Keyword index not included in metadata stats - fields.add(field) - - // Load sparse index to count entries (may trigger lazy load) - const sparseIndex = await this.loadSparseIndex(field) - if (sparseIndex) { - // Count entries and IDs from all chunks - for (const chunkId of sparseIndex.getAllChunkIds()) { - const chunk = await this.chunkManager.loadChunk(field, chunkId) - if (chunk) { - totalEntries += chunk.entries.size - for (const ids of chunk.entries.values()) { - totalIds += ids.size - } - } - } + if (this.columnStore) { + for (const f of this.columnStore.getIndexedFields()) { + if (f !== '__words__') fields.add(f) } } + // Legacy fallback: pre-7.20.0 workspaces may have sparse-index registry + // entries the column store doesn't know about yet. Surfacing them in the + // field list lets the rest of the system migrate them on read. + try { + const legacyFields = await this.getPersistedFieldList() + for (const f of legacyFields) { + if (f !== '__words__') fields.add(f) + } + } catch { + // Registry missing — nothing to add. + } - // Sanity check for index corruption (only metadata fields, not __words__ keyword index) - const entityCount = this.idMapper.size - if (entityCount > 0) { - const avgIdsPerEntity = totalIds / entityCount - if (avgIdsPerEntity > 100) { - prodLog.warn( - `⚠️ Metadata index may be corrupted: ${avgIdsPerEntity.toFixed(1)} avg entries/entity (expected ~30). ` + - `Try running brain.index.clearAllIndexData() followed by brain.index.rebuild() to fix.` - ) + // `totalEntries` semantically means "distinct entities tracked by this + // index". That's `idMapper.size`. `totalIds` is the sum of all + // (field, value) → entityId postings — proxied by the segment/tail size + // summary so we don't have to scan every bitmap. + let totalIds = 0 + if (this.columnStore) { + for (const summary of this.columnStore.getFieldSizeSummary()) { + if (summary.field === '__words__') continue + totalIds += summary.tailSize + // Segment count is a proxy; for a coarser-grained number we'd open + // each segment cursor. Avoided here because stats() is on the hot + // path for `brain.stats()` / health checks. + totalIds += summary.segmentCount * 1 // segments contribute at least 1 posting } } return { - totalEntries, + totalEntries: entityCount, totalIds, - fieldsIndexed: Array.from(fields), + fieldsIndexed: Array.from(fields).sort(), lastRebuild: Date.now(), - indexSize: totalEntries * 100 // rough estimate + indexSize: entityCount * 100 // rough estimate } } @@ -2996,9 +3071,9 @@ export class MetadataIndexManager { await this.addToIndex(noun.id, metadata, true, true) localCount++ // Periodic safety flush every 5000 entities to cap memory during rebuild - if (localCount % 5000 === 0) { - await this.flushDirtyMetadata() - } + // (Periodic safety flush removed in 7.22.0 — the underlying + // flushDirtyMetadata was a no-op since the 7.20.0 column-store + // refactor. Column store handles its own flushing.) } } @@ -3092,10 +3167,6 @@ export class MetadataIndexManager { await this.yieldToEventLoop() totalNounsProcessed += result.items.length - // Periodic safety flush every 5000 entities to cap memory during rebuild - if (totalNounsProcessed % 5000 === 0) { - await this.flushDirtyMetadata() - } hasMoreNouns = result.hasMore nounOffset += nounLimit @@ -3150,10 +3221,7 @@ export class MetadataIndexManager { if (metadata) { await this.addToIndex(verb.id, metadata, true, true) verbLocalCount++ - // Periodic safety flush every 5000 entities to cap memory during rebuild - if (verbLocalCount % 5000 === 0) { - await this.flushDirtyMetadata() - } + // (Periodic safety flush removed in 7.22.0 — see noun loop above.) } } @@ -3242,10 +3310,6 @@ export class MetadataIndexManager { await this.yieldToEventLoop() totalVerbsProcessed += result.items.length - // Periodic safety flush every 5000 entities to cap memory during rebuild - if (totalVerbsProcessed % 5000 === 0) { - await this.flushDirtyMetadata() - } hasMoreVerbs = result.hasMore verbOffset += verbLimit @@ -3262,11 +3326,8 @@ export class MetadataIndexManager { } } - // Flush remaining dirty chunks/sparse indices accumulated during rebuild - // (deferWrites=true prevented per-entity flushes, so dirty data accumulated) - await this.flushDirtyMetadata() - - // Flush to storage with final yield + // Flush to storage with final yield. The column store's flush() handles + // tail-buffer-to-segment promotion + manifest persistence. prodLog.debug('💾 Flushing metadata index to storage...') await this.flush() await this.yieldToEventLoop() diff --git a/tests/integration/cortex-compat.test.ts b/tests/integration/cortex-compat.test.ts new file mode 100644 index 00000000..15069d5c --- /dev/null +++ b/tests/integration/cortex-compat.test.ts @@ -0,0 +1,106 @@ +/** + * Cortex compatibility test (BR-DEFENSIVE-INTERFACE regression). + * + * Brainy 7.21.0 added several optional storage-adapter methods + * (`supportsMultiProcessLocking`, `acquireWriterLock`, etc.). Older plugins + * (notably `@soulcraft/cortex@2.2.0`) bundle a pre-7.21 `BaseStorage` that + * doesn't have them. Calling them unconditionally crashed boot. + * + * 7.22.0 fix: every call site is gated by `hasStorageMethod(name)` so older + * adapters degrade with a single warning at init. + * + * This test constructs a deliberately-stripped storage adapter (no new + * methods) and proves Brainy boots, can add/find/close normally, and logs + * the expected one-line warning. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import { Brainy } from '../../src/brainy.js' +import { MemoryStorage } from '../../src/storage/adapters/memoryStorage.js' +import { NounType } from '../../src/types/graphTypes.js' + +/** + * Mock storage adapter modeling pre-7.21 Cortex: extends MemoryStorage + * (which has full storage semantics) but deletes the new methods so Brainy + * sees a "method missing" surface. Class name is masked so the warning + * message names it as `LegacyCortexLikeStorage`. + */ +class LegacyCortexLikeStorage extends MemoryStorage { + constructor() { + super() + // Strip the 7.21+ methods. `delete` on prototype methods is awkward + // across TS strictness levels; assigning `undefined` produces the same + // `typeof === 'function'` false that older adapters exhibit. + ;(this as any).supportsMultiProcessLocking = undefined + ;(this as any).acquireWriterLock = undefined + ;(this as any).releaseWriterLock = undefined + ;(this as any).readWriterLock = undefined + ;(this as any).startFlushRequestWatcher = undefined + ;(this as any).stopFlushRequestWatcher = undefined + ;(this as any).requestFlushOverFilesystem = undefined + } +} + +describe('Cortex compatibility (BR-DEFENSIVE-INTERFACE)', () => { + let brain: Brainy | null = null + let warnSpy: ReturnType + + beforeEach(() => { + warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + }) + + afterEach(async () => { + if (brain) { + try { await brain.close() } catch { /* may already be closed */ } + brain = null + } + warnSpy.mockRestore() + }) + + it('boots without throwing against an adapter missing the 7.21+ methods', async () => { + brain = new Brainy({ storage: new LegacyCortexLikeStorage() as any }) + await expect(brain.init()).resolves.toBeUndefined() + }) + + it('logs a one-line warning naming the older adapter', async () => { + brain = new Brainy({ storage: new LegacyCortexLikeStorage() as any }) + await brain.init() + + const warnMessages = warnSpy.mock.calls + .map(call => String(call[0] ?? '')) + .join('\n') + expect(warnMessages).toMatch(/LegacyCortexLikeStorage|predates the 7\.21/i) + }) + + it('basic CRUD works normally (the missing methods are not on the hot path)', async () => { + brain = new Brainy({ storage: new LegacyCortexLikeStorage() as any }) + await brain.init() + + const id = await brain.add({ data: 'hello', type: NounType.Concept }) + const fetched = await brain.get(id) + expect(fetched).toBeTruthy() + expect(fetched!.id).toBe(id) + + await brain.flush() // No-op for memory-backed, but must not throw. + }) + + it('requestFlush() returns false when storage does not implement the RPC', async () => { + brain = new Brainy({ storage: new LegacyCortexLikeStorage() as any }) + await brain.init() + + // Same-process call would normally flush directly. Force the + // cross-process path by temporarily making this brain look read-only. + ;(brain as any).operationalMode = { canWrite: false, validateOperation: () => {} } + const ok = await brain.requestFlush({ timeoutMs: 500 }) + expect(ok).toBe(false) + }) + + it('stats() reports no writerLock for older adapters', async () => { + brain = new Brainy({ storage: new LegacyCortexLikeStorage() as any }) + await brain.init() + + const stats = await brain.stats() + expect(stats.mode).toBe('writer') + expect(stats.writerLock).toBeUndefined() + }) +}) diff --git a/tests/integration/find-where-zero.test.ts b/tests/integration/find-where-zero.test.ts new file mode 100644 index 00000000..053892bc --- /dev/null +++ b/tests/integration/find-where-zero.test.ts @@ -0,0 +1,219 @@ +/** + * BR-FIND-WHERE-ZERO regression test. + * + * Pre-7.22.0, the following failed silently: + * - Writer adds N entities of various types and flushes. + * - A fresh reader opens the same directory. + * - reader.find({ where: { ... } }) returned []. + * - reader.stats().entityCount returned 0. + * - reader.stats().entitiesByType reported all entities as 'thing'. + * + * Root cause was twofold: + * 1. The 7.20.0 column-store refactor deleted the sparse-index write path + * but left getStats() and the getIds()-fallback reading from sparse + * indices. New workspaces have no sparse-index files, so both surfaces + * returned 0 / []. + * 2. BaseStorage.getNounType() was hardcoded to return 'thing' after a + * type-cache removal, poisoning _system/type-statistics.json. + * + * 7.22.0 fix: getStats() reads from ColumnStore + idMapper, getIds() throws + * FIELD_NOT_INDEXED instead of silently returning [], and getNounType() reads + * from a write-time nounTypeByIdCache populated in saveNounMetadata_internal. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { Brainy } from '../../src/brainy.js' +import { NounType } from '../../src/types/graphTypes.js' +import { BrainyError } from '../../src/errors/brainyError.js' + +function makeTempDir(): string { + return mkdtempSync(join(tmpdir(), 'brainy-fwz-')) +} + +describe('BR-FIND-WHERE-ZERO regression', () => { + let dir: string + let writer: Brainy | null = null + let reader: Brainy | null = null + + beforeEach(() => { + dir = makeTempDir() + }) + + afterEach(async () => { + if (writer) { + try { await writer.close() } catch { /* may already be closed */ } + writer = null + } + if (reader) { + try { await reader.close() } catch { /* may already be closed */ } + reader = null + } + try { rmSync(dir, { recursive: true, force: true }) } catch { /* ignore */ } + }) + + describe('Reader sees correct entity counts after writer cold-start', () => { + it('preserves entity count across writer→close→reader-open', async () => { + writer = new Brainy({ storage: { type: 'filesystem', rootDirectory: dir }, silent: true }) + await writer.init() + const writerIds = new Set() + for (let i = 0; i < 10; i++) { + const id = await writer.add({ data: `alpha-${i}-payload`, type: NounType.Concept }) + writerIds.add(id) + } + await writer.flush() + await writer.close() + writer = null + + reader = await Brainy.openReadOnly({ + storage: { type: 'filesystem', rootDirectory: dir } + }) + // find() must return every entity we explicitly added — the historical + // bug was 0 results despite N being on disk. Anything beyond N from + // auto-extraction or VFS init is acceptable; the minimum guarantee is + // that no user-added entity is silently dropped. + const all = await reader.find({ type: NounType.Concept, limit: 1000 }) + const recovered = all.filter(e => writerIds.has(e.id)).map(e => e.id) + expect(recovered.length).toBe(10) + }) + + it('preserves type classification (no "all entities are thing" poisoning)', async () => { + writer = new Brainy({ storage: { type: 'filesystem', rootDirectory: dir }, silent: true }) + await writer.init() + // Three types. Each id is tracked individually so we don't conflate + // user-added entities with whatever VFS / auto-extraction inserts. + const conceptIds = new Set() + const eventIds = new Set() + const docIds = new Set() + for (let i = 0; i < 3; i++) conceptIds.add(await writer.add({ data: `alpha-${i}`, type: NounType.Concept })) + for (let i = 0; i < 5; i++) eventIds.add(await writer.add({ data: `beta-${i}`, type: NounType.Event })) + for (let i = 0; i < 2; i++) docIds.add(await writer.add({ data: `gamma-${i}`, type: NounType.Document })) + await writer.flush() + await writer.close() + writer = null + + reader = await Brainy.openReadOnly({ + storage: { type: 'filesystem', rootDirectory: dir } + }) + + // Every user-added id is recoverable when querying by its declared + // type. Pre-7.22 this failed because counts said 0 entries (which the + // rebuild trigger acted on) or all entities were attributed to 'thing'. + const concepts = await reader.find({ type: NounType.Concept, limit: 1000 }) + const events = await reader.find({ type: NounType.Event, limit: 1000 }) + const docs = await reader.find({ type: NounType.Document, limit: 1000 }) + + expect(concepts.filter(e => conceptIds.has(e.id)).length).toBe(3) + expect(events.filter(e => eventIds.has(e.id)).length).toBe(5) + expect(docs.filter(e => docIds.has(e.id)).length).toBe(2) + + // None of the user-added entities should leak into the 'thing' bucket. + const things = await reader.find({ type: NounType.Thing, limit: 1000 }) + const userInThings = things.filter( + e => conceptIds.has(e.id) || eventIds.has(e.id) || docIds.has(e.id) + ) + expect(userInThings.length).toBe(0) + }) + }) + + describe('find() consistency', () => { + it('returns entities that exist on disk (not silent empty)', async () => { + writer = new Brainy({ storage: { type: 'filesystem', rootDirectory: dir }, silent: true }) + await writer.init() + const conceptIds: string[] = [] + for (let i = 0; i < 4; i++) { + const id = await writer.add({ + data: `entity ${i}`, + type: NounType.Concept, + metadata: { entityType: 'booking', status: i % 2 === 0 ? 'paid' : 'pending' } + }) + conceptIds.push(id) + } + await writer.flush() + await writer.close() + writer = null + + reader = await Brainy.openReadOnly({ + storage: { type: 'filesystem', rootDirectory: dir } + }) + const all = await reader.find({ where: { entityType: 'booking' } }) + expect(all.length).toBe(4) + const paid = await reader.find({ where: { entityType: 'booking', status: 'paid' } }) + expect(paid.length).toBe(2) + }) + + it('returns [] with a logged warning for unindexed field', async () => { + writer = new Brainy({ storage: { type: 'filesystem', rootDirectory: dir }, silent: true }) + await writer.init() + await writer.add({ data: 'test', type: NounType.Concept }) + await writer.flush() + await writer.close() + writer = null + + reader = await Brainy.openReadOnly({ + storage: { type: 'filesystem', rootDirectory: dir } + }) + // Field that has never been written — production find() should degrade + // to [] (caught by getIdsForFilter), not throw upward. + const empty = await reader.find({ where: { nonExistentField: 'value' } }) + expect(empty).toEqual([]) + }) + + it('raw getIds() throws BrainyError(FIELD_NOT_INDEXED) for unindexed field', async () => { + writer = new Brainy({ storage: { type: 'filesystem', rootDirectory: dir }, silent: true }) + await writer.init() + await writer.add({ data: 'test', type: NounType.Concept }) + await writer.flush() + + // Direct getIds() on the metadata index throws — that's what + // getIdsForFilter() catches. Tests guarantee the contract holds. + const metadataIndex = (writer as any).metadataIndex + await expect( + metadataIndex.getIds('nonExistentField', 'anything') + ).rejects.toThrow(BrainyError) + await expect( + metadataIndex.getIds('nonExistentField', 'anything') + ).rejects.toMatchObject({ type: 'FIELD_NOT_INDEXED' }) + }) + }) + + describe('explain() and health() match the new contract', () => { + it('explain() returns column-store path for indexed fields', async () => { + writer = new Brainy({ storage: { type: 'filesystem', rootDirectory: dir }, silent: true }) + await writer.init() + await writer.add({ + data: 'test', + type: NounType.Concept, + metadata: { entityType: 'booking', priority: 1 } + }) + await writer.flush() + const plan = await writer.explain({ where: { entityType: 'booking' } }) + expect(plan.fieldPlan).toHaveLength(1) + expect(plan.fieldPlan[0].path).toBe('column-store') + expect(plan.warnings).toEqual([]) + }) + + it('health() reports pass for a clean writer + reader handoff', async () => { + writer = new Brainy({ storage: { type: 'filesystem', rootDirectory: dir }, silent: true }) + await writer.init() + for (let i = 0; i < 5; i++) { + await writer.add({ data: `entry ${i}`, type: NounType.Concept }) + } + await writer.flush() + await writer.close() + writer = null + + reader = await Brainy.openReadOnly({ + storage: { type: 'filesystem', rootDirectory: dir } + }) + const report = await reader.health() + // index-parity must pass (HNSW count matches metadata count) and + // field-registry must pass (every persisted field discoverable). + // No more 'index-parity warn: differ by N' regressions. + const indexParity = report.checks.find(c => c.name === 'index-parity') + expect(indexParity?.status).toBe('pass') + }) + }) +}) diff --git a/tests/integration/multi-process-safety.test.ts b/tests/integration/multi-process-safety.test.ts index 42c380e3..eaf1aba3 100644 --- a/tests/integration/multi-process-safety.test.ts +++ b/tests/integration/multi-process-safety.test.ts @@ -204,12 +204,13 @@ describe('Multi-process safety + read-only mode', () => { }) const stats = await reader.stats() expect(stats.mode).toBe('reader') - // NOTE: cross-instance count + type-classification drift between an - // in-process writer and a freshly-opened reader is a known issue with - // the reader-side index rebuild path. We verify shape + at-least-one- - // entity here; exact count/type fidelity is tracked as a separate fix. - expect(stats.entityCount).toBeGreaterThanOrEqual(1) - expect(Object.keys(stats.entitiesByType).length).toBeGreaterThan(0) + // Both Concept entities the writer added should be visible to the + // reader, classified correctly as 'concept' (not 'thing'). This is + // the BR-FIND-WHERE-ZERO regression test: stats() must read from the + // column store via idMapper.size, and getNounType() must use the + // write-time type cache instead of the old hardcoded 'thing'. + expect(stats.entityCount).toBeGreaterThanOrEqual(2) + expect(stats.entitiesByType.concept).toBeGreaterThanOrEqual(2) expect(stats.writerLock).toBeDefined() expect(stats.writerLock!.pid).toBe(process.pid) expect(stats.version).toBeTruthy()