Skip to content
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

Add tests/fix bug for single planning step #407

Closed
wants to merge 7 commits into from

Conversation

g-eoj
Copy link
Contributor

@g-eoj g-eoj commented Jan 28, 2025

Fixes issue hit while testing #300, which has flatten_messages_as_text set to True. Setting a planning_interval would fail.

Adds test coverage for all issues found.

agent = CodeAgent(model=mlx_model, tools=[UserInputTool()], add_base_tools=True, max_steps=3, planning_interval=1)
answer = agent.run("Hi!")
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[3], line 1
----> 1 answer = agent.run("Hi!")

File ~/.cache/uv/archive-v0/3MaM4p_ZnzhOGFENeDNZr/lib/python3.12/site-packages/smolagents/agents.py:565, in MultiStepAgent.run(self, task, stream, reset, single_step, images, additional_args)
    563     return self._run(task=self.task, images=images)
    564 # Outputs are returned only at the end as a string. We only look at the last step
--> 565 return deque(self._run(task=self.task, images=images), maxlen=1)[0]

File ~/.cache/uv/archive-v0/3MaM4p_ZnzhOGFENeDNZr/lib/python3.12/site-packages/smolagents/agents.py:586, in MultiStepAgent._run(self, task, images)
    584 try:
    585     if self.planning_interval is not None and self.step_number % self.planning_interval == 0:
--> 586         self.planning_step(
    587             task,
    588             is_first_step=(self.step_number == 0),
    589             step=self.step_number,
    590         )
    591     self.logger.log(
    592         Rule(
    593             f"[bold]Step {self.step_number}",
   (...)
    597         level=LogLevel.INFO,
    598     )
    600     # Run one step!

File ~/.cache/uv/archive-v0/3MaM4p_ZnzhOGFENeDNZr/lib/python3.12/site-packages/smolagents/agents.py:661, in MultiStepAgent.planning_step(self, task, is_first_step, step)
    648             message_prompt_facts = {
    649                 "role": MessageRole.SYSTEM,
    650                 "content": SYSTEM_PROMPT_FACTS,
    651             }
    652             message_prompt_task = {
    653                 "role": MessageRole.USER,
    654                 "content": f"""Here is the task:
   (...)
    658 Now begin!""",
    659             }
--> 661             answer_facts = self.model([message_prompt_facts, message_prompt_task]).content
    663             message_system_prompt_plan = {
    664                 "role": MessageRole.SYSTEM,
    665                 "content": SYSTEM_PROMPT_PLAN,
    666             }
    667             message_user_prompt_plan = {
    668                 "role": MessageRole.USER,
    669                 "content": USER_PROMPT_PLAN.format(
   (...)
    674                 ),
    675             }

File ~/.cache/uv/archive-v0/3MaM4p_ZnzhOGFENeDNZr/lib/python3.12/site-packages/smolagents/models.py:485, in MLXModel.__call__(self, messages, stop_sequences, tools_to_call_from, **kwargs)
    478 def __call__(
    479     self,
    480     messages: List[Dict[str, str]],
   (...)
    483     **kwargs,
    484 ) -> ChatMessage:
--> 485     completion_kwargs = self._prepare_completion_kwargs(
    486         flatten_messages_as_text=True,  # mlx-lm doesn't support vision models
    487         messages=messages,
    488         stop_sequences=stop_sequences,
    489         tools_to_call_from=tools_to_call_from,
    490         **kwargs,
    491     )
    492     messages = completion_kwargs.pop("messages")
    493     stop_sequences = completion_kwargs.pop("stop", [])

File ~/.cache/uv/archive-v0/3MaM4p_ZnzhOGFENeDNZr/lib/python3.12/site-packages/smolagents/models.py:263, in Model._prepare_completion_kwargs(self, messages, stop_sequences, grammar, tools_to_call_from, custom_role_conversions, convert_images_to_image_urls, flatten_messages_as_text, **kwargs)
    254 """
    255 Prepare parameters required for model invocation, handling parameter priorities.
    256 
   (...)
    260 3. Default values in self.kwargs
    261 """
    262 # Clean and standardize the message list
--> 263 messages = get_clean_message_list(
    264     messages,
    265     role_conversions=custom_role_conversions or tool_role_conversions,
    266     convert_images_to_image_urls=convert_images_to_image_urls,
    267     flatten_messages_as_text=flatten_messages_as_text,
    268 )
    270 # Use self.kwargs as the base configuration
    271 completion_kwargs = {
    272     **self.kwargs,
    273     "messages": messages,
    274 }

File ~/.cache/uv/archive-v0/3MaM4p_ZnzhOGFENeDNZr/lib/python3.12/site-packages/smolagents/models.py:228, in get_clean_message_list(message_list, role_conversions, convert_images_to_image_urls, flatten_messages_as_text)
    226 else:
    227     if flatten_messages_as_text:
--> 228         content = message["content"][0]["text"]
    229     else:
    230         content = message["content"]

TypeError: string indices must be integers, not 'str'

@g-eoj g-eoj changed the title Add tests/fix bugs for single planning step Add tests/fix bug for single planning step Jan 29, 2025
@g-eoj g-eoj marked this pull request as ready for review January 29, 2025 00:54
@g-eoj g-eoj requested a review from aymeric-roucher January 30, 2025 19:31
Copy link
Member

@albertvillanova albertvillanova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix, @g-eoj.

I am sorry, I missed your PR and fixed the issue with:

@g-eoj
Copy link
Contributor Author

g-eoj commented Feb 1, 2025

No worries.

@g-eoj g-eoj closed this Feb 1, 2025
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.

3 participants