soulcraft/brainy
2
0
Fork 0
Code Issues 1 Pull requests Projects Releases 335 Packages Wiki Activity Actions

feat: Remove dangerous getAllNouns/getAllVerbs methods, add safe pagination

Browse source 
BREAKING CHANGE: Removed getAllNouns() and getAllVerbs() from StorageAdapter interface
These methods could cause expensive full scans on cloud storage (S3/R2) leading to
high costs and performance issues. Replaced with safe paginated methods.

Changes:
- Remove getAllNouns/getAllVerbs from StorageAdapter interface and implementations
- Add internal optimization methods for intelligent preloading when safe
- Fix OPFS storage file naming consistency (.json extension)
- Fix S3 high-volume mode detection thresholds (was too aggressive)
- Fix TypeScript compilation errors with async methods
- Update all tests to use paginated methods

Performance:
- Add smart dataset size detection for automatic optimization
- Maintain all internal performance optimizations through safe preloading
- Only preload data in read-only mode or when dataset is small (<10k entities)

Fixes:
- Fix intelligent verb scoring tests metadata structure
- Fix S3 storage getVerbsBySource/Target/Type methods
- Fix memory usage in search operations using pagination

Docs:
- Add comprehensive storage architecture documentation
- Document known bash redirection issue
- Update README with architecture doc link

All affected tests passing
This commit is contained in:
David Snelling 2025-08-10 16:25:12 -07:00
parent 30fe350d54
commit abc17397b1
17 changed files with 998 additions and 390 deletions
Download patch file Download diff file Expand all files Collapse all files

15
src/storage/adapters/baseStorageAdapter.ts
View file

