From a94219e720a3564a599fd51fc6308b4cd66a95ea Mon Sep 17 00:00:00 2001 From: David Snelling Date: Mon, 26 Jan 2026 12:12:11 -0800 Subject: [PATCH] fix: update() field asymmetry causing index corruption CRITICAL: Fixed metadata index corruption on update() operations where removalMetadata only contained custom metadata + type, while entityForIndexing contained ALL indexed fields. This caused 7 fields to accumulate on every update, eventually making queries return 0 results. - Fix removalMetadata to include all indexed fields (src/brainy.ts) - Add validateIndexConsistency() and getIndexStats() public APIs - Add auto-corruption detection and repair on startup - Add getOrAssignSync() for EntityIdMapper persistence - Add comprehensive regression tests --- CHANGELOG.md | 57 ++++++ src/brainy.ts | 72 ++++++- src/utils/entityIdMapper.ts | 20 ++ src/utils/metadataIndex.ts | 99 +++++++++ tests/integration/update-asymmetry.test.ts | 226 +++++++++++++++++++++ 5 files changed, 470 insertions(+), 4 deletions(-) create mode 100644 tests/integration/update-asymmetry.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 572a738a..380544f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,63 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [7.5.0](https://github.com/soulcraftlabs/brainy/compare/v7.4.1...v7.5.0) (2026-01-26) + +### Bug Fixes + +**CRITICAL: Fixed metadata index corruption on update() operations** + +**Symptoms:** +- `find()` queries returning 0 results after many updates +- Index entry count growing with each update (7 extra entries per update) +- At scale (77+ updates), queries fail due to overcounting in intersection logic + +**Root Cause:** +In `update()`, the `removalMetadata` object only contained custom metadata + type, while `entityForIndexing` contained ALL indexed fields (confidence, weight, createdAt, updatedAt, service, data, createdBy). This asymmetry caused 7 fields to accumulate as orphaned index entries on every update. + +The `updatedAt` field was the worst offender - creating a NEW unique orphan on every update since the timestamp always changes. + +**Solution (src/brainy.ts:1163-1173):** +```typescript +// BEFORE (broken): Only removed custom metadata + type +const removalMetadata = { + ...existing.metadata, + type: existing.type +} + +// AFTER (fixed): Removes ALL indexed fields +const removalMetadata = { + type: existing.type, + confidence: existing.confidence, + weight: existing.weight, + createdAt: existing.createdAt, + updatedAt: existing.updatedAt, // CRITICAL: removes old timestamp + service: existing.service, + data: existing.data, + createdBy: existing.createdBy, + metadata: existing.metadata // Nested to match entityForIndexing structure +} +``` + +### Features + +**Index health monitoring and auto-repair** + +- `validateIndexConsistency()` - Public API to check index health +- `getIndexStats()` - Public API to get index statistics +- Auto-detection of index corruption on startup (>100 avg entries/entity) +- Automatic rebuild when corruption is detected + +**EntityIdMapper persistence improvements** + +- Added `getOrAssignSync()` for immediate persistence of UUID→int mappings +- Prevents mapping divergence on process crash + +### Tests + +- Added comprehensive integration tests for update field asymmetry fix +- Tests verify query accuracy, no duplicates, and entity integrity after many updates + ### [7.4.1](https://github.com/soulcraftlabs/brainy/compare/v7.4.0...v7.4.1) (2026-01-20) - fix: VFS readdir() no longer returns duplicate entries (2bd4031) diff --git a/src/brainy.ts b/src/brainy.ts index 635d3d22..01ecfb37 100644 --- a/src/brainy.ts +++ b/src/brainy.ts @@ -1147,11 +1147,29 @@ export class Brainy implements BrainyInterface { } // Operation 5-6: Update metadata index (remove old, add new) - // v6.2.1: Fix - Include type in removal metadata so noun index is properly updated - // existing.metadata only contains custom fields, not 'type' which maps to 'noun' + // v7.5.0 FIX: Include ALL indexed fields in removalMetadata (not just type) + // Previously, only metadata + type was removed, but entityForIndexing includes: + // confidence, weight, createdAt, updatedAt, service, data, createdBy + // This asymmetry caused 7 fields to accumulate on EVERY update, eventually + // making queries return 0 results (77x overcounting at scale). + // + // DEBUG: Log what we're removing and adding + // console.log('[UPDATE DEBUG] existing.metadata:', JSON.stringify(existing.metadata)) + // console.log('[UPDATE DEBUG] entityForIndexing keys:', Object.keys(entityForIndexing)) + // + // v7.5.0 FIX: removalMetadata must MATCH entityForIndexing structure + // entityForIndexing has: { type, confidence, ..., metadata: {...} } + // So removalMetadata must also have: { type, confidence, ..., metadata: {...} } const removalMetadata = { - ...existing.metadata, - type: existing.type // Include type so it maps to 'noun' during removal + type: existing.type, + confidence: existing.confidence, + weight: existing.weight, + createdAt: existing.createdAt, + updatedAt: existing.updatedAt, // CRITICAL: removes old timestamp + service: existing.service, + data: existing.data, + createdBy: existing.createdBy, + metadata: existing.metadata // CRITICAL: keep as nested 'metadata' property! } tx.addOperation( new RemoveFromMetadataIndexOperation(this.metadataIndex, params.id, removalMetadata) @@ -4694,6 +4712,52 @@ export class Brainy implements BrainyInterface { } } + /** + * v7.5.0: Validate metadata index consistency and detect corruption + * + * Returns health status and recommendations for repair. Corruption typically + * manifests as high avg entries/entity (expected ~30, corrupted can be 100+) + * caused by the update() field asymmetry bug (fixed in v7.5.0). + * + * @returns Promise resolving to validation results + * + * @example + * const validation = await brain.validateIndexConsistency() + * if (!validation.healthy) { + * console.log(validation.recommendation) + * // Run brain.rebuildIndex() to repair + * } + */ + async validateIndexConsistency(): Promise<{ + healthy: boolean + avgEntriesPerEntity: number + entityCount: number + indexEntryCount: number + recommendation: string | null + }> { + await this.ensureInitialized() + return this.metadataIndex.validateConsistency() + } + + /** + * v7.5.0: Get metadata index statistics + * + * Returns detailed statistics about the metadata index including + * total entries, IDs indexed, and fields indexed. + * + * @returns Promise resolving to index statistics + */ + async getIndexStats(): Promise<{ + totalEntries: number + totalIds: number + fieldsIndexed: string[] + lastRebuild: number + indexSize: number + }> { + await this.ensureInitialized() + return this.metadataIndex.getStats() + } + /** * Get graph neighbors of an entity * diff --git a/src/utils/entityIdMapper.ts b/src/utils/entityIdMapper.ts index d9cabc7b..1aff7e6b 100644 --- a/src/utils/entityIdMapper.ts +++ b/src/utils/entityIdMapper.ts @@ -86,6 +86,26 @@ export class EntityIdMapper { return newId } + /** + * v7.5.0: Get integer ID for UUID with immediate persistence guarantee + * Unlike getOrAssign(), this method flushes to storage immediately after assigning + * a new ID. This prevents UUID→int mapping divergence if the process crashes + * before a normal flush() occurs. + * + * Use this for critical operations where data integrity is paramount. + * Normal operations can use getOrAssign() with batched flushing for better performance. + */ + async getOrAssignSync(uuid: string): Promise { + const id = this.getOrAssign(uuid) + + // If a new ID was assigned, immediately persist to storage + if (this.dirty) { + await this.flush() + } + + return id + } + /** * Get UUID for integer ID */ diff --git a/src/utils/metadataIndex.ts b/src/utils/metadataIndex.ts index ccad225b..a49c78f3 100644 --- a/src/utils/metadataIndex.ts +++ b/src/utils/metadataIndex.ts @@ -223,6 +223,40 @@ export class MetadataIndexManager { // Phase 1b: Sync loaded counts to fixed-size arrays // Now correctly happens AFTER lazyLoadCounts() finishes this.syncTypeCountsToFixed() + + // v7.5.0: Detect index corruption and auto-rebuild if necessary + // The update() field asymmetry bug caused indexes to accumulate stale entries + // This check runs on startup to detect and repair corrupted indexes automatically + await this.detectAndRepairCorruption() + } + + /** + * v7.5.0: Detect index corruption and automatically repair via rebuild + * This catches the update() field asymmetry bug that causes 7 fields to accumulate per update + * Corruption threshold: 100 avg entries/entity (expected ~30) + */ + private async detectAndRepairCorruption(): Promise { + const validation = await this.validateConsistency() + + if (!validation.healthy) { + prodLog.warn(`⚠️ Index corruption detected (${validation.avgEntriesPerEntity.toFixed(1)} avg entries/entity)`) + prodLog.warn('🔄 Auto-rebuilding index to repair...') + + // Clear and rebuild + await this.clearAllIndexData() + await this.rebuild() + + // Re-validate after rebuild + const postRebuild = await this.validateConsistency() + if (postRebuild.healthy) { + prodLog.info(`✅ Index rebuilt successfully (${postRebuild.avgEntriesPerEntity.toFixed(1)} avg entries/entity)`) + } else { + prodLog.error( + `❌ Index still appears corrupted after rebuild (${postRebuild.avgEntriesPerEntity.toFixed(1)} avg entries/entity). ` + + `This may indicate a different issue.` + ) + } + } } /** @@ -2608,6 +2642,71 @@ export class MetadataIndexManager { } } + /** + * v7.5.0: Validate index consistency and detect corruption + * Returns health status and recommendations for repair + * + * Corruption typically manifests as high avg entries/entity (expected ~30, corrupted can be 100+) + * caused by the update() field asymmetry bug (fixed in v7.5.0) + */ + async validateConsistency(): Promise<{ + healthy: boolean + avgEntriesPerEntity: number + entityCount: number + indexEntryCount: number + recommendation: string | null + }> { + const entityCount = this.idMapper.size + + // If no entities, index is trivially healthy + if (entityCount === 0) { + return { + healthy: true, + avgEntriesPerEntity: 0, + entityCount: 0, + indexEntryCount: 0, + recommendation: null + } + } + + // Count total index entries across all fields + let indexEntryCount = 0 + for (const field of this.fieldIndexes.keys()) { + const sparseIndex = await this.loadSparseIndex(field) + if (sparseIndex) { + for (const chunkId of sparseIndex.getAllChunkIds()) { + const chunk = await this.chunkManager.loadChunk(field, chunkId) + if (chunk) { + for (const ids of chunk.entries.values()) { + indexEntryCount += ids.size + } + } + } + } + } + + const avgEntriesPerEntity = indexEntryCount / entityCount + + // Threshold: 100 entries/entity is clearly corrupted (expected ~30) + // This catches the update() asymmetry bug which causes 7 fields to accumulate per update + const CORRUPTION_THRESHOLD = 100 + const healthy = avgEntriesPerEntity <= CORRUPTION_THRESHOLD + + let recommendation: string | null = null + if (!healthy) { + recommendation = `Index corruption detected (${avgEntriesPerEntity.toFixed(1)} avg entries/entity, expected ~30). ` + + `Run brain.index.clearAllIndexData() followed by brain.index.rebuild() to repair.` + } + + return { + healthy, + avgEntriesPerEntity, + entityCount, + indexEntryCount, + recommendation + } + } + /** * Rebuild entire index from scratch using pagination * Non-blocking version that yields control back to event loop diff --git a/tests/integration/update-asymmetry.test.ts b/tests/integration/update-asymmetry.test.ts new file mode 100644 index 00000000..3b14233d --- /dev/null +++ b/tests/integration/update-asymmetry.test.ts @@ -0,0 +1,226 @@ +/** + * Update Field Asymmetry Regression Tests (v7.5.0) + * + * Tests for the critical bug where update() didn't remove all indexed fields, + * causing 7 fields to accumulate on EVERY update. This eventually made queries + * return 0 results (77x overcounting at scale). + * + * Root cause: removalMetadata only contained custom metadata + type, while + * entityForIndexing contained ALL indexed fields (confidence, weight, createdAt, + * updatedAt, service, data, createdBy). + * + * NO MOCKS - Real integration tests with actual storage + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import { Brainy } from '../../src/brainy.js' +import { NounType } from '../../src/types/graphTypes.js' +import * as fs from 'fs/promises' + +describe('Update Field Asymmetry Fix (v7.5.0)', () => { + let brain: Brainy + const testPath = './test-brainy-update-asymmetry' + + beforeEach(async () => { + // Clean up test directory thoroughly + try { + await fs.rm(testPath, { recursive: true, force: true }) + } catch { + // Ignore if doesn't exist + } + + // Create fresh Brainy instance + brain = new Brainy({ + storage: { type: 'filesystem', path: testPath }, + silent: true + }) + await brain.init() + }) + + afterEach(async () => { + // Clean up test directory + try { + await fs.rm(testPath, { recursive: true, force: true }) + } catch { + // Ignore cleanup errors + } + }) + + describe('Query Accuracy After Updates', () => { + it('should return correct results from find() by type after many updates', async () => { + // Create entity with specific type + const id = await brain.add({ + data: 'Findable Entity', + type: NounType.Person, + metadata: { name: 'Alice' } + }) + + // Update 20 times with changing metadata + for (let i = 0; i < 20; i++) { + await brain.update({ + id, + metadata: { name: 'Alice', updateCount: i } + }) + } + + await brain.flush() + + // find() by type should still return the entity + // (this would fail if index corruption caused overcounting) + // Note: 'type' is a top-level param, not inside 'where' + const byType = await brain.find({ + type: NounType.Person + }) + + expect(byType.length).toBeGreaterThanOrEqual(1) + const found = byType.find(r => r.id === id) + expect(found).toBeDefined() + expect(found?.id).toBe(id) + }) + + it('should return correct results from find() by metadata after many updates', async () => { + // Create entity with specific metadata + const id = await brain.add({ + data: 'Searchable Entity', + type: NounType.Document, + metadata: { category: 'important', status: 'active' } + }) + + // Update 20 times - changing some fields but keeping 'category' + for (let i = 0; i < 20; i++) { + await brain.update({ + id, + metadata: { category: 'important', status: 'active', version: i } + }) + } + + await brain.flush() + + // find() by metadata should still work + const results = await brain.find({ + where: { category: 'important' } + }) + + expect(results.length).toBeGreaterThanOrEqual(1) + const found = results.find(r => r.id === id) + expect(found).toBeDefined() + }) + + it('should not return duplicate results after multiple updates', async () => { + // Create 3 entities + const ids: string[] = [] + for (let i = 0; i < 3; i++) { + const id = await brain.add({ + data: `Entity ${i}`, + type: NounType.Document, + metadata: { group: 'test-group', index: i } + }) + ids.push(id) + } + + // Update each entity 10 times + for (const id of ids) { + for (let i = 0; i < 10; i++) { + await brain.update({ + id, + metadata: { group: 'test-group', version: i } + }) + } + } + + await brain.flush() + + // find() should return entities without duplicates + const results = await brain.find({ + where: { group: 'test-group' } + }) + + // Get unique IDs from results + const uniqueIds = new Set(results.map(r => r.id)) + + // Should have exactly 3 unique entities + expect(uniqueIds.size).toBe(3) + // No duplicates - results length should equal unique count + expect(results.length).toBe(uniqueIds.size) + }) + }) + + describe('Entity Integrity After Updates', () => { + it('should preserve entity data after many updates', async () => { + // Create entity + const id = await brain.add({ + data: 'Original Data', + type: NounType.Event, + metadata: { title: 'Original Title' } + }) + + const originalEntity = await brain.get(id) + const originalCreatedAt = originalEntity?.createdAt + + // Update 15 times + for (let i = 0; i < 15; i++) { + await brain.update({ + id, + metadata: { title: `Updated Title ${i}`, iteration: i } + }) + } + + await brain.flush() + + // Get final entity + const finalEntity = await brain.get(id) + + // Entity should still exist and be retrievable + expect(finalEntity).toBeDefined() + expect(finalEntity?.id).toBe(id) + + // Original data should be preserved + expect(finalEntity?.data).toBe('Original Data') + expect(finalEntity?.type).toBe(NounType.Event) + expect(finalEntity?.createdAt).toBe(originalCreatedAt) + + // Updated metadata should be reflected + expect(finalEntity?.metadata?.title).toBe('Updated Title 14') + expect(finalEntity?.metadata?.iteration).toBe(14) + + // updatedAt should have changed + expect(finalEntity?.updatedAt).not.toBe(originalCreatedAt) + }) + + it('should handle rapid consecutive updates', async () => { + // Create entity + const id = await brain.add({ + data: 'Rapid Update Test', + type: NounType.Task, + metadata: { status: 'pending' } + }) + + // Rapid-fire 50 updates without waiting + const updatePromises = [] + for (let i = 0; i < 50; i++) { + updatePromises.push( + brain.update({ + id, + metadata: { status: 'processing', step: i } + }) + ) + } + + // Wait for all updates to complete + await Promise.all(updatePromises) + await brain.flush() + + // Entity should still be findable and correct + const entity = await brain.get(id) + expect(entity).toBeDefined() + expect(entity?.type).toBe(NounType.Task) + + // Should be findable by type + // Note: 'type' is a top-level param, not inside 'where' + const results = await brain.find({ + type: NounType.Task + }) + expect(results.some(r => r.id === id)).toBe(true) + }) + }) +})