-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
chore(fix): supabase adapter #2258
base: develop
Are you sure you want to change the base?
Conversation
- **Enhanced Error Handling**: Improved logging and structured error messages for better debugging. - **Query Optimization**: Refactored how memory tables are queried dynamically based on embedding size. - **More Robust `getMemoryById` Implementation**: Now iterates through potential memory tables instead of assuming a single table. - **Ensuring Unique Entries in Relationships**: Improved relationship handling by checking for existing rooms before creating new ones. - **More Efficient Memory Creation**: Includes validation for embedding sizes and converts timestamps properly. - **Caching for Knowledge Search**: Implements a caching mechanism to improve performance. - **Better Handling of `maybeSingle()`**: Uses `maybeSingle()` in cases where a single record is expected, avoiding unnecessary errors.
Added functions that need to be pasted into supabase SQL editor along with this entire schema to create the db for Eliza.
Tried this PR as Supabase is broken on the latest tag and develop: Error: Error creating room: Could not find the function public.create_room(roomId) in the schema cache Looks like we're still missing some functions |
I wonder if I missed adding the create_room function to schema.sql. I'll check tonight. |
Added missing functions.
Yep, somehow I didn't add ANY of the functions to the schema.sql Fixed. |
did this work ongoing? |
On my end, the db builds fine. Everything like memories, cache, rooms, actors, etc are writing and reading properly. However, I had an error after I PR'sd with Twitter posting, but the error came back as an empty bracket. I will have to do some debuging tonight arfter work. If @RobertDoc , or someone else would like to test to make sure everything but twitter posting works on their end, that would be great confirmation. Whoever tests will need to copy and paste shcema.sql into their supabase sql editor and add the 2 Supabase variables to their |
fix(adapter-supabase): improve memory retrieval and participant state handling - Fix timestamp handling in getMemories - Use maybeSingle for participant state queries - Add proper error handling for memory table lookups
fix(supabase): update get_goals function to handle case sensitivity and column order - Add quotes to preserve case-sensitive column names - Fix RETURNS TABLE column order to match goals table structure - Add table alias to prevent ambiguous column references - Update WHERE clause to use table alias for column references
OK, everything works now. Tweets are generating without error. I made some changes to the get_goals function in the sdchema.sql, and some changes to ts.index. |
We ned a good way to perform robust database testing. I am getting zero errors, but it seems that chat client messaging and discord messaging are responding without context to the question when using supabase, but not with the local sqlite db. I am theorizing it has something to do with memories. |
{"code":"42883","details":null,"hint":"No function matches the given name and argument types. You might need to add explicit type casts.","message":"function levenshtein(text, text) does not exist"} getting this error too |
I added functions in the new commit. Can you please try again. Everything is working on my end with these changes. I just need to test RAGing knowledge. |
Added functionality for multiple knowledge tables. RAG works and is tested.
Working SChema for Supabase
Cleaned up logging and added some error handling
Added needed conditionals for RAG
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 WalkthroughThe pull request introduces comprehensive changes to the Supabase database adapter, focusing on enhancing memory and knowledge management. The modifications involve restructuring the SQL schema, dropping and recreating functions, and updating the database adapter's implementation. The changes aim to improve data integrity, performance, and flexibility in handling memories, knowledge chunks, and participant interactions. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/adapter-supabase/src/index.ts (2)
617-617
: Remove unnecessarycontinue
statementThe
continue
statement at the end of the loop is redundant and can be safely removed.Apply this diff to remove it:
- continue;
🧰 Tools
🪛 Biome (1.9.4)
[error] 617-617: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
512-517
: Validate embeddings upfront to simplify code logicConsider returning early if the embedding array is invalid or empty to reduce nesting and improve readability.
🧰 Tools
🪛 Biome (1.9.4)
[error] 512-512: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
[error] 514-514: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/adapter-supabase/schema.sql
(11 hunks)packages/adapter-supabase/src/index.ts
(21 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/adapter-supabase/src/index.ts
[error] 617-617: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 512-512: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 514-514: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
🔇 Additional comments (1)
packages/adapter-supabase/schema.sql (1)
354-370
:⚠️ Potential issueCorrect table reference in
count_memories
functionThe
count_memories
function incorrectly queriespublic.memories
instead of the dynamic table specified byquery_table_name
.Apply this diff to fix the table reference:
- SELECT COUNT(*) FROM public.memories - WHERE public.memories."type" = query_table_name + EXECUTE format(' + SELECT COUNT(*) FROM %I + WHERE (%L IS NULL OR "roomId" = %L) + AND (%L IS FALSE OR "unique" = TRUE) + ', query_table_name, query_roomid, query_roomid, query_unique) + INTO result; + RETURN result;Likely invalid or redundant comment.
Changed to Number.isNAN per bot suggestion.
@coderabbitai generate docstrings |
Caution No docstrings were generated. |
- Removed table name parameter from search_knowledge RPC call - Table selection is now handled internally by the function based on embedding size - Improved error logging for vector operations The changes fix two main issues: 1. Vector dimension checking using the correct pgvector function 2. JSONB text comparison operations in similarity search These updates make the vector similarity search more robust and fix compatibility issues with pgvector extension.
- Changed `array_length(query_embedding, 1)` to `vector_dims(query_embedding)` in search_knowledge function - array_length doesn't work with pgvector's vector type - vector_dims is the correct function for getting vector dimensions - Added proper text casting for JSONB comparisons in search_knowledge_table - Added `::text` casts to fix JSONB comparison operators - Fixed ILIKE operations on JSONB fields - Changed function delimiters from `$$` to `$func$` for better readability - Added CASCADE to function drops to ensure clean recreation
I'm having some trouble pulling a robust test together for this, but I ma working on it |
Relates to
This PR does not relate to a specific issue but introduces multiple enhancements and optimizations to the Supabase database adapter.
Risks
Risk Level: Low
Potential Impact:
Background
What does this PR do?
This PR introduces several improvements and optimizations to the
SupabaseDatabaseAdapter
inpackages/adapter-supabase/src/index.ts
. The key changes include:getMemoryById
Implementation: Now iterates through potential memory tables instead of assuming a single table.maybeSingle()
: UsesmaybeSingle()
in cases where a single record is expected, avoiding unnecessary errors.What kind of change is this?
Documentation changes needed?
Testing
Where should a reviewer start?
SupabaseDatabaseAdapter.ts
inpackages/adapter-supabase/src/index.ts
.getMemoryById
,createMemory
,getRoomsForParticipants
, andsearchKnowledge
methods.Detailed testing steps
searchKnowledge()
) and checking if cache retrieval works.384, 768, 1024, 1536
).Deploy Notes
Database changes
Deployment instructions
Discord username
4n7m4n