feat(memory): update session manager for Strands integration #170
Conversation
akshseh
temporarily deployed
to
manual-approval
GitHub Actions
Inactive
| class ShortTermRetrievalConfig(BaseModel): | ||
| """Configuration for Short term memory retrieval operations""" | ||
|
|
||
| branch_filter: Optional[bool] = True |
There was a problem hiding this comment.
Is there a plan to have branching on LTM? We can refactor later but then we should consider lifting the branch_filter into a base class
| default_branch: Optional[BranchConfig] = Field( | ||
| default_factory=lambda: BranchConfig(name="main", root_event_id="") | ||
| ) | ||
| short_term_retrieval_config: Optional[ShortTermRetrievalConfig] = ( |
There was a problem hiding this comment.
does this needs it's own variable? Can we allow for this to be set to the retrieval_config? Or do we anticipate users passing in both STM and LTM config for the same session?
| if self.config.metadata: | ||
| metadata.update(self.config.metadata) | ||
|
|
||
| if branch_name == "main": |
There was a problem hiding this comment.
since this is a special case let's make it a constant at the top
| created_event = self.memory_session_manager.create_event(**event_params) | ||
| else: | ||
| created_event = self.memory_session_manager.fork_conversation( | ||
| actor_id=self.config.actor_id, |
There was a problem hiding this comment.
is it necessary to pass in all these self parameters? This method signature is very bulky. Let's use the self data from inside that fork_conversation method
| def create_session(self, session: Session, **kwargs: Any) -> Session: | ||
| """Create a new session.""" | ||
| self._validate_session_id(session.session_id) | ||
| return session | ||
|
|
There was a problem hiding this comment.
where in agentcore is the session data stored?
| if payload and "blob" in payload[0]: | ||
| blob_data = json.loads(payload[0]["blob"]) | ||
| # Extract SessionAgent from blob. | ||
| session_data = blob_data.get("session_state") or blob_data.get( |
There was a problem hiding this comment.
what's with this or here? are we storing it either as a session_state or a session object?
There was a problem hiding this comment.
will remove session
| if role in self.config.message_types: | ||
| content = message.get("content", [{}])[0].get("text", "") | ||
| mapped_role = role_map.get(role, MessageRole.ASSISTANT) | ||
| if role == "user": |
There was a problem hiding this comment.
we can use the mapped_role here
| registry (HookRegistry): The hook registry to register callbacks with. | ||
| **kwargs: Additional keyword arguments. | ||
| """ | ||
| RepositorySessionManager.register_hooks(self, registry, **kwargs) |
There was a problem hiding this comment.
just to be clear, this line removes all the hooks that are expected as part of the session management approach in strands
| messages[-1]["content"][0][ | ||
| "text" | ||
| ] = f"<previous_context>\n{context_text}\n</previous_context>\n\n{original_text}\n" | ||
| event.agent.messages[-1]["content"][0]["text"] = context_text |
There was a problem hiding this comment.
what's the intent behind this why we are modifying the message directly? This will not get saved to LTM as the message added hook will not be triggered. Is this the expected behavior?
| except Exception as e: | ||
| logger.error("Failed to list messages from AgentCore Memory: %s", e) |
There was a problem hiding this comment.
I'm starting to see this error come up each time I initialize an agent now. Here is my code:
import uuid
import boto3
from datetime import date, datetime
from strands import Agent
from bedrock_agentcore.memory import MemoryClient
from bedrock_agentcore.memory.integrations.strands.config import AgentCoreMemoryConfig, RetrievalConfig
from bedrock_agentcore.memory.integrations.strands.session_manager import AgentCoreMemorySessionManager
MEM_ID = 'BasicTestMemory20251103223555-Mowu0s9pwM'
ACTOR_ID = "actor_id_test_%s" % datetime.now().strftime("%Y%m%d%H%M%S")
SESSION_ID = "testing_session_id_%s" % datetime.now().strftime("%Y%m%d%H%M%S")
config = AgentCoreMemoryConfig(
memory_id=MEM_ID,
session_id=SESSION_ID,
actor_id=ACTOR_ID
)
stm_session_manager = AgentCoreMemorySessionManager(config, region_name='us-east-1')
stm_agent_1 = Agent(session_manager=stm_session_manager)then
stm_agent_1("hello")
I see Failed to retrieve customer context: 'NoneType' object has no attribute 'items' printed out.
And then if I now initialize another agent with the same session manager I see:
stm_agent_2 = Agent(session_manager=stm_session_manager)
Failed to list messages from AgentCore Memory: Expecting value: line 1 column 1 (char 0)
2 participants
Issue #, if available: #149
Description of changes: Update Agentcore SessionManager for strands to use branching and metadata features of AgentCore Memory.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.