feat: database metrics setup with enhancing memory logic for downstream metrics #144
Conversation
WalkthroughIntegrates DB-backed user and conversation persistence into the DevRel pipeline: nodes now ensure users exist and retrieve user_uuid, optionally fetch prior conversation context, store interactions after response generation, cache user_uuid in summarization, and enrich Discord payloads with avatar and classification fields. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Gather as gather_context
participant DB as "DB Services"
participant Gen as generate_response
participant Discord
participant Sum as summarization
User->>Gather: Send message
Gather->>DB: ensure_user_exists(platform_id, platform, ...)
DB-->>Gather: user_uuid
alt no conversation_summary/key_topics in state
Gather->>DB: get_conversation_context(user_uuid)
DB-->>Gather: previous_conversation + summary + key_topics
Gather-->>Gather: assemble payload with previous context (early return)
else existing summary/topics present
Gather-->>Gather: merge context_data with state.context (normal flow)
end
Gather->>Gen: context + messages
Gen-->>Gen: generate final_response
Gen->>DB: store_interaction(user_uuid, platform_specific_id, content, intent, topics, metadata)
DB-->>Gen: success/failure (logged)
Gen->>Discord: send response + classification
Discord->>Sum: notify/trigger summarization
Sum->>DB: store_summary_to_database(user_uuid or lookup)
DB-->>Sum: success/failure (logged)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
backend/app/agents/devrel/nodes/summarization.py (1)
185-185: Use logger.exception with parameterized message (Ruff TRY400/RUF010).Capture traceback and avoid f-strings in logs.
- logger.error(f"Unexpected error while storing summary: {str(e)}") + logger.exception("Unexpected error while storing summary: %s", e)backend/integrations/discord/bot.py (3)
84-85: Avoid duplicating triage and classification in payload.Send a single field (e.g., classification) to reduce ambiguity; keep the other only if needed for backward compatibility and mark it deprecated in consumers.
If consumers still read triage, confirm and plan a follow-up removal.
88-91: Prefer display_avatar for consistent avatar URLs; guard attributes.More reliable across Users/Members and default avatars.
- "author": { - "username": message.author.name, - "display_name": message.author.display_name, - "avatar_url": str(message.author.avatar.url) if message.author.avatar else None - } + "author": { + "username": getattr(message.author, "name", None), + "display_name": getattr(message.author, "display_name", getattr(message.author, "global_name", None)), + "avatar_url": str(getattr(getattr(message.author, "display_avatar", None), "url", "")) or None + }
144-144: Use logger.exception to include stack trace (Ruff TRY400/RUF010).- logger.error(f"Error handling agent response: {str(e)}") + logger.exception("Error handling agent response: %s", e)backend/app/agents/devrel/nodes/generate_response.py (2)
128-129: Use a real platform message ID when available; timestamp fallback only if absent.Better deduplication and traceability downstream.
- platform_specific_id=f"{state.session_id}_{datetime.now().timestamp()}", + platform_specific_id=( + state.context.get("id") + or state.context.get("platform_message_id") + or f"{state.session_id}:{int(datetime.now().timestamp()*1000)}" + ),
143-144: Log exceptions with traceback (Ruff TRY400/RUF010); broad except is acceptable here but document rationale.- except Exception as e: - logger.error(f"Error storing interaction to database: {str(e)}") + except Exception as e: # Boundary to prevent DB errors from breaking response path + logger.exception("Error storing interaction to database: %s", e)backend/app/agents/devrel/nodes/gather_context.py (2)
45-46: Remove f-prefix on constant strings (Ruff F541).- logger.info(f"Retrieved previous conversation context from database") + logger.info("Retrieved previous conversation context from database") ... - logger.info(f"No previous conversation context found in database") + logger.info("No previous conversation context found in database")Also applies to: 58-58
16-23: Mask UUIDs in logs to reduce exposure of persistent identifiers.- logger.info(f"No existing context in state, fetching from database for user {user_uuid}") + logger.info("No existing context in state, fetching from database for user %s…", (str(user_uuid)[:8] if user_uuid else None))Also consider masking in any other UUID logs in this module.
Also applies to: 35-36, 41-43
backend/app/database/supabase/services.py (1)
79-81: Prefer logger.exception and parameterized messages; keep broad except only at boundary.Improves diagnostics and satisfies Ruff TRY400/RUF010.
- except Exception as e: - logger.error(f"Error ensuring user exists: {str(e)}") + except Exception as e: + logger.exception("Error ensuring user exists: %s", e) ... - except Exception as e: - logger.error(f"Error storing interaction: {str(e)}") + except Exception as e: + logger.exception("Error storing interaction: %s", e) ... - except Exception as e: - logger.error(f"Error retrieving conversation context: {str(e)}") + except Exception as e: + logger.exception("Error retrieving conversation context: %s", e)Also applies to: 156-159, 188-190
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/app/agents/devrel/nodes/gather_context.py(2 hunks)backend/app/agents/devrel/nodes/generate_response.py(3 hunks)backend/app/agents/devrel/nodes/summarization.py(2 hunks)backend/app/database/supabase/services.py(1 hunks)backend/integrations/discord/bot.py(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
backend/app/database/supabase/services.py (1)
backend/app/database/supabase/client.py (1)
get_supabase_client(9-13)
backend/app/agents/devrel/nodes/gather_context.py (2)
backend/app/database/supabase/services.py (2)
ensure_user_exists(11-81)get_conversation_context(161-190)backend/app/agents/state.py (1)
AgentState(18-74)
backend/integrations/discord/bot.py (1)
backend/app/core/orchestration/queue_manager.py (1)
QueuePriority(12-15)
backend/app/agents/devrel/nodes/generate_response.py (2)
backend/app/agents/state.py (1)
AgentState(18-74)backend/app/database/supabase/services.py (1)
store_interaction(84-158)
🪛 Ruff (0.14.0)
backend/app/database/supabase/services.py
79-79: Do not catch blind exception: Exception
(BLE001)
80-80: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
80-80: Use explicit conversion flag
Replace with conversion flag
(RUF010)
156-156: Do not catch blind exception: Exception
(BLE001)
157-157: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
157-157: Use explicit conversion flag
Replace with conversion flag
(RUF010)
188-188: Do not catch blind exception: Exception
(BLE001)
189-189: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
189-189: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/agents/devrel/nodes/summarization.py
185-185: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
185-185: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/agents/devrel/nodes/gather_context.py
45-45: f-string without any placeholders
Remove extraneous f prefix
(F541)
58-58: f-string without any placeholders
Remove extraneous f prefix
(F541)
backend/integrations/discord/bot.py
144-144: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
144-144: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/agents/devrel/nodes/generate_response.py
143-143: Do not catch blind exception: Exception
(BLE001)
144-144: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
144-144: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (3)
backend/app/agents/devrel/nodes/gather_context.py (1)
41-56: LGTM on conditional DB fetch and early return.Branching is clean and avoids unnecessary I/O when context already exists.
backend/app/agents/devrel/nodes/generate_response.py (1)
21-23: Verification confirms approval is correct.The persistence call executes exactly once per session in the main
generate_response_node()flow (line 22), with proper error handling at both the function level (lines 110+) and caller level, ensuring storage errors won't bubble to the user. No issues found.backend/app/database/supabase/services.py (1)
36-41: The review comment's race condition concern is invalid—UNIQUE constraints already exist on platform IDs.The schema at
backend/app/database/supabase/scripts/create_db.sqlalready definesdiscord_id,github_id, andslack_idasUNIQUEcolumns. This means the database prevents duplicate platform IDs at the constraint level, not via application logic. The check-then-insert pattern is suboptimal but does not create race conditions that bypass database constraints.The suggested upsert pattern remains valid for idempotency and cleaner semantics, but reframe it as improving code clarity rather than fixing a race condition. Additionally, the review comment does not flag the actual race condition present in the codebase: lines 145–150 in
store_interactionperform a non-atomic read-then-write ontotal_interactions_count, which is a genuine concurrency issue.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
backend/app/agents/devrel/nodes/generate_response.py (2)
130-130: Consider more robust platform_specific_id generation.The current timestamp-based approach could produce collisions if multiple messages occur within the same millisecond in a session. Consider using a UUID or combining with message index.
- platform_specific_id=f"{state.session_id}_{datetime.now().timestamp()}", + platform_specific_id=f"{state.session_id}_{uuid.uuid4()}",You'll need to add
import uuidat the top of the file.
145-146: Use logging.exception for better error diagnostics.Replace
logging.errorwithlogging.exceptionto automatically include the stack trace, making debugging easier.- except Exception as e: - logger.error(f"Error storing interaction to database: {str(e)}") + except Exception as e: + logger.exception("Error storing interaction to database")Based on static analysis hints.
backend/app/database/supabase/services.py (3)
79-81: Use logging.exception for better error diagnostics.Replace
logging.errorwithlogging.exceptionto automatically capture the stack trace.- except Exception as e: - logger.error(f"Error ensuring user exists: {str(e)}") + except Exception as e: + logger.exception("Error ensuring user exists") return NoneBased on static analysis hints.
151-157: Use logging.exception and fix control flow.The exception handlers should use
logging.exceptionfor stack traces. Also, the missing return after RPC failure causes an implicitNonereturn, which is now fixed in the previous comment.except Exception as e: - # Not failing the entire operation if incrementing the interaction count fails - logger.error(f"Error incrementing user interaction count: {str(e)}") + logger.exception("Error incrementing user interaction count") except Exception as e: - logger.error(f"Error storing interaction: {str(e)}") + logger.exception("Error storing interaction") return FalseBased on static analysis hints.
187-189: Use logging.exception for better error diagnostics.Replace
logging.errorwithlogging.exceptionto capture the stack trace.except Exception as e: - logger.error(f"Error retrieving conversation context: {str(e)}") + logger.exception("Error retrieving conversation context") return NoneBased on static analysis hints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/app/agents/devrel/nodes/generate_response.py(3 hunks)backend/app/database/supabase/scripts/create_db.sql(1 hunks)backend/app/database/supabase/services.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/app/database/supabase/services.py (1)
backend/app/database/supabase/client.py (1)
get_supabase_client(9-13)
backend/app/agents/devrel/nodes/generate_response.py (2)
backend/app/agents/state.py (1)
AgentState(18-74)backend/app/database/supabase/services.py (1)
store_interaction(84-157)
🪛 Ruff (0.14.1)
backend/app/database/supabase/services.py
79-79: Do not catch blind exception: Exception
(BLE001)
80-80: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
80-80: Use explicit conversion flag
Replace with conversion flag
(RUF010)
151-151: Do not catch blind exception: Exception
(BLE001)
153-153: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
153-153: Use explicit conversion flag
Replace with conversion flag
(RUF010)
155-155: Do not catch blind exception: Exception
(BLE001)
156-156: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
156-156: Use explicit conversion flag
Replace with conversion flag
(RUF010)
187-187: Do not catch blind exception: Exception
(BLE001)
188-188: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
188-188: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/agents/devrel/nodes/generate_response.py
145-145: Do not catch blind exception: Exception
(BLE001)
146-146: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
146-146: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (3)
backend/app/database/supabase/scripts/create_db.sql (1)
121-142: LGTM! Atomic counter implementation is solid.The function correctly uses
UPDATE ... RETURNINGto atomically increment the counter and return the new value. This addresses the previous concern about race conditions in the read-modify-write pattern.backend/app/agents/devrel/nodes/generate_response.py (2)
4-8: LGTM! Imports are appropriate.The new imports support the interaction persistence functionality.
21-22: LGTM! Interaction storage properly integrated.The storage call is correctly placed after response generation and properly awaited.
smokeyScraper
force-pushed
the
metrics_setup
branch
from
4fec495 to
688a83b
Compare
2 participants
Database Interactions
This PR ensures proper storage of context, user data, and interactions in the database for downstream metrics.
Summary by CodeRabbit