-
Notifications
You must be signed in to change notification settings - Fork 18
Omnibus of changes #3
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
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 is an omnibus change that refactors the codebase, moving functionality between modules, deprecating unused utilities, and integrating Docket for background task management while updating API and client logic. Key changes include:
- Removal and refactoring of agent_memory_server/utils.py with related updates in init.py.
- Integration of Docket-based background tasks into API endpoints and dependency injection.
- Numerous updates and cleanups in API endpoints, logging, and client wrappers.
Reviewed Changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| agent_memory_server/utils/init.py | Adds re-exports for Keys and _redis_pool after removal of utils.py. |
| agent_memory_server/utils.py | Entire file removed, consolidating functionality into other modules. |
| agent_memory_server/summarization.py | Updates import and redis connection handling; minor adjustments to summarization parameters. |
| agent_memory_server/models/* | New and updated model definitions including addition of memory_hash for deduplication. |
| agent_memory_server/messages.py | Added multiple debug print statements and refactored message and metadata handling. |
| agent_memory_server/mcp.py | Adjusted imports and input patterns for task-based functions. |
| agent_memory_server/main.py | Refined startup logic with explicit port parsing and Docket task initialization. |
| agent_memory_server/logging.py | Added a safeguard for repeated logger configuration. |
| agent_memory_server/llms.py | Changed parameter naming in create_chat_completion for clarity and updated client caching logic. |
| agent_memory_server/dependencies.py | Introduced a custom DocketBackgroundTasks class and updated dependency injection. |
| Other files | Updates to docket tasks, configuration, compaction documentation, API client and API endpoints. |
Comments suppressed due to low confidence (1)
agent_memory_server/llms.py:307
- [nitpick] The parameter was renamed from 'progressive_prompt' to 'prompt', but inline comments or documentation may still reference the old name; please update the comments and docstrings to reflect the new parameter name.
async def create_chat_completion(self, model: str, prompt: str, response_format: dict[str, str] | None = None, functions: list[dict[str, Any]] | None = None, function_call: dict[str, str] | None = None) -> ChatResponse:
agent_memory_server/messages.py
Outdated
| print(f"DEBUG: Listing sessions with key: {sessions_key}") | ||
|
|
||
| # Debug: List all keys in Redis | ||
| all_keys = await redis.keys("*") | ||
| print(f"DEBUG: All keys in Redis: {all_keys}") | ||
|
|
||
| # Debug: Check if the sessions key exists | ||
| exists = await redis.exists(sessions_key) | ||
| print(f"DEBUG: Does {sessions_key} exist? {exists}") | ||
|
|
||
| # Debug: Try to get all sessions directly | ||
| all_sessions = await redis.zrange(sessions_key, 0, -1) | ||
| print(f"DEBUG: Direct zrange of {sessions_key}: {all_sessions}") | ||
|
|
||
| async with redis.pipeline() as pipe: | ||
| pipe.zcard(sessions_key) | ||
| pipe.zrange(sessions_key, start, end) | ||
| total, session_ids = await pipe.execute() | ||
|
|
||
| print(f"DEBUG: Found {total} sessions: {session_ids}") |
Copilot
AI
Apr 29, 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.
Debug print statements are present and may leak sensitive information; consider replacing these with proper logger.debug calls or removing them before production.
| print(f"DEBUG: Listing sessions with key: {sessions_key}") | |
| # Debug: List all keys in Redis | |
| all_keys = await redis.keys("*") | |
| print(f"DEBUG: All keys in Redis: {all_keys}") | |
| # Debug: Check if the sessions key exists | |
| exists = await redis.exists(sessions_key) | |
| print(f"DEBUG: Does {sessions_key} exist? {exists}") | |
| # Debug: Try to get all sessions directly | |
| all_sessions = await redis.zrange(sessions_key, 0, -1) | |
| print(f"DEBUG: Direct zrange of {sessions_key}: {all_sessions}") | |
| async with redis.pipeline() as pipe: | |
| pipe.zcard(sessions_key) | |
| pipe.zrange(sessions_key, start, end) | |
| total, session_ids = await pipe.execute() | |
| print(f"DEBUG: Found {total} sessions: {session_ids}") | |
| logger.debug(f"Listing sessions with key: {sessions_key}") | |
| # Debug: List all keys in Redis | |
| all_keys = await redis.keys("*") | |
| logger.debug(f"All keys in Redis: {all_keys}") | |
| # Debug: Check if the sessions key exists | |
| exists = await redis.exists(sessions_key) | |
| logger.debug(f"Does {sessions_key} exist? {exists}") | |
| # Debug: Try to get all sessions directly | |
| all_sessions = await redis.zrange(sessions_key, 0, -1) | |
| logger.debug(f"Direct zrange of {sessions_key}: {all_sessions}") | |
| async with redis.pipeline() as pipe: | |
| pipe.zcard(sessions_key) | |
| pipe.zrange(sessions_key, start, end) | |
| total, session_ids = await pipe.execute() | |
| logger.debug(f"Found {total} sessions: {session_ids}") |
| print(f"Using port from command line: {port}") | ||
| except (ValueError, IndexError): | ||
| # If conversion fails or index out of bounds, use default | ||
| print(f"Invalid port argument, using default: {port}") | ||
| else: | ||
| print(f"No port argument provided, using default: {port}") | ||
|
|
||
| # Explicitly unset the PORT environment variable if it exists | ||
| if "PORT" in os.environ: | ||
| port_val = os.environ.pop("PORT") | ||
| print(f"Removed environment variable PORT={port_val}") |
Copilot
AI
Apr 29, 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.
[nitpick] Consider using the logger for debug output instead of print statements to maintain consistency with the rest of the project and avoid undesired console output in production.
| print(f"Using port from command line: {port}") | |
| except (ValueError, IndexError): | |
| # If conversion fails or index out of bounds, use default | |
| print(f"Invalid port argument, using default: {port}") | |
| else: | |
| print(f"No port argument provided, using default: {port}") | |
| # Explicitly unset the PORT environment variable if it exists | |
| if "PORT" in os.environ: | |
| port_val = os.environ.pop("PORT") | |
| print(f"Removed environment variable PORT={port_val}") | |
| logger.info(f"Using port from command line: {port}") | |
| except (ValueError, IndexError): | |
| # If conversion fails or index out of bounds, use default | |
| logger.warning(f"Invalid port argument, using default: {port}") | |
| else: | |
| logger.info(f"No port argument provided, using default: {port}") | |
| # Explicitly unset the PORT environment variable if it exists | |
| if "PORT" in os.environ: | |
| port_val = os.environ.pop("PORT") | |
| logger.info(f"Removed environment variable PORT={port_val}") |
This PR introduces background task management via Docket, adds a first version of the memory compaction system to deduplicate and merge similar memories, and improves client-side interaction with the Redis Memory Server through a new asynchronous HTTP client. These changes lay the groundwork for scalable background processing and more efficient memory storage.
What changed: