feat(plugin-sql): add CachedDatabaseAdapter with LRU caching and serv… #6329
Conversation
…erless optimizations - Add CachedDatabaseAdapter wrapper in plugin-sql that provides LRU caching for entities, rooms, worlds, agents, participants, components, relationships, and tasks with automatic cache invalidation on mutations - Support optional ExternalCacheAdapter interface for Redis/Upstash integration in serverless environments (two-tier L1/L2 caching) - Add embedding dimension caching to AgentRuntime with setEmbeddingDimension() method to skip model calls on serverless cold starts - Add getEmbeddingDimension() and setEmbeddingDimension() to runtime with validation against VECTOR_DIMS constants - Export CachedDatabaseAdapter, createCachedAdapter, CachedAdapterConfig, and ExternalCacheAdapter from plugin-sql - Add comprehensive integration tests with 100% code coverage (73 tests)
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| if (room?.worldId) { | ||
| await this.deleteFromCache(this.roomsByWorldCache, 'roomsByWorld', room.worldId); | ||
| } | ||
| } |
There was a problem hiding this comment.
deleteRoom fails to invalidate cache when room not cached
In deleteRoom, the code retrieves the room from cache only (via getFromCache) to determine its worldId for invalidating roomsByWorldCache. If the room was never cached (never read before deletion), the worldId won't be found and the roomsByWorldCache for that world won't be invalidated. This causes stale cache entries where getRoomsByWorld may return deleted rooms until TTL expiration. The method should fetch from the database (not just cache) to get the worldId before deletion.
| this.taskCache.clear(); | ||
| this.roomsByWorldCache.clear(); | ||
| this.entitiesForRoomCache.clear(); | ||
| } |
There was a problem hiding this comment.
clearAllCaches doesn't clear the external L2 cache
The clearAllCaches() method only clears the L1 in-memory caches but does not call this.externalCache.clear() when an external cache (Redis, Upstash) is configured. The ExternalCacheAdapter interface defines a clear(prefix?) method specifically for this purpose. After calling clearAllCaches(), subsequent reads will still retrieve stale data from the L2 external cache and repopulate L1, making the cache-clearing operation ineffective when external caching is enabled.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // ==================== Optional Methods ==================== | ||
|
|
||
| async runPluginMigrations?( | ||
| plugins: Array<{ | ||
| name: string; | ||
| schema?: Record<string, string | number | boolean | null | Record<string, unknown>>; | ||
| }>, | ||
| options?: { verbose?: boolean; force?: boolean; dryRun?: boolean } | ||
| ): Promise<void> { | ||
| if (this.baseAdapter.runPluginMigrations) { | ||
| return this.baseAdapter.runPluginMigrations(plugins, options); | ||
| } | ||
| } |
There was a problem hiding this comment.
syntax: Optional method signature uses deprecated syntax
The async runPluginMigrations?() syntax is non-standard. TypeScript optional methods should be declared as runPluginMigrations?: (params) => Promise<void> or implement the interface method properly.
| // ==================== Optional Methods ==================== | |
| async runPluginMigrations?( | |
| plugins: Array<{ | |
| name: string; | |
| schema?: Record<string, string | number | boolean | null | Record<string, unknown>>; | |
| }>, | |
| options?: { verbose?: boolean; force?: boolean; dryRun?: boolean } | |
| ): Promise<void> { | |
| if (this.baseAdapter.runPluginMigrations) { | |
| return this.baseAdapter.runPluginMigrations(plugins, options); | |
| } | |
| } | |
| async runPluginMigrations( | |
| plugins: Array<{ | |
| name: string; | |
| schema?: Record<string, string | number | boolean | null | Record<string, unknown>>; | |
| }>, | |
| options?: { verbose?: boolean; force?: boolean; dryRun?: boolean } | |
| ): Promise<void> { | |
| if (this.baseAdapter.runPluginMigrations) { | |
| return this.baseAdapter.runPluginMigrations(plugins, options); | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/plugin-sql/src/cached-adapter.ts
Line: 870:882
Comment:
**syntax:** Optional method signature uses deprecated syntax
The `async runPluginMigrations?()` syntax is non-standard. TypeScript optional methods should be declared as `runPluginMigrations?: (params) => Promise<void>` or implement the interface method properly.
```suggestion
async runPluginMigrations(
plugins: Array<{
name: string;
schema?: Record<string, string | number | boolean | null | Record<string, unknown>>;
}>,
options?: { verbose?: boolean; force?: boolean; dryRun?: boolean }
): Promise<void> {
if (this.baseAdapter.runPluginMigrations) {
return this.baseAdapter.runPluginMigrations(plugins, options);
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
| await this.setInCache(this.agentCache, 'agent', agent.id, agent as Agent); | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
createAgent caches partial data as complete Agent type
The createAgent method receives a Partial<Agent> and caches it cast to Agent. The Agent interface requires createdAt and updatedAt fields, which are typically set by the database during creation, not provided by the caller. When getAgent is subsequently called before any update/delete, it returns this incomplete cached data missing required timestamp fields. This violates the type contract and can cause runtime errors when code accesses the expected fields.
| } | ||
|
|
||
| return results.length > 0 ? results : null; | ||
| } |
There was a problem hiding this comment.
Batch fetch methods return items in wrong order
Both getEntitiesByIds and getRoomsByIds return results in incorrect order when some items are cached and others need database fetching. Cached items are pushed first, then database-fetched items are appended at the end. For example, requesting [id1, id2, id3] where only id2 is not cached returns [entity1, entity3, entity2] instead of preserving the input order. Code relying on result order matching request order will behave incorrectly.
Additional Locations (1)
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
| if (this.externalCache) { | ||
| await this.externalCache.delete(this.cacheKey(type, id)); | ||
| } | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Summary
Fixes 6 issues identified by AI code review in the
CachedDatabaseAdapter:Note
Introduces a caching wrapper and runtime optimization to reduce DB and model calls.
CachedDatabaseAdapter: L1 in-memory LRU with optional L2 external cache; read-through on misses, targeted invalidation on mutations; supports agents, entities, rooms, worlds, participants, components, relationships, tasks; passthrough for high-volume memory ops; exposed viaindex.ts.createCachedAdapter.AgentRuntimenow caches embedding dimension; addsgetEmbeddingDimension()/setEmbeddingDimension()(validated againstVECTOR_DIMS); init uses pre-set dimension or falls back to probing when embedding model exists.cached-adapter.test.ts.Written by Cursor Bugbot for commit 0ca8e4e. This will update automatically on new commits. Configure here.
Greptile Summary
This PR adds a
CachedDatabaseAdapterwrapper that provides LRU caching with optional external cache support (Redis/Upstash) for serverless environments, plus embedding dimension caching in the runtime to avoid redundant model calls.Key Changes:
CachedDatabaseAdapterclass implementing two-tier caching strategy (in-memory L1 + optional external L2)setEmbeddingDimension()entitiesForRoom) are clearedCache Strategy:
Note: This is marked as a DRAFT PR and should NOT be merged yet.
Confidence Score: 3/5
createAgentthat could cache incomplete data, and (3) this being a DRAFT PR explicitly marked "DO NOT MERGE". The core caching logic is sound and thoroughly tested, but the syntax issues must be fixed for production readiness.packages/plugin-sql/src/cached-adapter.tsrequires syntax fixes for optional method declarations (lines 870-909) and logic review for type casting on line 297Important Files Changed
Sequence Diagram
sequenceDiagram participant Runtime as AgentRuntime participant CachedAdapter as CachedDatabaseAdapter participant L1Cache as In-Memory LRU Cache participant L2Cache as External Cache (Redis/Upstash) participant BaseAdapter as Base Database Adapter participant DB as PostgreSQL/PGLite Note over Runtime: Initialization Runtime->>Runtime: Check embeddingDimension cache alt Pre-configured dimension Runtime->>CachedAdapter: ensureEmbeddingDimension(dimension) CachedAdapter->>BaseAdapter: ensureEmbeddingDimension(dimension) BaseAdapter->>DB: Configure vector dimension else Dimension not cached Runtime->>Runtime: getModel(TEXT_EMBEDDING) Runtime->>Runtime: Generate test embedding Runtime->>Runtime: Cache embedding.length Runtime->>CachedAdapter: ensureEmbeddingDimension(embedding.length) CachedAdapter->>BaseAdapter: ensureEmbeddingDimension(embedding.length) BaseAdapter->>DB: Configure vector dimension end Note over Runtime,DB: Read Operations (Cache Hit) Runtime->>CachedAdapter: getAgent(agentId) CachedAdapter->>L1Cache: get(agentId) L1Cache-->>CachedAdapter: Agent data CachedAdapter-->>Runtime: Agent data Note over Runtime,DB: Read Operations (L1 Miss, L2 Hit) Runtime->>CachedAdapter: getRoom(roomId) CachedAdapter->>L1Cache: get(roomId) L1Cache-->>CachedAdapter: undefined CachedAdapter->>L2Cache: get(cacheKey) L2Cache-->>CachedAdapter: Room data CachedAdapter->>L1Cache: set(roomId, room) CachedAdapter-->>Runtime: Room data Note over Runtime,DB: Read Operations (Cache Miss) Runtime->>CachedAdapter: getEntity(entityId) CachedAdapter->>L1Cache: get(entityId) L1Cache-->>CachedAdapter: undefined CachedAdapter->>L2Cache: get(cacheKey) L2Cache-->>CachedAdapter: undefined CachedAdapter->>BaseAdapter: getEntity(entityId) BaseAdapter->>DB: SELECT entity DB-->>BaseAdapter: Entity data BaseAdapter-->>CachedAdapter: Entity data CachedAdapter->>L1Cache: set(entityId, entity) CachedAdapter->>L2Cache: set(cacheKey, entity, ttl) CachedAdapter-->>Runtime: Entity data Note over Runtime,DB: Write Operations (Cache Invalidation) Runtime->>CachedAdapter: updateAgent(agentId, updates) CachedAdapter->>BaseAdapter: updateAgent(agentId, updates) BaseAdapter->>DB: UPDATE agent DB-->>BaseAdapter: Success BaseAdapter-->>CachedAdapter: Success CachedAdapter->>L1Cache: delete(agentId) CachedAdapter->>L2Cache: delete(cacheKey) CachedAdapter-->>Runtime: Success Note over Runtime,DB: Batch Operations Runtime->>CachedAdapter: getRoomsByIds([id1, id2, id3]) CachedAdapter->>L1Cache: get(id1) L1Cache-->>CachedAdapter: Room1 CachedAdapter->>L1Cache: get(id2) L1Cache-->>CachedAdapter: undefined CachedAdapter->>L1Cache: get(id3) L1Cache-->>CachedAdapter: undefined CachedAdapter->>BaseAdapter: getRoomsByIds([id2, id3]) BaseAdapter->>DB: SELECT rooms WHERE id IN (...) DB-->>BaseAdapter: [Room2, Room3] BaseAdapter-->>CachedAdapter: [Room2, Room3] CachedAdapter->>L1Cache: set(id2, Room2) CachedAdapter->>L1Cache: set(id3, Room3) CachedAdapter-->>Runtime: [Room1, Room2, Room3]