feat: add getMemoryByIds to database adapters #2293
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi @bbopar! Welcome to the elizaOS community. Thanks for submitting your first pull request; your efforts are helping us accelerate towards AGI. We'll review it shortly. You are now an elizaOS contributor!
|
@coderabbitai review |
📝 WalkthroughWalkthroughA new method Changes
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/core/__tests__/database.test.ts (1)
18-29: Enhance mock implementation to utilize tableName parameter.The mock implementation currently ignores the
tableNameparameter. Consider filtering memories based on the providedtableNameto better simulate real database behavior.async getMemoriesByIds( memoryIds: UUID[], - _tableName?: string + tableName?: string ): Promise<Memory[]> { - return memoryIds.map((id) => ({ + let memories = memoryIds.map((id) => ({ id: id, content: { text: "Test Memory" }, roomId: "room-id" as UUID, userId: "user-id" as UUID, agentId: "agent-id" as UUID, + type: "default-type" })) as Memory[]; + return tableName ? memories.filter(m => m.type === tableName) : memories; }packages/adapter-sqlite/src/index.ts (1)
207-232: Strong implementation with minor type improvement needed.The implementation is robust with proper SQL injection prevention and data type handling. Consider using a more specific type for queryParams.
-const queryParams: any[] = []; +const queryParams: (UUID | string)[] = [];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/adapter-pglite/src/index.ts(1 hunks)packages/adapter-postgres/src/index.ts(1 hunks)packages/adapter-sqlite/src/index.ts(1 hunks)packages/adapter-sqljs/src/index.ts(1 hunks)packages/adapter-supabase/src/index.ts(1 hunks)packages/core/__tests__/database.test.ts(1 hunks)packages/core/src/database.ts(2 hunks)packages/core/src/types.ts(4 hunks)
👮 Files not reviewed due to content moderation or server errors (4)
- packages/adapter-sqljs/src/index.ts
- packages/core/src/types.ts
- packages/adapter-pglite/src/index.ts
- packages/adapter-postgres/src/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: smoke-tests
- GitHub Check: integration-tests
🔇 Additional comments (1)
packages/core/src/database.ts (1)
103-112: LGTM! Well-documented abstract method.The method signature and documentation clearly define the contract for retrieving multiple memories by their IDs.
Relates to
#1186 - Add getMemoriesByIds
Risks
Low - Added new feature and tested (non-breaking change which adds functionality)
Background
What does this PR do?
Adds
getMemoriesByIdsmethod to all database adapters (pglite, postgres, sqlite, sqljs and supabase) to support batch memory retrieval. This enables fetching multiple memories by their IDs in a single query, with optional filtering by table name.What kind of change is this?
Features (non-breaking change which adds functionality)
Documentation changes needed?
Documentation update is needed but there's a current issue with typedoc generation:
This will need to be addressed in a separate PR to fix the documentation build process.
Testing
Where should a reviewer start?
getMemoriesByIdsin each database adapter:pgliteimplementationpostgresimplementationsqliteimplementationsqljsimplementationsupabaseimplementation1186-add-get-memories-by-ids-testwhere you can test the impl for all adapters.Detailed testing steps
git clone https://github.com/elizaOS/eliza.git cd eliza git checkout 1186-add-get-memories-by-ids-testbranch for testing: https://github.com/bbopar/eliza/tree/1186-add-get-memories-by-ids-test
Choose and set up the database adapter you want to test (pglite/postgres/sqlite/sqljs/supabase)
Test cases (using temporary API endpoint for validation):
Test Case 1: Basic memory retrieval with table filter
Expected output:
Test Case 2: Invalid table name handling
Expected output:
Test Case 3: Empty memory IDs array
Expected output:
Test Case 4: Missing table name (no type filter)
Expected output:
Test Case 5: Large batch retrieval
Expected output:
Discord username:
.0xbbjoker