-
Notifications
You must be signed in to change notification settings - Fork 505
Add dummy toolUse message when its missing due to pagination in session management #1274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add dummy toolUse message when its missing due to pagination in session management #1274
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| Fixed list of messages with proper tool use/result pairs | ||
| """ | ||
| # First, check if the first message has orphaned toolResult (no preceding toolUse) | ||
| if messages: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the bug report - Isn't the problem the opposite?
It's not that the latest message is broken; it's that the oldest message is broken - that's because we have N messages:
- 55
- 54
- 53
- 52
- 51
- 50
- 49
- 48
And 48 is a toolResult with 47 being toolUse, but it's not included
In this case messages[0] is 55, while messages[-1] == 48
And I think the fix in this case is to just not include 48
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So here's a full example:
from strands import Agent, tool
@tool
def get_user_location() -> str:
"""Get the user's location."""
return "Seattle, USA"
MEM_ID='ComprehensiveAgentMemory_sep16-YDe9F76vbr'
ACTOR_ID='actor_id_test_20251201135435'
SESSION_ID='testing_session_id_20251201135435'
config = AgentCoreMemoryConfig(
memory_id=MEM_ID,
session_id=SESSION_ID,
actor_id=ACTOR_ID,
)
session_manager = AgentCoreMemorySessionManager(config, region_name='us-east-1')
agent_1 = Agent(session_manager=session_manager, tools=[get_user_location])Let's view the agent_1 messages:
agent_1.messages
[{'role': 'assistant',
'content': [{'toolUse': {'toolUseId': 'tooluse_f09Y0LwyT2yteCYshTzb_Q',
'name': 'unknown_tool',
'input': {'error': 'Tool use was truncated due to message limit.'}}}]},
{'role': 'user',
'content': [{'toolResult': {'toolUseId': 'tooluse_f09Y0LwyT2yteCYshTzb_Q',
'status': 'success',
'content': [{'text': 'Seattle, USA'}]}}]},
{'role': 'assistant', 'content': [{'text': 'You live in Seattle, USA.'}]},
{'role': 'user', 'content': [{'text': 'I like pizza'}]},
{'role': 'user', 'content': [{'text': 'where do I live?'}]},
{'role': 'user', 'content': [{'text': 'where do I live?'}]},
{'role': 'assistant',
'content': [{'text': '\n\nI can see you mentioned that you like pizza again! As I mentioned before, you live in **Seattle, USA**. \n\nSeattle has a fantastic pizza scene! Are you looking for pizza recommendations in your area, or did you have a specific question about pizza places in Seattle?'}]},
{'role': 'user', 'content': [{'text': 'I like pizza'}]}]
The session manager is replaying the conversation so the -1 message is the newest one (I like pizza) whereas the closer to 0 the further back we go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right; I was thinking of it backwards.
Still, can we remove the first message instead creating a fake one? IMHO it's better to trim than to synthesize
(and if you do ^, can we update the comment to indicate "check if the oldest message")
Description
This pull request improves the robustness of session message history handling by ensuring that orphaned tool use and tool result messages are properly paired, preventing broken conversation states. It introduces a new helper for generating missing tool use content, updates the session manager to handle both types of orphaned messages, and adds comprehensive unit tests for these helpers.
Session message history fixes:
_fix_broken_tool_useinrepository_session_manager.pyto handle both orphaned toolUse messages (no toolResult) and orphaned toolResult messages (no toolUse), ensuring valid message history even after pagination or legacy bugs.generate_missing_tool_use_contenthelper to retroactively insert dummy toolUse messages when needed.Tool helper enhancements:
generate_missing_tool_use_contentto_tool_helpers.py, which generates dummy toolUse content blocks for orphaned toolResult messages.Testing improvements:
test_tool_helpers.pywith thorough tests for bothgenerate_missing_tool_result_contentand the newgenerate_missing_tool_use_content, covering single, multiple, empty, and realistic ID scenarios.Related Issues
#1272
Documentation PR
Type of Change
Bug fix
Testing
How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.