Fix and refactor final answer checks

[aymeric-roucher]

This refactor fixes #1440 and refactors the logic to address underlying issues such as "agent cannot return None in final_answer"

[aymeric-roucher]

I think porting this logic into a separate function obfuscates the logic more than it clarifies.

[aymeric-roucher]

This is from #1442, will not appear after merging pr 1442.

[Copilot]

Pull Request Overview

This PR refactors the final answer handling in agents and updates related tests to use PIL.Image instances and the renamed step_number field.

• Refactored ActionStep.dict() to serialize images, add is_final_answer, and replace "step" with "step_number".
• Replaced the old FinalOutput type with ActionOutput and updated streaming logic in agents.py.
• Updated tests to use Image.new(...), renamed keys in dict assertions, and added scenarios for final answer checks.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
tests/test_memory.py	Switched from string paths to PIL.Image objects, renamed "step" to "step_number", added checks for observations_images.
tests/test_agents.py	Added a test case for final_answer_checks, updated max_steps usage, and renamed planning step variables.
src/smolagents/memory.py	Updated ActionStep.dict() to output step_number, serialize observations_images, and include is_final_answer.
src/smolagents/agents.py	Removed FinalOutput, introduced ActionOutput, and refactored the streaming loop to validate and flag final answers.

Comments suppressed due to low confidence (2)

tests/test_memory.py:101

• Consider adding an assertion to verify that action_step_dict["observations_images"] contains the expected byte data (e.g., matching Image.new(...).tobytes()).

    assert "observations_images" in action_step_dict

tests/test_memory.py:203

• Add assertions in test_task_step_to_messages to confirm that the generated messages include an image entry (e.g., check for a content dict with "type": "image").

    task_step = TaskStep(task="This is a task.", task_images=[Image.new("RGB", (100, 100))])

[Copilot]

The ActionOutput class is missing the @dataclass decorator, so instances won’t accept constructor arguments. Add @dataclass above this class definition.

[aymeric-roucher]

No it isn't, go home copilot you're drunk

[albertvillanova]

Thanks for addressing these issues.

Not sure about the need to replace FinalOutput with ActionOutput and the need to add the is_final_answer...

[albertvillanova]

I think this test passes before the fixes introduced in this PR.

[aymeric-roucher]

You're right, just fixed it to not be passing before!

[albertvillanova]

If I understand correctly, the ActionOutput class will be:

• Either ActionOutput(output: Any, is_final_answer=True) for final answer
• Or ActionOutput(output=None, is_final_answer=False) for non final answer

IMHO it would be simpler to return:

• Either FinalOutput(output: Any) for final answer
• Or None for non final answer

I find the ActionOutput is non-optimal because it adds complexity without adding relevant information, but feel free to ignore it!

[aymeric-roucher]

@Albert pasting my slack message here for visibility:

Thinking in terms of clients that will need to consume streaming events, I think it's simpler for frontends to consume a general object like ActionOutput and use their attributes rather than consuming FinalOutput | None : but I might still rework this, i'm still making lots of changes! The idea is to serve the same kind of streaming agents as other packages like copilotkit or openai agents) do.

[albertvillanova]

@aymeric-roucher I think when streaming, users are already consuming Generator[ChatMessageStreamDelta | FinalOutput].

The alternative approach just adds a None: Generator[ChatMessageStreamDelta | FinalOutput | None].