feat:Implement Supabase storage for conversation summaries #105
Conversation
|
""" WalkthroughThe code implements Supabase integration for storing conversation summaries. It loads Supabase credentials from environment variables, initializes a client, and fully implements the Changes
Sequence Diagram(s)sequenceDiagram
participant AgentState
participant SummarizationNode
participant Supabase
AgentState->>SummarizationNode: Trigger store_summary_to_database(state)
SummarizationNode->>Supabase: Upsert conversation summary (user/session info, summary, topics, etc.)
Supabase-->>SummarizationNode: Response (success or error)
SummarizationNode->>SummarizationNode: Log result
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/agents/devrel/nodes/summarization.py(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#75
File: backend/app/agents/devrel/agent.py:34-35
Timestamp: 2025-06-13T21:56:19.183Z
Learning: In the Devr.AI backend, the DevRelAgent follows a singleton pattern where only one instance exists for the entire application lifetime, using InMemorySaver with thread-based conversation management to persist user conversations across sessions.
backend/app/agents/devrel/nodes/summarization.py (2)
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#72
File: backend/app/agents/devrel/nodes/handle_technical_support_node.py:6-17
Timestamp: 2025-06-08T13:15:40.536Z
Learning: The handle_technical_support_node function in backend/app/agents/devrel/nodes/handle_technical_support_node.py is intentionally minimal and will be extended after database configuration is completed.
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#75
File: backend/app/agents/devrel/agent.py:34-35
Timestamp: 2025-06-13T21:56:19.183Z
Learning: In the Devr.AI backend, the DevRelAgent follows a singleton pattern where only one instance exists for the entire application lifetime, using InMemorySaver with thread-based conversation management to persist user conversations across sessions.
🧬 Code Graph Analysis (1)
backend/app/agents/devrel/nodes/summarization.py (1)
backend/app/agents/state.py (1)
AgentState(18-73)
🔇 Additional comments (3)
backend/app/agents/devrel/nodes/summarization.py (3)
17-18: LGTM! Clean utility function.The helper function handles the datetime to ISO format conversion safely with null checking.
145-175: Data Type Compatibility for key_topics
Supabase-py automatically serializes Pythonlistobjects into JSON arrays when inserting intojson/jsonbcolumns. No manualjson.dumpsis required for thekey_topicsfield.
164-166: Ignore null‐check for memory_thread_id – it’s always populated
Thememory_thread_idused in the upsert is driven by the orchestrator, which falls back touser_idwhen not explicitly provided, and the database schema enforces it as NOT NULL UNIQUE. In practice:
- In
agent_coordinator.py,guarantees a non-empty string.memory_thread_id = message_data.get("memory_thread_id") or message_data.get("user_id", "")- The LangGraph workflow maps that value into
state.thread_idbefore the summarization node runs.- The
conversation_contextschema declaresmemory_thread_id TEXT NOT NULL UNIQUE.Since
memory_thread_idcan never beNoneat this point, the upsert conflict strategy on"memory_thread_id"is safe and idempotent.Likely an incorrect or invalid review comment.
DhruvK278
changed the title
DhruvK278
changed the title
|
I'm facing this error, could you please check once @DhruvK278 attachment
could you please let me know how you were using this? (some dtype mismatch) PS: really sorry, I mistakenly clicked on "close with comment". Rest, the implementation seems very clean and well aligned as per your attached references. |
|
Hi, @smokeyScraper , this error is likely because of invalid uuid, uuid should look like this 006ccfe9-7662-4427-bfa3-2266009f6fe9 |
|
hey @DhruvK278, I see, tested a bit and realized the discord ID |
|
I'll try to resolve this by tomorrow. We probably have to change the IDs that are being stored and used in the DB. Also, your PR is upserts these based on Instead, we could use the user_id in conversation_context to directly refer users(id), this way we can simply get the Discord ID from the users table (a bit of a tradeoff on extra call/storage for a field). Could you please look at this once? I, too, will try aligning this throughout and will probably fix it by EOD. But yeah, I'm up for any suggestions. |
|
I see, as of now the user_id uuid is not UNIQUE in db. For this to work we will need to update the DB as you mentioned. After you update it I will use the change the upsert logic to |
|
yes, you've got it. rest, for the use case of |
|
hey @DhruvK278, I have fixed the schema. Now we are first looking for the ID of an user in the users table via a lookup using the platform-specific ID for that user, which will be |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/app/agents/devrel/nodes/summarization.py (2)
2-2: Remove unused import.The
osmodule is imported but not used in the code.-import os
8-10: Consider using centralized Supabase client initialization.The code directly calls
get_supabase_client()at module level. While this works, it creates a dependency on the Supabase client being available at import time.Consider moving the client initialization inside the function that uses it, or add proper error handling if the client initialization fails:
-supabase = get_supabase_client() + +def _get_supabase_client(): + """Get Supabase client with error handling""" + try: + return get_supabase_client() + except Exception as e: + logger.error(f"Failed to initialize Supabase client: {e}") + raiseThen use
supabase = _get_supabase_client()inside the function.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/agents/devrel/nodes/summarization.py(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
backend/app/agents/devrel/nodes/summarization.py (5)
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#72
File: backend/app/agents/devrel/nodes/handle_technical_support_node.py:6-17
Timestamp: 2025-06-08T13:15:40.536Z
Learning: The handle_technical_support_node function in backend/app/agents/devrel/nodes/handle_technical_support_node.py is intentionally minimal and will be extended after database configuration is completed.
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#87
File: backend/app/api/v1/health.py:1-12
Timestamp: 2025-06-28T23:14:21.089Z
Learning: In the Devr.AI project, the application is designed to run from the backend directory, making import paths like `from main import DevRAIApplication` correct for the current setup. Import path adjustments for deployment will be configured later during the deployment process.
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#72
File: backend/app/agents/devrel/nodes/handle_web_search_node.py:31-42
Timestamp: 2025-06-08T13:31:11.572Z
Learning: In backend/app/agents/devrel/tools/search_tool.py, the TavilySearchTool.search() method has partial error handling for missing API key, AttributeError, ConnectionError, and TimeoutError, but lacks a comprehensive Exception catch-all block, so calling functions may still need additional error handling for other potential exceptions.
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#72
File: backend/app/agents/shared/classification_router.py:0-0
Timestamp: 2025-06-08T13:08:48.469Z
Learning: The user plans to migrate the JSON parsing in backend/app/agents/shared/classification_router.py from manual JSON extraction to using Pydantic parser for better validation and type safety.
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#85
File: backend/app/services/auth/management.py:83-83
Timestamp: 2025-06-28T14:44:34.399Z
Learning: In the backend/app/services/auth/management.py file, the team prefers to use datetime.now() (local timezone/IST) during development and testing for easier debugging, with plans to change to UTC timezone-aware datetime (datetime.now(timezone.utc)) during deployment.
🧬 Code Graph Analysis (1)
backend/app/agents/devrel/nodes/summarization.py (2)
backend/app/agents/state.py (1)
AgentState(18-73)backend/app/database/supabase/client.py (1)
get_supabase_client(9-13)
🪛 Ruff (0.12.2)
backend/app/agents/devrel/nodes/summarization.py
2-2: os imported but unused
Remove unused import: os
(F401)
🔇 Additional comments (7)
backend/app/agents/devrel/nodes/summarization.py (7)
12-13: Helper function implementation looks good.The
isoformat_or_nonehelper function correctly handles optional datetime objects and provides consistent datetime formatting.
143-148: Validation logic is appropriate.The validation checks ensure required fields are present before proceeding with database operations.
149-161: User lookup implementation aligns with PR discussion.The code correctly implements the user lookup approach discussed in the PR comments, where it queries the users table using the platform-specific ID to get the internal UUID. This addresses the database schema alignment issue mentioned in the PR discussion.
The implementation properly:
- Constructs platform-specific column name (
discord_id,slack_id, etc.)- Queries the users table to get the internal UUID
- Validates that the user exists before proceeding
- Logs the found UUID for debugging
162-171: Record structure follows new schema approach.The record structure correctly uses the
user_idfield with the UUID from the users table, implementing the 1:1 relationship approach discussed in the PR comments. The removal ofmemory_thread_idfrom the record aligns with the updated database schema.
172-174: Upsert operation uses correct conflict resolution.The upsert operation correctly uses
on_conflict="user_id"which implements the 1:1 relationship between users and conversation contexts as discussed in the PR comments.
175-180: Response validation is improved.The response validation checks both the existence of
response.dataand that it contains at least one record, which is more robust than the previous implementation.
181-182: Error handling is comprehensive.The exception handling properly catches and logs any unexpected errors during the storage operation.
|
@smokeyScraper Done!! |
|
To maintain a clean commit history, could you please undo a few of your recent commits, make the necessary changes, and then commit with a force push. This keeps the commit history clean. I'm preferring "'n' commits for edits" + "1 commit for coderabbit fix" (Easiest is to use editor's (VS Code/Cursor) GitHub section for this) |
DhruvK278
force-pushed
the
enhancement/persistentDB-#78
branch
from
bd378ca to
007f905
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/app/agents/devrel/nodes/summarization.py (1)
2-2: Remove unused import.The
osimport is not used in the file since Supabase client initialization was moved to a separate module.-import os
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/agents/devrel/nodes/summarization.py(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#85
File: tests/test_supabase.py:1-3
Timestamp: 2025-06-28T14:45:55.244Z
Learning: In the Devr.AI project, smokeyScraper prefers to defer comprehensive test refactoring to separate PRs/efforts when doing major backend restructuring, rather than expanding the scope of the current refactoring PR to include test updates.
backend/app/agents/devrel/nodes/summarization.py (5)
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#72
File: backend/app/agents/devrel/nodes/handle_technical_support_node.py:6-17
Timestamp: 2025-06-08T13:15:40.536Z
Learning: The handle_technical_support_node function in backend/app/agents/devrel/nodes/handle_technical_support_node.py is intentionally minimal and will be extended after database configuration is completed.
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#87
File: backend/app/api/v1/health.py:1-12
Timestamp: 2025-06-28T23:14:21.089Z
Learning: In the Devr.AI project, the application is designed to run from the backend directory, making import paths like `from main import DevRAIApplication` correct for the current setup. Import path adjustments for deployment will be configured later during the deployment process.
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#72
File: backend/app/agents/devrel/nodes/handle_web_search_node.py:31-42
Timestamp: 2025-06-08T13:31:11.572Z
Learning: In backend/app/agents/devrel/tools/search_tool.py, the TavilySearchTool.search() method has partial error handling for missing API key, AttributeError, ConnectionError, and TimeoutError, but lacks a comprehensive Exception catch-all block, so calling functions may still need additional error handling for other potential exceptions.
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#72
File: backend/app/agents/shared/classification_router.py:0-0
Timestamp: 2025-06-08T13:08:48.469Z
Learning: The user plans to migrate the JSON parsing in backend/app/agents/shared/classification_router.py from manual JSON extraction to using Pydantic parser for better validation and type safety.
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#85
File: backend/app/services/auth/management.py:83-83
Timestamp: 2025-06-28T14:44:34.399Z
Learning: In the backend/app/services/auth/management.py file, the team prefers to use datetime.now() (local timezone/IST) during development and testing for easier debugging, with plans to change to UTC timezone-aware datetime (datetime.now(timezone.utc)) during deployment.
🧬 Code Graph Analysis (1)
backend/app/agents/devrel/nodes/summarization.py (2)
backend/app/agents/state.py (1)
AgentState(18-73)backend/app/database/supabase/client.py (1)
get_supabase_client(9-13)
🪛 Ruff (0.12.2)
backend/app/agents/devrel/nodes/summarization.py
2-2: os imported but unused
Remove unused import: os
(F401)
🔇 Additional comments (2)
backend/app/agents/devrel/nodes/summarization.py (2)
8-10: LGTM: Clean separation of concerns with centralized Supabase client.The import and initialization of the Supabase client through the centralized
get_supabase_client()function is a good architectural decision that promotes reusability and consistent configuration management.
12-13: LGTM: Useful helper function for datetime serialization.The
isoformat_or_nonehelper function properly handles optional datetime objects, providing consistent serialization for database storage while gracefully handling None values.
| async def store_summary_to_database(state: AgentState) -> None: | ||
| # TODO: Implement database storage to user_interactions | ||
| """Store the summary in PostgreSQL database""" | ||
| logger.info(f"[PLACEHOLDER] Storing summary for session {state.session_id}") | ||
| pass | ||
| """Store the summary in Supabase database""" | ||
| logger.info(f"Storing summary for session {state.session_id} into conversation_context") | ||
|
|
||
| try: | ||
| # Validation | ||
| if not state.user_id or not state.platform: | ||
| logger.error(f"Missing required fields: user_id={state.user_id}, platform={state.platform}") | ||
| return | ||
|
|
||
| platform_id = state.user_id | ||
| platform_column = f"{state.platform}_id" | ||
|
|
||
| # UUID fetch | ||
| user_response = await supabase.table("users").select("id").eq(platform_column, platform_id).limit(1).execute() | ||
|
|
||
| if not user_response.data: | ||
| logger.error(f"User with {platform_column} '{platform_id}' not found in users table.") | ||
| return | ||
|
|
||
| user_uuid = user_response.data[0]['id'] | ||
| logger.info(f"Found user UUID: {user_uuid} for {platform_column}: {platform_id}") | ||
|
|
||
| # Records | ||
| record = { | ||
| "user_id": user_uuid, | ||
| "conversation_summary": state.conversation_summary, | ||
| "key_topics": state.key_topics, | ||
| "total_interactions": state.interaction_count, | ||
| "session_start_time": isoformat_or_none(state.session_start_time), | ||
| "session_end_time": isoformat_or_none(datetime.now()), | ||
| } | ||
|
|
||
| # Upsert | ||
| response = await supabase.table("conversation_context").upsert(record, on_conflict="user_id").execute() | ||
|
|
||
| # Checks | ||
| if response.data and len(response.data) > 0: | ||
| logger.info(f"✅ Summary stored successfully for session {state.session_id}") | ||
| else: | ||
| logger.error(f"❌ Supabase upsert failed for session {state.session_id}: {response}") | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Unexpected error while storing summary: {str(e)}") |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify database schema alignment and consider additional error handling.
The implementation correctly follows the updated approach discussed in PR comments - looking up user UUID from the users table using platform-specific ID and upserting by user_id. The validation, logging, and error handling look solid.
However, there are a few areas to verify:
-
Database schema alignment: Ensure the
conversation_contexttable has the expected structure withuser_idas a unique constraint for the upsert operation. -
Platform ID validation: The code assumes
state.user_idcontains the platform-specific ID (e.g., Discord ID), but there's no validation that this ID format is appropriate for the platform.
Run the following script to verify the database schema and usage patterns:
🏁 Script executed:
#!/bin/bash
# Description: Verify database schema alignment and potential usage patterns
# Expected: Confirm conversation_context table structure and user_id uniqueness
# Check if there are any references to the old memory_thread_id approach
echo "=== Checking for old memory_thread_id usage ==="
rg -A 3 -B 3 "memory_thread_id" --type py
# Check for other usages of store_summary_to_database to understand call patterns
echo "=== Checking store_summary_to_database usage patterns ==="
rg -A 5 -B 5 "store_summary_to_database" --type py
# Check for platform ID validation patterns in the codebase
echo "=== Checking platform ID validation patterns ==="
rg -A 3 -B 3 "discord_id|slack_id|github_id" --type pyLength of output: 32486
I’ve reviewed the Supabase models and found that the ConversationContext model still defines a required memory_thread_id and platform field, but our upsert payload only includes user_id, conversation_summary, key_topics, total_interactions, session_start_time, and session_end_time. To align the code with the schema, we should:
- Include
platformandmemory_thread_idin the upsert record (to satisfy non-null constraints). - Confirm that
user_idhas a unique constraint on theconversation_contexttable for on_conflict upserts. - Add a simple format check for
state.user_id(e.g., ensure Discord IDs are numeric, GitHub IDs are numeric, Slack IDs match^U[A-Z0-9]+$, etc.) before querying.
File to update:
• backend/app/agents/devrel/nodes/summarization.py (in store_summary_to_database)
Suggested diff:
--- a/backend/app/agents/devrel/nodes/summarization.py
+++ b/backend/app/agents/devrel/nodes/summarization.py
@@ async def store_summary_to_database(state: AgentState) -> None:
- record = {
+ record = {
"user_id": user_uuid,
+ "platform": state.platform,
+ "memory_thread_id": state.session_id,
"conversation_summary": state.conversation_summary,
"key_topics": state.key_topics,
"total_interactions": state.interaction_count,
"session_start_time": isoformat_or_none(state.session_start_time),
"session_end_time": isoformat_or_none(datetime.now()),
}Also add just-before-upsert:
# Quick platform ID format validation
if state.platform == "discord" and not state.user_id.isdigit():
logger.error(f"Invalid Discord ID format: {state.user_id}")
return
# Add similar checks for slack_id and github_id as needed...With these adjustments, the upsert will satisfy the table schema and guard against malformed platform IDs.
🤖 Prompt for AI Agents
In backend/app/agents/devrel/nodes/summarization.py from lines 139 to 182, the
upsert record is missing the required non-null fields platform and
memory_thread_id according to the ConversationContext model. Add these two
fields to the record dictionary before the upsert call. Also, before querying
the users table, add validation checks on state.user_id format based on
state.platform (e.g., check if Discord and GitHub IDs are numeric, Slack IDs
match the regex ^U[A-Z0-9]+$) and log an error and return early if the format is
invalid. Finally, verify that user_id has a unique constraint in the
conversation_context table to safely use it in the on_conflict parameter of the
upsert.
Replaces the placeholder in store_summary_to_database with logic to upsert conversation summaries into the Supabase 'conversation_context' table. Fetches user UUID, validates required fields, and ensures one summary per user using upsert. Adds error handling and logging for database operations.
DhruvK278
force-pushed
the
enhancement/persistentDB-#78
branch
from
007f905 to
172401d
Compare
|
@coderabbitai review please |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll review the current state of the implementation after the database schema fixes and changes you and DhruvK278 have made. @smokeyScraper The implementation looks much better now! The database schema issues have been properly addressed. Here's what I can see: ✅ Issues Resolved:
✅ Implementation Quality:
Minor Suggestions:
The core database integration issues you and @DhruvK278 discussed have been well addressed. The implementation should now work correctly with the updated schema and avoid the UUID format errors you encountered earlier. 🏁 Script executed: Length of output: 6931 @smokeyScraper The implementation looks excellent now! All the database schema issues have been properly resolved. Here's my review: ✅ Issues Successfully Resolved:
✅ Implementation Quality:
Code Quality Highlights:
The implementation now correctly handles the Discord ID → UUID lookup flow and maintains the 1:1 user-to-conversation relationship as intended. The database integration should work smoothly with your updated schema. Great work on resolving the schema alignment issues! 🎉 🧠 Learnings used |
|
|
||
| supabase_url = os.getenv("SUPABASE_URL") | ||
| supabase_key = os.getenv("SUPABASE_KEY") | ||
| supabase: Client = create_client(supabase_url, supabase_key) |
There was a problem hiding this comment.
This leads to sync calls btw, refer backend/app/database/supabase/client.py and use it directly.
|
merged! |
2 participants
Replaces the placeholder database storage in store_summary_to_database with actual Supabase upsert logic. Adds environment variable loading, Supabase client initialization, and improved logging for summary storage operations.
Closes #78
📝 Description
This pull request implements persistent storage of conversation summaries by replacing the placeholder store_summary_to_database function with working Supabase logic. It ensures the DevRel agent can remember previous user interactions by upserting conversation state data into the conversation_context table on Supabase.
This enables long-term memory across sessions, improves personalization, and allows memory to persist even after restarts. The code uses environment variables to securely initialize the Supabase client and includes logging for both success and failure cases.
🔧 Changes Made
store_summary_to_databasewith functional Supabase upsert.conversation_contexttable usingmemory_thread_idto ensure idempotency..env.📷 Screenshots or Visual Changes (if applicable)
✅ Checklist
Summary by CodeRabbit
New Features
Chores