fix: Clean old session if needed - check is done on create new session

Author: dori

src/mcp_as_a_judge/constants.py

@@ -15,4 +15,3 @@
 DATABASE_URL = "sqlite://:memory:"
 MAX_SESSION_RECORDS = 20  # Maximum records to keep per session (FIFO)
 MAX_TOTAL_SESSIONS = 50  # Maximum total sessions to keep (LRU cleanup)
-RECORD_RETENTION_DAYS = 10  # Optional time-based cleanup (fallback)

src/mcp_as_a_judge/db/cleanup_service.py

@@ -1,16 +1,16 @@
 """
 Database cleanup service for conversation history records.
 
-This service handles time-based cleanup operations for conversation history records,
-removing records older than the retention period (default: 1 day).
+This service handles LRU-based cleanup operations for conversation history records,
+removing least recently used sessions when session limits are exceeded.
 """
 
-from datetime import UTC, datetime, timedelta
+from datetime import UTC, datetime
 
 from sqlalchemy import Engine, func
 from sqlmodel import Session, select
 
-from mcp_as_a_judge.constants import MAX_TOTAL_SESSIONS, RECORD_RETENTION_DAYS
+from mcp_as_a_judge.constants import MAX_TOTAL_SESSIONS
 from mcp_as_a_judge.db.interface import ConversationRecord
 from mcp_as_a_judge.logging_config import get_logger
 
@@ -20,11 +20,11 @@
 
 class ConversationCleanupService:
     """
-    Service for cleaning up old conversation history records.
+    Service for cleaning up conversation history records.
 
     Implements session-based LRU cleanup strategy:
-    - Maintains max 2000 sessions by removing least recently used sessions
-    - Runs once per day to avoid performance overhead
+    - Maintains session limit by removing least recently used sessions
+    - Runs immediately when new sessions are created and limit is exceeded
 
     LRU vs FIFO for Better User Experience:
     - LRU (Least Recently Used): Keeps sessions that users are actively using,
@@ -45,53 +45,8 @@ def __init__(self, engine: Engine) -> None:
         """
         self.engine = engine
         self.max_total_sessions = MAX_TOTAL_SESSIONS
-        self.retention_days = RECORD_RETENTION_DAYS
-        self.last_cleanup_time = datetime.now(UTC)
-        self.last_session_cleanup_time = datetime.now(UTC)
 
-    def cleanup_old_records(self) -> int:
-        """
-        Remove records older than retention_days.
-        This runs once per day to avoid excessive cleanup operations.
-
-        Returns:
-            Number of records deleted
-        """
-        # Only run cleanup once per day
-        if (datetime.now(UTC) - self.last_cleanup_time).days < 1:
-            return 0
-
-        cutoff_date = datetime.now(UTC) - timedelta(days=self.retention_days)
-
-        with Session(self.engine) as session:
-            # Count old records
-            old_count_stmt = select(ConversationRecord).where(
-                ConversationRecord.timestamp < cutoff_date
-            )
-            old_records = session.exec(old_count_stmt).all()
-            old_count = len(old_records)
-
-            if old_count == 0:
-                logger.info(
-                    f"🧹 Daily cleanup: No records older than {self.retention_days} days"
-                )
-                self.last_cleanup_time = datetime.now(UTC)
-                return 0
-
-            # Delete old records
-            for record in old_records:
-                session.delete(record)
 
-            session.commit()
-
-            # Reset cleanup tracking
-            self.last_cleanup_time = datetime.now(UTC)
-
-            logger.info(
-                f"🧹 Daily cleanup: Deleted {old_count} records older than "
-                f"{self.retention_days} days"
-            )
-            return old_count
 
     def get_session_count(self) -> int:
         """
@@ -178,34 +133,27 @@ def cleanup_excess_sessions(self) -> int:
 
         This implements LRU (Least Recently Used) cleanup strategy:
         - Keeps sessions that users are actively using (better UX than FIFO)
-        - Only runs once per day to avoid excessive cleanup operations
-        - During the day, session count can exceed limit
-          (e.g., 5000 sessions is not a memory issue)
-        - Daily cleanup brings it back to the target limit (2000 sessions)
+        - Runs immediately when session limit is exceeded (no daily restriction)
         - Removes entire sessions (all records for those session_ids)
+        - Called every time a new session is created to maintain session limit
 
         Returns:
             Number of records deleted
         """
-        # Only run session cleanup once per day
-        if (datetime.now(UTC) - self.last_session_cleanup_time).days < 1:
-            return 0
-
         current_session_count = self.get_session_count()
 
         if current_session_count <= self.max_total_sessions:
             logger.info(
-                f"🧹 Daily session LRU cleanup: {current_session_count} sessions "
+                f"🧹 Session LRU cleanup: {current_session_count} sessions "
                 f"(max: {self.max_total_sessions}) - no cleanup needed"
             )
-            self.last_session_cleanup_time = datetime.now(UTC)
             return 0
 
         # Calculate how many sessions to remove
         sessions_to_remove = current_session_count - self.max_total_sessions
 
         logger.info(
-            f"🧹 Daily session LRU cleanup: {current_session_count} sessions exceeds limit "
+            f"🧹 Session LRU cleanup: {current_session_count} sessions exceeds limit "
             f"({self.max_total_sessions}), removing {sessions_to_remove} "
             f"least recently used sessions"
         )
