-
Notifications
You must be signed in to change notification settings - Fork 19k
feat(langchain): finishing up HITL implementation #32962
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
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. |
If this works for now, it's probably fine? Behavior is correct as far as an end user is concerned. We could add a TODO for this refactor (and potentially apply the patch I made before for ToolNode).
Is the tool call id part of the human interrupt? if so any reason not to do it? I suspect even if we disable parallel tool calling, this could be an issue in workflows that use multiple agents in parallel? |
Sounds great |
Pasting here, doesn't belong in description anymore: Main question here is what we do with multiple tool calls and how we send them to tool node: If we use
If we don't use
Will open an issue advocating for revisiting this |
…ping tool call id -> resume
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
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.
Flash review, looking at the code
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
libs/langchain_v1/tests/unit_tests/agents/test_middleware_agent.py
Outdated
Show resolved
Hide resolved
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
66a370c
to
6745b79
Compare
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
… structured output (#32980) This enables parallel tool calling w/ a combo of 1. Standard and structured response tool calls 2. Deferred (requiring human approval / edits) tool calls and structured response tool calls Hard to unit test w/ HITL right now end to end, so here's a repro of things working w/ an integration test: ```py from pydantic import BaseModel, Field from langchain_core.tools import tool from langchain_core.messages import HumanMessage from langgraph.types import Command from langgraph.checkpoint.memory import InMemorySaver from langchain.agents.middleware_agent import create_agent from langchain.agents.middleware.human_in_the_loop import HumanInTheLoopMiddleware from langchain_openai import ChatOpenAI class WeatherBaseModel(BaseModel): temperature: float = Field(description="Temperature in fahrenheit") condition: str = Field(description="Weather condition") @tool def add_numbers(a: int, b: int) -> int: """Add two numbers.""" return a + b model = ChatOpenAI(model="gpt-4o-mini", temperature=0) checkpointer = InMemorySaver() agent = create_agent( model=model, tools=[add_numbers], response_format=WeatherBaseModel, middleware=[HumanInTheLoopMiddleware(tool_configs={"add_numbers": True})], ) agent = agent.compile(checkpointer=checkpointer) # First invocation should be interrupted due to human-in-the-loop middleware response = agent.invoke( { "messages": [ HumanMessage( "Add 1 and 2, then return the weather forecast with temperature 72 and condition sunny." ) ] }, config={"configurable": {"thread_id": "1"}}, ) interrupt_description = response["__interrupt__"][0].value[0]["description"] print(interrupt_description) """ Tool execution requires approval Tool: add_numbers Args: {'a': 1, 'b': 2} """ # Resume the agent with approval response = agent.invoke( Command(resume=[{"type": "approve"}]), config={"configurable": {"thread_id": "1"}} ) for msg in response["messages"]: msg.pretty_print() """ ================================ Human Message ================================= Add 1 and 2, then return the weather forecast with temperature 72 and condition sunny. ================================== Ai Message ================================== Tool Calls: WeatherBaseModel (call_u6nXsEYRJbqNx4AEHgiQMpE2) Call ID: call_u6nXsEYRJbqNx4AEHgiQMpE2 Args: temperature: 72 condition: sunny add_numbers (call_nuQEZF7PwfYDlVpnSt8eaInI) Call ID: call_nuQEZF7PwfYDlVpnSt8eaInI Args: a: 1 b: 2 ================================= Tool Message ================================= Name: WeatherBaseModel Returning structured response: temperature=72.0 condition='sunny' ================================= Tool Message ================================= Name: add_numbers 3 """ print(repr(response["response"])) """ WeatherBaseModel(temperature=72.0, condition='sunny') """ ```
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 think we should remove this. Folks wanting to use this old style can use what's in LangGraph. Should be deprecated.
|
||
message: str | ||
args: dict | ||
allowed_actions: list[Literal["approve", "reject", "edit"]] |
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.
Using a more common pattern w/ access control where we use a simple list.
One point of confusion here is that we actually don't have an "edit"
action below, it's rolled into approve
.
I propose we maybe use approve_with_edits
instead to make this more clear.
We can expand this in the future to have things like:
"approve_with_persistence"
I don't see a case where rejecting wouldn't expose the option for a message.
message_prefix: str = "Tool execution requires approval", | ||
tool_configs: dict[str, bool | list[AllowedActions]], | ||
*, | ||
action_request_prefix: str = "Tool execution requires approval", |
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 think this is more clear
|
||
request: ActionRequest = { | ||
"message": message, | ||
"args": {"tool_name": tool_name, "tool_args": tool_args}, |
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.
we could just dump the tool call here and expect a tool call in return? wdyt @eyurtsev
class ApproveResponse(TypedDict): | ||
"""Response when a human approves the action.""" | ||
|
||
action: Literal["approve"] |
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.
name collision in action
. action
refers the action taken by the reviewer / HIL as well as to the action taken by the agent.
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 really like action here bc it matches w/ allowed_actions
. Maybe we could find a better name for the request.
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.
But, if allowed_actions
is going to bloat to have stuff like edits, persist, then my above point becomes less valuable. So maybe fine to use type here.
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
libs/langchain_v1/langchain/agents/middleware/human_in_the_loop.py
Outdated
Show resolved
Hide resolved
…i/langchain into sr/hitl-improvements
# Main changes / new features ## Better support for parallel tool calls 1. Support for multiple tool calls requiring human input 2. Support for combination of tool calls requiring human input + those that are auto-approved 3. Support structured output w/ tool calls requiring human input 4. Support structured output w/ standard tool calls ## Shortcut for allowed actions Adds a shortcut where tool config can be specified as a `bool`, meaning "all actions allowed" ```py HumanInTheLoopMiddleware(tool_configs={"expensive_tool": True}) ``` ## A few design decisions here * We only raise one interrupt w/ all `HumanInterrupt`s, currently we won't be able to execute all tools until all of these are resolved. This isn't super blocking bc we can't re-invoke the model until all tools have finished execution. That being said, if you have a long running auto-approved tool, this could slow things down. ## TODOs * Ideally, we would rename `accept` -> `approve` * Ideally, we would rename `respond` -> `reject` * Docs update (@sydney-runkle to own) * In another PR I'd like to refactor testing to have one file for each prebuilt middleware :) Fast follow to #32962 which was deemed as too breaking
Main changes / new features
Better support for parallel tool calls
Enhanced action request / response primitives
Simplified the primitives we use for action responses --
accept
andreject
patterns.edit
is rolled intoaccept
w/ edits.These primitives have no notion of tool calls (no ids etc) so we can generically use them for HITL patterns independent of tool call approvals.
Shortcut for allowed actions
Adds a shortcut where tool config can be specified as a
bool
, meaning "all actions allowed"A few design decisions here
HumanInterrupt
s, currently we won't be able to execute all tools until all of these are resolved. This isn't super blocking bc we can't re-invoke the model until all tools have finished execution. That being said, if you have a long running auto-approved tool, this could slow things down.args
for an action request w/tool_name
andtool_args
and expect the same for edits.allowed_actions
is now represented as a list to be consistent with most other access control patternsTODO