@ -50,19 +50,8 @@ export abstract class BaseStorageAdapter implements StorageAdapter {
details?: Record<string, any>
}>
/**
* Get all nouns from storage
* @returns Promise that resolves to an array of all nouns
* @deprecated This method loads all data into memory and may cause performance issues. Use getNouns() with pagination instead.
*/
abstract getAllNouns(): Promise<any[]>
/**
* Get all verbs from storage
* @returns Promise that resolves to an array of all HNSWVerbs
* @deprecated This method loads all data into memory and may cause performance issues. Use getVerbs() with pagination instead.
*/
abstract getAllVerbs(): Promise<any[]>
// NOTE: getAllNouns and getAllVerbs have been removed to prevent expensive full scans.
// Use getNouns() and getVerbs() with pagination instead.
/**
* Get nouns with pagination and filtering

52
src/storage/adapters/opfsStorage.ts
View file

@ -206,7 +206,7 @@ export class OPFSStorage extends BaseStorage {
}
// Create or get the file for this noun
const fileHandle = await this.nounsDir!.getFileHandle(noun.id, {
const fileHandle = await this.nounsDir!.getFileHandle(`${noun.id}.json`, {
create: true
})
@ -230,7 +230,7 @@ export class OPFSStorage extends BaseStorage {
try {
// Get the file handle for this noun
const fileHandle = await this.nounsDir!.getFileHandle(id)
const fileHandle = await this.nounsDir!.getFileHandle(`${id}.json`)
// Read the noun data from the file
const file = await fileHandle.getFile()
@ -331,7 +331,7 @@ export class OPFSStorage extends BaseStorage {
await this.ensureInitialized()
try {
await this.nounsDir!.removeEntry(id)
await this.nounsDir!.removeEntry(`${id}.json`)
} catch (error: any) {
// Ignore NotFoundError, which means the file doesn't exist
if (error.name !== 'NotFoundError') {
@ -364,7 +364,7 @@ export class OPFSStorage extends BaseStorage {
}
// Create or get the file for this verb
const fileHandle = await this.verbsDir!.getFileHandle(edge.id, {
const fileHandle = await this.verbsDir!.getFileHandle(`${edge.id}.json`, {
create: true
})
@ -393,7 +393,7 @@ export class OPFSStorage extends BaseStorage {
try {
// Get the file handle for this edge
const fileHandle = await this.verbsDir!.getFileHandle(id)
const fileHandle = await this.verbsDir!.getFileHandle(`${id}.json`)
// Read the edge data from the file
const file = await fileHandle.getFile()
@ -488,12 +488,12 @@ export class OPFSStorage extends BaseStorage {
protected async getVerbsBySource_internal(
sourceId: string
): Promise<GraphVerb[]> {
// This method is deprecated and would require loading metadata for each edge
// For now, return empty array since this is not efficiently implementable with new storage pattern
console.warn(
'getVerbsBySource_internal is deprecated and not efficiently supported in new storage pattern'
)
return []
// Use the paginated approach to properly handle HNSWVerb to GraphVerb conversion
const result = await this.getVerbsWithPagination({
filter: { sourceId: [sourceId] },
limit: Number.MAX_SAFE_INTEGER // Get all matching results
})
return result.items
}
/**
@ -514,12 +514,12 @@ export class OPFSStorage extends BaseStorage {
protected async getVerbsByTarget_internal(
targetId: string
): Promise<GraphVerb[]> {
// This method is deprecated and would require loading metadata for each edge
// For now, return empty array since this is not efficiently implementable with new storage pattern
console.warn(
'getVerbsByTarget_internal is deprecated and not efficiently supported in new storage pattern'
)
return []
// Use the paginated approach to properly handle HNSWVerb to GraphVerb conversion
const result = await this.getVerbsWithPagination({
filter: { targetId: [targetId] },
limit: Number.MAX_SAFE_INTEGER // Get all matching results
})
return result.items
}
/**
@ -538,12 +538,12 @@ export class OPFSStorage extends BaseStorage {
* Get verbs by type (internal implementation)
*/
protected async getVerbsByType_internal(type: string): Promise<GraphVerb[]> {
// This method is deprecated and would require loading metadata for each edge
// For now, return empty array since this is not efficiently implementable with new storage pattern
console.warn(
'getVerbsByType_internal is deprecated and not efficiently supported in new storage pattern'
)
return []
// Use the paginated approach to properly handle HNSWVerb to GraphVerb conversion
const result = await this.getVerbsWithPagination({
filter: { verbType: [type] },
limit: Number.MAX_SAFE_INTEGER // Get all matching results
})
return result.items
}
/**
@ -572,7 +572,7 @@ export class OPFSStorage extends BaseStorage {
await this.ensureInitialized()
try {
await this.verbsDir!.removeEntry(id)
await this.verbsDir!.removeEntry(`${id}.json`)
} catch (error: any) {
// Ignore NotFoundError, which means the file doesn't exist
if (error.name !== 'NotFoundError') {
@ -590,7 +590,7 @@ export class OPFSStorage extends BaseStorage {
try {
// Create or get the file for this metadata
const fileHandle = await this.metadataDir!.getFileHandle(id, {
const fileHandle = await this.metadataDir!.getFileHandle(`${id}.json`, {
create: true
})
@ -612,7 +612,7 @@ export class OPFSStorage extends BaseStorage {
try {
// Get the file handle for this metadata
const fileHandle = await this.metadataDir!.getFileHandle(id)
const fileHandle = await this.metadataDir!.getFileHandle(`${id}.json`)
// Read the metadata from the file
const file = await fileHandle.getFile()

130
src/storage/adapters/s3CompatibleStorage.ts
View file

@ -532,16 +532,26 @@ export class S3CompatibleStorage extends BaseStorage {
const backpressureStatus = this.backpressure.getStatus()
const socketMetrics = this.socketManager.getMetrics()
// EXTREMELY aggressive detection - activate on ANY load
// Reasonable high-volume detection - only activate under real load
const isTestEnvironment = process.env.NODE_ENV === 'test'
const explicitlyDisabled = process.env.BRAINY_FORCE_BUFFERING === 'false'
// Use reasonable thresholds instead of emergency aggressive ones
const reasonableThreshold = Math.max(threshold, 10) // At least 10 pending operations
const highSocketUtilization = 0.8 // 80% socket utilization
const highRequestRate = 50 // 50 requests per second
const significantErrors = 5 // 5 consecutive errors
const shouldEnableHighVolume =
this.forceHighVolumeMode || // Environment override
backpressureStatus.queueLength >= threshold || // Configurable threshold (>= 0 by default!)
socketMetrics.pendingRequests >= threshold || // Socket pressure
this.pendingOperations >= threshold || // Any pending ops
socketMetrics.socketUtilization >= 0.01 || // Even 1% socket usage
(socketMetrics.requestsPerSecond >= 1) || // Any request rate
(this.consecutiveErrors >= 0) || // Always true - any system activity
true // FORCE ENABLE for emergency debugging
!isTestEnvironment && // Disable in test environment
!explicitlyDisabled && // Allow explicit disabling
(this.forceHighVolumeMode || // Environment override
backpressureStatus.queueLength >= reasonableThreshold || // High queue backlog
socketMetrics.pendingRequests >= reasonableThreshold || // Many pending requests
this.pendingOperations >= reasonableThreshold || // Many pending ops
socketMetrics.socketUtilization >= highSocketUtilization || // High socket pressure
(socketMetrics.requestsPerSecond >= highRequestRate) || // High request rate
(this.consecutiveErrors >= significantErrors)) // Significant error pattern
if (shouldEnableHighVolume && !this.highVolumeMode) {
this.highVolumeMode = true
@ -1672,66 +1682,88 @@ export class S3CompatibleStorage extends BaseStorage {
}
}
// Apply filtering at GraphVerb level since HNSWVerb filtering is not supported
let filteredGraphVerbs = graphVerbs
if (options.filter) {
filteredGraphVerbs = graphVerbs.filter((graphVerb) => {
// Filter by sourceId
if (options.filter!.sourceId) {
const sourceIds = Array.isArray(options.filter!.sourceId)
? options.filter!.sourceId
: [options.filter!.sourceId]
if (!sourceIds.includes(graphVerb.sourceId)) {
return false
}
}
// Filter by targetId
if (options.filter!.targetId) {
const targetIds = Array.isArray(options.filter!.targetId)
? options.filter!.targetId
: [options.filter!.targetId]
if (!targetIds.includes(graphVerb.targetId)) {
return false
}
}
// Filter by verbType (maps to type field)
if (options.filter!.verbType) {
const verbTypes = Array.isArray(options.filter!.verbType)
? options.filter!.verbType
: [options.filter!.verbType]
if (graphVerb.type && !verbTypes.includes(graphVerb.type)) {
return false
}
}
return true
})
}
return {
items: graphVerbs,
items: filteredGraphVerbs,
hasMore: result.hasMore,
nextCursor: result.nextCursor
}
}
/**
* Get verbs by source (internal implementation)
*/
protected async getVerbsBySource_internal(
sourceId: string
): Promise<GraphVerb[]> {
return this.getEdgesBySource(sourceId)
}
/**
* Get edges by source
*/
protected async getEdgesBySource(sourceId: string): Promise<GraphVerb[]> {
// This method is deprecated and would require loading metadata for each edge
// For now, return empty array since this is not efficiently implementable with new storage pattern
this.logger.trace('getEdgesBySource is deprecated and not efficiently supported in new storage pattern')
return []
protected async getVerbsBySource_internal(sourceId: string): Promise<GraphVerb[]> {
// Use the paginated approach to properly handle HNSWVerb to GraphVerb conversion
const result = await this.getVerbsWithPagination({
filter: { sourceId: [sourceId] },
limit: Number.MAX_SAFE_INTEGER // Get all matching results
})
return result.items
}
/**
* Get verbs by target (internal implementation)
*/
protected async getVerbsByTarget_internal(
targetId: string
): Promise<GraphVerb[]> {
return this.getEdgesByTarget(targetId)
}
/**
* Get edges by target
*/
protected async getEdgesByTarget(targetId: string): Promise<GraphVerb[]> {
// This method is deprecated and would require loading metadata for each edge
// For now, return empty array since this is not efficiently implementable with new storage pattern
this.logger.trace('getEdgesByTarget is deprecated and not efficiently supported in new storage pattern')
return []
protected async getVerbsByTarget_internal(targetId: string): Promise<GraphVerb[]> {
// Use the paginated approach to properly handle HNSWVerb to GraphVerb conversion
const result = await this.getVerbsWithPagination({
filter: { targetId: [targetId] },
limit: Number.MAX_SAFE_INTEGER // Get all matching results
})
return result.items
}
/**
* Get verbs by type (internal implementation)
*/
protected async getVerbsByType_internal(type: string): Promise<GraphVerb[]> {
return this.getEdgesByType(type)
}
/**
* Get edges by type
*/
protected async getEdgesByType(type: string): Promise<GraphVerb[]> {
// This method is deprecated and would require loading metadata for each edge
// For now, return empty array since this is not efficiently implementable with new storage pattern
this.logger.trace('getEdgesByType is deprecated and not efficiently supported in new storage pattern')
return []
// Use the paginated approach to properly handle HNSWVerb to GraphVerb conversion
const result = await this.getVerbsWithPagination({
filter: { verbType: [type] },
limit: Number.MAX_SAFE_INTEGER // Get all matching results
})
return result.items
}
/**

34
src/storage/baseStorage.ts
View file

@ -203,30 +203,24 @@ export abstract class BaseStorage extends BaseStorageAdapter {
}
/**
* Get all verbs from storage
* @returns Promise that resolves to an array of all HNSWVerbs
* @deprecated This method loads all data into memory and may cause performance issues. Use getVerbs() with pagination instead.
* Internal method for loading all verbs - used by performance optimizations
* @internal - Do not use directly, use getVerbs() with pagination instead
*/
public async getAllVerbs(): Promise<HNSWVerb[]> {
protected async _loadAllVerbsForOptimization(): Promise<HNSWVerb[]> {
await this.ensureInitialized()
console.warn('getAllVerbs() is deprecated and may cause memory issues with large datasets. Consider using getVerbs() with pagination instead.')
// Get all verbs using the paginated method with a very large limit
// Only use this for internal optimizations when safe
const result = await this.getVerbs({
pagination: {
limit: Number.MAX_SAFE_INTEGER
}
pagination: { limit: Number.MAX_SAFE_INTEGER }
})
// Convert GraphVerbs back to HNSWVerbs since that's what this method should return
// Convert GraphVerbs back to HNSWVerbs for internal use
const hnswVerbs: HNSWVerb[] = []
for (const graphVerb of result.items) {
// Create an HNSWVerb from the GraphVerb (reverse conversion)
const hnswVerb: HNSWVerb = {
id: graphVerb.id,
vector: graphVerb.vector,
connections: new Map() // HNSWVerbs need connections, but GraphVerbs don't have them
connections: new Map()
}
hnswVerbs.push(hnswVerb)
}
@ -274,19 +268,15 @@ export abstract class BaseStorage extends BaseStorageAdapter {
}
/**
* Get all nouns from storage
* @returns Promise that resolves to an array of all nouns
* @deprecated This method loads all data into memory and may cause performance issues. Use getNouns() with pagination instead.
* Internal method for loading all nouns - used by performance optimizations
* @internal - Do not use directly, use getNouns() with pagination instead
*/
public async getAllNouns(): Promise<HNSWNoun[]> {
protected async _loadAllNounsForOptimization(): Promise<HNSWNoun[]> {
await this.ensureInitialized()
console.warn('getAllNouns() is deprecated and may cause memory issues with large datasets. Consider using getNouns() with pagination instead.')
// Only use this for internal optimizations when safe
const result = await this.getNouns({
pagination: {
limit: Number.MAX_SAFE_INTEGER
}
pagination: { limit: Number.MAX_SAFE_INTEGER }
})
return result.items