From 5f10f8d9abed4b9006f065ca725be7609e7ef35d Mon Sep 17 00:00:00 2001 From: David Snelling Date: Tue, 16 Sep 2025 10:35:07 -0700 Subject: [PATCH] fix: prevent infinite loop in pagination when storage returns phantom items - Fix FileSystemStorage counting non-existent files in totalCount - Add safety check in BaseStorage to prevent hasMore:true with empty items - Ensure pagination terminates correctly even with corrupted storage state --- scripts/buildEmbeddedPatterns.ts | 4 +- src/brainy.ts | 18 ++++- src/neural/embeddedPatterns.ts | 4 +- src/storage/adapters/fileSystemStorage.ts | 35 +++++++-- src/storage/baseStorage.ts | 14 +++- src/types/brainy.types.ts | 4 + src/utils/metadataIndex.ts | 63 +++++++++++++-- test-brainy-init-sequence.js | 94 +++++++++++++++++++++++ test-infinite-loop.js | 47 ++++++++++++ test-storage-pagination.js | 79 +++++++++++++++++++ 10 files changed, 340 insertions(+), 22 deletions(-) create mode 100644 test-brainy-init-sequence.js create mode 100644 test-infinite-loop.js create mode 100644 test-storage-pagination.js diff --git a/scripts/buildEmbeddedPatterns.ts b/scripts/buildEmbeddedPatterns.ts index b19a9a5f..746c130f 100644 --- a/scripts/buildEmbeddedPatterns.ts +++ b/scripts/buildEmbeddedPatterns.ts @@ -189,7 +189,9 @@ export const PATTERNS_METADATA = { } } -console.log(\`🧠 Brainy Pattern Library loaded: \${EMBEDDED_PATTERNS.length} patterns, \${(PATTERNS_METADATA.sizeBytes.total / 1024).toFixed(1)}KB total\`) +// Only log if not suppressed - controlled by logging configuration +import { prodLog } from '../utils/logger.js' +prodLog.info(\`🧠 Brainy Pattern Library loaded: \${EMBEDDED_PATTERNS.length} patterns, \${(PATTERNS_METADATA.sizeBytes.total / 1024).toFixed(1)}KB total\`) ` // Write the TypeScript file diff --git a/src/brainy.ts b/src/brainy.ts index 0c686171..88938bb5 100644 --- a/src/brainy.ts +++ b/src/brainy.ts @@ -22,6 +22,7 @@ import { NaturalLanguageProcessor } from './neural/naturalLanguageProcessor.js' import { TripleIntelligenceSystem } from './triple/TripleIntelligenceSystem.js' import { MetadataIndexManager } from './utils/metadataIndex.js' import { GraphAdjacencyIndex } from './graph/graphAdjacencyIndex.js' +import { configureLogger, LogLevel } from './utils/logger.js' import { Entity, Relation, @@ -94,15 +95,24 @@ export class Brainy { storage: { ...this.config.storage, ...configOverrides.storage }, model: { ...this.config.model, ...configOverrides.model }, index: { ...this.config.index, ...configOverrides.index }, - augmentations: { ...this.config.augmentations, ...configOverrides.augmentations } + augmentations: { ...this.config.augmentations, ...configOverrides.augmentations }, + verbose: configOverrides.verbose ?? this.config.verbose, + silent: configOverrides.silent ?? this.config.silent } - + // Set dimensions if provided if (dimensions) { this.dimensions = dimensions } } + // Configure logging based on config options + if (this.config.silent) { + configureLogger({ level: -1 as LogLevel }) // Suppress all logs + } else if (this.config.verbose) { + configureLogger({ level: LogLevel.DEBUG }) // Enable verbose logging + } + try { // Setup and initialize storage this.storage = await this.setupStorage() @@ -1487,7 +1497,9 @@ export class Brainy { warmup: config?.warmup ?? false, realtime: config?.realtime ?? false, multiTenancy: config?.multiTenancy ?? false, - telemetry: config?.telemetry ?? false + telemetry: config?.telemetry ?? false, + verbose: config?.verbose ?? false, + silent: config?.silent ?? false } } diff --git a/src/neural/embeddedPatterns.ts b/src/neural/embeddedPatterns.ts index ba7288ef..7a0631ad 100644 --- a/src/neural/embeddedPatterns.ts +++ b/src/neural/embeddedPatterns.ts @@ -4053,4 +4053,6 @@ export const PATTERNS_METADATA = { } } -console.log(`🧠 Brainy Pattern Library loaded: ${EMBEDDED_PATTERNS.length} patterns, ${(PATTERNS_METADATA.sizeBytes.total / 1024).toFixed(1)}KB total`) +// Only log if not suppressed - controlled by logging configuration +import { prodLog } from '../utils/logger.js' +prodLog.info(`🧠 Brainy Pattern Library loaded: ${EMBEDDED_PATTERNS.length} patterns, ${(PATTERNS_METADATA.sizeBytes.total / 1024).toFixed(1)}KB total`) diff --git a/src/storage/adapters/fileSystemStorage.ts b/src/storage/adapters/fileSystemStorage.ts index 34310be7..39f51c45 100644 --- a/src/storage/adapters/fileSystemStorage.ts +++ b/src/storage/adapters/fileSystemStorage.ts @@ -623,9 +623,25 @@ export class FileSystemStorage extends BaseStorage { // Get page of files const pageFiles = nounFiles.slice(startIndex, startIndex + limit) - - // Load nouns + + // Load nouns - count actual successfully loaded items const items: HNSWNoun[] = [] + let successfullyLoaded = 0 + let totalValidFiles = 0 + + // First pass: count total valid files (for accurate totalCount) + // This is necessary to fix the pagination bug + for (const file of nounFiles) { + try { + // Just check if file exists and is readable + await fs.promises.access(path.join(this.nounsDir, file), fs.constants.R_OK) + totalValidFiles++ + } catch { + // File not readable, skip + } + } + + // Second pass: load the current page for (const file of pageFiles) { try { const data = await fs.promises.readFile( @@ -633,7 +649,7 @@ export class FileSystemStorage extends BaseStorage { 'utf-8' ) const noun = JSON.parse(data) - + // Apply filter if provided if (options.filter) { // Simple filter implementation @@ -646,21 +662,24 @@ export class FileSystemStorage extends BaseStorage { } if (!matches) continue } - + items.push(noun) + successfullyLoaded++ } catch (error) { console.warn(`Failed to read noun file ${file}:`, error) } } - - const hasMore = startIndex + limit < nounFiles.length + + // CRITICAL FIX: hasMore should be based on actual valid files, not just file count + // Also check if we actually loaded any items from this page + const hasMore = (startIndex + limit < totalValidFiles) && (successfullyLoaded > 0 || startIndex === 0) const nextCursor = hasMore && pageFiles.length > 0 ? pageFiles[pageFiles.length - 1].replace('.json', '') : undefined - + return { items, - totalCount: nounFiles.length, + totalCount: totalValidFiles, // Use actual valid file count, not all files hasMore, nextCursor } diff --git a/src/storage/baseStorage.ts b/src/storage/baseStorage.ts index a3897d9c..0c0612b4 100644 --- a/src/storage/baseStorage.ts +++ b/src/storage/baseStorage.ts @@ -414,10 +414,15 @@ export abstract class BaseStorage extends BaseStorageAdapter { // Apply offset if needed (some adapters might not support offset) const items = result.items.slice(offset) + // CRITICAL SAFETY CHECK: Prevent infinite loops + // If we have no items but hasMore is true, force hasMore to false + // This prevents pagination bugs from causing infinite loops + const safeHasMore = items.length > 0 ? result.hasMore : false + return { items, totalCount: result.totalCount || totalCount, - hasMore: result.hasMore, + hasMore: safeHasMore, nextCursor: result.nextCursor } } @@ -606,10 +611,15 @@ export abstract class BaseStorage extends BaseStorageAdapter { // Apply offset if needed (some adapters might not support offset) const items = result.items.slice(offset) + // CRITICAL SAFETY CHECK: Prevent infinite loops + // If we have no items but hasMore is true, force hasMore to false + // This prevents pagination bugs from causing infinite loops + const safeHasMore = items.length > 0 ? result.hasMore : false + return { items, totalCount: result.totalCount || totalCount, - hasMore: result.hasMore, + hasMore: safeHasMore, nextCursor: result.nextCursor } } diff --git a/src/types/brainy.types.ts b/src/types/brainy.types.ts index 1a5315f6..1201263c 100644 --- a/src/types/brainy.types.ts +++ b/src/types/brainy.types.ts @@ -343,6 +343,10 @@ export interface BrainyConfig { realtime?: boolean // Enable real-time updates multiTenancy?: boolean // Enable service isolation telemetry?: boolean // Send anonymous usage stats + + // Logging configuration + verbose?: boolean // Enable verbose logging + silent?: boolean // Suppress all logging output } // ============= Neural API Types ============= diff --git a/src/utils/metadataIndex.ts b/src/utils/metadataIndex.ts index 153a792a..969b8558 100644 --- a/src/utils/metadataIndex.ts +++ b/src/utils/metadataIndex.ts @@ -1507,12 +1507,33 @@ export class MetadataIndexManager { const nounLimit = 25 // Even smaller batches during initialization to prevent socket exhaustion let hasMoreNouns = true let totalNounsProcessed = 0 - - while (hasMoreNouns) { + let consecutiveEmptyBatches = 0 + const MAX_ITERATIONS = 10000 // Safety limit to prevent infinite loops + let iterations = 0 + + while (hasMoreNouns && iterations < MAX_ITERATIONS) { + iterations++ const result = await this.storage.getNouns({ pagination: { offset: nounOffset, limit: nounLimit } }) - + + // CRITICAL SAFETY CHECK: Prevent infinite loop on empty results + if (result.items.length === 0) { + consecutiveEmptyBatches++ + if (consecutiveEmptyBatches >= 3) { + prodLog.warn('⚠️ Breaking metadata rebuild loop: received 3 consecutive empty batches') + break + } + // If hasMore is true but items are empty, it's likely a bug + if (result.hasMore) { + prodLog.warn(`⚠️ Storage returned empty items but hasMore=true at offset ${nounOffset}`) + hasMoreNouns = false // Force exit + break + } + } else { + consecutiveEmptyBatches = 0 // Reset counter on non-empty batch + } + // CRITICAL FIX: Use batch metadata reading to prevent socket exhaustion const nounIds = result.items.map(noun => noun.id) @@ -1581,12 +1602,32 @@ export class MetadataIndexManager { const verbLimit = 25 // Even smaller batches during initialization to prevent socket exhaustion let hasMoreVerbs = true let totalVerbsProcessed = 0 - - while (hasMoreVerbs) { + let consecutiveEmptyVerbBatches = 0 + let verbIterations = 0 + + while (hasMoreVerbs && verbIterations < MAX_ITERATIONS) { + verbIterations++ const result = await this.storage.getVerbs({ pagination: { offset: verbOffset, limit: verbLimit } }) - + + // CRITICAL SAFETY CHECK: Prevent infinite loop on empty results + if (result.items.length === 0) { + consecutiveEmptyVerbBatches++ + if (consecutiveEmptyVerbBatches >= 3) { + prodLog.warn('⚠️ Breaking verb metadata rebuild loop: received 3 consecutive empty batches') + break + } + // If hasMore is true but items are empty, it's likely a bug + if (result.hasMore) { + prodLog.warn(`⚠️ Storage returned empty verb items but hasMore=true at offset ${verbOffset}`) + hasMoreVerbs = false // Force exit + break + } + } else { + consecutiveEmptyVerbBatches = 0 // Reset counter on non-empty batch + } + // CRITICAL FIX: Use batch verb metadata reading to prevent socket exhaustion const verbIds = result.items.map(verb => verb.id) @@ -1647,11 +1688,19 @@ export class MetadataIndexManager { await this.yieldToEventLoop() } + // Check if we hit iteration limits + if (iterations >= MAX_ITERATIONS) { + prodLog.error(`❌ Metadata noun rebuild hit maximum iteration limit (${MAX_ITERATIONS}). This indicates a bug in storage pagination.`) + } + if (verbIterations >= MAX_ITERATIONS) { + prodLog.error(`❌ Metadata verb rebuild hit maximum iteration limit (${MAX_ITERATIONS}). This indicates a bug in storage pagination.`) + } + // Flush to storage with final yield prodLog.debug('💾 Flushing metadata index to storage...') await this.flush() await this.yieldToEventLoop() - + prodLog.info(`✅ Metadata index rebuild completed! Processed ${totalNounsProcessed} nouns and ${totalVerbsProcessed} verbs`) prodLog.info(`🎯 Initial indexing may show minor socket timeouts - this is expected and doesn't affect data processing`) diff --git a/test-brainy-init-sequence.js b/test-brainy-init-sequence.js new file mode 100644 index 00000000..8d0d6165 --- /dev/null +++ b/test-brainy-init-sequence.js @@ -0,0 +1,94 @@ +import { FileSystemStorage } from './dist/storage/adapters/fileSystemStorage.js'; +import { HNSWIndexOptimized } from './dist/hnsw/hnswIndexOptimized.js'; +import { MetadataIndexManager } from './dist/utils/metadataIndex.js'; +import { GraphAdjacencyIndex } from './dist/graph/graphAdjacencyIndex.js'; +import { cosineDistance } from './dist/utils/index.js'; +import fs from 'fs/promises'; + +async function testBrainyInitSequence() { + const testDir = './test-init-sequence'; + + // Clean up + try { + await fs.rm(testDir, { recursive: true }); + } catch (e) {} + + console.log('=== Step 1: Create FileSystemStorage ==='); + const storage = new FileSystemStorage(testDir); + + console.log('=== Step 2: Initialize storage ==='); + await storage.init(); + + // Check if any files were created + console.log('\nChecking files after storage.init():'); + try { + const nounsFiles = await fs.readdir(`${testDir}/nouns`); + console.log('Nouns files:', nounsFiles); + } catch (e) { + console.log('Nouns dir error:', e.message); + } + + console.log('\n=== Step 3: Create HNSWIndexOptimized ==='); + const index = new HNSWIndexOptimized({ + m: 16, + efConstruction: 200, + efSearch: 50, + distanceFunction: cosineDistance + }, cosineDistance, storage); + + console.log('\n=== Step 4: Create MetadataIndexManager ==='); + const metadataIndex = new MetadataIndexManager(storage); + + console.log('\n=== Step 5: Create GraphAdjacencyIndex ==='); + const graphIndex = new GraphAdjacencyIndex(storage); + + console.log('\n=== Step 6: Check for existing data (like rebuildIndexesIfNeeded) ==='); + const entities = await storage.getNouns({ pagination: { limit: 1 } }); + console.log('Initial check result:', { + itemCount: entities.items.length, + totalCount: entities.totalCount, + hasMore: entities.hasMore + }); + + if (entities.totalCount === 0 || entities.items.length === 0) { + console.log('No data found - would skip rebuild'); + } else { + console.log('Data found - would trigger rebuild!'); + + console.log('\n=== Step 7: Rebuild metadata index ==='); + console.log('Starting metadata index rebuild...'); + + // This is where the infinite loop happens + // Let's trace what happens in the rebuild + let iterations = 0; + let offset = 0; + const limit = 25; + + while (iterations < 5) { // Limit iterations for testing + iterations++; + console.log(`\nIteration ${iterations}: offset=${offset}, limit=${limit}`); + + const result = await storage.getNouns({ + pagination: { offset, limit } + }); + + console.log(`Result: ${result.items.length} items, hasMore=${result.hasMore}`); + + if (result.items.length === 0 && result.hasMore) { + console.log('🔴 BUG DETECTED: Empty items but hasMore=true!'); + } + + if (!result.hasMore || result.items.length === 0) { + console.log('Breaking loop'); + break; + } + + offset += limit; + } + } + + // Clean up + await fs.rm(testDir, { recursive: true }); +} + +testBrainyInitSequence().catch(console.error); \ No newline at end of file diff --git a/test-infinite-loop.js b/test-infinite-loop.js new file mode 100644 index 00000000..80136cfd --- /dev/null +++ b/test-infinite-loop.js @@ -0,0 +1,47 @@ +import { Brainy } from './dist/index.js'; +import fs from 'fs/promises'; + +async function test() { + // Clean up any existing test data + const testDir = './.test-brainy'; + try { + await fs.rm(testDir, { recursive: true }); + } catch (e) { + // Ignore if doesn't exist + } + + console.log('Testing Brainy initialization...'); + + const brain = new Brainy({ + storage: { + type: 'filesystem', + options: { + path: testDir + } + }, + model: { + type: 'accurate' // Same as Sage + }, + cache: true, + verbose: false, + silent: true + }); + + console.log('Calling brain.init()...'); + + try { + await brain.init(); + console.log('✅ Initialization completed successfully!'); + } catch (error) { + console.error('❌ Initialization failed:', error); + } + + // Clean up + try { + await fs.rm(testDir, { recursive: true }); + } catch (e) { + // Ignore + } +} + +test().catch(console.error); \ No newline at end of file diff --git a/test-storage-pagination.js b/test-storage-pagination.js new file mode 100644 index 00000000..ab6d5c68 --- /dev/null +++ b/test-storage-pagination.js @@ -0,0 +1,79 @@ +import { FileSystemStorage } from './dist/storage/adapters/fileSystemStorage.js'; +import fs from 'fs/promises'; +import path from 'path'; + +async function testPagination() { + const testDir = './test-storage-debug'; + + // Clean up and create fresh directory + try { + await fs.rm(testDir, { recursive: true }); + } catch (e) {} + await fs.mkdir(testDir, { recursive: true }); + + console.log('Creating FileSystemStorage...'); + + const storage = new FileSystemStorage(testDir); + + console.log('Initializing storage...'); + await storage.init(); + + console.log('\n=== First getNouns call (limit: 1) ==='); + const result1 = await storage.getNouns({ + pagination: { limit: 1 } + }); + console.log('Result 1:', { + itemCount: result1.items.length, + items: result1.items.map(i => i.id), + hasMore: result1.hasMore, + totalCount: result1.totalCount + }); + + console.log('\n=== Second getNouns call (offset: 0, limit: 25) ==='); + const result2 = await storage.getNouns({ + pagination: { offset: 0, limit: 25 } + }); + console.log('Result 2:', { + itemCount: result2.items.length, + items: result2.items.map(i => i.id), + hasMore: result2.hasMore, + totalCount: result2.totalCount + }); + + console.log('\n=== Third getNouns call (offset: 25, limit: 25) ==='); + const result3 = await storage.getNouns({ + pagination: { offset: 25, limit: 25 } + }); + console.log('Result 3:', { + itemCount: result3.items.length, + items: result3.items.map(i => i.id), + hasMore: result3.hasMore, + totalCount: result3.totalCount + }); + + console.log('\n=== Fourth getNouns call (offset: 50, limit: 25) ==='); + const result4 = await storage.getNouns({ + pagination: { offset: 50, limit: 25 } + }); + console.log('Result 4:', { + itemCount: result4.items.length, + items: result4.items.map(i => i.id), + hasMore: result4.hasMore, + totalCount: result4.totalCount + }); + + // Check what's actually in the directories + console.log('\n=== Checking actual files ==='); + const nounsDir = path.join(testDir, 'nouns'); + try { + const files = await fs.readdir(nounsDir); + console.log('Files in nouns directory:', files); + } catch (e) { + console.log('nouns directory error:', e.message); + } + + // Clean up + await fs.rm(testDir, { recursive: true }); +} + +testPagination().catch(console.error); \ No newline at end of file