-
Notifications
You must be signed in to change notification settings - Fork 16.4k
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
Harrison/agents rewrite #15028
Harrison/agents rewrite #15028
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
if llm, tools, prompt are all we need for every agent maybe we can have a generic create_agent(type, llm, tools, prompt)
method. don't need to do rn, just thinking aloud
|
||
|
||
def create_openai_functions_agent( | ||
llm: BaseLanguageModel, tools: Sequence[BaseTool], prompt: ChatPromptTemplate |
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.
does prompt need to be typed as ChatPromptTemplate
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.
guessing it's an intentional choice not to make it optional?
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.
[Not design related] Since they're deprecating the functions API shouldn't we be pushing a "tools" agent?
messages: List[Union[BaseMessagePromptTemplate, BaseMessage]] | ||
messages = [SystemMessage(content=system_message)] | ||
if include_chat_history: | ||
messages.append(MessagesPlaceholder(variable_name="chat_history")) | ||
messages.extend( | ||
[ | ||
HumanMessagePromptTemplate.from_template("{input}"), | ||
MessagesPlaceholder(variable_name="agent_scratchpad"), | ||
] | ||
) | ||
return ChatPromptTemplate(messages=messages) |
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.
nit: find ChatPromptTemplate.from_messages cleaner
messages = [("system", system_message),]
if include_chat_history:
messages.append(MessagesPlaceholder())
messages.extend([
("human", "{input}"),
MessagesPlaceholder()
])
return ChatPromptTemplate.from_messages(messages)
messages: List[Union[BaseMessagePromptTemplate, BaseMessage]] | ||
messages = [SystemMessage(content=system_message)] | ||
if include_chat_history: | ||
messages.append(MessagesPlaceholder(variable_name="chat_history")) |
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 would really love to find a good way to standardize or reserve this name - always struck me as a bit arbitrary and inconsistent across modules
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.
Possibly as memory becomes more defined
agent = ( | ||
RunnablePassthrough.assign( | ||
agent_scratchpad=lambda x: format_to_openai_function_messages( | ||
x["intermediate_steps"] |
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.
for trying to run agent manually (while experimenting) would be nice if "intermediate_steps" wasn't required (ie do x.get('intermediate_steps', [])
)
functions=[format_tool_to_openai_function(t) for t in tools] | ||
) | ||
agent = ( | ||
RunnablePassthrough.assign( |
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.
should we add a nice run_name
- `input`: this is the user input | ||
- `agent_scratchpad`: this is the history of steps the agent has taken so far. | ||
This gets populated by the AgentExecutor. | ||
- `chat_history`: This is optionally required as input, if |
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.
"optionally required" is confusing
return ChatPromptTemplate(messages=messages) | ||
|
||
|
||
def create_openai_functions_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.
Worth renaming to create_functions_agent
and dropping the openai
and having typing be some kind of FunctionCallingChatModel
since you could do technically do this with any functions-capable model?
|
||
prompt = prompt.partial( | ||
tools=render_text_description(list(tools)), | ||
tool_names=", ".join([t.name for t in tools]), |
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.
Don't think you need this here? It's not in the hub prompt
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.
your right
|
||
|
||
def create_xml_agent( | ||
llm: BaseLanguageModel, tools: Sequence[BaseTool], prompt: ChatPromptTemplate |
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.
No reason this one can't be BasePromptTemplate
?
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.
your right
cookbook/human_approval.ipynb
Outdated
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.
Do we need a redirect for these?
CC @baskaryan
This agent uses the [ReAct](https://arxiv.org/pdf/2210.03629) framework to determine which tool to use | ||
based solely on the tool's description. Any number of tools can be provided. | ||
This agent requires that a description is provided for each tool. | ||
Whether this agent is intended for Chat Models (takes in messages, outputs message) or LLMs (takes in string, outputs string). The main thing this affects is the prompting strategy used. You can use an agent with a different type of model than it is intended for, but it likely won't product as good of results. |
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.
product
-> produce results of the same quality
<!-- Thank you for contributing to LangChain! Please title your PR "<package>: <description>", where <package> is whichever of langchain, community, core, experimental, etc. is being modified. Replace this entire comment with: - **Description:** a description of the change, - **Issue:** the issue # it fixes if applicable, - **Dependencies:** any dependencies required for this change, - **Twitter handle:** we announce bigger features on Twitter. If your PR gets announced, and you'd like a mention, we'll gladly shout you out!
Made a stupid typo in the last PR which got already merged😅 revamp getting started (#15070) Harrison/agents rewrite (#15028) havent marked the class as deprecated yet, will likely want to do all in one go (with other classes) --------- Co-authored-by: Nuno Campos <[email protected]> ModelIO revamp (#15230) Retrieval Docs Revamp (#15238) [core, langchain] modelio code improvements (#15277) [langchain] agents code changes (#15278) <!-- Thank you for contributing to LangChain! Please title your PR "<package>: <description>", where <package> is whichever of langchain, community, core, experimental, etc. is being modified. Replace this entire comment with: - **Description:** a description of the change, - **Issue:** the issue # it fixes if applicable, - **Dependencies:** any dependencies required for this change, - **Twitter handle:** we announce bigger features on Twitter. If your PR gets announced, and you'd like a mention, we'll gladly shout you out! move stuff
havent marked the class as deprecated yet, will likely want to do all in one go (with other classes)