-
Notifications
You must be signed in to change notification settings - Fork 21
feat: Add Human in the Loop utilities for tool execution in Agent #369
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
base: main
Are you sure you want to change the base?
Conversation
Some renaming and example how I'd like the interface to look Alternative implementation formatting Continue work on alternative Fixes Add more examples Change name of file
Pull Request Test Coverage Report for Build 18090924811Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some high-level comments:
-
Skimming other frameworks, it doesn't seem like there's a single right way to do HITL (Tool level or Agent level?). If this approach also helps with serialization headaches, it could be a good direction.
-
I see a design tension between giving users freedom to provide their own
ConfirmationStrategy
(there is a protocol for this andConfirmationResult.action
is a string) and requiring a specific stricter strategy (the current implementation of_handle_confirmation_strategies
only expects confirm/reject/modify). If we want to let users free to implement their strategies, I would suggest delegating most of the logic in_handle_confirmation_strategies
to theConfirmationStrategy
implementation. (Otherwise, we could be stricter in definingConfirmationResult
and removing the protocol...) -
about names: maybe we can rename
UserInterface
→ConfirmationUI
but if we go with the approach proposed in this PR, I recommend checking in with Bilge.
#371 should fix the failing docs workflow |
…erimental into hitl-alternative
haystack_experimental/components/agents/human_in_the_loop/strategies.py
Outdated
Show resolved
Hide resolved
haystack_experimental/components/agents/human_in_the_loop/user_interfaces.py
Outdated
Show resolved
Hide resolved
haystack_experimental/components/agents/human_in_the_loop/protocol.py
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,117 @@ | |||
# SPDX-FileCopyrightText: 2022-present deepset GmbH <[email protected]> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran the examples. They work properly and confirm that implementation is solid.
In a future notebook, as a user, I would also expect an introduction to the classes introduced, their meaning, and customization options.
ProtocolsI see that in this PR we are using a protocol ( For consistency, I would use protocols. They can also have default implementations for some methods and other classes can inherit from them to use the default implementations. See Python docs. EDIT: we should verify if this idea can work well... Code organizationIf we make both
|
…ating a Policy's state
This idea seems to work. I'll switch to this structure for now and keep testing. |
…to hitl-alternative
…ctly take in tool name and description instead of a tool instance. Should be better for exchanging information over a RestAPI
Related Issues
Proposed Changes:
This PR introduces a human-in-the-loop (HITL) mechanism for tool execution in Agent, allowing users to confirm, reject, or modify tool invocations interactively.
TODOs
Key Changes
haystack_experimental/components/agents/human_in_the_loop/
dataclasses.py
ConfirmationUIResult
andToolExecutionDecision
dataclasses to standardize user feedback and tool execution decisionspolicies.py
ConfirmationPolicy
base class and concrete policies:AlwaysAskPolicy
,NeverAskPolicy
, andAskOncePolicy
user_interfaces.py
ConfirmationUI
base class and two implementations:RichConsoleUI
(using Rich library) andSimpleConsoleUI
(using standard input), supporting interactive user feedback and parameter modification.hitl_multi_agent_example.py
to see a case where this occurs.protocol.py
ConfirmationStrategy
protocol. I'm not 100% if this should be a protocol or a base class.strategies.py
HumanInTheLoopStrategy
to orchestrate the confirmation policy and the confirmation UI, returning aToolExecutionDecision
.haystack_experimental/components/tools/tool_invoker.py
ToolInvoker
to support per-tool confirmation strategies.haystack_experimental/components/agents/agent.py
Agent
use the new experimentalToolInvoker
and added theconfirmation_strategies
explicitly.How did you test it?
Provided an example script called
hitl_intro_example.py
which shows how to use these new utility functions to convert existing tools into ones that ask for confirmation. I've also reproduced it hereSingle Agent Example highlighting different Confirmation Policies and Confirmation UIs
Multi-Agent Example: Tools within Sub-Agents use HiTL Confirmation
Notes for the reviewer
@julian-risch this now follows more closely your idea of passing the tool confirmation strategies to the Agent (or ToolInvoker) rather than applying it at the tool level. I think I like this design better since it has better separation and it makes the various policies easy to understand.
Checklist
fix:
,feat:
,build:
,chore:
,ci:
,docs:
,style:
,refactor:
,perf:
,test:
.