Skip to content

RFC: Add return_direct to Tools, Actions, and FunctionCalling Agents#532

Merged
eob merged 6 commits intomainfrom
add-return-direct-to-tools
Sep 1, 2023
Merged

RFC: Add return_direct to Tools, Actions, and FunctionCalling Agents#532
eob merged 6 commits intomainfrom
add-return-direct-to-tools

Conversation

@eob
Copy link
Copy Markdown
Contributor

@eob eob commented Aug 27, 2023

This PR adds a return_direct flag to Tool the style of LangChain that permits a tool to signal that its response should be delivered as the "final answer" to the user.

This prevents a common error condition in which a reasoning agent runs a "terminal" tool, but then instead of returning its output, it either:

  • Summarizes for the user what it did
  • Runs other tools unnecessarily

Here's an example of that in action with the Prompt Golf bot:

image

What happened here is:

  • The Agent correctly ran the GameTurnTool which generated a new image
  • The Agent then replied with a summary of what it did rather than replying with the output of the tool
image

@eob eob requested a review from douglas-reid August 27, 2023 19:06
@eob
Copy link
Copy Markdown
Contributor Author

eob commented Aug 27, 2023

@douglas-reid does this seem like a reasonable way to achieve this? It would result in:

  • FinishAction being the way the Agent signals completion.
  • Tool.return_direct being a way a Tool can signal that it always should end reasoning

@douglas-reid
Copy link
Copy Markdown
Contributor

@eob my first reaction is that this is fine, as it seems to address an immediate need. it feels like "llm smell" in the sense that a summarizing action seems like something that should be ideally smoothed out of workflows at a different level. and i worry that there are likely tools that don't always want to return FinishActions. but both of those concerns seem minimal in the face of our existing reality. i'll take a closer look at the code impl now.

Copy link
Copy Markdown
Contributor

@douglas-reid douglas-reid left a comment

Choose a reason for hiding this comment

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

Some initial reactions.

Comment thread src/steamship/agents/schema/action.py Outdated
output: Optional[List[Block]]
"""Any direct output produced by the Tool."""

return_direct: bool = False
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

curious: should we label this more like is_final or tool_is_direct similar to track that state (and update the comment -- as i'm not expecting users to set return_direct on Actions) ?

aside: Would this be "simpler" if the Action returned by an Agent/AgentExecutor when Tool.return_direct == True was a FinishAction instead? Or even a new type of Action (ex: DirectReturnAction) ? Maybe that would be more complex (as the AgentExecutor would have to be involved) ??

Comment thread src/steamship/agents/schema/tool.py Outdated
return_direct: bool = False
"""Whether to return the tool's output directly.

Setting this to True means that after the tool is called, the AgentExecutor will stop looping.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
Setting this to True means that after the tool is called, the AgentExecutor will stop looping.
Setting this to True means that after the Tool is run, the Tool's output will be returned without further Agent interactions. This will preempt the execution of other tools, as well as any subsequent calls to LLMs, etc., to interpret the Tool output.

self.run_action(agent=agent, action=action, context=context)

# If this action wishes to end the loop, comply with that wish
if action.return_direct:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

aside (take it or leave it): my api spidey-sense is tingling seeing this code block. i think seeing this, i'd prefer something like action.is_final (or even if isinstance(action, DirectReturnAction)) for clarity (as Actions aren't really "returning" anything per se).


# If this action wishes to end the loop, comply with that wish
if action.return_direct:
logging.info(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

curious: do you think we need this log statement for full coverage? WDYT about just adding "is_final" or similar to the existing "next tool" log? that might lead to more readable code (no floating break) and fewer lines of logging code?

# If this action wishes to end the loop, comply with that wish
if action.return_direct:
logging.info(
f"Exciting reasoning loop by request of: {action.tool}",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
f"Exciting reasoning loop by request of: {action.tool}",
f"Exiting Agent execution by request of: {action.tool}",

@eob
Copy link
Copy Markdown
Contributor Author

eob commented Aug 28, 2023

@douglas-reid thanks a ton -- will do some tinkering down these threads.

A few thoughts I had that are in the same vein as yours:

  • I'm not sure if it should feel weird that a the is_final property of Tool is static as opposed to part of its response.
  • I like the idea of Action.is_final... that feels cleaner and also aligns it with FinalAction

@eob eob enabled auto-merge September 1, 2023 17:53
@eob eob added this pull request to the merge queue Sep 1, 2023
@eob eob removed this pull request from the merge queue due to a manual request Sep 1, 2023
@eob eob merged commit 655ae07 into main Sep 1, 2023
@eob eob deleted the add-return-direct-to-tools branch September 1, 2023 20:41
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.

2 participants