-
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
Adds the option to pass the original prompt into the AgentExecutor for PlanAndExecute agents #5401
Adds the option to pass the original prompt into the AgentExecutor for PlanAndExecute agents #5401
Conversation
…nal objective passed into the agent
… position instead of keywords
langchain/experimental/plan_and_execute/executors/agent_executor.py
Outdated
Show resolved
Hide resolved
…tting `PlanAndExecute.input_key` key from `inputs`
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.
Leaving a message to close the review I created on accident. Sorry I've made this PR a bit of a mess!
@hwchase17 or @vowelparrot would be great to get your input on the current state of the PR, and if any other actions/changes I need to make or whether this is ready to merge :)
_new_inputs = { | ||
"previous_steps": self.step_container, | ||
"current_step": step, | ||
"objective": inputs[self.input_key], |
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.
@hwchase17 after looking into LLMChain.prep_prompts, I think this is the cleanest solution. We can remove the if statement, since in LLMChain.prep_prompt, selected_inputs explicitly searches for keys present in prompt.input_variables. Thus there should be no problem always including the "objective" item, even if include_task_in_prompt is False.
I think having this line and referring to self.input_key is important, in case for whatever reason a user changes the input_key.
Let me know if you're happy with this, I'll stop spamming commits until then 😆
(Sorry I accidentally created a review instead of making a normal comment)
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.
lgtm! thanks
…r PlanAndExecute agents (langchain-ai#5401) # Adds the option to pass the original prompt into the AgentExecutor for PlanAndExecute agents This PR allows the user to optionally specify that they wish for the original prompt/objective to be passed into the Executor agent used by the PlanAndExecute agent. This solves a potential problem where the plan is formed referring to some context contained in the original prompt, but which is not included in the current prompt. Currently, the prompt format given to the Executor is: ``` System: Respond to the human as helpfully and accurately as possible. You have access to the following tools: <Tool and Action Description> <Output Format Description> Begin! Reminder to ALWAYS respond with a valid json blob of a single action. Use tools if necessary. Respond directly if appropriate. Format is Action:```$JSON_BLOB```then Observation:. Thought: Human: <Previous steps> <Current step> ``` This PR changes the final part after `Human:` to optionally insert the objective: ``` Human: <objective> <Previous steps> <Current step> ``` I have given a specific example in langchain-ai#5400 where the context of a database path is lost, since the plan refers to the "given path". The PR has been linted and formatted. So that existing behaviour is not changed, I have defaulted the argument to `False` and added it as the last argument in the signature, so it does not cause issues for any users passing args positionally as opposed to using keywords. Happy to take any feedback or make required changes! Fixes langchain-ai#5400 ## Who can review? Community members can review the PR once tests pass. Tag maintainers/contributors who might be interested: @vowelparrot --------- Co-authored-by: Nathan Azrak <[email protected]>
Adds the option to pass the original prompt into the AgentExecutor for PlanAndExecute agents
This PR allows the user to optionally specify that they wish for the original prompt/objective to be passed into the Executor agent used by the PlanAndExecute agent. This solves a potential problem where the plan is formed referring to some context contained in the original prompt, but which is not included in the current prompt.
Currently, the prompt format given to the Executor is:
This PR changes the final part after
Human:
to optionally insert the objective:I have given a specific example in #5400 where the context of a database path is lost, since the plan refers to the "given path".
The PR has been linted and formatted. So that existing behaviour is not changed, I have defaulted the argument to
False
and added it as the last argument in the signature, so it does not cause issues for any users passing args positionally as opposed to using keywords.Happy to take any feedback or make required changes!
Fixes #5400
Who can review?
Community members can review the PR once tests pass. Tag maintainers/contributors who might be interested:
@vowelparrot