-
Notifications
You must be signed in to change notification settings - Fork 0
Replace custom memory with Memori integration #1
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
Replace custom memory with Memori integration #1
Conversation
Replaced custom memory implementation with the memori library (https://github.com/GibsonAI/memori), an SQL-native memory engine for AI agents. This provides automatic conversation history management and context injection for LLM calls. Key changes: - Added memorisdk>=2.3.0 as a dependency - Rewrote memory.py to use Memori for conversation history - Memori automatically handles conversation context injection into LLM calls - Kept user metadata (preferences, context, message counts) in JSON storage - Updated agent.py to enable Memori after initialization - Conversation history now managed by Memori's SQLite database - Updated tests to work with new memory architecture - Fixed deadlock issues in memory manager lock handling - Fixed KnowledgeChunk hashability issue in search functionality - All 12 tests passing successfully Features: - SQL-based conversation storage (default: SQLite) - Automatic context injection (conscious_ingest and auto_ingest modes) - Cost-effective alternative to vector databases - Graceful degradation when OpenAI API key not provided
Removed knowledge base functionality as requested - context can be provided through custom prompt files/text instead. Changes: - Removed KnowledgeConfig from config.py - Removed knowledge base initialization from agent.py - Removed search_knowledge tool from agent - Removed knowledge base tests (2 tests) - Removed knowledge-dir CLI option from main.py - Removed knowledge base references from message_router help/status - Removed optional knowledge dependencies from pyproject.toml - Removed KNOWLEDGE_DIR and related config from .env.example - Simplified agent dependencies (removed knowledge parameter) All tests passing (10/10 tests)
Implemented smart message filtering and custom context features: Features: - Channel-based message filtering (default: channel 0 / General) - Activation phrase for channel messages (default: "@bot") - DMs always trigger a response (no activation phrase needed) - Channel messages require activation phrase to trigger response - Custom prompt file support for domain-specific knowledge - Load custom instructions from text file - Configurable via CUSTOM_PROMPT_FILE environment variable Configuration: - ACTIVATION_PHRASE: Phrase to trigger bot in channels (default: "@bot") - LISTEN_CHANNEL: Channel to monitor (default: "0" for General) - CUSTOM_PROMPT_FILE: Path to custom prompt/context file Message Handling Logic: - Direct messages (DMs): Always respond, no activation phrase needed - Channel messages: Only respond if: 1. Message is on the configured listen_channel 2. Message contains activation_phrase (case-insensitive) - Broadcast messages: Ignore by default Implementation: - Added channel field to MeshCoreMessage dataclass - Added _should_respond_to_message() method to filter messages - Enhanced AIConfig with activation_phrase, listen_channel, custom_prompt_file - Custom prompt loaded from file and appended to agent instructions - User identification via public key for per-user memory (Memori) Example custom prompt file included in prompts/example.txt All tests passing (10/10)
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.
Pull request overview
This PR replaces the custom conversation history management system with Memori library integration, which provides automatic conversation memory injection for LLM calls. The changes also remove the knowledge base functionality in favor of custom prompt files and add message filtering capabilities (activation phrase and channel-based filtering) for more controlled bot responses.
Key Changes
- Integrated Memori SDK for automated conversation history management, replacing custom in-memory conversation tracking
- Removed knowledge base functionality and replaced it with custom prompt file support for domain-specific knowledge
- Added activation phrase and channel filtering for bot responses in channel messages
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/meshbot/memory.py | Integrated Memori library for conversation history, separated metadata storage from conversation tracking, added enable_memori() method |
| src/meshbot/agent.py | Removed knowledge base dependencies, added message filtering logic (_should_respond_to_message), integrated custom prompt loading |
| src/meshbot/config.py | Removed KnowledgeConfig, added activation_phrase, listen_channel, and custom_prompt_file to AIConfig |
| src/meshbot/main.py | Removed knowledge_dir CLI option, added custom prompt file loading |
| src/meshbot/message_router.py | Removed knowledge base search command and statistics from help/status messages |
| src/meshbot/knowledge.py | Fixed bug where KnowledgeChunk objects were used as dict keys (now uses id(chunk)) |
| src/meshbot/meshcore_interface.py | Added channel field to MeshCoreMessage dataclass, reordered imports |
| tests/test_basic.py | Updated tests for Memori integration, removed knowledge base tests, added user preferences and context tests |
| pyproject.toml | Added memorisdk>=2.3.0 dependency, removed knowledge optional dependencies |
| prompts/example.txt | New file with example custom prompt for domain-specific knowledge |
| .env.example | Added ACTIVATION_PHRASE, LISTEN_CHANNEL, CUSTOM_PROMPT_FILE, and MEMORI_DATABASE_URL configuration options |
Comments suppressed due to low confidence (1)
src/meshbot/memory.py:12
- Import of 'ConfigManager' is not used.
from memori import ConfigManager, Memori
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Update user info (inline to avoid lock issues) | ||
| if message.sender_name and message.sender_name != memory.user_name: | ||
| memory.user_name = message.sender_name | ||
| self._dirty = True | ||
|
|
||
| # Keep only last 100 messages to prevent memory bloat | ||
| if memory.conversation_history and len(memory.conversation_history) > 100: | ||
| memory.conversation_history = memory.conversation_history[-100:] | ||
| current_time = asyncio.get_event_loop().time() | ||
| if memory.first_seen is None: | ||
| memory.first_seen = current_time | ||
| self._dirty = True | ||
|
|
||
| memory.last_seen = current_time | ||
|
|
Copilot
AI
Nov 22, 2025
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.
Maintainability: This code duplicates the user info update logic that was previously in update_user_info. Consider calling update_user_info instead of inlining the logic to reduce code duplication, or ensure the lock is properly managed.
| from collections import defaultdict | ||
| from typing import Any, Dict, List, Optional | ||
|
|
||
| from memori import ConfigManager, Memori |
Copilot
AI
Nov 22, 2025
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.
Maintainability: ConfigManager is imported but never used in this file. Consider removing it from the import statement.
| from memori import ConfigManager, Memori | |
| from memori import Memori |
| # MEMORI_DATABASE_URL=sqlite:///memori_conversations.db | ||
| # MEMORI_DATABASE_URL=postgresql://user:pass@localhost/memori |
Copilot
AI
Nov 22, 2025
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.
Documentation: The .env.example file documents MEMORI_DATABASE_URL environment variable on lines 27-28, but this variable is not used anywhere in the code. The MemoryManager.__init__ accepts a database_url parameter but doesn't read it from environment variables. Either remove these lines from .env.example or add support for reading this environment variable in memory.py.
| # MEMORI_DATABASE_URL=sqlite:///memori_conversations.db | |
| # MEMORI_DATABASE_URL=postgresql://user:pass@localhost/memori |
| auto_ingest=True, # Enable dynamic search | ||
| openai_api_key=openai_api_key, | ||
| ) | ||
| self.memori_enabled = False |
Copilot
AI
Nov 22, 2025
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.
Bug: memori_enabled is set to False after successfully initializing Memori. This should be set to True to indicate that Memori is available and can be enabled.
| self.memori_enabled = False | |
| self.memori_enabled = True |
| async def update_user_info( | ||
| self, user_id: str, user_name: Optional[str] = None | ||
| ) -> None: | ||
| """Update user information.""" | ||
| async with self._lock: | ||
| memory = await self.get_user_memory(user_id) | ||
| # Note: This method should be called from within a lock context | ||
| if user_id not in self._metadata: | ||
| self._metadata[user_id] = UserMemory(user_id=user_id) | ||
| self._dirty = True | ||
|
|
||
| if user_name and user_name != memory.user_name: | ||
| memory.user_name = user_name | ||
| self._dirty = True | ||
| memory = self._metadata[user_id] | ||
|
|
||
| current_time = asyncio.get_event_loop().time() | ||
| if user_name and user_name != memory.user_name: | ||
| memory.user_name = user_name | ||
| self._dirty = True | ||
|
|
||
| if memory.first_seen is None: | ||
| memory.first_seen = current_time | ||
| self._dirty = True | ||
| current_time = asyncio.get_event_loop().time() | ||
|
|
||
| memory.last_seen = current_time | ||
| if memory.first_seen is None: | ||
| memory.first_seen = current_time | ||
| self._dirty = True | ||
|
|
||
| memory.last_seen = current_time | ||
| self._dirty = True |
Copilot
AI
Nov 22, 2025
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.
Bug: The update_user_info method is no longer properly guarded by a lock. The comment on line 195 states "This method should be called from within a lock context", but the method is accessing and modifying self._metadata and self._dirty without acquiring self._lock. This creates a race condition. Either acquire the lock within this method, or ensure all callers hold the lock (which they currently don't - see line 233 in add_message).
| from dataclasses import dataclass, field | ||
| from pathlib import Path | ||
| from typing import Optional, Dict, Any | ||
| from typing import Any, Dict, Optional |
Copilot
AI
Nov 22, 2025
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.
Import of 'Any' is not used.
Import of 'Dict' is not used.
| from typing import Any, Dict, Optional | |
| from typing import Optional |
| from dataclasses import dataclass | ||
| from pathlib import Path | ||
| from typing import Dict, List, Optional, Tuple, Any | ||
| from typing import Any, Dict, List, Optional, Tuple |
Copilot
AI
Nov 22, 2025
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.
Import of 'Tuple' is not used.
| from typing import Any, Dict, List, Optional, Tuple | |
| from typing import Any, Dict, List, Optional |
| import re | ||
| from typing import Dict, List, Optional, Callable, Any | ||
| from dataclasses import dataclass | ||
| from typing import Any, Callable, Dict, List, Optional |
Copilot
AI
Nov 22, 2025
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.
Import of 'Callable' is not used.
| from typing import Any, Callable, Dict, List, Optional | |
| from typing import Any, Dict, List, Optional |
| import asyncio | ||
| from pathlib import Path | ||
| from unittest.mock import Mock, AsyncMock | ||
| from unittest.mock import AsyncMock, Mock |
Copilot
AI
Nov 22, 2025
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.
Import of 'AsyncMock' is not used.
Import of 'Mock' is not used.
| from unittest.mock import AsyncMock, Mock |
| from meshbot.memory import MemoryManager, ConversationMessage | ||
| from meshbot.knowledge import SimpleKnowledgeBase | ||
| from meshbot.meshcore_interface import MockMeshCoreInterface, MeshCoreMessage | ||
| from meshbot.memory import ConversationMessage, MemoryManager |
Copilot
AI
Nov 22, 2025
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.
Import of 'ConversationMessage' is not used.
| from meshbot.memory import ConversationMessage, MemoryManager | |
| from meshbot.memory import MemoryManager |
No description provided.