Skip to content

Conversation

@OpenGaet
Copy link
Contributor

Issue #, if available:

Description of changes:

Added Async LLM Callback Support

New Features:
• Added process_turn_with_llm_async() method to MemorySessionManager and MemorySession classes
• Supports async LLM callbacks that return Awaitable[str]
• Maintains full backwards compatibility with existing sync callbacks

Code Improvements:
• Extracted common logic into helper methods (_retrieve_memories_for_llm() and _save_conversation_turn())
• Added test coverage for async functionality

Files Modified:
• src/bedrock_agentcore/memory/session.py (+122/-16 lines)
• tests/bedrock_agentcore/memory/test_session.py (+35 lines)

Usage:

# Async callback support
await manager.process_turn_with_llm_async(                                                                                                                                                                         
    actor_id="user-123",                                                                                                                                                                                           
    session_id="session-456",                                                                                                                                                                                      
    user_input="Hello",                                                                                                                                                                                            
    llm_callback=async_callback,  # async function                                                                                                                                                                 
    retrieval_config=config                                                                                                                                                                                        
)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

) -> List[Dict[str, Any]]:
"""Helper method to retrieve memories for LLM context."""
retrieved_memories = []
if retrieval_config:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if instead, we make it so _retrieve_memories_for_llm requires the retrieval_config but then this if retrieval_config check happens where we call _retrieve_memories_for_llm? I think that would be a less accident prone API since then there is no way the user tries to call _retrieve_memories_for_llm without a retrieval config and is surprised that they get no memory records

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the existing logic, just moved to a separate function to reduce code duplication. On top of this, it is a "private" function, so the user should not be calling _retrieve_memories_for_llm directly. But feel free to make changes as desired.

@codecov-commenter
Copy link

codecov-commenter commented Oct 22, 2025

Codecov Report

❌ Patch coverage is 80.64516% with 6 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@f30e281). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/bedrock_agentcore/memory/session.py 80.64% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #131   +/-   ##
=======================================
  Coverage        ?   89.39%           
=======================================
  Files           ?       24           
  Lines           ?     2414           
  Branches        ?      342           
=======================================
  Hits            ?     2158           
  Misses          ?      151           
  Partials        ?      105           
Flag Coverage Δ
unittests 89.39% <80.64%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bergjaak bergjaak merged commit 1e3fd0c into aws:main Oct 22, 2025
13 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants