Open
Conversation
…g.py is lookinf for different env variables than the readme for django
…hen trying to contact openai?
This reverts commit 53c4a04.
…t has access to, as well as giving it some guidelines to try to avoid duplicate memories.
Changed memory.get_all_memories() and search returns to a more explicit list of memories.
…y-per-user' into 172-long-term-memory-per-user
…ixed the prompt where it wasn't stated that it has the tool search_memories
…based off of unused memories
…zy memory cleanup strategy(every 10 stores it will call the cleanup)
Member
|
Finally, @kkwan0 can you fix the linter and formatter errors? |
Ar-temis
reviewed
Mar 27, 2026
Member
|
@munishlohani Can you help with this pls? |
munishlohani
reviewed
Apr 1, 2026
munishlohani
approved these changes
Apr 1, 2026
Member
|
@kkwan0 @munishlohani So in the future we want to decouple the agent and the backend by developing a separate FastAPI server for the agent that the backend can send requests to. If we do that, the persistent memory can be called after an answer on the agent side. The problem is that, we need to develop the FastAPI server first. @kkwan0 Do you wanna work on this? This would mean that this feature will have to wait until the fastapi server is done. |
Collaborator
Author
|
yeah I can work on that |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughAdds a mem0-backed long-term memory subsystem and DSPy memory modules, a FastAPI app exposing memory endpoints with Pydantic request models and tests, refactors planner/agent to use conversation history/summary strings and include memory tools, updates configuration defaults and dependencies, removes several legacy DSPy modules, and adjusts prompts and docs. No public API function signatures beyond new memory classes/models and the FastAPI app were removed except module deletions and new exported classes described. Sequence Diagram(s)sequenceDiagram
participant Agent as Agent
participant PermanentMem as PermanentMemory
participant MemTool as MemoryTools
participant Mem0 as mem0.Memory
participant LLM as LLM/DSPy
Agent->>PermanentMem: forward(session_conversation, most_recent_conversation)
PermanentMem->>LLM: plan_action(conversation_history, conversation_summary, user_memories)
LLM-->>PermanentMem: next_thought / next_tool_name / next_tool_args
alt next_tool_name != "finish"
PermanentMem->>MemTool: call tool (search/store/update/delete)
MemTool->>Mem0: search/add/update/delete (user_id, run_id, metadata)
Mem0-->>MemTool: results / ack / error
MemTool-->>PermanentMem: formatted_result (or error string)
PermanentMem->>LLM: continue planning with observation
else next_tool_name == "finish"
PermanentMem->>LLM: finish
end
PermanentMem-->>Agent: trajectory / final_output
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
chatdku/chatdku/core/dspy_classes/prompt_settings.py (1)
35-44:⚠️ Potential issue | 🟠 MajorDon’t hard-code the current semester in a static prompt.
This will silently become wrong after the next term rollover, and the assistant will keep answering with stale calendar context. Derive it at runtime or remove the “current semester” claim from the base prompt.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/core/dspy_classes/prompt_settings.py` around lines 35 - 44, The base prompt variable role_str in prompt_settings.py currently hard-codes "We are in the second session of the Spring 2026 Semester..." which will become stale; either remove that sentence from role_str or replace it with a dynamic value generated at runtime (e.g., call a helper like get_current_semester() or get_academic_session(Date.now()) and inject its result into role_str), and ensure any function you add (get_current_semester/get_academic_session) uses the system date and your academic calendar rules so prompts always reflect the real current semester.chatdku/chatdku/django/readme.md (1)
76-112:⚠️ Potential issue | 🔴 CriticalRemove live-looking secrets from the README sample.
This example includes actual-looking application secrets and an API key. Keeping them in versioned docs is a credential leak and also encourages copy/pasting shared secrets into local environments. Please replace them with obvious placeholders and rotate anything that was ever valid.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/django/readme.md` around lines 76 - 112, The README sample currently contains real-looking secrets (e.g., SECRET_KEY, FIELD_ENCRYPTION_KEY, REDIS_PASSWORD, DB_PASSWORD, LLM_API_KEY, EMAIL_HOST_USER, EMAIL_TO, etc.); replace all concrete values with clearly marked placeholders (e.g., SECRET_KEY="<your-secret-key>", FIELD_ENCRYPTION_KEY="<base64-key>", REDIS_PASSWORD="<redis-password>", DB_PASSWORD="<db-password>", LLM_API_KEY="<openai-or-other-api-key>", EMAIL_TO=["you@example.com"]) and remove any stray quoting/spacing issues so the file only shows example placeholders; also add a short note to rotate any keys that may have been used in the repo or environment.
🧹 Nitpick comments (8)
.gitignore (1)
3-3: Consider adding a trailing slash for consistency.The
.ruff_cacheentry would be more consistent with other tool cache directories in this file (e.g.,.pytest_cache/on line 63,.mypy_cache/on line 165) if written as.ruff_cache/. While both forms work, the trailing slash explicitly indicates it's a directory and aligns with the pattern used for similar entries.Minor consistency improvement
-.ruff_cache +.ruff_cache/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 3, Update the .gitignore entry for the ruff cache from ".ruff_cache" to ".ruff_cache/" to match the trailing-slash convention used by other tool cache directories (e.g., ".pytest_cache/" and ".mypy_cache/") and explicitly indicate it's a directory.chatdku/pyproject.toml (1)
55-55: Pinmem0aito a compatible version range.This is a new core runtime dependency for persistent memory, but it is currently unbounded. The recommended range is
>=1.0.0,<2.0.0to ensure consistent support for theMemoryAPI with Chroma and OpenAI-compatible backends (v1.x releases maintain API stability with no breaking changes). Pinning prevents unexpected behavior changes from futuremem0aireleases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/pyproject.toml` at line 55, Update the unpinned dependency "mem0ai" in the pyproject.toml dependencies list to a constrained version range ">=1.0.0,<2.0.0": locate the dependency entry for mem0ai and replace the bare name with the pinned range to ensure compatibility with the Memory API and avoid unexpected breaking changes from future v2+ releases.chatdku/chatdku/backend/fastAPI/main.py (3)
48-58: Async endpoints calling synchronous blocking methods.All endpoints are declared
asyncbutMemoryToolsmethods perform synchronous I/O (Chroma vector store, LLM calls). This blocks the event loop and limits concurrency.Consider wrapping blocking calls with
asyncio.to_thread()orloop.run_in_executor():import asyncio `@app.post`("/memory/search") async def search_memories(request: SearchMemoryRequest): tools = get_memory_tools(request.user_id, request.session_id) result = await asyncio.to_thread( tools.search_memories, request.query, limit=request.limit, filters=request.filters ) return {"result": result}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/backend/fastAPI/main.py` around lines 48 - 58, The async endpoints search_memories and store_memory are calling synchronous, blocking MemoryTools methods (search_memories and store_memory) which will block the event loop; update both endpoints to run those blocking calls in a thread executor (e.g., await asyncio.to_thread(...) or loop.run_in_executor(...)) so the calls to tools.search_memories(...) and tools.store_memory(...) are executed off the event loop and awaited; keep using get_memory_tools(request.user_id, request.session_id) as-is if it is cheap, otherwise also dispatch it to a thread similarly.
74-78: Fragile error detection via string prefix.Relying on
result.startswith("Error")couples the API to the exact error message format inMemoryTools. If the message changes or is localized, this check silently fails.Consider having
MemoryToolsraise exceptions or return a structured result (e.g., a tuple or dataclass with success/error fields) that the endpoint can inspect reliably.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/backend/fastAPI/main.py` around lines 74 - 78, The endpoint currently treats any delete failure by checking result.startswith("Error"), which is fragile; update MemoryTools.delete_memory (from get_memory_tools) to either raise a well-defined exception (e.g., custom MemoryNotFoundError/MemoryOperationError) or return a structured result (e.g., dataclass or dict with success and error fields), then change the handler in main.delete endpoint to catch the specific exception(s) and raise HTTPException with the exception message or to inspect the structured result.success field and raise HTTPException when false; refer to get_memory_tools, MemoryTools.delete_memory, the result variable and the HTTPException raise site to locate and update both producer and consumer so the error flow is reliable.
35-37: Unused Pydantic modelDeleteMemoryRequest.The
DeleteMemoryRequestmodel is defined but never used. Thedelete_memoryendpoint at line 68 uses query parameters instead. Either remove this unused model or refactor the delete endpoint to use it for consistency with other endpoints.♻️ Option 1: Remove unused model
-class DeleteMemoryRequest(MemoryRequestBase): - memory_id: str = Field(..., description="Memory ID to delete") -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/backend/fastAPI/main.py` around lines 35 - 37, DeleteMemoryRequest is defined but never used; update the delete_memory endpoint to accept a DeleteMemoryRequest Pydantic body (replace current query params) so the endpoint signature uses DeleteMemoryRequest and reads memory_id from the model, and adjust any callers/tests accordingly; alternatively, if you prefer to keep query params, remove the unused DeleteMemoryRequest class (`DeleteMemoryRequest`) to avoid dead code and delete its import/usages. Ensure the chosen change is applied to the endpoint function named delete_memory and related tests/clients.chatdku/chatdku/core/dspy_classes/memory.py (1)
218-228: Redundantisinstance(i, dict)check.
self.historyis typed aslist[ConversationMemoryEntry], so items will never bedictinstances. The filterif not isinstance(i, dict)is always true and adds confusion. The same pattern appears at lines 249 and 269-271.If this is defensive coding for a specific scenario, consider adding a comment. Otherwise, remove the checks for clarity.
♻️ Simplified version
def history_str(self, left: int = 0, right: Optional[int] = None): if right is None: right = len(self.history) return "\n".join( - [ - i.model_dump_json(indent=4) - for i in self.history[left:right] - if not isinstance(i, dict) - ] + [i.model_dump_json(indent=4) for i in self.history[left:right]] )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/core/dspy_classes/memory.py` around lines 218 - 228, The filter "if not isinstance(i, dict)" in history_str is redundant because self.history is typed as list[ConversationMemoryEntry]; remove that isinstance check from history_str (and the identical redundant checks at the other occurrences around the history-related methods at lines referenced in the review) so each list comprehension simply iterates over self.history[left:right] and calls i.model_dump_json(indent=4); if you intended defensive handling of stray dicts instead, replace the check with a short inline comment explaining why dicts may appear or explicitly cast/validate items before calling model_dump_json on ConversationMemoryEntry.chatdku/chatdku/core/tools/memory_tool.py (2)
160-161: Redundanthasattrcheck.
memory_access_logis initialized in__init__(line 18-20), so this check is unnecessary. Consider removing it for clarity, or add a comment if there's a specific reason for defensive initialization.♻️ Remove redundant check
- if not hasattr(self, "memory_access_log"): - self.memory_access_log = {} - for idx, mem in enumerate(results["results"]):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/core/tools/memory_tool.py` around lines 160 - 161, Remove the redundant defensive initialization of memory_access_log in the method where it checks "if not hasattr(self, 'memory_access_log'): self.memory_access_log = {}", because memory_access_log is already set in the class __init__; delete that if-block in chatdku/core/tools/memory_tool.py (or replace it with a short comment only if you intend to keep defensive code) so references to memory_access_log rely on the __init__ initialization and avoid duplicate logic.
119-120: Consider catching more specific exceptions.All methods catch bare
Exceptionand return error strings. While this pattern works for the agent interface, it can mask programming errors. Consider catching more specific exceptions (e.g.,mem0specific errors,KeyError,ValueError) and letting unexpected errors propagate for debugging.This applies to similar handlers at lines 183, 206, 225, 233, and 291.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/core/tools/memory_tool.py` around lines 119 - 120, Replace the broad "except Exception as e" handlers in chatdku/chatdku/core/tools/memory_tool.py (the six handlers flagged) with targeted exception handling: catch only the expected, recoverable errors (for example KeyError, ValueError and any mem0-specific exception class used by your memory backend) and return the existing error strings for those cases; for any other unexpected exceptions re-raise them so they bubble up (do not swallow ProgrammingError/TypeError or other unexpected exceptions). Locate each handler by its current "except Exception as e" occurrence and change them to multiple except clauses (or a conditional re-raise) that explicitly lists known exceptions and uses "raise" for anything else.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@chatdku/chatdku/backend/fastAPI/main.py`:
- Line 81: Resolve the informal TODO by either removing the redundant manual
endpoint or documenting its purpose: if you want to keep the POST
/memory/cleanup endpoint, update the route handler and its docstring to explain
why an administrative/manual cleanup (the endpoint defined at
`@app.post`("/memory/cleanup")) is kept even though auto-cleanup runs inside
store_memory (function store_memory) — e.g., for manual/forced cleanup, testing,
or admin control — otherwise delete the endpoint and any tests/clients
referencing it to avoid confusion. Ensure references to store_memory and the
cleanup route are consistent in comments and API docs.
In `@chatdku/chatdku/core/agent.py`:
- Around line 247-255: The bug is that recent_conversation is a list of message
dicts but conversations.append(recent_conversation) nests it, making
session_conversation a list of lists; change the update so conversations remains
a flat list: replace the append call with extending the list (e.g., use
conversations.extend(recent_conversation)) or otherwise flatten before calling
permanent_memory; ensure the value passed to
permanent_memory(session_conversation=...) is the flat conversations list and
most_recent_conversation stays as the list recent_conversation so
PermanentMemory receives the expected shapes.
In `@chatdku/chatdku/core/dspy_classes/memory.py`:
- Around line 153-177: The truncation path assumes session_conversation is a
dict but forward() passes a list[dict], causing an AttributeError when
truncate_conversation calls .keys(); update the signatures and logic to be
consistent: change _call_with_potential_conversation_truncation and
truncate_conversation to accept the actual type (list[dict[str,str]]), or make
truncate_conversation robust to both types by detecting whether conversation is
a list or dict and truncating appropriately (e.g., pop the first N elements for
a list or remove first N keys for a dict); reference the functions
_call_with_potential_conversation_truncation, truncate_conversation and the
forward call site to align types and prevent the runtime error.
- Around line 235-279: ConversationMemory.forward currently ends without
returning anything; update it to return a dspy.Prediction like
PermanentMemory.forward does: after setting span attributes and status,
construct a dspy.Prediction that contains the same payload you put into
SpanAttributes.OUTPUT_VALUE (e.g., a dict with history and summary or the
JSON-parsed equivalent) and return it; ensure you import and use dspy.Prediction
(or the project's Prediction class) and keep the span.status(StatusCode.OK) call
before returning so the span reflects success.
In `@chatdku/chatdku/core/dspy_classes/prompt_settings.py`:
- Around line 79-88: The sample examples in prompt_settings.py under the "DKU
student examples" and "DKU faculty examples" sections are producing transient
support requests as entries in the "facts" field (e.g., "Needs guidance on
classes to take with Stats302", "Question about underload requirement") which
will permanently pollute per-user memory; update those examples so they no
longer create durable memory by either removing/replacing them with truly
persistent profile/preferences facts or adding an explicit non-persistence flag
in the example output (e.g., change the output JSON schema to include a field
like "persist": false or "ephemeral": true instead of storing transient items in
"facts"), and ensure any code that serializes example outputs (the prompt
examples in prompt_settings.py) respects that flag and does not write those
entries to memory.
In `@chatdku/chatdku/core/tools/memory_tool.py`:
- Around line 258-263: The sorting is using m.get("last_accessed", ...) but
last_accessed is only kept in the instance-local memory_access_log (not
persisted), so either persist last_accessed on access or read from
memory_access_log when sorting; to fix, when a memory is accessed update its
metadata (call the memory store's update method from where memory_access_log is
set) to write last_accessed into the memory object, and/or change the sort key
in the long_mems_sorted computation (the lambda in sorted at long_mems_sorted)
to consult self.memory_access_log.get(m.get("id")) first and fall back to
m.get("created_at") and use self._to_timestamp accordingly so access-based
cleanup works as intended (see memory_access_log, get_all(), _to_timestamp, and
the code that updates access logs).
- Around line 215-217: The current check in memory_tool.py only guards against
idx >= len(self.last_memory_search) but allows negative indices; update the
validation in the method containing this try block (look for references to
self.last_memory_search and the variable idx) to also reject idx < 0 (and
optionally non-integer values) before accessing the list. Concretely, ensure the
code returns the same "Invalid memory index..." message when idx is negative
(i.e., if not isinstance(idx, int) or idx < 0 or idx >=
len(self.last_memory_search)), so negative indices no longer resolve to
unintended list items.
- Around line 294-305: In _to_timestamp, datetime.fromisoformat will raise
AttributeError because datetime is the module; change the call to the datetime
class (e.g., use datetime.datetime.fromisoformat(val)) or import the class
directly (from datetime import datetime) and then call
datetime.fromisoformat(val); update the except to include ValueError (and
optionally AttributeError) only if you choose to keep defensive handling,
ensuring the function returns a float timestamp on success and 0.0 on failure.
- Around line 40-46: The embedder dict currently hardcodes embedding_dims: 1024
which ties the code to a single model; update this to use a configurable value
(e.g., config.embedding_dims) instead of the literal 1024 and ensure callers
that build schemas (Redis/Postgres/Chroma) read the same config.embedding_dims
so schemas and vector lengths stay consistent with config.embedding; reference
the embedder dict and the symbols embedding_dims, config.embedding and
config.tei_url in memory_tool.py and add validation/fallback (e.g., infer or
error) if config.embedding_dims is missing, or alternatively add a short comment
documenting that embedding_dims must match all DB schemas and that changing
config.embedding requires migrations.
In `@chatdku/tests/test_fastAPI.py`:
- Around line 6-10: Tests currently spin up the real FastAPI app with a global
TestClient and fixed USER_ID/SESSION_ID which couples them to persistent state;
change tests to use app dependency overrides or mocks for external storage
layers (e.g., the functions/classes used for Mem0/Chroma access) and instantiate
a fresh TestClient per test, and generate unique USER_ID/SESSION_ID values (use
UUIDs or fixtures) inside each test to avoid cross-test interference; refer to
the TestClient instantiation and the constants USER_ID and SESSION_ID to locate
where to replace global values with per-test fixtures and where to inject
dependency_overrides on the FastAPI app.
- Around line 59-71: The test_delete_memory is too permissive; modify it to
create a memory first (POST to the same API used by tests, e.g., call
client.post("/memory", json={"memory_id": memory_id, "user_id": USER_ID,
"session_id": SESSION_ID", ...}) or the app's expected body) then call
client.delete(f"/memory/{memory_id}", params={"user_id": USER_ID, "session_id":
SESSION_ID}) to delete that exact resource and assert the expected success
status (e.g., assert response.status_code == 200 or 204) and that the response
JSON contains the successful key (e.g., "result") rather than allowing 400; use
the existing test_delete_memory function and the same memory_id to locate where
to add the create step and the stricter assertions.
- Around line 46-56: The test_update_memory currently assumes an existing stable
memory at idx=0; make the test deterministic by arranging a known memory first:
call the memory creation endpoint (e.g., via client.post("/memory/create") or
whichever function creates memories for USER_ID/SESSION_ID), then either call
the memory search/list endpoint to obtain the exact index/id for that created
memory or use the creation response's identifier, and finally call
client.post("/memory/update", ...) using that returned index/id instead of a
hardcoded idx=0; update assertions to verify the updated content using the
returned identifier.
---
Outside diff comments:
In `@chatdku/chatdku/core/dspy_classes/prompt_settings.py`:
- Around line 35-44: The base prompt variable role_str in prompt_settings.py
currently hard-codes "We are in the second session of the Spring 2026
Semester..." which will become stale; either remove that sentence from role_str
or replace it with a dynamic value generated at runtime (e.g., call a helper
like get_current_semester() or get_academic_session(Date.now()) and inject its
result into role_str), and ensure any function you add
(get_current_semester/get_academic_session) uses the system date and your
academic calendar rules so prompts always reflect the real current semester.
In `@chatdku/chatdku/django/readme.md`:
- Around line 76-112: The README sample currently contains real-looking secrets
(e.g., SECRET_KEY, FIELD_ENCRYPTION_KEY, REDIS_PASSWORD, DB_PASSWORD,
LLM_API_KEY, EMAIL_HOST_USER, EMAIL_TO, etc.); replace all concrete values with
clearly marked placeholders (e.g., SECRET_KEY="<your-secret-key>",
FIELD_ENCRYPTION_KEY="<base64-key>", REDIS_PASSWORD="<redis-password>",
DB_PASSWORD="<db-password>", LLM_API_KEY="<openai-or-other-api-key>",
EMAIL_TO=["you@example.com"]) and remove any stray quoting/spacing issues so the
file only shows example placeholders; also add a short note to rotate any keys
that may have been used in the repo or environment.
---
Nitpick comments:
In @.gitignore:
- Line 3: Update the .gitignore entry for the ruff cache from ".ruff_cache" to
".ruff_cache/" to match the trailing-slash convention used by other tool cache
directories (e.g., ".pytest_cache/" and ".mypy_cache/") and explicitly indicate
it's a directory.
In `@chatdku/chatdku/backend/fastAPI/main.py`:
- Around line 48-58: The async endpoints search_memories and store_memory are
calling synchronous, blocking MemoryTools methods (search_memories and
store_memory) which will block the event loop; update both endpoints to run
those blocking calls in a thread executor (e.g., await asyncio.to_thread(...) or
loop.run_in_executor(...)) so the calls to tools.search_memories(...) and
tools.store_memory(...) are executed off the event loop and awaited; keep using
get_memory_tools(request.user_id, request.session_id) as-is if it is cheap,
otherwise also dispatch it to a thread similarly.
- Around line 74-78: The endpoint currently treats any delete failure by
checking result.startswith("Error"), which is fragile; update
MemoryTools.delete_memory (from get_memory_tools) to either raise a well-defined
exception (e.g., custom MemoryNotFoundError/MemoryOperationError) or return a
structured result (e.g., dataclass or dict with success and error fields), then
change the handler in main.delete endpoint to catch the specific exception(s)
and raise HTTPException with the exception message or to inspect the structured
result.success field and raise HTTPException when false; refer to
get_memory_tools, MemoryTools.delete_memory, the result variable and the
HTTPException raise site to locate and update both producer and consumer so the
error flow is reliable.
- Around line 35-37: DeleteMemoryRequest is defined but never used; update the
delete_memory endpoint to accept a DeleteMemoryRequest Pydantic body (replace
current query params) so the endpoint signature uses DeleteMemoryRequest and
reads memory_id from the model, and adjust any callers/tests accordingly;
alternatively, if you prefer to keep query params, remove the unused
DeleteMemoryRequest class (`DeleteMemoryRequest`) to avoid dead code and delete
its import/usages. Ensure the chosen change is applied to the endpoint function
named delete_memory and related tests/clients.
In `@chatdku/chatdku/core/dspy_classes/memory.py`:
- Around line 218-228: The filter "if not isinstance(i, dict)" in history_str is
redundant because self.history is typed as list[ConversationMemoryEntry]; remove
that isinstance check from history_str (and the identical redundant checks at
the other occurrences around the history-related methods at lines referenced in
the review) so each list comprehension simply iterates over
self.history[left:right] and calls i.model_dump_json(indent=4); if you intended
defensive handling of stray dicts instead, replace the check with a short inline
comment explaining why dicts may appear or explicitly cast/validate items before
calling model_dump_json on ConversationMemoryEntry.
In `@chatdku/chatdku/core/tools/memory_tool.py`:
- Around line 160-161: Remove the redundant defensive initialization of
memory_access_log in the method where it checks "if not hasattr(self,
'memory_access_log'): self.memory_access_log = {}", because memory_access_log is
already set in the class __init__; delete that if-block in
chatdku/core/tools/memory_tool.py (or replace it with a short comment only if
you intend to keep defensive code) so references to memory_access_log rely on
the __init__ initialization and avoid duplicate logic.
- Around line 119-120: Replace the broad "except Exception as e" handlers in
chatdku/chatdku/core/tools/memory_tool.py (the six handlers flagged) with
targeted exception handling: catch only the expected, recoverable errors (for
example KeyError, ValueError and any mem0-specific exception class used by your
memory backend) and return the existing error strings for those cases; for any
other unexpected exceptions re-raise them so they bubble up (do not swallow
ProgrammingError/TypeError or other unexpected exceptions). Locate each handler
by its current "except Exception as e" occurrence and change them to multiple
except clauses (or a conditional re-raise) that explicitly lists known
exceptions and uses "raise" for anything else.
In `@chatdku/pyproject.toml`:
- Line 55: Update the unpinned dependency "mem0ai" in the pyproject.toml
dependencies list to a constrained version range ">=1.0.0,<2.0.0": locate the
dependency entry for mem0ai and replace the bare name with the pinned range to
ensure compatibility with the Memory API and avoid unexpected breaking changes
from future v2+ releases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8239f6ef-18f0-4fb5-b9a8-18ff7df0f043
📒 Files selected for processing (16)
.gitignorechatdku/chatdku/backend/fastAPI/main.pychatdku/chatdku/config.pychatdku/chatdku/core/agent.pychatdku/chatdku/core/dspy_classes/conversation_memory.pychatdku/chatdku/core/dspy_classes/judge.pychatdku/chatdku/core/dspy_classes/memory.pychatdku/chatdku/core/dspy_classes/plan.pychatdku/chatdku/core/dspy_classes/prompt_settings.pychatdku/chatdku/core/dspy_classes/query_rewrite.pychatdku/chatdku/core/dspy_classes/synthesizer.pychatdku/chatdku/core/dspy_classes/tool_memory.pychatdku/chatdku/core/tools/memory_tool.pychatdku/chatdku/django/readme.mdchatdku/pyproject.tomlchatdku/tests/test_fastAPI.py
💤 Files with no reviewable changes (4)
- chatdku/chatdku/core/dspy_classes/tool_memory.py
- chatdku/chatdku/core/dspy_classes/conversation_memory.py
- chatdku/chatdku/core/dspy_classes/judge.py
- chatdku/chatdku/core/dspy_classes/query_rewrite.py
Comment on lines
+59
to
+71
| def test_delete_memory(): | ||
| memory_id = "test-memory-id" | ||
|
|
||
| response = client.delete( | ||
| f"/memory/{memory_id}", | ||
| params={"user_id": USER_ID, "session_id": SESSION_ID} | ||
| ) | ||
|
|
||
| # Could be success or failure depending on backend state | ||
| assert response.status_code in [200, 400] | ||
|
|
||
| data = response.json() | ||
| assert "result" in data or "detail" in data |
There was a problem hiding this comment.
The delete assertion is too permissive to catch regressions.
Accepting either 200 or 400 means an endpoint that always fails still passes this test. Create a memory in the test, delete that exact ID, and assert one expected outcome.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@chatdku/tests/test_fastAPI.py` around lines 59 - 71, The test_delete_memory
is too permissive; modify it to create a memory first (POST to the same API used
by tests, e.g., call client.post("/memory", json={"memory_id": memory_id,
"user_id": USER_ID, "session_id": SESSION_ID", ...}) or the app's expected body)
then call client.delete(f"/memory/{memory_id}", params={"user_id": USER_ID,
"session_id": SESSION_ID}) to delete that exact resource and assert the expected
success status (e.g., assert response.status_code == 200 or 204) and that the
response JSON contains the successful key (e.g., "result") rather than allowing
400; use the existing test_delete_memory function and the same memory_id to
locate where to add the create step and the stricter assertions.
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Member
|
@kkwan0 damn bro wtf are these errors |
Collaborator
Author
|
This thing is the devil |
…ligence-Lab/ChatDKU into 172-long-term-memory-per-user
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
chatdku/chatdku/core/tools/memory_tool.py (3)
12-51: Hardcodedlocalhostlimits deployment flexibility.The Chroma host is hardcoded to
"localhost"on line 27, but other services like LLM and TEI use configurable URLs fromconfig. This will fail in containerized or distributed deployments where Chroma runs on a different host.♻️ Proposed fix
"vector_store": { "provider": "chroma", "config": { "collection_name": config.memory_collection, - "host": "localhost", + "host": getattr(config, "chroma_db_host", "localhost"), "port": config.chroma_db_port, }, },Alternatively, add
chroma_db_hosttoconfig.pydefaults similar to other service URLs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/core/tools/memory_tool.py` around lines 12 - 51, The constructor (__init__) builds memory_config with a hardcoded Chroma host "localhost"; replace that literal with a configurable value (e.g., use config.chroma_db_host or a fallback like config.chroma_db_host or config.chroma_db_url) so deployments can point to a remote Chroma service; update the memory_config["vector_store"]["config"]["host"] assignment and ensure config exposes the new chroma host variable (or add it to config) before calling Memory.from_config.
122-184:search_memoriessignature differs from relevant snippet.The method signature here includes
limit: int = 5, but Context snippet 3 showssearch_memories(self, query: str, filters: dict | None = None)without alimitparameter. Verify if this is an intentional enhancement or if callers need updating.The
hasattrcheck on line 160 is redundant sincememory_access_logis initialized in__init__.♻️ Remove redundant hasattr check
- if not hasattr(self, "memory_access_log"): - self.memory_access_log = {} - for idx, mem in enumerate(results["results"]):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/core/tools/memory_tool.py` around lines 122 - 184, The search_memories method signature was changed to include limit (search_memories(self, query: str, limit: int = 5, filters: dict | None = None)) which may be incompatible with other callers expecting search_memories(self, query: str, filters: dict | None = None); decide whether to keep limit—if so, update all callers and any referenced context snippet to pass the new parameter (or provide a default in call sites); otherwise remove limit from this signature to match existing usage. Also remove the redundant hasattr(self, "memory_access_log") check inside search_memories and rely on memory_access_log being initialized in __init__ (ensure __init__ indeed creates self.memory_access_log). Reference: search_memories, memory_access_log, and __init__.
236-294: Cleanup logic has quadratic complexity and potential race conditions.
Complexity: Each
delete()call in the while loops (lines 273, 286) likely makes a network call, making this O(n) network calls in the worst case. For 100+ memories, this could be slow.Race condition: Between
get_all()and the delete loop, another process could modify memories, causing stale deletions or missed items.No limit on deletions per cleanup: If
max_memories=100and there are 1000 memories, this will make 900 delete calls in one invocation.Consider batching deletions if Mem0 supports it, or adding a per-cleanup deletion cap to avoid long-running operations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/core/tools/memory_tool.py` around lines 236 - 294, The cleanup_memory function currently issues many individual memory.delete calls after memory.get_all, causing O(n) network calls, race conditions, and unbounded deletions; modify cleanup_memory to (1) compute how many memories to remove but cap deletions per invocation (e.g., max_deletes param or fixed cap), (2) collect IDs to delete from the sorted short_mems_sorted/long_mems_sorted lists instead of deleting inside the loop, (3) if the memory backend provides a batch delete API use that (refer to memory.delete_many or similar) to delete IDs in one network call, otherwise delete IDs up to the cap and stop, (4) before performing deletion re-fetch or call a lightweight count/exists check to reduce stale race deletions, and (5) remove corresponding entries from self.memory_access_log for each deleted id (references: cleanup_memory, self.memory.get_all, self.memory.delete / batch delete, self.memory_access_log, and _to_timestamp).chatdku/chatdku/core/dspy_classes/memory.py (4)
216-226: Redundantisinstance(i, dict)check in list comprehension.Lines 222-225 and 247 filter with
if not isinstance(i, dict), butself.historyis typed aslist[ConversationMemoryEntry]. If the list only containsConversationMemoryEntryobjects, this check is unnecessary. If dict objects can appear in history, the type annotation is incorrect.♻️ Remove redundant check or fix type annotation
If history only contains
ConversationMemoryEntry:return "\n".join( [ i.model_dump_json(indent=4) for i in self.history[left:right] - if not isinstance(i, dict) ] )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/core/dspy_classes/memory.py` around lines 216 - 226, The list comprehension in history_str (and the similar filter at the other occurrence) contains a redundant if not isinstance(i, dict) check because self.history is annotated as list[ConversationMemoryEntry]; either remove that filter so history_str simply joins i.model_dump_json(...) for i in self.history[left:right], or if dicts can actually appear, correct the self.history annotation to include dict (e.g., list[ConversationMemoryEntry | dict]) and update history_str to handle dict entries (e.g., convert dicts to ConversationMemoryEntry or format them appropriately) — locate history_str and the other filtered comprehension by name and apply one of these fixes consistently.
107-151: Broadexcept Exceptionswallows all errors during tool execution.Line 144 catches all exceptions during tool calls. While this prevents crashes, it may hide programming bugs (e.g.,
TypeErrorfrom wrong arguments). Consider logging the full traceback or re-raising after recording.Also,
dspy.Prediction()on line 151 returns an empty prediction with no useful output. Consider including the trajectory or a success indicator.♻️ Return trajectory in prediction
span.set_attribute("output.value", safe_json_dumps(trajectory)) - return dspy.Prediction() + return dspy.Prediction(trajectory=trajectory)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/core/dspy_classes/memory.py` around lines 107 - 151, The forward method in PermanentMemoryAgent uses a broad except Exception when invoking self.tools[plan.next_tool_name](...) which can swallow programming errors—replace the blanket catch with more specific handling: catch known runtime/tool exceptions (e.g., ToolExecutionError or RuntimeError) to record the observation, and for unexpected exceptions (TypeError, AttributeError, etc.) log the full traceback via span.set_attribute or a logger and re-raise (or wrap) so bugs surface; also include the captured trajectory (and a success/failure flag or final observation) in the returned dspy.Prediction instead of returning an empty prediction so callers can inspect the execution trace (update references in forward, trajectory, and the dspy.Prediction return).
280-282:register_historyhas no return value and no span tracing.Unlike
forward(), this method doesn't record telemetry or return anything. If this is intentional for a lightweight append operation, consider adding a brief docstring explaining when to use this vsforward().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/core/dspy_classes/memory.py` around lines 280 - 282, register_history currently just appends a ConversationMemoryEntry and neither returns the created entry nor records any tracing/telemetry like forward(); update register_history to start a tracing span (matching the tracing pattern used by forward()), attach relevant attributes (e.g., role and content length), append the new ConversationMemoryEntry(role=..., content=...), end the span, and return the created ConversationMemoryEntry; additionally add a one-line docstring clarifying when to use register_history vs forward().
92-106:create_react_signaturereturns a tuple but variable is shadowed.Line 102 assigns:
react_signature, tools = create_react_signature(...), buttoolswas already defined on line 96. This shadows the original list with the tool dictionary returned bycreate_react_signature, which is likely intentional (the dict includes thefinishtool). However, this could be clearer.♻️ Clarify variable naming
- react_signature, tools = create_react_signature(PermanentMemorySignature, tools) - self.tools = tools + react_signature, tool_dict = create_react_signature(PermanentMemorySignature, tools) + self.tools = tool_dict🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@chatdku/chatdku/core/dspy_classes/memory.py` around lines 92 - 106, The constructor in PermanentMemory shadows the initial tools list when calling create_react_signature; rename the initial list (e.g., initial_tools or tool_list) and call create_react_signature with that name, then unpack as react_signature, tools_map = create_react_signature(PermanentMemorySignature, initial_tools) and assign self.tools = tools_map so PermanentMemory.__init__, create_react_signature, PermanentMemorySignature, self.tools and self.planner remain clear and unambiguous.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@chatdku/chatdku/core/dspy_classes/memory.py`:
- Around line 33-90: The PermanentMemorySignature field types don't match how
they're passed: update the class fields in PermanentMemorySignature so
session_conversation and most_recent_conversation are typed as list[dict[str,
str]] (to match how PermanentMemory.forward() receives them) and change
user_memories to the actual return type from self.memory.get_all_memories()
(string) or to list[str] if you change get_all_memories to return a list; adjust
the signature fields accordingly and re-run type checks to ensure
PermanentMemory.forward() and any callers use the same types.
In `@chatdku/chatdku/core/tools/memory_tool.py`:
- Around line 53-120: The store_memory function's docstring only describes a
single-string content but the signature allows str | list[dict[str,str]] and
memory.add supports both; update the Args section of store_memory to state
content may be either a single string or a list of message dicts with "role" and
"content" keys and give an example of the list form, and then make the signature
in the other implementation (store_memory in dspy_classes/memory.py) consistent
with this union type so both places accept str | list[dict[str,str]]; optionally
replace the broad except Exception in store_memory with mem0-specific exception
types from memory.add to narrow error handling.
---
Nitpick comments:
In `@chatdku/chatdku/core/dspy_classes/memory.py`:
- Around line 216-226: The list comprehension in history_str (and the similar
filter at the other occurrence) contains a redundant if not isinstance(i, dict)
check because self.history is annotated as list[ConversationMemoryEntry]; either
remove that filter so history_str simply joins i.model_dump_json(...) for i in
self.history[left:right], or if dicts can actually appear, correct the
self.history annotation to include dict (e.g., list[ConversationMemoryEntry |
dict]) and update history_str to handle dict entries (e.g., convert dicts to
ConversationMemoryEntry or format them appropriately) — locate history_str and
the other filtered comprehension by name and apply one of these fixes
consistently.
- Around line 107-151: The forward method in PermanentMemoryAgent uses a broad
except Exception when invoking self.tools[plan.next_tool_name](...) which can
swallow programming errors—replace the blanket catch with more specific
handling: catch known runtime/tool exceptions (e.g., ToolExecutionError or
RuntimeError) to record the observation, and for unexpected exceptions
(TypeError, AttributeError, etc.) log the full traceback via span.set_attribute
or a logger and re-raise (or wrap) so bugs surface; also include the captured
trajectory (and a success/failure flag or final observation) in the returned
dspy.Prediction instead of returning an empty prediction so callers can inspect
the execution trace (update references in forward, trajectory, and the
dspy.Prediction return).
- Around line 280-282: register_history currently just appends a
ConversationMemoryEntry and neither returns the created entry nor records any
tracing/telemetry like forward(); update register_history to start a tracing
span (matching the tracing pattern used by forward()), attach relevant
attributes (e.g., role and content length), append the new
ConversationMemoryEntry(role=..., content=...), end the span, and return the
created ConversationMemoryEntry; additionally add a one-line docstring
clarifying when to use register_history vs forward().
- Around line 92-106: The constructor in PermanentMemory shadows the initial
tools list when calling create_react_signature; rename the initial list (e.g.,
initial_tools or tool_list) and call create_react_signature with that name, then
unpack as react_signature, tools_map =
create_react_signature(PermanentMemorySignature, initial_tools) and assign
self.tools = tools_map so PermanentMemory.__init__, create_react_signature,
PermanentMemorySignature, self.tools and self.planner remain clear and
unambiguous.
In `@chatdku/chatdku/core/tools/memory_tool.py`:
- Around line 12-51: The constructor (__init__) builds memory_config with a
hardcoded Chroma host "localhost"; replace that literal with a configurable
value (e.g., use config.chroma_db_host or a fallback like config.chroma_db_host
or config.chroma_db_url) so deployments can point to a remote Chroma service;
update the memory_config["vector_store"]["config"]["host"] assignment and ensure
config exposes the new chroma host variable (or add it to config) before calling
Memory.from_config.
- Around line 122-184: The search_memories method signature was changed to
include limit (search_memories(self, query: str, limit: int = 5, filters: dict |
None = None)) which may be incompatible with other callers expecting
search_memories(self, query: str, filters: dict | None = None); decide whether
to keep limit—if so, update all callers and any referenced context snippet to
pass the new parameter (or provide a default in call sites); otherwise remove
limit from this signature to match existing usage. Also remove the redundant
hasattr(self, "memory_access_log") check inside search_memories and rely on
memory_access_log being initialized in __init__ (ensure __init__ indeed creates
self.memory_access_log). Reference: search_memories, memory_access_log, and
__init__.
- Around line 236-294: The cleanup_memory function currently issues many
individual memory.delete calls after memory.get_all, causing O(n) network calls,
race conditions, and unbounded deletions; modify cleanup_memory to (1) compute
how many memories to remove but cap deletions per invocation (e.g., max_deletes
param or fixed cap), (2) collect IDs to delete from the sorted
short_mems_sorted/long_mems_sorted lists instead of deleting inside the loop,
(3) if the memory backend provides a batch delete API use that (refer to
memory.delete_many or similar) to delete IDs in one network call, otherwise
delete IDs up to the cap and stop, (4) before performing deletion re-fetch or
call a lightweight count/exists check to reduce stale race deletions, and (5)
remove corresponding entries from self.memory_access_log for each deleted id
(references: cleanup_memory, self.memory.get_all, self.memory.delete / batch
delete, self.memory_access_log, and _to_timestamp).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 75f2b193-8842-489a-8a71-4f76bd9bafc6
📒 Files selected for processing (3)
chatdku/chatdku/core/dspy_classes/memory.pychatdku/chatdku/core/dspy_classes/prompt_settings.pychatdku/chatdku/core/tools/memory_tool.py
🚧 Files skipped from review as they are similar to previous changes (1)
- chatdku/chatdku/core/dspy_classes/prompt_settings.py
Comment on lines
+33
to
+90
| class PermanentMemorySignature(dspy.Signature): | ||
| """ | ||
| You are a Memory Management Agent. | ||
| Your goal is to store, update, or delete long-term useful information about the user. | ||
|
|
||
| You have access to the following tools to manage the long-term memory: | ||
| - store_memory(content: str, metadata: dict | None = None): Store the content in the long-term memory. | ||
| - search_memories(query: str, filters: dict | None = None): Search for memories based on the query and filters. | ||
| - update_memory(idx: int, new_content: str): Update the memory at the given index to have the new_content. | ||
| - delete_memory(memory_id: str): Delete the memory with the given ID. | ||
| - finish(): stop when no action is needed | ||
|
|
||
|
|
||
| And you can see your past trajectory so far. Your goal is to use one or more of the | ||
| supplied tools to store OR update OR delete any useful facts about the user from the | ||
| most_recent_conversation. | ||
| To do this, you will produce next_thought, next_tool_name, and next_tool_args in each turn, | ||
| and also when finishing the task. | ||
| After each tool call, you receive a resulting observation, which gets appended to your trajectory. | ||
| When writing next_thought, you may reason about the current situation and plan for future steps. | ||
| When selecting the next_tool_name and its next_tool_args, the tool must be one of the provided tools. | ||
|
|
||
| For your convenience, all the user_memories are given to you. Based on the latest conversation, | ||
| you may update any memory that needs updating and may also delete any memory that is no longer relevant. | ||
|
|
||
| When storing memories: | ||
| 1. ALWAYS call search_memories first to check if a similar memory already exists to avoid duplicates. | ||
| - Use a descriptive query that matches the content or metadata of the memory you want to update or delete | ||
| - You may also use optional metadata filters to narrow down results (e.g., {"category": "academic"}) | ||
| 2. If a similar memory is found, update it instead of creating a new one. | ||
| 3. If the new information is a correction of an existing memory, delete the old one and create a new one | ||
| 4. If no relevant memories are found, then store the memory. | ||
| 5. Only call one tool per turn and wait for the observation before next action | ||
|
|
||
| When updating or deleting memories: | ||
| 1. ALWAYS call search_memories first to get the relevant memories and their indices. | ||
| - Use a descriptive query that matches the content or metadata of the memory you want to update or delete | ||
| - You may also use optional metadata filters to narrow down results (e.g., {"category": "academic"}) | ||
| 2. Then use the index (idx) from the search results to specify which memory to update or delete. | ||
| 3. Memory IDs are for reference only. Do NOT generate or guess memory IDs. | ||
| 4. Only call one tool per turn and wait for the observation before next action | ||
|
|
||
| Guidelines: | ||
| - Avoid duplicate memories | ||
| - if a similar memory already exists, update it instead of creating a new one. | ||
| - Delete memories only if they are no longer relevant or if the information is incorrect | ||
| - For example, if the user has changed their major, you should delete the old memory and store the new one. | ||
|
|
||
| If the most_recent_conversation does not contain any useful information, | ||
| you should immediately use "finish" tool. | ||
| """ | ||
|
|
||
| # need to tweak prompt to include guidelines for temp and long term memories | ||
|
|
||
| session_conversation: dict[str, str] = dspy.InputField() | ||
| user_memories: list[str] = dspy.InputField() | ||
| most_recent_conversation: dict[str, str] = dspy.InputField() | ||
|
|
There was a problem hiding this comment.
PermanentMemorySignature input field types are inconsistent with actual usage.
Line 87-89 declares:
session_conversation: dict[str, str]most_recent_conversation: dict[str, str]
But PermanentMemory.forward() (lines 109-110) receives these as list[dict[str, str]]. This type mismatch in the signature will cause DSPy to potentially serialize/format these incorrectly.
🐛 Proposed fix
- session_conversation: dict[str, str] = dspy.InputField()
- user_memories: list[str] = dspy.InputField()
- most_recent_conversation: dict[str, str] = dspy.InputField()
+ session_conversation: list[dict[str, str]] = dspy.InputField()
+ user_memories: str = dspy.InputField()
+ most_recent_conversation: list[dict[str, str]] = dspy.InputField()Note: user_memories is assigned from self.memory.get_all_memories() (line 119), which returns a str, not list[str].
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@chatdku/chatdku/core/dspy_classes/memory.py` around lines 33 - 90, The
PermanentMemorySignature field types don't match how they're passed: update the
class fields in PermanentMemorySignature so session_conversation and
most_recent_conversation are typed as list[dict[str, str]] (to match how
PermanentMemory.forward() receives them) and change user_memories to the actual
return type from self.memory.get_all_memories() (string) or to list[str] if you
change get_all_memories to return a list; adjust the signature fields
accordingly and re-run type checks to ensure PermanentMemory.forward() and any
callers use the same types.
Comment on lines
+53
to
+120
| def store_memory( | ||
| self, | ||
| content: str | list[dict[str, str]], | ||
| metadata: dict | None = None, | ||
| ) -> str: | ||
| """Store information in memory along with metadata. | ||
|
|
||
| Args: | ||
| content: The fact to be stored in memory. | ||
| metadata: optional dictionary of metadata to associate with the memory. | ||
| All metadata values must be a single primitive (str, int, float, bool), or None | ||
| If you store multiple items(e.g., multiple tags), encode them as a comma-separated string. | ||
|
|
||
| You should store information related to the user. For example it could be: | ||
| - name of the user | ||
| - user's major | ||
| - user's graduation year | ||
| - etc | ||
| You should store the information you have asked from the user also. | ||
|
|
||
| Guidelines for time relevance: | ||
| - "long-term": stable facts that are useful across conversations | ||
| Examples: | ||
| - "User is a computer science major" | ||
| - "User prefers evening classes" | ||
| - "short-term": recent or context-specific information | ||
| Examples: | ||
| - "User is currently stressed about upcoming exams" | ||
| - "User is going to be late on an assignment today" | ||
|
|
||
| In addition to storing memory content, you should extract metadata from the content and store it as well. | ||
| Metadata can include: | ||
| - category (e.g., academic, personal, preference) | ||
| - entities (e.g., course names, majors, locations) | ||
| - tags (keywords) | ||
| - time relevance (e.g., short-term, long-term) | ||
|
|
||
| Do NOT store: | ||
| - task-specific requests (e.g., "help me plan my schedule") | ||
| - one-time clarifications (e.g., "I meant Bio110, not Bio101") | ||
| - general questions or instructions | ||
| - weak or irrelevant information | ||
|
|
||
|
|
||
| Example Usage: | ||
| store_memory( | ||
| "User will attend a guest lecture today.", | ||
| metadata={ | ||
| "category": "academic", | ||
| "entities": "lecture", | ||
| "tags": "user_info", | ||
| "time_relevance": "short-term" | ||
| } | ||
| ) | ||
| Returns: | ||
| str: The result of the operation. | ||
| """ | ||
| try: | ||
| self.memory.add( | ||
| content, user_id=self.user_id, run_id=self.session_id, metadata=metadata | ||
| ) | ||
| self.op_count += 1 | ||
|
|
||
| if self.op_count % 10 == 0: | ||
| self.cleanup_memory() | ||
| return f"Stored memory: {content}" | ||
| except Exception as e: | ||
| return f"Error storing memory: {str(e)}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if mem0's add() method supports list[dict] content
rg -n "\.add\(" --type py -A 3 -B 2Repository: Edge-Intelligence-Lab/ChatDKU
Length of output: 8309
🏁 Script executed:
#!/bin/bash
# First, let's look at the memory_tool.py file around lines 53-120
head -130 chatdku/chatdku/core/tools/memory_tool.py | tail -80Repository: Edge-Intelligence-Lab/ChatDKU
Length of output: 3059
🏁 Script executed:
#!/bin/bash
# Also check the imports and class initialization to understand what memory library is being used
head -60 chatdku/chatdku/core/tools/memory_tool.pyRepository: Edge-Intelligence-Lab/ChatDKU
Length of output: 2028
🏁 Script executed:
#!/bin/bash
# Search for all method signatures in memory_tool.py
rg "def store_memory|def retrieve_memory|def cleanup_memory" chatdku/chatdku/core/tools/memory_tool.py -A 5Repository: Edge-Intelligence-Lab/ChatDKU
Length of output: 562
🏁 Script executed:
#!/bin/bash
# Search for actual calls to store_memory to see what type of content is passed
rg "store_memory\(" -A 2 -B 2Repository: Edge-Intelligence-Lab/ChatDKU
Length of output: 2194
🏁 Script executed:
#!/bin/bash
# Check if there are any list inputs being passed to store_memory
rg "store_memory\(" --type py -A 5 | grep -E "(store_memory|^\s*\[|list)"Repository: Edge-Intelligence-Lab/ChatDKU
Length of output: 711
🏁 Script executed:
#!/bin/bash
# Look for mem0 library files in dependencies to understand add() method
find . -path "*/site-packages/mem0*" -o -path "*/mem0*" -type f -name "*.py" 2>/dev/null | head -20Repository: Edge-Intelligence-Lab/ChatDKU
Length of output: 55
🏁 Script executed:
#!/bin/bash
# Check requirements or setup files for mem0 version
rg "mem0" -i --type txt --type toml --type yaml --type cfgRepository: Edge-Intelligence-Lab/ChatDKU
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Check the StoreMemoryRequest definition
rg "class StoreMemoryRequest" -A 10Repository: Edge-Intelligence-Lab/ChatDKU
Length of output: 943
🏁 Script executed:
#!/bin/bash
# Look at the test_store_memory test
rg "def test_store_memory" -A 20 chatdku/tests/test_fastAPI.pyRepository: Edge-Intelligence-Lab/ChatDKU
Length of output: 564
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation or comments about list support
rg "list\[dict" -B 3 -A 3Repository: Edge-Intelligence-Lab/ChatDKU
Length of output: 3909
🏁 Script executed:
#!/bin/bash
# Search for mem0 in requirements or any dependency files
find . -name "requirements*.txt" -o -name "pyproject.toml" -o -name "setup.py" | xargs grep -l "mem0" 2>/dev/nullRepository: Edge-Intelligence-Lab/ChatDKU
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Check the mem0 version in pyproject.toml
cat chatdku/pyproject.toml | grep -A 5 -B 5 "mem0"Repository: Edge-Intelligence-Lab/ChatDKU
Length of output: 328
🌐 Web query:
mem0 library add() method signature documentation
💡 Result:
The mem0 library provides two main classes with an add method: Memory (open-source/self-hosted) and MemoryClient (platform/hosted). For Memory.add (Python open-source SDK): - Signature: add(messages, *, user_id=None, agent_id=None, run_id=None, metadata=None, infer=True, memory_type=None, prompt=None) - messages: str, dict, or list of message dicts (e.g., [{"role": "user", "content": "text"}]) - Returns: Dict with "results" key containing memory operations. Example: from mem0 import Memory m = Memory m.add(messages, user_id="alice", metadata={"category": "example"}, infer=False) For MemoryClient.add (Python platform SDK): - Signature: add(messages, kwargs) where kwargs include user_id, agent_id, app_id, metadata, filters, etc. - messages: list of message dicts. - Returns: API response with memory IDs. Example: from mem0 import MemoryClient client = MemoryClient(api_key="your-api-key") client.add(messages, user_id="alice") Similar signatures exist in JavaScript/TypeScript SDKs, with userId instead of user_id. Official documentation: https://docs.mem0.ai/core-concepts/memory-operations/add and GitHub LLM.md.
Citations:
- 1: https://docs.mem0.ai/core-concepts/memory-operations/add
- 2: https://github.com/mem0ai/mem0/blob/main/LLM.md
- 3: https://docs.mem0.ai/api-reference/memory/add-memories
- 4: https://github.com/mem0ai/mem0/blob/69a832dc/docs/core-concepts/memory-operations/add.mdx
- 5: https://docs.mem0.ai/open-source/python-quickstart
🏁 Script executed:
#!/bin/bash
# Check if mem0 has specific exceptions defined
python3 << 'EOF'
try:
import mem0
# Check for exception classes in mem0
import inspect
members = inspect.getmembers(mem0, inspect.isclass)
exceptions = [name for name, obj in members if issubclass(obj, Exception)]
print("mem0 exceptions:", exceptions if exceptions else "No specific exceptions found")
except ImportError:
print("mem0 not installed in sandbox, checking documentation")
EOFRepository: Edge-Intelligence-Lab/ChatDKU
Length of output: 126
🏁 Script executed:
#!/bin/bash
# Check the dspy_classes/memory.py to see the docstring signature mismatch
cat -n chatdku/chatdku/core/dspy_classes/memory.py | grep -A 5 -B 5 "store_memory"Repository: Edge-Intelligence-Lab/ChatDKU
Length of output: 1352
🏁 Script executed:
#!/bin/bash
# Check if there's any conversion or processing of list input before passing to memory.add
sed -n '50,125p' chatdku/chatdku/core/tools/memory_tool.pyRepository: Edge-Intelligence-Lab/ChatDKU
Length of output: 2949
Update docstring to document list support for content parameter.
The type hint correctly allows str | list[dict[str, str]] matching mem0's Memory.add() method, which supports both formats. However, the docstring only describes storing a single fact and omits any reference to list input. Update the Args section to clarify that content can be either a string or a list of message dictionaries with "role" and "content" keys.
Additionally, note the inconsistency: chatdku/core/dspy_classes/memory.py (line 39) defines the tool signature as store_memory(content: str, ...) without the list variant, which conflicts with the actual implementation.
For error handling, consider catching more specific exceptions from mem0 instead of broad Exception, though this is a minor refactor.
🧰 Tools
🪛 Ruff (0.15.9)
[warning] 119-119: Do not catch blind exception: Exception
(BLE001)
[warning] 120-120: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@chatdku/chatdku/core/tools/memory_tool.py` around lines 53 - 120, The
store_memory function's docstring only describes a single-string content but the
signature allows str | list[dict[str,str]] and memory.add supports both; update
the Args section of store_memory to state content may be either a single string
or a list of message dicts with "role" and "content" keys and give an example of
the list form, and then make the signature in the other implementation
(store_memory in dspy_classes/memory.py) consistent with this union type so both
places accept str | list[dict[str,str]]; optionally replace the broad except
Exception in store_memory with mem0-specific exception types from memory.add to
narrow error handling.
…at might not be needed" This reverts commit 87e45d6.
Ar-temis
force-pushed
the
main
branch
2 times, most recently
from
b7ed236 to
9d10f0a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Long-Term Memory Per User
Description
This PR implements long-term memory capabilities for the ChatDKU agent using Mem0, allowing the agent to remember user-specific facts across conversations. Users' information such as class year, major, and preferences are now persisted and recalled intelligently.
Issue Reference
Closes #172
Changes
New Features
Persistent User Memory: Implemented
MemoryToolsclass using Mem0 to store and retrieve user factsMemory Operations:
Memory Cleanup:
Key Files Modified
New Files Created:
chatdku/core/tools/memory_tool.py- Memory management interface using Mem0chatdku/core/dspy_classes/memory.py- Temporary and permanent memory modulesConfiguration Updates:
chatdku/pyproject.toml: Addedmem0aidependencychatdku/core/dspy_classes/prompt_settings.py: Added custom fact extraction promptTechnical Details
Memory Management
Agent Integration
search_memories,get_all_memories) added to agent's tool setPermanentMemoryagent runs after each conversation to extract and store relevant factsPrompt Enhancements
custom_fact_extraction_promptfor intelligent fact extraction from conversationsRelated
Summary by CodeRabbit
New Features
Refactor
Documentation
Tests
Chores