cleanup ! #147

harshalmore31 · 2025-11-11T17:46:09Z

This pull request introduces several improvements across the documentation and the MemoryAgent implementation, focusing on reliability, clarity, and performance. The documentation now provides clearer guidance on memory modes, SQL-based architecture advantages, and the interceptor pattern, while the codebase enhances error handling, duplicate detection, and logging in the memory agent.

Documentation Enhancements:

Memory Mode Selection Guidance:
- Added a comprehensive decision matrix and quick selection guide to help users choose between Conscious, Auto, and Combined memory modes, including performance trade-offs and upgrade/downgrade scenarios.
SQL-Native Architecture Explanation:
- Added a section comparing SQL-native memory storage to vector databases, highlighting cost, transparency, query speed, and portability benefits, along with best-use recommendations.
Interceptor Pattern and Internal Flow:
- Detailed the interceptor pattern used by Memori for transparent LLM API call interception, including a three-phase process (context injection, memory recording, background analysis) and flow diagrams.
- Expanded quick-start documentation with step-by-step explanations, flow diagrams, and instructions for inspecting and testing memory persistence in the SQL database.

MemoryAgent Reliability and Logging Improvements:

Robust Async API Calls:
- Introduced an async retry mechanism with exponential backoff for handling connection and timeout errors in all OpenAI API calls, reducing transient failure rates. [1] [2] [3] [4]
- Set explicit timeout and retry parameters for OpenAI client initialization to improve resilience.
Duplicate Detection Refinements:
- Increased the similarity threshold for duplicate detection to 0.92 (from 0.8) to reduce false positives.
- Skipped deduplication for conversational/query memories to avoid suppressing valid repeated queries.
- Enhanced logging for duplicate detection, including detailed match information for easier debugging.
General Code Improvements:
- Added missing asyncio import for async operations.

These changes collectively make Memori more reliable, transparent, and user-friendly, both in its documentation and core memory management logic.

Updated the README to change the section title from 'Overview' to 'What is Memori'.

- Applied Black formatting (line-length: 88) - Sorted imports with isort (black profile) - Applied Ruff auto-fixes Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

github-actions · 2025-11-11T17:46:48Z

Auto-Formatting Applied

Your code has been automatically formatted with:

Black (line-length: 88)
isort (import sorting)
Ruff (auto-fixes)

The changes have been committed to this PR. Please review and continue with your development!

github-actions · 2025-11-11T17:47:39Z

Summary Security Scan Results

All security scans passed!

Scan Type	Status
Dependency Scan	[SUCCESS] Passed
Code Security	[SUCCESS] Passed
Secrets Scan	[SUCCESS] Passed

View detailed reports in the workflow artifacts (if available).

[FIX] Workflow Information

Workflow Run: #268
Triggered by: pull_request
Branch: new_cleanup
Commit: c86021e

github-actions · 2025-11-11T17:49:13Z

Code Formatting Check

Great job! Your code is already properly formatted according to our standards:

Black formatting
Import sorting
Ruff checks

No changes needed!

github-actions · 2025-11-11T17:50:03Z

Summary Security Scan Results

All security scans passed!

Scan Type	Status
Dependency Scan	[SUCCESS] Passed
Code Security	[SUCCESS] Passed
Secrets Scan	[SUCCESS] Passed

View detailed reports in the workflow artifacts (if available).

[FIX] Workflow Information

Workflow Run: #269
Triggered by: pull_request
Branch: new_cleanup
Commit: c245fff

