[BugFix] Fix Agent update planning logic #1417
Conversation
|
@albertvillanova Can you take a look at this? this fixes this bug. |
There was a problem hiding this comment.
Thanks for the proposed fix.
Just a general comment:
If I understand correctly, your proposed solution assumes that 2 consecutive tasks are related, so the 2nd task can skip planning if the first one has already handled it.
That makes sense in some scenarios, but what about if the second task is completely unrelated to the 1st one?
|
The fix ensures that when an agent has reset to false and has previous memory (whether injected or restored), that memory is actually used during planning with the update planning prompt. Currently, the code checks step_number == 1 instead of checking for existing memory, so agent memory always gets ignored. |
| @@ -617,7 +617,8 @@ def _generate_planning_step( | |||
| } | |||
| ], | |||
| } | |||
| input_messages = [plan_update_pre] + memory_messages + [plan_update_post] | |||
| # remove last message from memory_messages because it is the current task | |||
| input_messages = [plan_update_pre] + memory_messages[:-1] + [plan_update_post] | |||
There was a problem hiding this comment.
the last message from memory_messages isn't necessarily the current task; it could be an action step I believe?
Describe the bug
Start agent.run(reset=False), the injected memory is not used in planning, llm doesn't have any of the previous conversation context.
Proposed fix
Added a test