-
Notifications
You must be signed in to change notification settings - Fork 183
# 🔥🕊️ Rise of the Phoenix: Event Loop Refactor #106
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
Conversation
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.
Hi @fede-dash, great to see you here! Thank you for your contribution ❤️
Would it be possible to separate out the formatting changes from this PR? We're using ruff with hatch fmt
for formatting and linting (https://github.com/strands-agents/sdk-python/blob/main/CONTRIBUTING.md) so it seems the black rules that were applied in this PR don't match with what we have configured in the project (https://github.com/strands-agents/sdk-python/blob/main/pyproject.toml).
The event_loop improvements and modularization are great, would love to get those merged.
Comments addressed |
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.
Thanks for the updates!
hatch run test-lint
is showing these errors:
cmd [1] | ruff check
All checks passed!
cmd [2] | mypy -p src
src/strands/event_loop/event_loop.py:213: error: Argument 1 to "EventLoopException" has incompatible type "str"; expected "Exception" [arg-type]
src/strands/event_loop/event_loop.py:224: error: Argument 6 to "_handle_tool_execution" has incompatible type "ToolConfig | None"; expected "ToolConfig" [arg-type]
Found 2 errors in 1 file (checked 51 source files)
Other than that, looks great. I gave it a test locally as well
lints passing now - will do that moving forward ❯ hatch run test-lint
cmd [1] | ruff check
All checks passed!
cmd [2] | mypy -p src
Success: no issues found in 51 source files |
Thank you @fede-dash 🚀 !! |
🔥🕊️ Always will be! |
* models - openai - argument none (#97) * docs(readme): add open PRs badge + link to samples repo + change 'Docs' to 'Documentation' (#100) * docs(readme): add logo (#101) * docs(readme): add logo, title, badges, links to other repos, standardize headings (#102) * style(readme): use dark logo for clearer visibility when system is using light color scheme (#104) * fix(readme): use logo that changes color automatically depending on user's color preference scheme (#105) * feat(handlers): add reasoning text to callback handler and related tests (#109) * feat(handlers): add reasoning text to callback handler and related tests * feat(handlers): removed redundant comment in .gitignore file * feat(handlers): Updated reasoningText type as (Optional[str] * feat: Add dynamic system prompt override functionality (#108) * Modularizing Event Loop (#106) * fix(telemetry): fix agent span start and end when using Agent.stream_async() (#119) * feat: Update SlidingWindowConversationManager (#120) * v0.1.5 --------- Co-authored-by: Patrick Gray <[email protected]> Co-authored-by: Gokhan (Joe) Gultekin <[email protected]> Co-authored-by: Shubham Raut <[email protected]> Co-authored-by: fede-dash <[email protected]> Co-authored-by: Nick Clegg <[email protected]>
🔨 Modularizing Event Loop Logic
Commit: 8cb8422
Author: Fede Kamelhar
Date: May 25, 2025
Message: Modularizing Event Loop
⸻
✨ What’s Included
Moved all logic related to handling tool_use into a dedicated private function: _handle_tool_execution().
Benefit:
Improves readability, simplifies event_loop_cycle(), and separates concerns for better maintainability and testing.
New function:
Introduced constants for retry settings, replacing hardcoded values:
Benefit:
Easier tuning of retry behavior and removal of magic numbers.
Refined function signatures for stronger typing:
• initialize_state() now accepts a Dict[str, Any] instead of **kwargs.
• callback_handler is now typed as Callable[..., Any].
Benefit:
Improves IDE support, readability, and static analysis.
⸻
Small Refactors & Cleanup
• Replaced inline retry configuration with named constants.
• Removed redundant state handling and improved control flow.
• Added a detailed docstring to _handle_tool_execution().
Updated Tests
Updated test_initialize_state() to use the new function signature:
Benefit:
Keeps tests aligned with refactored API for better coverage and accuracy.