memori/agents/memory_agent.py


 Focus on extracting information that would genuinely help provide better context and assistance in future conversations."""

+    async def _retry_with_backoff(self, func, *args, max_retries=3, **kwargs):


To fix the problem, we should ensure that every code path in _retry_with_backoff both returns an explicit value or raises. At present, if the function exhausts all attempts in the for-loop without a successful return (if all attempts fail with a retryable error, but for some reason do not raise), it will reach the end of the function and return None implicitly. The correct solution is to add an explicit return None (or optionally raise an exception or return a sentinel value) after the for-loop, making the function behavior clear: it returns a result from the function if successful, raises if an exception occurs, or returns None if all retry attempts are exhausted. This edit is only required in memori/agents/memory_agent.py, at the end of the _retry_with_backoff function.

memori/config/memory_manager.py

+            # Destructors shouldn't raise, but log for debugging
+            try:
+                logger.debug(f"Cleanup error in destructor: {e}")
+            except:


To fix this issue, we should avoid using a bare except: clause. Instead, we should explicitly catch Exception in the inner try...except block in the destructor (__del__), which will avoid catching KeyboardInterrupt, SystemExit, and other exceptions derived directly from BaseException but not from Exception. This will still log any ordinary errors that occur in cleanup() or logger.debug(), but will not suppress signals to terminate or interrupt the Python process.

Specifically:

Locate the bare except: at line 326 in def __del__(self).

Change it to except Exception:.

No additional imports or structural changes are needed.

memori/core/memory.py

+            # Destructors shouldn't raise, but log for debugging
+            try:
+                logger.debug(f"Cleanup error in destructor: {e}")
+            except:


To fix this problem, replace the except: block on line 2801 in memori/core/memory.py with a more restrictive except block that catches only Exception. This way, only regular errors are caught during logging in __del__, while critical signals like KeyboardInterrupt and SystemExit are allowed to propagate. Edit the block such that:

The line except: is changed to except Exception:.

No change is required to the body of this except block, as the intent is to silence secondary failures in logging within the destructor.

You only need to edit this particular block in the code region shown. No additional new methods or imports are required.

memori/utils/logging.py

+                # Set litellm's logger to ERROR level to prevent duplicate logs
+                litellm_logger = logging.getLogger("LiteLLM")
+                litellm_logger.setLevel(logging.ERROR)
+            except ImportError:


To fix this issue, we should not leave the except ImportError: block empty. The best practice is to log an informative message explaining that the optional dependency (litellm) was not found, so its specific logging suppression steps will be skipped. This achieves two things: future readers understand the reason for ignoring the exception, and diagnostic information is available if the absence of litellm matters. The minimal required change is to insert a logger statement in the except ImportError: block at line 43 (or, if logging is not yet configured at that point, a comment explaining the intent). Given that the loguru logger is available, logging a warning or info message is preferable.

All changes must be made in the file memori/utils/logging.py, specifically inside the except block at line 43.

tests/integration/test_memory_modes.py

+        time.sleep(0.5)
+
+        # ASPECT 2: Persistence - Chat history stored, but no memory ingestion
+        stats = memori.db_manager.get_memory_stats(memori.user_id)


To fix the problem: simply remove the assignment to the unused variable stats. Since memori.db_manager.get_memory_stats(memori.user_id) most likely does not have required side effects, we can remove the entire line (line 60). No other changes to the surrounding code are needed, and no additional imports or method changes are necessary.

tests/integration/test_memory_modes.py

+        )
+
+        # ASPECT 2: Persistence - Data persists across mode change
+        initial_stats = memori_sqlite.db_manager.get_memory_stats(memori_sqlite.user_id)


To properly fix the "unused local variable" warning for initial_stats on line 625 of tests/integration/test_memory_modes.py, we should remove the variable assignment if the result is genuinely not needed elsewhere. However, if the function call must stay for potential required side-effects (such as for documenting, clearing a cache, or triggering a logging action), we should remove the left-hand side only, keeping the function call as a standalone line. If the unused variable is kept for documentation or placeholder purposes, it must be renamed following unused-variable conventions (e.g., _ or unused_initial_stats). In this case, the right side function call should be kept as a standalone line, and the assignment should be deleted.

tests/integration/test_multi_tenancy.py

+        - Integration: Users isolated per namespace
+        """
+        users = multi_user_memori
+        namespace1 = f"{test_namespace}_ns1"


To fix the problem, simply remove the line assigning to namespace1, as it is not used anywhere in the function. No additional imports, function definitions, or variable assignments are needed elsewhere. The removal is limited to the single line within tests/integration/test_multi_tenancy.py, inside the test_namespace_user_isolation method.

tests/integration/test_multi_tenancy.py

+        """
+        users = multi_user_memori
+        namespace1 = f"{test_namespace}_ns1"
+        namespace2 = f"{test_namespace}_ns2"


To fix the problem, we should remove the assignment to the unused variable namespace2 in the test_namespace_user_isolation method of the class TestNamespaceAndUserIDCombination in file tests/integration/test_multi_tenancy.py. The right-hand side (RHS) of the assignment, which evaluates to a simple formatted string, does not produce side effects and can be safely deleted along with the variable. No further changes (such as import changes or method definitions) are necessary.

tests/integration/test_multi_tenancy.py

+                )
+
+        # Test search performance for each user
+        search_times = {}


To fix this problem, the unused variable assignment should be removed from the function. Specifically, in the code block of test 11 on line 565, the line search_times = {} can simply be deleted. No changes to imports or other code are necessary, as there are no side effects from the assignment or loss of documentation value. Ensure only this line is removed and other surrounding functionality is untouched.

tests/integration/test_mysql_comprehensive.py

+        memory = create_simple_memory(
+            content=content, summary="Replication test", classification="knowledge"
+        )
+        memory_id = memori_mysql.db_manager.store_long_term_memory_enhanced(


The best way to fix the problem is to remove the unused variable assignment to memory_id on line 534, leaving only the function call to memori_mysql.db_manager.store_long_term_memory_enhanced(...) as a statement for its possible side effect (storing the memory in the database). This way, we retain the functionality (the memory is stored), avoid superfluous code, and satisfy CodeQL that there is no unused variable. Only lines 534–535 need to be edited in the given region of the file tests/integration/test_mysql_comprehensive.py.

Copilot

Pull Request Overview

This pull request introduces comprehensive improvements to both documentation and core functionality, with a strong focus on reliability, error handling, and testing infrastructure. The changes enhance the MemoryAgent's resilience through async retry mechanisms, improve duplicate detection to reduce false positives, and add extensive integration tests across multiple databases and LLM providers.

Key Changes:

Added async retry mechanism with exponential backoff for OpenAI API calls to handle transient connection failures
Increased duplicate detection similarity threshold from 0.8 to 0.92 to reduce false positives
Enhanced error handling by replacing bare except clauses with specific exception types
Introduced comprehensive integration test suites for SQLite, PostgreSQL, MySQL, and multiple LLM providers
Added connection pool monitoring and configuration improvements

Reviewed Changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
`tests/pytest.ini`	New pytest configuration with markers for test categorization and coverage settings
`tests/integration/test_*.py`	Comprehensive integration tests for databases (SQLite, PostgreSQL, MySQL) and LLM providers (OpenAI, Ollama, LiteLLM, Azure)
`memori/agents/memory_agent.py`	Added async retry logic with exponential backoff and improved duplicate detection
`memori/utils/async_bridge.py`	New persistent background event loop for efficient async task execution
`memori/utils/logging.py`	Enhanced logging to suppress noisy third-party library logs
`memori/database/search_service.py`	Improved error logging with structured messages
`memori/database/sqlalchemy_manager.py`	Added connection pool status monitoring and health checks
`memori/core/memory.py`	Implemented conversation fingerprinting for deduplication and improved async processing
`docs/*`	Added troubleshooting guide, architecture documentation, and expanded quick-start guide

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-11T18:17:45Z

tests/integration/test_memory_modes.py

+        time.sleep(0.5)
+
+        # ASPECT 2: Persistence - Chat history stored, but no memory ingestion
+        stats = memori.db_manager.get_memory_stats(memori.user_id)


Variable stats is not used.

Copilot · 2025-11-11T18:17:45Z

tests/integration/test_memory_modes.py

+            classification="context",
+            importance="high",
+        )
+        memory_id = memori.db_manager.store_long_term_memory_enhanced(


Variable memory_id is not used.

Suggested change

memory_id = memori.db_manager.store_long_term_memory_enhanced(

memori.db_manager.store_long_term_memory_enhanced(

Copilot · 2025-11-11T18:17:46Z

tests/integration/test_memory_modes.py

+        with patch.object(
+            client.chat.completions, "create", return_value=mock_openai_response
+        ):
+            response = client.chat.completions.create(


Variable response is not used.

Copilot · 2025-11-11T18:17:46Z

tests/integration/test_memory_modes.py

+        )
+
+        # ASPECT 2: Persistence - Data persists across mode change
+        initial_stats = memori_sqlite.db_manager.get_memory_stats(memori_sqlite.user_id)


Variable initial_stats is not used.

Suggested change

initial_stats = memori_sqlite.db_manager.get_memory_stats(memori_sqlite.user_id)

Copilot · 2025-11-11T18:17:46Z

tests/integration/test_mysql_comprehensive.py

+        memory = create_simple_memory(
+            content=content, summary="Replication test", classification="knowledge"
+        )
+        memory_id = memori_mysql.db_manager.store_long_term_memory_enhanced(


Variable memory_id is not used.

Suggested change

memory_id = memori_mysql.db_manager.store_long_term_memory_enhanced(

memori_mysql.db_manager.store_long_term_memory_enhanced(

Copilot · 2025-11-11T18:17:47Z

tests/integration/test_multi_tenancy.py

+                )
+
+        # Test search performance for each user
+        search_times = {}


Variable search_times is not used.

Suggested change

search_times = {}

Copilot · 2025-11-11T18:17:48Z

memori/agents/memory_agent.py


 Focus on extracting information that would genuinely help provide better context and assistance in future conversations."""

+    async def _retry_with_backoff(self, func, *args, max_retries=3, **kwargs):


Mixing implicit and explicit returns may indicate an error, as implicit returns always return None.

Copilot · 2025-11-11T18:17:48Z

memori/config/memory_manager.py

+            # Destructors shouldn't raise, but log for debugging
+            try:
+                logger.debug(f"Cleanup error in destructor: {e}")
+            except:


Except block directly handles BaseException.

Suggested change

except:

except Exception:

Copilot · 2025-11-11T18:17:48Z

memori/core/memory.py

+            # Destructors shouldn't raise, but log for debugging
+            try:
+                logger.debug(f"Cleanup error in destructor: {e}")
+            except:


Except block directly handles BaseException.

Suggested change

except:

except Exception:

Copilot · 2025-11-11T18:17:49Z

memori/utils/logging.py

+                # Set litellm's logger to ERROR level to prevent duplicate logs
+                litellm_logger = logging.getLogger("LiteLLM")
+                litellm_logger.setLevel(logging.ERROR)
+            except ImportError:


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except ImportError:

except ImportError:

# litellm is optional; skip configuration if not installed

harshalmore31 and others added 18 commits November 9, 2025 13:22

cleanup

925391f

cleanup !

c7364ac

updates !

1173174

updates !

2669821

fixes !

1720f40

cleanup !

1a611ca

Fixes !

ca7f582

updates !

864ab03

cleanup !

fcc40f8

fixed !

39ee3b7

Change section title in README

ffcbad0

Updated the README to change the section title from 'Overview' to 'What is Memori'.

fixed logging !

f3a71c9

fixes !

1bf2975

fix !

96b738d

expections fixed !

4b27fb7

docs updated !

537d4b5

feat. pytest for testing !

86fe78f

Auto-format code with Black, isort, and Ruff

ae152d6

- Applied Black formatting (line-length: 88) - Sorted imports with isort (black profile) - Applied Ruff auto-fixes Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Merge branch 'main' into new_cleanup

3008e99

github-advanced-security bot found potential problems Nov 11, 2025

View reviewed changes

harshalmore31 requested a review from Copilot November 11, 2025 18:14

Copilot started reviewing on behalf of harshalmore31 November 11, 2025 18:15 View session

Copilot finished reviewing on behalf of harshalmore31 November 11, 2025 18:15

Copilot AI reviewed Nov 11, 2025

View reviewed changes

@@ -562,7 +562,6 @@
                             )
                     # Test search performance for each user
-                    search_times = {}
                     for user_id in ["alice", "bob", "charlie"]:
                         with performance_tracker.track(f"search_{user_id}"):


		Focus on extracting information that would genuinely help provide better context and assistance in future conversations."""

		async def _retry_with_backoff(self, func, args, max_retries=3, *kwargs):

@@ -173,6 +173,8 @@
                                     continue
                             # Re-raise if not a retryable error or max retries reached
                             raise
+                    # Explicitly return None if all retries are exhausted and no exception was raised
+                    return None
                 async def process_conversation_async(
                     self,

@@ -323,5 +323,5 @@
                         # Destructors shouldn't raise, but log for debugging
                         try:
                             logger.debug(f"Cleanup error in destructor: {e}")
-                        except:
+                        except Exception:
                             pass  # Can't do anything if logging fails in destructor

@@ -40,7 +40,7 @@
                             litellm_logger = logging.getLogger("LiteLLM")
                             litellm_logger.setLevel(logging.ERROR)
                         except ImportError:
-                            pass
+                            logger.info("LiteLLM not installed; skipping LiteLLM log suppression.")
                         if verbose:
                             logger.add(

	memory_id = memori.db_manager.store_long_term_memory_enhanced(
	memori.db_manager.store_long_term_memory_enhanced(

	memory_id = memori_mysql.db_manager.store_long_term_memory_enhanced(
	memori_mysql.db_manager.store_long_term_memory_enhanced(

	except ImportError:
	except ImportError:
	# litellm is optional; skip configuration if not installed

cleanup ! #147

Are you sure you want to change the base?

cleanup ! #147

Uh oh!

Conversation

harshalmore31 commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 11, 2025

Auto-Formatting Applied

Uh oh!

github-actions bot commented Nov 11, 2025

Summary Security Scan Results

Uh oh!

github-actions bot commented Nov 11, 2025

Code Formatting Check

Uh oh!

github-actions bot commented Nov 11, 2025

Summary Security Scan Results

Uh oh!

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

harshalmore31 commented Nov 11, 2025 •

edited

Loading