Add the add_tool(), remove_tool() and remove_all_tool() me…

[JMLX42]

…thods for AssitantAgent

Why are these changes needed?

Allow the user to add/remove tools dynamically.

Related issue number

#2666

Checks

• I've included any doc changes needed for https://microsoft.github.io/autogen/. See https://microsoft.github.io/autogen/docs/Contribute#documentation to build and test documentation locally.
• I've added tests (if relevant) corresponding to the changes introduced in this PR.
• I've made sure all auto checks have passed.

[lokitoth]

What happens (or should happen?) if one thread adds / removes tools while another thread is running a task?

[ekzhu]

@lokitoth Currently, there is a note on AssistantAgent's API doc that says it is not thread/coroutine safe.

https://microsoft.github.io/autogen/dev/reference/python/autogen_agentchat.agents.html#autogen_agentchat.agents.AssistantAgent

[ekzhu]

@JMLX42 thanks for the PR, right now this needs a bit more design. If you are in a hurry to depend on this feature you are welcome to create a community package that extends the BaseAgent and implement a custom agent. I will include this for consideration for 0.4.1.

[JMLX42]

thanks for the PR, right now this needs a bit more design

@ekzhu thank you for the feedback.

AFAIK it meets the original goal: one can extend AssistantAgent and override on_messages_stream() to have a dynamic tool list:

class MyAgent(AssistantAgent):
    async def on_messages_stream(
        self, messages: Sequence[ChatMessage], cancellation_token: CancellationToken
    ) -> AsyncGenerator[AgentMessage | Response, None]:
        query = "\n".join([msg.content for msg in messages if isinstance(msg, TextMessage)])
        logger.debug(f"query: {query}")

        self.remove_all_tools()
        self.add_tools(
            get_tools_for_query(
                index=self.index,
                tools=self._openapi.tools,
                query=query,
                embedding_func=self._embedding_func,
                top_k=self._top_k,
            )
        )
        logger.info(f"selected tools: {[tool.name for tool in self._tools]}")

        async for msg in super().on_messages_stream(messages, cancellation_token):
            yield msg

        self.remove_all_tools()

And it does not break anything.

If you are in a hurry to depend on this feature you are welcome to create a community package that extends the BaseAgent and implement a custom agent.

I can do that. But since it does not break anything and it just moved some code out of __init__ into add_tools() it sounds a bit extreme to make a completely separate 3rd party package.

[ekzhu]

Thanks @JMLX42

I can do that. But since it does not break anything and it just moved some code out of init into add_tools() it sounds a bit extreme to make a completely separate 3rd party package.

It may not break anything but put additional constraints potentially more surface area for bugs. If it turns out to be a popular feature and has a lot of usage cases we can add it. Right now, we can point folks to your extension package when need arises.

[husseinmozannar]

I like the idea to add and remove tools dynamically, I think this will be useful for many users. We just need to add appropriate tests to detect any unforeseen issues. Otherwise, seems like not a bad idea to merge to main

[JMLX42]

We just need to add appropriate tests to detect any unforeseen issues

I would be happy to add tests. But I am not sure what "unforeseen issues" we're talking about here.

Tools being removed during the tool execution phase of on_messages_stream()? IMO that can easily be deal with by copying self._tools before executing the tool calls. Since AFAIK that's the only place where self._tools is read it should work just fine.

Anything else we should test for?

[rysweet]

@ekzhu or @husseinmozannar any suggestions on what tests would be appropriate? @JMLX42 maybe test for get_tools_for_query permutations/failures and for cases where on_messages_stream doesn't have the expected content?

[ekzhu]

@JMLX42

Question:

1. add_tool vs add_tools? Is there are a reason to use plular? same for remove_tools
2. Can we use save_state on an existing AssistantAgent and load_state on a new instance of AssistantAgent with different tools?

For unit tests, we can have unit tests for basic usage of these API methods. But I would like to understand the usage scenarios for these API methods.

