Skip to content

Refactor event loop to use Agent object rather than individual parameters #359

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

Merged

Conversation

zastrowm
Copy link
Member

@zastrowm zastrowm commented Jul 5, 2025

Description

Update the event_loop_cycle function to accept the agent directly instead of the separate parameters that were coming directly from the agent. This simplifies + clarifies exactly what the event loop is doing with the agent. Long-term this sets us up for more easily access other agent capabilities like hooks.

There is a downside here of tighter coupling between the agent & the event_loop - an alternative implementation of this would be to abstract the object being passed - maybe something like EventLoopContext. I originally started implementing this approach before revisiting it as it seemed like an unnecessary abstraction.

I am open to feedback regarding the tighter coupling/however.

Notes

With this the ToolHandler abstraction is un-necessary since we're not passing that in and we're binding the actual tool call to pass into the tool-executor anyways. After some discussion on the PR, I moved the ToolHandler.run directly into the event loop, as this seemed the best place and sets us up for future changes w.r.t. hooks.

Related Issues

N/A

Documentation PR

N/A

Type of Change

Other (please describe): Refactor

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli

  • I ran hatch run prepare
  • I verified that the OTEL traces are the same before & after

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

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

… parameters

Update the event_loop_cycle function to accept the agent directly instead of the separate parameters that were coming directly from the agent. This simplifies + clarifies exactly what the event loop is doing with the agent.  Long-term this sets us up for more easily access other agent capabilities like hooks.

There is a downside here of tighter coupling between the agent & the event_loop - an alternative implementation of this would be to abstract the object being passed - maybe something like `EventLoopContext`.  I originally started implementing this approach before revisiting it as it seemed like an unnecessary abstraction.
ToolHandler's requirements have been reduced enough where it probably doesn't warrant a separate class, especially given that event into executor a partial function is passed in to conform to the requirement.
@zastrowm zastrowm merged commit 46f66be into strands-agents:main Jul 7, 2025
12 checks passed
jsamuel1 pushed a commit to jsamuel1/sdk-python that referenced this pull request Jul 9, 2025
…ters (strands-agents#359)

Update the event_loop_cycle function to accept the agent directly instead of the separate parameters that were coming directly from the agent. This simplifies + clarifies exactly what the event loop is doing with the agent.  Long-term this sets us up for more easily access other agent capabilities like hooks.

There is a downside here of tighter coupling between the agent & the event_loop - an alternative implementation of this would be to abstract the object being passed - maybe something like `EventLoopContext`.  I originally started implementing this approach before revisiting it as it seemed like an unnecessary abstraction.
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.

2 participants