@@ -215,17 +163,13 @@ def cleanup_excess_sessions(self) -> int:
 
         if not lru_session_ids:
             logger.warning("🧹 No sessions found for LRU cleanup")
-            self.last_session_cleanup_time = datetime.now(UTC)
             return 0
 
         # Delete all records for these sessions
         deleted_count = self.delete_sessions(lru_session_ids)
 
-        # Reset cleanup tracking
-        self.last_session_cleanup_time = datetime.now(UTC)
-
         logger.info(
-            f"✅ Daily session LRU cleanup completed: removed {sessions_to_remove} sessions, "
+            f"✅ Session LRU cleanup completed: removed {sessions_to_remove} sessions, "
             f"deleted {deleted_count} records"
         )
 

src/mcp_as_a_judge/db/db_config.py

@@ -9,7 +9,6 @@
     DATABASE_URL,
     MAX_SESSION_RECORDS,
     MAX_TOTAL_SESSIONS,
-    RECORD_RETENTION_DAYS,
 )
 
 
@@ -63,7 +62,6 @@ def __init__(self) -> None:
         self.url = DATABASE_URL
         self.max_session_records = MAX_SESSION_RECORDS
         self.max_total_sessions = MAX_TOTAL_SESSIONS
-        self.record_retention_days = RECORD_RETENTION_DAYS
 
 
 class Config:

src/mcp_as_a_judge/db/providers/sqlite_provider.py

@@ -30,7 +30,7 @@ class SQLiteProvider(ConversationHistoryDB):
     - SQLModel with SQLAlchemy for type safety
     - In-memory or file-based SQLite storage
     - Two-level cleanup strategy:
-      1. Daily session-based LRU cleanup (max 2000 sessions, removes least recently used)
+      1. Session-based LRU cleanup (runs when new sessions are created, removes least recently used)
       2. Per-session FIFO cleanup (max 20 records per session, runs on every save)
     - Session-based conversation retrieval
     """
@@ -51,7 +51,7 @@ def __init__(self, max_session_records: int = 20, url: str = "") -> None:
 
         self._max_session_records = max_session_records
 
-        # Initialize cleanup service for time-based cleanup
+        # Initialize cleanup service for LRU session cleanup
         self._cleanup_service = ConversationCleanupService(engine=self.engine)
 
         # Create tables
@@ -60,8 +60,7 @@ def __init__(self, max_session_records: int = 20, url: str = "") -> None:
         logger.info(
             f"🗄️ SQLModel SQLite provider initialized: {connection_string}, "
             f"max_records_per_session={max_session_records}, "
-            f"max_total_sessions={self._cleanup_service.max_total_sessions}, "
-            f"retention_days={self._cleanup_service.retention_days}"
+            f"max_total_sessions={self._cleanup_service.max_total_sessions}"
         )
 
     def _parse_sqlite_url(self, url: str) -> str:
@@ -82,18 +81,13 @@ def _create_tables(self) -> None:
         SQLModel.metadata.create_all(self.engine)
         logger.info("📋 Created conversation_history table with SQLModel")
 
-    def _cleanup_old_records(self) -> int:
-        """
-        Remove records older than retention_days using the cleanup service.
-        This runs once per day to avoid excessive cleanup operations.
-        """
-        return self._cleanup_service.cleanup_old_records()
+
 
     def _cleanup_excess_sessions(self) -> int:
         """
         Remove least recently used sessions when total sessions exceed limit.
-        This implements daily LRU cleanup to maintain max 2000 sessions for better memory management.
-        Runs once per day - during the day session count can exceed limit without issues.
+        This implements LRU cleanup to maintain session limit for better memory management.
+        Runs immediately when new sessions are created and limit is exceeded.
         """
         return self._cleanup_service.cleanup_excess_sessions()
 
@@ -147,6 +141,16 @@ def _cleanup_old_messages(self, session_id: str) -> int:
             )
             return len(old_records)
 
+    def _is_new_session(self, session_id: str) -> bool:
+        """Check if this is a new session (no existing records)."""
+        with Session(self.engine) as session:
+            existing_record = session.exec(
+                select(ConversationRecord).where(
+                    ConversationRecord.session_id == session_id
+                ).limit(1)
+            ).first()
+            return existing_record is None
+
     async def save_conversation(
         self, session_id: str, source: str, input_data: str, output: str
     ) -> str:
@@ -159,6 +163,9 @@ async def save_conversation(
             f"for session {session_id}, source {source} at {timestamp}"
         )
 
+        # Check if this is a new session before saving
+        is_new_session = self._is_new_session(session_id)
+
         # Create new record
         record = ConversationRecord(
             id=record_id,
@@ -175,8 +182,10 @@ async def save_conversation(
 
         logger.info("✅ Successfully inserted record into conversation_history table")
 
-        # Daily session LRU cleanup: maintain max 2000 sessions (runs once per day)
-        self._cleanup_excess_sessions()
+        # Session LRU cleanup: only run when a new session is created
+        if is_new_session:
+            logger.info(f"🆕 New session detected: {session_id}, running LRU cleanup")
+            self._cleanup_excess_sessions()
 
         # Per-session FIFO cleanup: maintain max 20 records per session (runs on every save)
         self._cleanup_old_messages(session_id)