[JMLX42]

@ekzhu

add_tool vs add_tools? Is there are a reason to use plular? same for remove_tools

Because add_tools([MyTool]) works for a single tool, but making add_tool() work with multiple tools involves a lot more code/work.

Can we use save_state on an existing AssistantAgent and load_state on a new instance of AssistantAgent with different tools?

I am not sure how save_state() is supposed to work. Is there some existing up to date documentation?

[husseinmozannar]

It makes sense to have add_tools instead of add_tool

Along with this, maybe we should have a way for the agent to export tools (because now tools could be created dynamically and we want to re-use them in the future)

Some use cases for this I imagine:

• teams creating new tools and adding them on the fly for a use case of a continuously learning agent. One agent generates a tool and adds it to another agent
• teams figure out to remove tools because the tools don't work or are no longer needed.
• teams re-writing existing tools to improve them. a kind of dynamic tool re-writing

Copilot reviewed 1 out of 1 changed files in this pull request and generated no suggestions.

Comments skipped due to low confidence (1)

python/packages/autogen-agentchat/src/autogen_agentchat/agents/_assistant_agent.py:235

• The remove_tools method should check if the tool name exists in the lists before attempting to remove it to avoid unexpected behavior.

def remove_tools(self, tool_names: List[str]) -> None:

[gagb]

In tinyRA here was the workflow I used:

1. user request arrives
2. backend constructs an assistant agent with given tools
3. the agent does the work
4. the user clicks a button which triggers an LLM call to extracts a new tool (e.g., python function)
5. ui displays the tool draft, user edits it and saves it
6. a bunch of backend checks validate the tool
7. append the valid tool to the list of available tools
8. a new user request arrives, and repeat step 2

[gagb]

The PR simplifies the constructor and adds useful methods.

[ekzhu]

I have some bias toward singular API e.g., remove_tool instead of remove_tools, for simplicity of the API and avoid creating unnecessary objects (thinking about a parallel API in .NET). Please convince me. cc @lokitoth

[ekzhu]

I also think instead of remove all tools we should offer an api for listing the tools, and for removal we always remove one by one.

So:

1. Add
2. Remove
3. List

[ekzhu]

Using Python, it's trivial to do collection modification with the List method and singular add and remove.

[jackgerrits]

I agree on singular APIs to avoid unnecessary list creation. I know it's more code to add a list but it could easily be added as a mixin and it is the simplest primitive required to implement the functionality.

[JMLX42]

I agree on singular APIs to avoid unnecessary list creation. I know it's more code to add a list but it could easily be added as a mixin and it is the simplest primitive required to implement the functionality.

@jackgerrits @ekzhu to summarize:

• add_tool(self, tools: List[Tool]) => add_tool(self, tool: Tool)
• remove_tools(self, tool_names: List[str]) => remove_tool(self, tool_names: str)
• remove_all_tools(self) is left unchanged
• add a @property tools(self) -> List[Tool]

Correct?

[ekzhu]

add_tool(self, tools: List[Tool]) => add_tool(self, tool: Tool)
remove_tools(self, tool_names: List[str]) => remove_tool(self, tool_names: str)
remove_all_tools(self) is left unchanged
add a @Property tools(self) -> List[Tool]

yes. thank you

[JMLX42]

yes. thank you

@ekzhu done in db76ae3

[ekzhu]

Thanks for the update. I think the remaining issue is updating the doc string and add a usage example in the doc string.

[JMLX42]

Thanks for the update. I think the remaining issue is updating the doc string and add a usage example in the doc string.

@ekzhu done in 9b9ad51

[ekzhu]

@JMLX42 thanks again for the PR. After some discussion and thoughts, I think I am for adding a list_tools API and removing remove_all_tools. Because if you want to add or remove tools, you still need to know if the tools exist.

I can make the changes directly. Apologies for the back-and-forth.