[feat]: InMemorySaver() introduction with custom reducers and summarization node for context handling #75
Conversation
WalkthroughThis update introduces comprehensive conversation summarization and memory management to the DevRel agent system. It adds summarization nodes, state persistence keyed by thread ID, and memory clearing logic. Node return values are standardized to dictionaries, and the agent state model is extended for session tracking. Discord integration now supports explicit memory resets. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DiscordBot
participant AgentCoordinator
participant DevRelAgent
participant InMemorySaver
participant LLM
User->>DiscordBot: Sends message or !reset
DiscordBot->>AgentCoordinator: Sends devrel request (with memory_thread_id) or clear_thread_memory
AgentCoordinator->>DevRelAgent: run(initial_state, thread_id)
DevRelAgent->>InMemorySaver: Retrieve or save state by thread_id
DevRelAgent->>LLM: Generate response / Summarize conversation (if needed)
DevRelAgent-->>AgentCoordinator: Returns updated state
AgentCoordinator->>InMemorySaver: Clear memory if timeout or manual reset
AgentCoordinator->>DiscordBot: Returns response
DiscordBot->>User: Sends final response or reset confirmation
Possibly related PRs
Suggested labels
Poem
✨ 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 (
|
|
Warning Gemini encountered an error creating the summary. You can try again by commenting |
|
/gemini summary |
Summary of ChangesThis pull request significantly enhances the DevRel agent's ability to maintain context across multiple interactions by introducing state persistence using LangGraph's Highlights
Changelog
Activity
|
![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
🔭 Outside diff range comments (3)
backend/app/agents/devrel/nodes/handle_technical_support_node.py (1)
6-17:⚠️ Potential issueReturn-type annotation is now wrong – update to
dictThe body now returns a plain dictionary, but the signature still advertises
-> AgentState. This will confuse static analysers, IDEs, and anyone consuming the function.-async def handle_technical_support_node(state: AgentState) -> AgentState: +async def handle_technical_support_node(state: AgentState) -> dict:(Optionally refine the doc-string to mention the dictionary contract.)
backend/app/agents/devrel/nodes/handle_onboarding_node.py (1)
6-17:⚠️ Potential issueMismatch between declared and actual return type
Same issue as in the technical-support node – function returns a
dictwhile the annotation saysAgentState.-async def handle_onboarding_node(state: AgentState) -> AgentState: +async def handle_onboarding_node(state: AgentState) -> dict:Aligning signatures avoids mypy / pyright errors and keeps the codebase consistent.
backend/app/agents/devrel/nodes/gather_context_node.py (1)
8-9:⚠️ Potential issueReturn-type annotation is now incorrect – update to reflect
dictresult
gather_context_nodewas refactored to return a raw dictionary, yet the signature still advertises-> AgentState.
Any downstream static analysis or type-checking will flag this as an error and IDEs will offer the wrong completions.-async def gather_context_node(state: AgentState) -> AgentState: +async def gather_context_node(state: AgentState) -> dict:Remember to adjust doc-strings and any callers that relied on the old contract.
🧹 Nitpick comments (6)
backend/app/agents/devrel/nodes/handle_faq_node.py (1)
6-26: Add minimal typing + clarifyfaq_toolexpectations
faq_toolis untyped, so editors can’t surface async/attr mistakes. Consider a lightweight Protocol:from typing import Protocol, runtime_checkable @runtime_checkable class FAQTool(Protocol): async def get_response(self, query: str) -> str: ...and change the signature:
-async def handle_faq_node(state: AgentState, faq_tool) -> dict: +async def handle_faq_node(state: AgentState, faq_tool: FAQTool) -> dict:Nice to have, but it will improve correctness and discoverability.
backend/app/agents/devrel/nodes/gather_context_node.py (1)
32-35: Consider appending to existingstate.messagesinstead of starting a new listReturning
{"messages": [new_message], ...}discards the prior history held instate.messages.
If the caller simply merges dicts (state |= outputpattern), you’ll lose the existing messages array.
Safer option:- "messages": [new_message], + "messages": state.messages + [new_message],…unless the orchestration layer intentionally rebuilds history elsewhere.
backend/bots/discord/discord_bot.py (1)
242-243: Minor: help text still says “chat thread and memory” – maybe mention the!resetdelayUsers might appreciate a hint that the next message will spawn a new thread automatically (mirrors the confirmation you send).
Totally optional.backend/app/agents/devrel/nodes/generate_response_node.py (1)
50-63: Potential zero/negative session-duration & naive-datetime issue
state.last_interaction_timeandstate.session_start_timeare naivedatetime.now()instances.
If the orchestrator later switches to timezone-aware stamps this subtraction will raise.
Also, first interaction yields0.0 minutes, which may not read nicely to end-users.- f"Session duration: {(state.last_interaction_time - state.session_start_time).total_seconds() / 60:.1f} minutes" + f"Session duration: {max(1, int((state.last_interaction_time - state.session_start_time).total_seconds() // 60))} min"Or simply omit on first message.
backend/app/core/orchestration/agent_coordinator.py (1)
4-4: Remove unuseddatetimeimport (Ruff F401)The module no longer references
datetime; keeping the import will fail the linter.-from datetime import datetime🧰 Tools
🪛 Ruff (0.11.9)
4-4:
datetime.datetimeimported but unusedRemove unused import:
datetime.datetime(F401)
backend/app/agents/devrel/nodes/summarization_node.py (1)
131-136: Placeholder function needs implementation.The TODO indicates this function needs to store summaries to PostgreSQL. This is important for persistent memory across sessions.
Would you like me to help implement the database storage logic or create an issue to track this task?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
backend/app/agents/devrel/agent.py(6 hunks)backend/app/agents/devrel/nodes/gather_context_node.py(2 hunks)backend/app/agents/devrel/nodes/generate_response_node.py(1 hunks)backend/app/agents/devrel/nodes/handle_faq_node.py(2 hunks)backend/app/agents/devrel/nodes/handle_onboarding_node.py(1 hunks)backend/app/agents/devrel/nodes/handle_technical_support_node.py(1 hunks)backend/app/agents/devrel/nodes/handle_web_search_node.py(2 hunks)backend/app/agents/devrel/nodes/summarization_node.py(1 hunks)backend/app/agents/devrel/prompts/base_prompt.py(1 hunks)backend/app/agents/devrel/prompts/summarization_prompt.py(1 hunks)backend/app/agents/shared/base_agent.py(2 hunks)backend/app/agents/shared/state.py(2 hunks)backend/app/core/orchestration/agent_coordinator.py(5 hunks)backend/bots/discord/discord_bot.py(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
backend/app/agents/devrel/nodes/handle_faq_node.py (1)
backend/app/agents/shared/state.py (1)
AgentState(18-73)
backend/bots/discord/discord_bot.py (1)
backend/app/core/orchestration/queue_manager.py (2)
enqueue(48-65)QueuePriority(9-12)
backend/app/agents/shared/base_agent.py (1)
backend/app/agents/shared/state.py (1)
AgentState(18-73)
backend/app/agents/devrel/nodes/summarization_node.py (1)
backend/app/agents/shared/state.py (1)
AgentState(18-73)
backend/app/agents/devrel/nodes/handle_web_search_node.py (1)
backend/app/agents/shared/state.py (1)
AgentState(18-73)
backend/app/agents/devrel/nodes/generate_response_node.py (1)
backend/app/agents/shared/state.py (1)
AgentState(18-73)
backend/app/agents/devrel/agent.py (2)
backend/app/agents/devrel/nodes/summarization_node.py (3)
check_summarization_needed(13-45)summarize_conversation_node(47-108)store_summary_to_database(131-135)backend/app/agents/shared/state.py (1)
AgentState(18-73)
🪛 Ruff (0.11.9)
backend/app/core/orchestration/agent_coordinator.py
4-4: datetime.datetime imported but unused
Remove unused import: datetime.datetime
(F401)
backend/app/agents/devrel/agent.py
4-4: datetime.datetime imported but unused
Remove unused import
(F401)
4-4: datetime.timedelta imported but unused
Remove unused import
(F401)
🪛 Pylint (3.3.7)
backend/app/agents/devrel/agent.py
[refactor] 149-164: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
🔇 Additional comments (16)
backend/app/agents/devrel/prompts/summarization_prompt.py (1)
1-18: Prompt addition looks solidThe template is clear, well-structured, and follows existing constant-naming conventions. No issues spotted.
backend/app/agents/devrel/nodes/handle_web_search_node.py (1)
31-43: 🛠️ Refactor suggestionWrap external call in try/except to avoid node hard-failure
search_tool.searchis an external dependency and may raise (network timeouts, quota, etc.). A guarded call keeps the graph alive and allows graceful fallback.- search_results = await search_tool.search(search_query) + try: + search_results = await search_tool.search(search_query) + except Exception as exc: + logger.error("Search tool failed: %s", exc) + search_results = []Returning an empty list lets downstream nodes decide how to respond without crashing the run.
⛔ Skipped due to learnings
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 already includes comprehensive error handling that catches all exceptions and returns an empty list instead of raising them, so calling functions don't need additional try-catch blocks.backend/bots/discord/discord_bot.py (2)
87-88: Verify thatmemory_thread_id = user_idwon’t leak context across guildsA single Discord user can participate in multiple servers.
Using plainuser_idas the memory key means conversations from different guilds (and even DMs) share one long-term memory blob.
If that is not desirable, consider namespacing:memory_thread_id = f"{message.guild.id}:{user_id}" if message.guild else f"dm:{user_id}"
193-201: Nice touch – explicit high-priority memory-clear message looks solid
The enqueue call includes a clear, high-priority payload; coordinator side already registers the handler.
LGTM.backend/app/agents/devrel/prompts/base_prompt.py (1)
5-27: Prompt restructuring looks clean and self-explanatory
The new sections should help the model leverage both short- and long-term context.
No issues spotted.backend/app/agents/devrel/nodes/generate_response_node.py (1)
96-99:final_responseis not written back toAgentStateDown-stream logic (e.g. summarisation, coordinator) still inspects
state.final_response.
Currently you return a dict with that key, but the originalAgentStateinstance remains unchanged.
Ensure the orchestrator merges this dict into the state or, easier, also set the field:- return { - "final_response": final_response, - "current_task": "response_generated" - } + state.final_response = final_response + state.current_task = "response_generated" + return {"final_response": final_response, "current_task": state.current_task}Confirm whichever merging strategy you adopted picks this up.
backend/app/core/orchestration/agent_coordinator.py (2)
38-40: Emptymemory_thread_idfallback path can disable persistenceIf
user_idis missing (edge integrations) you may pass an empty string to the agent’s saver, creating orphaned checkpoints.
Better to guard:memory_thread_id = message_data.get("memory_thread_id") or message_data.get("user_id") if not memory_thread_id: memory_thread_id = f"anon_{uuid.uuid4()}"
71-101: Robust & well-logged memory-clear workflowThe handler stores a final summary before wiping the saver – exactly what we want to avoid data loss.
Good defensive logging.backend/app/agents/shared/base_agent.py (2)
22-44: Well-implemented thread-based state persistence!The state recovery logic with comprehensive logging provides excellent visibility into the agent's memory lifecycle. The implementation correctly handles both existing and new thread states.
56-76: Consistent implementation of thread persistence for streaming mode.The streaming method properly mirrors the state recovery pattern from the run method, with added step counting for debugging streaming workflows.
backend/app/agents/devrel/nodes/summarization_node.py (2)
98-98: Clever use of negative count to reset interaction counter!Using
-current_countwith the additive merge semantics effectively resets the counter to 0. This is a nice pattern for state updates.
110-129: Well-structured topic extraction with appropriate limits.The function properly extracts topics with error handling and a sensible limit of 5 topics to prevent unbounded growth.
backend/app/agents/devrel/agent.py (2)
35-93: Well-integrated summarization workflow!The summarization nodes and conditional routing are properly integrated into the agent's workflow graph. The use of InMemorySaver for checkpointing enables the thread-based state persistence.
122-137: Clean helper methods for state management.Both
_should_summarizeandget_thread_stateare well-implemented with appropriate error handling.backend/app/agents/shared/state.py (2)
6-16: Well-designed custom merge functions!The
replace_summaryandreplace_topicsfunctions provide appropriate merge semantics for the summarization feature, ensuring newer values replace older ones.
29-45: Comprehensive state model extensions for memory management.The new fields effectively support conversation summarization and session management. The use of appropriate type annotations with custom merge semantics (additive for
interaction_count, replace forconversation_summaryandkey_topics) is well thought out.
|
resolving issues... |
|
Refers #73 (can be closed once I implement persistent memory in a separate PR) Will scope out DB issues soon :) |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
backend/app/agents/devrel/agent.py (1)
138-170:⚠️ Potential issue
clear_thread_memoryis still outside the class – causesNameError: name 'self' is not definedThe function is defined at module level but references
self, so the first call will explode.
Indent it so it becomes a method ofDevRelAgent:-async def clear_thread_memory(self, thread_id: str, force_clear: bool = False) -> bool: + async def clear_thread_memory(self, thread_id: str, force_clear: bool = False) -> bool: """Clear memory for a specific thread using memory_timeout_reached flag""" ...(Indent the entire body by 4 spaces.)
Follow-ups:
- Verify
langgraph.checkpoint.memory.InMemorySaverexposes adelete_thread()(or equivalent) method; if not, implement a wrapper or usedel self.checkpointer[thread_id].- Consider the Pylint hint: after
return True, theelse:at 161-163 is redundant.🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 148-163: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
🧹 Nitpick comments (2)
backend/app/agents/devrel/agent.py (2)
34-34:InMemorySaver()instance is local to each agent – consider sharing or injectingCreating a new
InMemorySaver()perDevRelAgentinstance isolates checkpoints between concurrent agent instances or workers.
If your deployment spins up multiple agents (e.g., per HTTP request), conversation continuity will break because each agent keeps a private store.Prefer one of:
- self.checkpointer = InMemorySaver() + # Re-use a global/singleton checkpointer so all agent instances share the same store. + self.checkpointer = shared_checkpointer or InMemorySaver()or accept a
checkpointerin__init__and default to the singleton.
This keeps thread memory consistent across processes.
158-163: Redundantelseblock after earlyreturnLines 158-160 already
return True; the subsequentelse:(161-163) can be safely de-indented to reduce nesting, satisfying R1705.- logger.info(f"Successfully cleared memory for thread {thread_id}") - return True - else: - logger.info(f"Thread {thread_id} has not timed out, memory preserved") - return False + logger.info(f"Successfully cleared memory for thread {thread_id}") + return True + logger.info(f"Thread {thread_id} has not timed out, memory preserved") + return False🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 148-163: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/app/agents/devrel/agent.py(6 hunks)backend/app/agents/devrel/nodes/summarization_node.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/agents/devrel/nodes/summarization_node.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/app/agents/devrel/agent.py (2)
backend/app/agents/devrel/nodes/summarization_node.py (3)
check_summarization_needed(14-46)summarize_conversation_node(48-109)store_summary_to_database(132-136)backend/app/agents/shared/state.py (1)
AgentState(18-73)
🪛 Pylint (3.3.7)
backend/app/agents/devrel/agent.py
[refactor] 148-163: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/app/agents/devrel/agent.py (1)
161-166: 🛠️ Refactor suggestionRemove redundant
elsebranches afterreturnPylint is right: lines 161-166 are dead code paths nested under unnecessary
elseclauses.
Flattening them improves readability and eliminates the R1705 warning:- else: - logger.info(f"Thread {thread_id} has not timed out, memory preserved") - return False - else: - logger.info(f"No state found for thread {thread_id}, nothing to clear") - return True + logger.info(f"Thread {thread_id} has not timed out, memory preserved") + return False + + logger.info(f"No state found for thread {thread_id}, nothing to clear") + return True
🧹 Nitpick comments (1)
backend/app/agents/devrel/agent.py (1)
128-136: Consider returning an immutable snapshot fromget_thread_state
state.valuesis passed by reference; callers mutating this dict will silently bypass LangGraph’s merge logic and may corrupt the checkpoint.
Protect against accidental mutation by returning a shallow copy:- return state.values if state else {} + return dict(state.values) if state else {}This is a low-overhead safety net.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/agents/devrel/agent.py(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/app/agents/devrel/agent.py (2)
backend/app/agents/devrel/nodes/summarization_node.py (3)
check_summarization_needed(14-46)summarize_conversation_node(48-109)store_summary_to_database(132-136)backend/app/agents/shared/state.py (1)
AgentState(18-73)
🪛 Pylint (3.3.7)
backend/app/agents/devrel/agent.py
[refactor] 148-163: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
| self.checkpointer = InMemorySaver() | ||
| super().__init__("DevRelAgent", self.config) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Share a single InMemorySaver instead of instantiating per-agent
self.checkpointer = InMemorySaver() creates a brand-new store for every DevRelAgent instance.
If the application spins up a fresh agent per request (very common in serverless / web frameworks), each request will operate on an isolated store and no memory carry-over will occur, defeating the whole purpose of the saver.
Create one saver at module level (or inject it from the orchestrator) and reuse it across agents:
- self.checkpointer = InMemorySaver()
+# backend/app/agents/devrel/agent.py
+CHECKPOINTER = InMemorySaver() # shared singleton
+
+ self.checkpointer = CHECKPOINTERNo other code changes are required because workflow.compile(checkpointer=…) already accepts the instance.
This minor refactor preserves true cross-session memory and avoids silent data loss.
Also applies to: 91-92
🤖 Prompt for AI Agents
In backend/app/agents/devrel/agent.py around lines 34 to 35, the code creates a
new InMemorySaver instance for each DevRelAgent, causing isolated memory stores
per agent. To fix this, move the InMemorySaver instantiation to the module level
so that a single shared instance is reused across all DevRelAgent instances.
This change preserves memory across sessions without modifying other code since
the saver instance is already accepted by workflow.compile. Apply the same
refactor to lines 91-92 as well.
There was a problem hiding this comment.
Not a problem as only one instance lives for the entire application lifetime with thread and InMemorySaver() based system to remember user conversations throughout.
There was a problem hiding this comment.
@smokeyScraper, thank you for the clarification! That makes perfect sense - with a singleton DevRelAgent instance living for the entire application lifetime, the InMemorySaver created in the constructor will indeed be shared across all conversations through the thread-based system. This architecture effectively achieves memory persistence without the isolation concerns I raised. I appreciate you explaining the application's design pattern.
✏️ Learnings added
Learnt from: smokeyScraper
PR: AOSSIE-Org/Devr.AI#75
File: backend/app/agents/devrel/agent.py:34-35
Timestamp: 2025-06-13T21:56:19.160Z
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.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
2 participants
Refers to a few issues of #70
Attached Interactions
Summary by CodeRabbit
New Features
Improvements
Bug Fixes