Skip to content

fix: use async with for learning context manager#1252

Merged
zzstoatzz merged 2 commits intomainfrom
fix/async-learning-context
Dec 9, 2025
Merged

fix: use async with for learning context manager#1252
zzstoatzz merged 2 commits intomainfrom
fix/async-learning-context

Conversation

@zzstoatzz
Copy link
Copy Markdown
Collaborator

Summary

The learning SDK's context manager needs to be used with async with when wrapping async code. Using sync with creates a sync client, but the async interceptor expects an async client for memory operations.

This was causing the learning SDK to silently fail - no memory was being injected or captured because the sync client doesn't work properly in an async context.

Change

# Before (broken)
with learning(agent=f"slackbot-{user_context['user_id']}"):
    result = await create_agent(...).run(...)

# After (fixed)  
async with learning(agent=f"slackbot-{user_context['user_id']}"):
    result = await create_agent(...).run(...)

Test plan

  • Deploy and verify memory persistence works across conversations

🤖 Generated with Claude Code

the learning SDK's context manager needs to be used with `async with`
when wrapping async code. using sync `with` creates a sync client but
the async interceptor expects an async client for memory operations.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings December 9, 2025 21:50
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a critical bug in the slackbot example where the learning context manager from the agentic-learning SDK was being used incorrectly with synchronous with instead of asynchronous async with. Since the wrapped code uses await to run async operations, the async context manager is required for proper memory injection and capture.

Key changes:

  • Changed with learning(...) to async with learning(...) in the slackbot's async agent run function
  • Ensures memory persistence works correctly across conversations by using the proper async client

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@zzstoatzz zzstoatzz merged commit 47da4ce into main Dec 9, 2025
3 of 4 checks passed
@zzstoatzz zzstoatzz deleted the fix/async-learning-context branch December 9, 2025 21:53
zzstoatzz added a commit that referenced this pull request Dec 10, 2025
Reverts commits:
- 47da4ce fix: use async with for learning context manager (#1252)
- 36da4c1 chore: consolidate slackbot memory on letta learning-sdk (#1250)
- 30aed2d feat: integrate learning-sdk into slackbot for persistent memory (#1249)

The learning SDK's async context manager has a bug where it tries to
await pending tasks in __aexit__ after the event loop has already
closed, causing "Event loop is closed" errors that crash the slackbot.

This restores the original TurboPuffer-based user facts system until
the upstream bug is fixed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
zzstoatzz added a commit that referenced this pull request Dec 10, 2025
Reverts commits:
- 47da4ce fix: use async with for learning context manager (#1252)
- 36da4c1 chore: consolidate slackbot memory on letta learning-sdk (#1250)
- 30aed2d feat: integrate learning-sdk into slackbot for persistent memory (#1249)

The learning SDK's async context manager has a bug where it tries to
await pending tasks in __aexit__ after the event loop has already
closed, causing "Event loop is closed" errors that crash the slackbot.

This restores the original TurboPuffer-based user facts system until
the upstream bug is fixed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants