fix: address PR review feedback

- Auto-normalize plain string metadata values to {"stringValue": ...}
  so users can write {"project": "atlas"} instead of the verbose form.
  Applied via pydantic validator on default_metadata and at runtime for
  metadata_provider return values.
- Move inline datetime imports to top of test file (nit from Hweinstock)
- Fix lint/format issues that caused CI Lint and Format check to fail
- Add tests for normalization in both config and session manager

Author: tejaskash

src/bedrock_agentcore/memory/integrations/strands/README.md

@@ -257,15 +257,18 @@ config = AgentCoreMemoryConfig(
     session_id=SESSION_ID,
     actor_id=ACTOR_ID,
     default_metadata={
-        "project": {"stringValue": "atlas"},
-        "env": {"stringValue": "production"},
+        "project": "atlas",
+        "env": "production",
     },
 )
 session_manager = AgentCoreMemorySessionManager(config, region_name='us-east-1')
 agent = Agent(session_manager=session_manager)
 agent("Hello!")  # This event will have project=atlas and env=production metadata
 ```
 
+> Plain strings are auto-wrapped to `{"stringValue": "..."}`. The explicit form
+> `{"project": {"stringValue": "atlas"}}` also works.
+
 ### Dynamic Metadata (metadata_provider)
 
 For values that change per invocation (e.g., traceId for Langfuse), use `metadata_provider` —
@@ -275,7 +278,7 @@ a callable invoked at each event creation:
 from langfuse.decorators import langfuse_context
 
 def get_trace_metadata():
-    return {"traceId": {"stringValue": langfuse_context.get_current_trace_id() or ""}}
+    return {"traceId": langfuse_context.get_current_trace_id() or ""}
 
 config = AgentCoreMemoryConfig(
     memory_id=MEM_ID,
@@ -296,7 +299,7 @@ with `default_metadata` and `metadata_provider` (per-call values override both f
 ```python
 session_manager.create_message(
     session_id, agent_id, message,
-    metadata={"priority": {"stringValue": "high"}},
+    metadata={"priority": "high"},
 )
 ```
 

src/bedrock_agentcore/memory/integrations/strands/config.py

@@ -2,7 +2,12 @@
 
 from typing import Any, Callable, Dict, Optional
 
-from pydantic import BaseModel, ConfigDict, Field
+from pydantic import BaseModel, ConfigDict, Field, field_validator
+
+
+def normalize_metadata(raw: Dict[str, Any]) -> Dict[str, Any]:
+    """Normalize metadata values: plain strings become {"stringValue": value}."""
+    return {k: {"stringValue": v} if isinstance(v, str) else v for k, v in raw.items()}
 
 
 class RetrievalConfig(BaseModel):
@@ -40,10 +45,12 @@ class AgentCoreMemoryConfig(BaseModel):
             restored messages before loading them into Strands runtime memory. Default is False.
         default_metadata: Optional default metadata key-value pairs to attach to every message event.
             Merged with any per-call metadata. Maximum 15 total keys per event (including internal keys).
-            Example: {"location": {"stringValue": "NYC"}}
+            Accepts plain strings (auto-wrapped) or explicit MetadataValue dicts.
+            Example: {"location": "NYC"} or {"location": {"stringValue": "NYC"}}
         metadata_provider: Optional callable that returns metadata key-value pairs. Called at each
             event creation, so it can return dynamic values (e.g. current traceId). The returned
             dict is merged after default_metadata but before per-call metadata.
+            Accepts plain strings (auto-wrapped) or explicit MetadataValue dicts.
     """
 
     model_config = ConfigDict(arbitrary_types_allowed=True)
@@ -58,3 +65,10 @@ class AgentCoreMemoryConfig(BaseModel):
     filter_restored_tool_context: bool = Field(default=False)
     default_metadata: Optional[Dict[str, Any]] = None
     metadata_provider: Optional[Callable[[], Dict[str, Any]]] = None
+
+    @field_validator("default_metadata", mode="before")
+    @classmethod
+    def _normalize_default_metadata(cls, v: Optional[Dict[str, Any]]) -> Optional[Dict[str, Any]]:
+        if v is None:
+            return None
+        return normalize_metadata(v)

src/bedrock_agentcore/memory/integrations/strands/session_manager.py

@@ -29,7 +29,7 @@
 )
 
 from .bedrock_converter import AgentCoreMemoryConverter
-from .config import AgentCoreMemoryConfig, RetrievalConfig
+from .config import AgentCoreMemoryConfig, RetrievalConfig, normalize_metadata
 from .converters import MemoryConverter
 
 if TYPE_CHECKING:
@@ -215,7 +215,7 @@ def _build_metadata(
             merged.update(self.config.default_metadata)
 
         if self.config.metadata_provider:
-            merged.update(self.config.metadata_provider())
+            merged.update(normalize_metadata(self.config.metadata_provider()))
 
         if per_call_metadata:
             merged.update(per_call_metadata)
@@ -224,17 +224,14 @@ def _build_metadata(
         user_reserved = RESERVED_METADATA_KEYS & merged.keys()
         if user_reserved:
             raise ValueError(
-                f"Metadata keys {user_reserved} are reserved for internal use. "
-                f"Reserved keys: {RESERVED_METADATA_KEYS}"
+                f"Metadata keys {user_reserved} are reserved for internal use. Reserved keys: {RESERVED_METADATA_KEYS}"
             )
 
         if internal_metadata:
             merged.update(internal_metadata)
 
         if len(merged) > MAX_METADATA_KEYS:
-            raise ValueError(
-                f"Combined metadata has {len(merged)} keys, exceeding the maximum of {MAX_METADATA_KEYS}."
-            )
+            raise ValueError(f"Combined metadata has {len(merged)} keys, exceeding the maximum of {MAX_METADATA_KEYS}.")
 
         return merged or None
 

tests/bedrock_agentcore/memory/integrations/strands/test_agentcore_memory_config.py

@@ -87,3 +87,39 @@ def test_with_retrieval_config(self):
             memory_id="mem-123", session_id="sess-456", actor_id="actor-789", retrieval_config={"namespace1": retrieval}
         )
         assert config.retrieval_config["namespace1"].top_k == 5
+
+    def test_default_metadata_plain_strings_normalized(self):
+        """Plain string values are auto-wrapped to {"stringValue": ...}."""
+        config = AgentCoreMemoryConfig(
+            memory_id="mem-123",
+            session_id="sess-456",
+            actor_id="actor-789",
+            default_metadata={"project": "atlas", "env": "prod"},
+        )
+        assert config.default_metadata == {
+            "project": {"stringValue": "atlas"},
+            "env": {"stringValue": "prod"},
+        }
+
+    def test_default_metadata_explicit_format_unchanged(self):
+        """Explicit {"stringValue": ...} dicts pass through unchanged."""
+        config = AgentCoreMemoryConfig(
+            memory_id="mem-123",
+            session_id="sess-456",
+            actor_id="actor-789",
+            default_metadata={"project": {"stringValue": "atlas"}},
+        )
+        assert config.default_metadata == {"project": {"stringValue": "atlas"}}
+
+    def test_default_metadata_mixed_formats(self):
+        """Mixed plain strings and explicit dicts both work."""
+        config = AgentCoreMemoryConfig(
+            memory_id="mem-123",
+            session_id="sess-456",
+            actor_id="actor-789",
+            default_metadata={"project": "atlas", "env": {"stringValue": "prod"}},
+        )
+        assert config.default_metadata == {
+            "project": {"stringValue": "atlas"},
+            "env": {"stringValue": "prod"},
+        }

tests/bedrock_agentcore/memory/integrations/strands/test_agentcore_memory_session_manager.py

@@ -2,6 +2,7 @@
 
 import logging
 import time
+from datetime import datetime, timezone
 from unittest.mock import Mock, patch
 
 import pytest
@@ -1680,8 +1681,6 @@ def test__flush_messages_multiple_sessions_grouped_into_separate_api_calls(
         so all messages go to one session. This test verifies the internal grouping logic
         by directly manipulating the buffer.
         """
-        from datetime import datetime, timezone
-
         calls_by_session = {}
 
         def track_create_event(**kwargs):
@@ -1749,8 +1748,6 @@ def track_create_event(**kwargs):
         # The combined event should use the latest timestamp (12:10:00)
         assert len(captured_timestamps) == 1
         # The timestamp should be the latest one (12:10:00)
-        from datetime import datetime, timezone
-
         expected_latest = datetime(2024, 1, 1, 12, 10, 0, tzinfo=timezone.utc)
         # Account for monotonic timestamp adjustment (may add microseconds)
         assert captured_timestamps[0] >= expected_latest
@@ -1762,7 +1759,6 @@ def test__flush_messages_partial_failure_multiple_sessions_preserves_buffer(
 
         Note: Tests internal grouping logic by directly manipulating buffer.
         """
-        from datetime import datetime, timezone
 
         def fail_on_second_session(**kwargs):
             session_id = kwargs.get("sessionId")
@@ -1830,8 +1826,6 @@ def test_mixed_sessions_with_blobs_and_conversational(self, batching_session_man
 
         Note: Tests internal grouping logic by directly manipulating buffer.
         """
-        from datetime import datetime, timezone
-
         calls_by_session = {}
 
         def track_create_event(**kwargs):
@@ -2543,7 +2537,12 @@ def test_after_invocation_hook_flushes_buffer(self, batching_session_manager, mo
         # Add messages to buffer
         with batching_session_manager._message_lock:
             batching_session_manager._message_buffer.append(
-                BufferedMessage("test-session", [("user", "test message")], False, batching_session_manager._get_monotonic_timestamp())
+                BufferedMessage(
+                    "test-session",
+                    [("user", "test message")],
+                    False,
+                    batching_session_manager._get_monotonic_timestamp(),
+                )
             )
 
         assert batching_session_manager.pending_message_count() == 1
@@ -2766,7 +2765,9 @@ def test_interval_flush_callback_flushes_when_buffer_has_messages(self):
             # Add messages to buffer
             with manager._message_lock:
                 manager._message_buffer.append(
-                    BufferedMessage("test-session", [("user", "test message")], False, manager._get_monotonic_timestamp())
+                    BufferedMessage(
+                        "test-session", [("user", "test message")], False, manager._get_monotonic_timestamp()
+                    )
                 )
 
             assert manager.pending_message_count() == 1
@@ -2921,7 +2922,9 @@ def test_interval_flush_callback_flushes_when_both_buffers_have_data(self):
             # Add both messages and agent state to buffers
             with manager._message_lock:
                 manager._message_buffer.append(
-                    BufferedMessage("test-session", [("user", "test message")], False, manager._get_monotonic_timestamp())
+                    BufferedMessage(
+                        "test-session", [("user", "test message")], False, manager._get_monotonic_timestamp()
+                    )
                 )
 
             from strands.types.session import SessionAgent
@@ -3079,8 +3082,6 @@ def test_create_message_no_metadata_passes_none(self, session_manager, mock_memo
 
     def test_batched_messages_include_metadata(self, mock_memory_client):
         """Metadata flows through the batching path and appears in the flushed event."""
-        from datetime import datetime, timezone
-
         config = AgentCoreMemoryConfig(
             memory_id="test-memory-123",
             session_id="test-session-456",
@@ -3199,3 +3200,37 @@ def test_metadata_provider_reserved_keys_rejected(self, mock_memory_client):
         msg = SessionMessage.from_message({"role": "user", "content": [{"text": "hello"}]}, 0)
         with pytest.raises(ValueError, match="reserved"):
             manager.create_message("test-session-456", "agent-1", msg)
+
+    def test_metadata_provider_plain_strings_normalized(self, mock_memory_client):
+        """metadata_provider returning plain strings gets auto-normalized."""
+        config = AgentCoreMemoryConfig(
+            memory_id="test-memory-123",
+            session_id="test-session-456",
+            actor_id="test-actor-789",
+            metadata_provider=lambda: {"traceId": "trace-abc"},
+        )
+        manager = _create_session_manager(config, mock_memory_client)
+        mock_memory_client.create_event.return_value = {"eventId": "evt_1"}
+
+        msg = SessionMessage.from_message({"role": "user", "content": [{"text": "hello"}]}, 0)
+        manager.create_message("test-session-456", "agent-1", msg)
+
+        kwargs = mock_memory_client.create_event.call_args[1]
+        assert kwargs["metadata"]["traceId"] == {"stringValue": "trace-abc"}
+
+    def test_default_metadata_plain_strings_normalized(self, mock_memory_client):
+        """default_metadata with plain strings gets auto-normalized at config time."""
+        config = AgentCoreMemoryConfig(
+            memory_id="test-memory-123",
+            session_id="test-session-456",
+            actor_id="test-actor-789",
+            default_metadata={"project": "atlas"},
+        )
+        manager = _create_session_manager(config, mock_memory_client)
+        mock_memory_client.create_event.return_value = {"eventId": "evt_1"}
+
+        msg = SessionMessage.from_message({"role": "user", "content": [{"text": "hello"}]}, 0)
+        manager.create_message("test-session-456", "agent-1", msg)
+
+        kwargs = mock_memory_client.create_event.call_args[1]
+        assert kwargs["metadata"]["project"] == {"stringValue": "atlas"}