[RFC] Question of Tooling Consistency Check Between Agent<>AgentExecutor

[skcoirz]

This is a follow up of https://github.com/hwchase17/langchain/pull/3840

Question: Why do we allow None allowed_tools in Agent when there are tools provided in AgentExecutor?

AgentExecutor is able to fetch the allowed_tools from Agent directly. It doesn’t look like we need to provide tools again at AgentExecutor entry functions when Agent is already provided as a parameter.

I’m new to the logics. I could be wrong. Would like to get some help here. Thank you so much.

[skcoirz]

Thank you for the review, dev2049. I have some questions here as described in the PR summary. Can I ask why you think it’s not needed?

On Wed, May 3, 2023 at 6:56 PM Davis Chase @.***> wrote:

@.**** commented on this pull request.

In langchain/agents/agent.py https://github.com/hwchase17/langchain/pull/4022#discussion_r1184470355:

@@ -629,12 +629,15 @@ def validate_tools(cls, values: Dict) -> Dict: agent = values["agent"] tools = values["tools"] allowed_tools = agent.get_allowed_tools()

•    if allowed_tools is not None:
  

•        if set(allowed_tools) != set([tool.name for tool in tools]):
  

•            raise ValueError(
  

•                f"Allowed tools ({allowed_tools}) different than "
  

•                f"provided tools ({[tool.name for tool in tools]})"
  

•            )
  

•    if allowed_tools is None and len(tools) == 0:
  

don't think this is needed

— Reply to this email directly, view it on GitHub https://github.com/hwchase17/langchain/pull/4022#pullrequestreview-1412158827, or unsubscribe https://github.com/notifications/unsubscribe-auth/AO64MH6T23C6JYP4EOQHDHDXEMEDXANCNFSM6AAAAAAXT4VB2Q . You are receiving this because you authored the thread.Message ID: @.***>

[hwchase17]

i also dont think this is neccessary. allowed_tools can be None for a myriad of reasons. if its None we should just ignore

[skcoirz]

@hwchase17 , @dev2049 , I’m okay with closing this PR, but when you have time, can you explain a bit when allowed_tools should be None? Ideally with a few real use cases? I have checked the logics of Agent and AgentExecutor, but I couldn’t understand why we allow it to be None. Would appreciate your context sharing. Thank you.

[skcoirz]

not hearing back from the core team. close for now.