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

Simplify Agents #160

Open
hardikjshah opened this issue Feb 21, 2025 · 4 comments
Open

Simplify Agents #160

hardikjshah opened this issue Feb 21, 2025 · 4 comments

Comments

@hardikjshah
Copy link
Contributor

hardikjshah commented Feb 21, 2025

Multiple places where client_tools are needed

  1. User implements a client tool
### [1] Define the tool 
@client_tool
def get_ticker_data(ticker_symbol: str, start: str, end: str) -> str:
    """
    Get yearly closing prices for a given ticker symbol

    :param ticker_symbol: The ticker symbol for which to get the data. eg. '^GSPC'
    :param start: Start date, eg. '2021-01-01'
    :param end: End date, eg. '2024-12-31'
    :return: JSON string of yearly closing prices
    """
    ...
    return some_json

[2] Provide tool def in the config

agent_config = AgentConfig(
    model=selected_model,
    instructions="You are a helpful assistant",
    sampling_params={
        "strategy": {"type": "top_p", "temperature": 1.0, "top_p": 0.9},
    },
    toolgroups=["builtin::rag"],
    # --> we need to mention tool defs here <-- # 
    client_tools=[
        client_tool.get_tool_definition() for client_tool in client_tools
    ],
    tool_config: ..., 
    input_shields=available_shields if available_shields else [],
    output_shields=available_shields if available_shields else [],
    enable_session_persistence=False,
)

[3] again pass client tools here

agent = Agent(client, agent_config, client_tools)

Step [2] should not be necessary and we should just take client tools provided in [3] and inject them into the config before sending to server.

Proposal

@client_tool
def get_ticker_data(ticker_symbol: str, start: str, end: str) -> str:
    """
    Get yearly closing prices for a given ticker symbol

    :param ticker_symbol: The ticker symbol for which to get the data. eg. '^GSPC'
    :param start: Start date, eg. '2021-01-01'
    :param end: End date, eg. '2024-12-31'
    :return: JSON string of yearly closing prices
    """
    ...
    return some_json

# simplify by taking all tools in one place
tools = [
    # can be a simple str 
    "builltin::websearch",
    # can be a dict that takes additional params for builtin tools
    {
        "name": "builtin::rag",
        "args": {
            "vector_db_ids": [vector_db_id],
        },
    },
    # can be a client tool 
    get_ticker_data,
]

# no tool info in agent-config 
agent_config = AgentConfig(
    model=selected_model,
    instructions="You are a helpful assistant, keep answers short and concise",
    sampling_params={
        "strategy": {"type": "top_p", "temperature": 1.0, "top_p": 0.9},
    },
    tool_config: {
         "system_message_behavior": "append",
    }, 
    input_shields=[],
    output_shields=[],
    enable_session_persistence=True,
)

# we will take tools and extract tool defs and pass them to the server appropriately 
agent = Agent(client, agent_config, tools) 
@raghotham
Copy link
Contributor

Why do we want to pass tools directly to Agent(). Can't we just get developers to add tools to agent_config?

@ehhuang
Copy link
Contributor

ehhuang commented Feb 21, 2025

#142 (review) is related

@yanxi0830
Copy link
Contributor

yanxi0830 commented Feb 21, 2025

I see this as the high-level question of how to unify the concept of "client tools" + "server tools". IIRC, right now we can have tools be defined as Union[str, dict, ClientTool]. ClientTool need to be passed in as constructor to Agent as it is a specific instance and not

I think the proposal simplifies the amount of code user have to write. The ReAct agent kind of does it with client_tools and construct the agent_config with client_tools.

class ReActAgent(Agent):
"""ReAct agent.
Simple wrapper around Agent to add prepare prompts for creating a ReAct agent from a list of tools.
"""
def __init__(
self,
client: LlamaStackClient,
model: str,
builtin_toolgroups: Tuple[str] = (),
client_tools: Tuple[ClientTool] = (),
tool_parser: ToolParser = ReActToolParser(),
json_response_format: bool = False,
custom_agent_config: Optional[AgentConfig] = None,
):
def get_tool_defs():

However, the downside of this user may not have the full picture of agent_config that gets send. Would it be better to introduce helpers to take care of the config creation?

agent_config = Agent.get_config_from_tools(tools) -> AgentConfig
react_agent_config = ReActAgent.get_config_from_tools(tools) -> AgentConfig
# modify config with models, sampling params etc
agent = Agent(client, agent_config, client_tools)

-- we would still need to pass in tools to the agent in any case though (unless we introduce some notion of client side tools registration)

@ehhuang
Copy link
Contributor

ehhuang commented Feb 21, 2025

Yea, I like having it in agent_config. Also going one step further, I don't think user should care whether a tool is client or server. They should all be passed in to a tools param.

Also relatedly, I'm thinking of just changing toolgroups to tools. Why? It's another concept for user to learn, and I found this confusing and not very useful:

  1. The model itself doesn't make use of the groups concept.
  2. User can just get the tools from a group with list_tools, then pass the list in to the AgentConfig. This allows the user to be more explicit about what tools they want to expose. Also makes the tools more transparent, e.g. I don't know what tool_name I can refer to in my instructions when I use a toolgroup, so might as well make this explicit.

Created separate discussion: #165

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

No branches or pull requests

4 participants