-
Notifications
You must be signed in to change notification settings - Fork 123
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
Adding support for a new runnable for InlineAgents #340
base: main
Are you sure you want to change the base?
Adding support for a new runnable for InlineAgents #340
Conversation
1d963b7
to
10ad334
Compare
10ad334
to
5f9ad6c
Compare
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.
Good start, I really like how inline agents fits the interfaces better than the create agent/invoke agent pattern. Left a comment on the interface itself. We should adhere to the messages input/output contract similar to how the chat models do it.
prompt_override_configuration: Optional[Dict] | ||
inline_session_state: Optional[Dict] | ||
|
||
class BedrockInlineAgentsRunnable(RunnableSerializable[Dict, OutputType]): |
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.
The contract seems to deviate from how other Runnables are implemented. Notice here in the Anthropic Chat Model that they use message as input and output: https://github.com/langchain-ai/langchain/blob/master/libs/core/langchain_core/language_models/chat_models.py#L127
If we support message in and message out then this will easily plugin to LCEL, LG and other constructs they have.
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 like this better than the state based approach. Left a comment to remove the older executor based approach.
get_boto_session, | ||
) | ||
|
||
class BedrockInlineAgentsChatModel(BaseChatModel): |
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.
Let's not have both BedrockInlineAgentRunnable and BedrockInlineAgentsChatModel. Remove the state based BedrockInlineAgentRunnable implementation and rename BedrockInlineAgentsChatModel to BedrockInlineAgentRunnable. I like how the new implementation supports the standardized message interface and will integrate with LangGraph better.
Please update the inline_agent_langgraph notebook to use this implementation instead of the current state based approach. I would like use to completely remove "should_continue" just like here in this sample they have no should_continue: https://langchain-ai.github.io/langgraph/how-tos/create-react-agent/#usage
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.
Thanks, merged the two interfaces and added a langgraph integration notebook.
We'd still need the custom parsers for running the graph because of the way RoC inputs are passed to the agent. We can investigate more on how to handle this natively within the runnable itself in next iterations.
Description
This PR introduces a new Bedrock Inline-Agents Runnable that allows using InvokeInlineAgent API with return of control functions as tools and configurable agentic properties.
Usage
See Jupyter notebook for usage with
AgentExecutor
Closes #309