This is an important change, thanks for this.

🎯 Review - Automatic Prompt Caching

Excellent implementation of automatic prompt caching for Bedrock! This addresses #1432 nicely and will provide significant performance and cost benefits for multi-turn conversations.

What I Really Like ✅

1. Smart Cache Point Strategy: The _inject_cache_point() logic is elegant - automatically moving the cache point to the last assistant message ensures optimal cache utilization without manual management.

2. Comprehensive Tests: 155 lines of new tests in test_bedrock.py covering edge cases like:

   • Cache point injection
   • Cache point movement across turns
   • Cleanup of stale cache points

3. Backward Compatibility: Deprecating cache_prompt while still supporting it shows good API stewardship.

4. Documentation: Clear docstrings and usage examples in the PR description.

Minor Suggestions 💡

1. Cache Point Detection Could Be More Explicit

In _inject_cache_point() around line 344, the logic for detecting an existing cache point at the right position relies on the loop continuing. Consider making this more explicit:

# Check if cache point was already found at the right position
last_assistant_content = messages[last_assistant_idx]["content"]
if last_assistant_content and "cachePoint" in last_assistant_content[-1]:
    logger.debug(f"Cache point already exists at end of last assistant message {last_assistant_idx}")
    return

# Add cache point at the end of the last assistant message
last_assistant_content.append({"cachePoint": {"type": "default"}})
logger.debug(f"Added cache point at end of assistant message {last_assistant_idx}")

2. Model Support Detection

The supports_caching property checks for "claude" or "anthropic" in the model ID. Consider if future Bedrock models might support caching:

@property
def supports_caching(self) -> bool:
    """Whether this model supports prompt caching.
    
    Returns True for Claude models on Bedrock that support caching.
    Add other models as they become available.
    """
    model_id = self.config.get("model_id", "").lower()
    # Claude 3 Opus, Sonnet, and Haiku on Bedrock support caching
    # Claude 3.5 Sonnet and later also support caching
    return "claude" in model_id or "anthropic" in model_id

3. Integration Test Clarity

The integration tests are great! One suggestion - add a comment explaining the cache hit verification:

# After second call, verify cache hit (cache_read_input_tokens > 0)
# This confirms the cache point strategy is working
assert result.metadata["converse_metrics"]["cache_read_input_tokens"] > 0

Questions for Discussion 🤔

1. Cache Invalidation: What happens if the system prompt or tools change between calls? Does the cache automatically invalidate, or should there be explicit cache busting?

2. Multiple Cache Points: The current strategy uses a single cache point. Are there scenarios where multiple cache points (e.g., one for system prompt, one for conversation) would be beneficial?

3. Performance Metrics: Would it be valuable to expose cache hit/miss metrics in the AgentResult.metadata for users to monitor cache effectiveness?

CI Status

I see CI is still pending. Once it passes, this looks ready for maintainer review!

Overall Assessment

This is a high-quality PR that will be valuable for the community. The automatic cache management removes complexity from users while providing real performance benefits. Great work, @kevmyung! 🎉

🦆

🤖 This is an experimental AI agent response from the Strands team, powered by Strands Agents. We're exploring how AI agents can help with community support and development. Your feedback helps us improve! If you'd prefer human assistance, please let us know.

Codecov Report

❌ Patch coverage is 93.75000% with 2 lines in your changes missing coverage. Please review.

📢 Thoughts on this report? Let us know!