OpenBMB / ChatDev

Create Customized Software using Natural Language Idea (through LLM-powered Multi-Agent Collaboration)
https://arxiv.org/abs/2307.07924
Apache License 2.0
24.38k stars 3.06k forks source link

There is a bug in the break_cycle logic in CodeReview #387

Open snakeytm opened 2 weeks ago

snakeytm commented 2 weeks ago

composed_phase.py

class CodeReview(ComposedPhase):
    def break_cycle(self, phase_env) -> bool:
        if "<INFO> Finished".lower() in phase_env['modification_conclusion'].lower():
            return True
        else:
            return False

According to the prompt, " Finished" is only returned during the CodeReviewComment stage, and the conclusion of this stage will be stored in chat_env.env_dict['review_comments']. There seems to be a bug in the current code logic. I believe the correct code should be as follows:

class CodeReview(ComposedPhase):
    def break_cycle(self, phase_env) -> bool:
        # 修复bug,这个标识来自于review_comments
        if "<INFO> Finished".lower() in phase_env['review_comments'].lower():
            return True
        else:
            return False
thinkwee commented 2 weeks ago

thank you! you are right. could u please make a pr on it?

snakeytm commented 2 weeks ago

"Okay, my solution above still has some issues. I'll think about how to fix it."

snakeytm commented 2 weeks ago

I've found the issue. Here's a better solution: Move the code line self.phase_env['modification_conclusion'] = self.seminar_conclusion from the update_chat_env method in the CodeReviewModification class in phase.py to the update_chat_env method in the CodeReviewComment class.

snakeytm commented 2 weeks ago

Based on the previous issue, another problem has been identified: In the chatting method of the Phase class in the phase.py file, there is a line of code: seminar_conclusion = seminar_conclusion.split("")[-1]. Even if the agent returns " Finished", this line of code will result in "Finished", which does not conform to the break_cycle judgment logic, causing the loop to be unable to exit.

snakeytm commented 2 weeks ago

I have an idea that might solve this problem. We can categorize the dialogue conclusion markers in the prompt into two types. One type needs to be included in the context to provide information for downstream use, such as " PowerPoint" in DemandAnalysis. The other type is just a marker for ending, such as " Finished" in CodeReviewComment. I suggest changing " Finished" in CodeReviewComment to " Finished". @thinkwee What do you think?