Skip to content

Conversation

albertvillanova
Copy link
Member

Call finalize_step after planning step to support callbacks.

Fix #1435.

@aymeric-roucher
Copy link
Collaborator

aymeric-roucher commented Jun 17, 2025

@albertvillanova one potential difficulty : many people will have step checks that rely on particular attributes of ActionStep, such as the observations, so this would break their workflows by trying to use attributes that do not exist on PlanningStep. Maybe we should differentiate callbacks for planning and for action steps.

@albertvillanova albertvillanova requested a review from Copilot June 19, 2025 07:39
Copy link
Contributor

@Copilot 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 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):

@albertvillanova
Copy link
Member Author

@aymeric-roucher, thanks for your comment! You're absolutely right about the potential issue with the callbacks assuming just ActionStep.

To address this, I have tried to implement a typed callback registry:

  • this allows registering callbacks per step type

Benefits:

  • Backward compatible: Existing action-step callbacks remain unaffected.
  • Flexible: Advanced users can register callbacks for planning, action, or all steps.
  • Type-safe and extensible: Easy to add support for new step types in the future without modifying core agent logic.
  • Encourages separation of concerns: Callbacks for planning and action steps remain clearly scoped.

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.

@aymeric-roucher
Copy link
Collaborator

I really like the solution you propose @albertvillanova!

@albertvillanova albertvillanova merged commit c63606f into huggingface:main Jun 30, 2025
3 checks passed
@albertvillanova albertvillanova deleted the fix-1435 branch June 30, 2025 15:42
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.

Is there a way to return plan of a CodeAgent?
2 participants