search

Pythagora-io / gpt-pilot

The first real AI developer
Other
29.08k stars 2.91k forks source link

Prevent loops in task reviewer #941

Closed gperetin closed 1 month ago

gperetin commented 1 month ago

I'm not sure this is correct, having a hard time with the verification.

I was able to get it to have a review source, and it doesn't get stuck, but I wasn't able to get a reproduction on the current state.

I also have some error entries that seem related, but I've had those before this change as well, which is interesting.

2024-05-23 14:35:53,981 WARNING [core.agents.developer] Unfinished steps found before the next iteration is broken down: [{'id': '53a7b3eb566a433bb1579eeb39565a38', 'completed': False, 'type': 'review_task', 'source': 'feature'}, {'id': 'f114d468eca54614b5bba715ebd6aaa9', 'completed': False, 'type': 'create_readme', 'source': 'feature'}]

README does get updated at the end of the task so I think that part is correct.

senko commented 1 month ago

Unfinished steps found before the next iteration is broken down

This happens because earlier the only time iteration breakdown function should be called is after all the steps are done and then Troubleshooter is run. Then the Troubleshooter asks the user to test the app and if there's a problem, creates an "iteration" which is an attempt at fixing the problem.

However, now this function is also run because TaskReviewer asks Developer to do so (via an AgentResponse). At the time when The Task Reviewer does so, there are two remaining steps:

These are the two steps visible in the warning. Now, the important bit here is that if one agent calls another (or rather, tells Orchestrator it needs something, and then Orchestrator calls the aother agent), the current step is not committed. So even though TaskReviewer marked its own step as complete, that's till in the "next state" by the time the iteration breakdown function is ran.

The solution is simply to remove the warning. It's now obsolete :)

LeonOstrez commented 1 month ago

@gperetin you can remove the warning then and this is ready for merge. Did you manage to test this works as intended?

gperetin commented 1 month ago

@gperetin you can remove the warning then and this is ready for merge. Did you manage to test this works as intended?

I have not, got stuck on that missing prompt template issue. I just sent out a PR for the LLM check, so getting back on this now.

gperetin commented 1 month ago

I was able to test this and works as expected, meaning, after the troubleshooter step, README was not updated.