Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: (db) add limit param to memory retrieval across adapters #2264

Conversation

augchan42
Copy link
Contributor

@augchan42 augchan42 commented Jan 13, 2025

  • Add limit parameter to getMemoriesByRoomIds in SQLite adapter
  • Add limit parameter to getMemoriesByRoomIds in SQLjs adapter
  • Add limit parameter to getMemoriesByRoomIds in PGLite adapter
  • Add limit parameter to getMemoriesByRoomIds in Supabase adapter
  • Fix query parameter ordering in SQLjs adapter
  • Add consistent DESC ordering across all adapters

Without this, all memories are retrieved for that room since the limit being passed from getRecentInteractions in the core runtime is ignored silently:

    // Check the existing memories in the database
            return this.messageManager.getMemoriesByRoomIds({
                // filter out the current room id from rooms
                roomIds: rooms.filter((room) => room !== roomId),
                limit: 20,
            });

Closes #2253

Discord username: hosermage

…apters

- Add limit parameter to getMemoriesByRoomIds in SQLite adapter
- Add limit parameter to getMemoriesByRoomIds in SQLjs adapter
- Add limit parameter to getMemoriesByRoomIds in PGLite adapter
- Add limit parameter to getMemoriesByRoomIds in Supabase adapter
- Fix query parameter ordering in SQLjs adapter
- Add consistent DESC ordering across all adapters

Closes elizaOS#2253"
@augchan42
Copy link
Contributor Author

Oh, the postgres adapter does already have limit support, so those using postgres do not experience this issue.
in packages/adapter-postgres/src/index.ts, getMemoriesByRoomIds function:

            // Add sorting, and conditionally add LIMIT if provided
            query += ` ORDER BY "createdAt" DESC`;
            if (params.limit) {
                query += ` LIMIT $${queryParams.length + 1}`;
                queryParams.push(params.limit.toString());
            }

@shakkernerd
Copy link
Member

Oh, the postgres adapter does already have limit support, so those using postgres do not experience this issue. in packages/adapter-postgres/src/index.ts, getMemoriesByRoomIds function:

            // Add sorting, and conditionally add LIMIT if provided
            query += ` ORDER BY "createdAt" DESC`;
            if (params.limit) {
                query += ` LIMIT $${queryParams.length + 1}`;
                queryParams.push(params.limit.toString());
            }

Yes, the limit implementation was missing in the sqlite adapter and most likely other too.

Copy link
Member

@shakkernerd shakkernerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good one!

@shakkernerd shakkernerd merged commit a0d8537 into elizaOS:develop Jan 14, 2025
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getRecentPostInteractions returning all memory
2 participants