refactor: resolve PR review comments on memory editing functionality

- Replace inefficient model_dump() with model_copy() for updates
- Improve datetime parsing to handle all ISO 8601 timezone formats
- Refactor verbose field setting to use dictionary-first approach
- Extract hash regeneration logic into reusable helper function

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>

Author: abrookins

agent_memory_server/long_term_memory.py

@@ -278,6 +278,30 @@ def generate_memory_hash(memory: MemoryRecord) -> str:
     return hashlib.sha256(content_json.encode()).hexdigest()
 
 
+def update_memory_hash_if_text_changed(memory: MemoryRecord, updates: dict) -> dict:
+    """
+    Helper function to regenerate memory hash if text field was updated.
+
+    This avoids code duplication of the hash regeneration logic across
+    different update flows (like memory creation, merging, and editing).
+
+    Args:
+        memory: The original memory record
+        updates: Dictionary of updates to apply
+
+    Returns:
+        Dictionary with updated memory_hash added if text was in the updates
+    """
+    result_updates = dict(updates)
+
+    # If text was updated, regenerate the hash
+    if "text" in updates:
+        temp_memory = memory.model_copy(update=updates)
+        result_updates["memory_hash"] = generate_memory_hash(temp_memory)
+
+    return result_updates
+
+
 async def merge_memories_with_llm(
     memories: list[MemoryRecord], llm_client: Any = None
 ) -> MemoryRecord:
@@ -1574,17 +1598,10 @@ async def update_long_term_memory(
             f"Cannot update fields: {invalid_fields}. Valid fields: {updatable_fields}"
         )
 
-    # Create updated memory record
-    updated_data = existing_memory.model_dump()
-    updated_data.update(updates)
-    updated_data["updated_at"] = datetime.now(UTC)
-
-    # If text was updated, regenerate the hash
-    if "text" in updates:
-        temp_memory = MemoryRecord(**updated_data)
-        updated_data["memory_hash"] = generate_memory_hash(temp_memory)
-
-    updated_memory = MemoryRecord(**updated_data)
+    # Create updated memory record using efficient model_copy and hash helper
+    base_updates = {**updates, "updated_at": datetime.now(UTC)}
+    update_dict = update_memory_hash_if_text_changed(existing_memory, base_updates)
+    updated_memory = existing_memory.model_copy(update=update_dict)
 
     # Update in the vectorstore
     adapter = await get_vectorstore_adapter()

agent_memory_server/mcp.py

@@ -1,4 +1,5 @@
 import logging
+from datetime import datetime
 from typing import Any
 
 import ulid
@@ -47,6 +48,29 @@
 logger = logging.getLogger(__name__)
 
 
+def _parse_iso8601_datetime(event_date: str) -> datetime:
+    """
+    Parse ISO 8601 datetime string with robust handling of different timezone formats.
+
+    Args:
+        event_date: ISO 8601 formatted datetime string
+
+    Returns:
+        Parsed datetime object
+
+    Raises:
+        ValueError: If the datetime format is invalid
+    """
+    try:
+        # Handle 'Z' suffix (UTC indicator)
+        if event_date.endswith("Z"):
+            return datetime.fromisoformat(event_date.replace("Z", "+00:00"))
+        # Let fromisoformat handle other timezone formats like +05:00, -08:00, etc.
+        return datetime.fromisoformat(event_date)
+    except ValueError as e:
+        raise ValueError(f"Invalid ISO 8601 datetime format '{event_date}': {e}") from e
+
+
 class FastMCP(_FastMCPBase):
     """Extend FastMCP to support optional URL namespace and default STDIO namespace."""
 
@@ -948,27 +972,22 @@ async def edit_long_term_memory(
     )
     ```
     """
-    # Build the update request, converting event_date string to datetime if provided
-    updates = EditMemoryRecordRequest()
-
-    if text is not None:
-        updates.text = text
-    if topics is not None:
-        updates.topics = topics
-    if entities is not None:
-        updates.entities = entities
-    if memory_type is not None:
-        updates.memory_type = memory_type
-    if namespace is not None:
-        updates.namespace = namespace
-    if user_id is not None:
-        updates.user_id = user_id
-    if session_id is not None:
-        updates.session_id = session_id
-    if event_date is not None:
-        from datetime import datetime
-
-        # Parse ISO 8601 datetime string
-        updates.event_date = datetime.fromisoformat(event_date.replace("Z", "+00:00"))
+    # Build the update request dictionary, handling event_date parsing
+    update_dict = {
+        "text": text,
+        "topics": topics,
+        "entities": entities,
+        "memory_type": memory_type,
+        "namespace": namespace,
+        "user_id": user_id,
+        "session_id": session_id,
+        "event_date": (
+            _parse_iso8601_datetime(event_date) if event_date is not None else None
+        ),
+    }
+
+    # Filter out None values to only include fields that should be updated
+    update_dict = {k: v for k, v in update_dict.items() if v is not None}
+    updates = EditMemoryRecordRequest(**update_dict)
 
     return await core_update_long_term_memory(memory_id=memory_id, updates=updates)