-
Notifications
You must be signed in to change notification settings - Fork 397
feat: integrate Mem0 memory management into code_scratch #730
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary of ChangesHello @vinci-grape, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the project's memory management by integrating Mem0AI. It introduces a robust system for shared and persistent memory within the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request successfully integrates Mem0 for memory management, introducing a SharedMemoryManager
and the Mem0Memory
implementation. The changes enable the agent to handle both conversational and procedural memory, which is a great enhancement. My review focuses on improving code quality and robustness. I've identified a few issues, including an unused import, leftover commented code, a brittle string manipulation that could lead to bugs, and a typo in a configuration file's prompt. Addressing these points will make the new memory feature more reliable and the codebase cleaner.
ms_agent/agent/memory/mem0ai.py
Outdated
|
||
if system_message and hasattr(system_message, 'content'): | ||
# Append memories to existing system message | ||
system_message.content = system_message.content.split("\n\nUser Memories:\n")[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using split("\n\nUser Memories:\n")
to remove previous memories before injecting new ones is brittle. If the original system message contains this string for any other reason, it will be unintentionally truncated, which could lead to incorrect agent behavior. A more robust approach should be considered, such as storing the base system prompt separately and reconstructing it with the latest memories in each turn.
f'which supports: {list(memory_mapping.keys())}') | ||
self.memory_tools.append(memory_mapping[_memory.name]( | ||
self.config)) | ||
if _memory.name == 'mem0': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the memory
module to parent dir as an independent module.
Refer to this PR: #721
requirements/framework.txt
Outdated
json5 | ||
markdown | ||
mcp | ||
mem0ai |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be optional dependency ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
"""Load and initialize memory components from the config.""" | ||
if hasattr(self.config, 'memory'): | ||
for _memory in (self.config.memory or []): | ||
assert _memory.name in memory_mapping, ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint:
pip install pre-commit
pre-commit run --all-files (if needed)
def check_runtime(): | ||
try: | ||
os.system('pkill -f node') | ||
# os.system('pkill -f node') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete code, not comment code if they are not needed
Change Summary
Related issue number
Checklist
pre-commit install
andpre-commit run --all-files
before git commit, and passed lint check.