Address code review suggestions for PR #2243: Add timestamp validation, logging, configurable time decay, and edge case testing

Co-Authored-By: Joe Moura <joao@crewai.com>

src/crewai/memory/short_term/short_term_memory.py

@@ -1,3 +1,4 @@
+from datetime import datetime
 from typing import Any, Dict, Optional
 
 from pydantic import PrivateAttr
@@ -52,7 +53,26 @@ class ShortTermMemory(Memory):
         metadata: Optional[Dict[str, Any]] = None,
         agent: Optional[str] = None,
     ) -> None:
+        """
+        Save a memory item to the storage.
+        
+        Args:
+            value: The data to save.
+            metadata: Optional metadata to associate with the memory.
+            agent: Optional agent identifier.
+            
+        Raises:
+            ValueError: If the item's timestamp is in the future.
+        """
+        import logging
+        
         item = ShortTermMemoryItem(data=value, metadata=metadata, agent=agent)
+        
+        if item.timestamp > datetime.now():
+            raise ValueError("Cannot save memory item with future timestamp")
+            
+        logging.debug(f"Saving memory item with timestamp: {item.timestamp}")
+        
         if self._memory_provider == "mem0":
             item.data = f"Remember the following insights from Agent run: {item.data}"
 

src/crewai/memory/short_term/short_term_memory_item.py

@@ -10,6 +10,8 @@ class ShortTermMemoryItem:
         metadata: Optional[Dict[str, Any]] = None,
         timestamp: Optional[datetime] = None,
     ):
+        if timestamp is not None and timestamp > datetime.now():
+            raise ValueError("Timestamp cannot be in the future")
         self.data = data
         self.agent = agent
         self.metadata = metadata if metadata is not None else {}

src/crewai/memory/storage/rag_storage.py

@@ -115,7 +115,25 @@ class RAGStorage(BaseRAGStorage):
         filter: Optional[dict] = None,
         score_threshold: float = 0.35,
         recency_weight: float = 0.3,
+        time_decay_days: float = 1.0,
     ) -> List[Any]:
+        """
+        Search for entries in the storage based on semantic similarity and recency.
+        
+        Args:
+            query: The search query string.
+            limit: Maximum number of results to return.
+            filter: Optional filter to apply to the search.
+            score_threshold: Minimum score threshold for results.
+            recency_weight: Weight given to recency vs. semantic similarity (0.0-1.0).
+                Higher values prioritize recent memories more strongly.
+            time_decay_days: Number of days over which recency factor decays to zero.
+                Smaller values make older memories lose relevance faster.
+        
+        Returns:
+            List of search results, each containing id, metadata, context, and score.
+            Results are sorted by combined semantic similarity and recency score.
+        """
         if not hasattr(self, "app"):
             self._initialize_app()
 
@@ -140,7 +158,7 @@ class RAGStorage(BaseRAGStorage):
                         now = datetime.now()
                         # Calculate recency factor (newer = higher score)
                         time_diff_seconds = (now - timestamp).total_seconds()
-                        recency_factor = max(0, 1 - (time_diff_seconds / (24 * 60 * 60)))  # Normalize to 1 day
+                        recency_factor = max(0, 1 - (time_diff_seconds / (time_decay_days * 24 * 60 * 60)))
                         # Adjust score with recency factor
                         result["score"] = result["score"] * (1 - recency_weight) + recency_factor * recency_weight
                     except (ValueError, TypeError):

tests/memory/test_memory_topic_changes.py

@@ -85,3 +85,46 @@ def test_memory_prioritizes_recent_topic(short_term_memory):
             
             # Verify that the scores reflect the recency prioritization
             assert results[0]["score"] > results[1]["score"], "Recent topic should have higher score"
+
+
+def test_future_timestamp_validation():
+    """Test that ShortTermMemoryItem raises ValueError for future timestamps."""
+    # Setup agent and task for memory
+    agent = Agent(
+        role="Tutor",
+        goal="Teach programming concepts",
+        backstory="You are a programming tutor helping students learn.",
+        tools=[],
+        verbose=True,
+    )
+    
+    task = Task(
+        description="Explain programming concepts to students.",
+        expected_output="Clear explanations of programming concepts.",
+        agent=agent,
+    )
+    
+    # Create a future timestamp
+    future_timestamp = datetime.now() + timedelta(days=1)
+    
+    # Test constructor validation
+    with pytest.raises(ValueError, match="Timestamp cannot be in the future"):
+        ShortTermMemoryItem(data="Test data", timestamp=future_timestamp)
+    
+    # Test save method validation
+    memory = ShortTermMemory(crew=Crew(agents=[agent], tasks=[task]))
+    
+    # Create a memory item with a future timestamp
+    future_data = "Test data with future timestamp"
+    
+    # We need to pass the data directly to the save method
+    # The save method will create a ShortTermMemoryItem internally
+    # and then we'll modify its timestamp before it's saved
+    
+    # Mock datetime.now to return a fixed time
+    with patch('crewai.memory.short_term.short_term_memory_item.datetime') as mock_datetime:
+        # Set up the mock to return our future timestamp when now() is called
+        mock_datetime.now.return_value = future_timestamp
+        
+        with pytest.raises(ValueError, match="Cannot save memory item with future timestamp"):
+            memory.save(value=future_data)