-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/prompt caching #17
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |
| from strands.handlers.callback_handler import PrintingCallbackHandler, null_callback_handler | ||
| from strands.hooks import BeforeToolCallEvent | ||
| from strands.interrupt import Interrupt | ||
| from strands.models import CacheConfig | ||
| from strands.models.bedrock import DEFAULT_BEDROCK_MODEL_ID, BedrockModel | ||
| from strands.session.repository_session_manager import RepositorySessionManager | ||
| from strands.telemetry.tracer import serialize | ||
|
|
@@ -2182,3 +2183,44 @@ def test_agent_skips_fix_for_valid_conversation(mock_model, agenerator): | |
| # Should not have added any toolResult messages | ||
| # Only the new user message and assistant response should be added | ||
| assert len(agent.messages) == original_length + 2 | ||
|
|
||
|
|
||
| def test_cache_config_does_not_mutate_original_messages(mock_model, agenerator): | ||
| """Test that cache_config injection does not mutate the original agent.messages.""" | ||
| mock_model.mock_stream.return_value = agenerator( | ||
| [ | ||
| {"messageStart": {"role": "assistant"}}, | ||
| {"contentBlockStart": {"start": {"text": ""}}}, | ||
| {"contentBlockDelta": {"delta": {"text": "Response"}}}, | ||
| {"contentBlockStop": {}}, | ||
| {"messageStop": {"stopReason": "end_turn"}}, | ||
| ] | ||
| ) | ||
|
|
||
| # Simulate a mock BedrockModel with cache_config | ||
| mock_model.get_config = unittest.mock.MagicMock( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Issue: This test uses Suggestion: Move this test to def test_format_bedrock_messages_does_not_mutate_original(bedrock_client):
"""Test that _format_bedrock_messages doesn't mutate the original messages."""
model = BedrockModel(
model_id="us.anthropic.claude-sonnet-4-20250514-v1:0",
cache_config=CacheConfig(strategy="auto")
)
messages = [
{"role": "user", "content": [{"text": "Hello"}]},
{"role": "assistant", "content": [{"text": "Hi there!"}]},
]
original_content_len = len(messages[1]["content"])
model._format_bedrock_messages(messages)
# Original messages should be unchanged
assert len(messages[1]["content"]) == original_content_len |
||
| return_value={"cache_config": CacheConfig(strategy="auto"), "model_id": "us.anthropic.claude-sonnet-4-v1:0"} | ||
| ) | ||
|
|
||
| # Initial messages with assistant response (no cache point) | ||
| initial_messages = [ | ||
| {"role": "user", "content": [{"text": "Hello"}]}, | ||
| {"role": "assistant", "content": [{"text": "Hi there!"}]}, | ||
| ] | ||
|
|
||
| agent = Agent(model=mock_model, messages=copy.deepcopy(initial_messages)) | ||
|
|
||
| # Store deep copy of messages before invocation | ||
| messages_before = copy.deepcopy(agent.messages) | ||
|
|
||
| # Invoke agent | ||
| agent("Follow up question") | ||
|
|
||
| # Check that original assistant message content was not mutated with cachePoint | ||
| # The assistant message at index 1 should still only have the text block | ||
| original_assistant_content = messages_before[1]["content"] | ||
| current_assistant_content = agent.messages[1]["content"] | ||
|
|
||
| # Both should have the same structure (no cache point added to agent.messages) | ||
| assert len(original_assistant_content) == len(current_assistant_content) | ||
| assert "cachePoint" not in current_assistant_content[-1] | ||
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.
Issue:
_inject_cache_pointmutates the original messages list in-place, which can lead to unexpected side effects onagent.messages.Suggestion: Make a deep copy of the messages before injecting cache points:
This ensures that
agent.messagesremains unmodified and cache points are only in the request payload sent to Bedrock.