-
Notifications
You must be signed in to change notification settings - Fork 2k
Support callbacks after planning step #1445
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
@albertvillanova one potential difficulty : many people will have step checks that rely on particular attributes of |
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.
Pull Request Overview
This PR updates the agent's execution flow to call the finalize_step callback after the planning step, supporting callbacks for both planning and action steps.
- Introduces a call to _finalize_step after completing the planning step.
- Updates the _finalize_step method signature to accept a PlanningStep in addition to an ActionStep.
Comments suppressed due to low confidence (2)
src/smolagents/agents.py:476
- Ensure that planning_step provides the timing attribute expected by _finalize_step, as the function now accepts both ActionStep and PlanningStep.
self._finalize_step(planning_step)
src/smolagents/agents.py:525
- [nitpick] Consider renaming the parameter 'memory_step' to a more generic term like 'step' since it can now represent both ActionStep and PlanningStep.
def _finalize_step(self, memory_step: ActionStep | PlanningStep):
@aymeric-roucher, thanks for your comment! You're absolutely right about the potential issue with the callbacks assuming just To address this, I have tried to implement a typed callback registry:
Benefits:
This also removes the need to guess whether a callback is safe to run after planning, since users explicitly declare the expected step type when registering the callback. Let me know what you think. |
I really like the solution you propose @albertvillanova! |
Call
finalize_step
after planning step to support callbacks.Fix #1435.