修复QA-Engineer在RunCode(执行用例)结束后，要求Engineer WriteCode(修改代码）, Engineer未修改，业务流直接结束的问题

[bobooscar]

User description

Features
修复QA-Engineer在RunCode(执行用例)结束后，要求Engineer WriteCode(修改代码）, Engineer未修改，业务流直接结束的问题

Influence
导致过早结束业务流程，Engineer代码有问题而直接结束流程；修复后会增加代码修改流程，增加成功率，但同时会降低完成率。请慎重评估是否合入。

Result
修改后，在QA测试发现原码有问题时，Engineer继续修改代码，进行重试。

---

PR Type

Bug fix, Enhancement

---

Description

• Added and enhanced debug and warning logs across multiple roles to improve traceability and debugging.
• Increased the test rounds allowed for QA engineers to ensure thorough testing and bug fixing.
• Included RunCode in the actions that can trigger code writing, enhancing the engineer's role responsiveness.

---

Changes walkthrough 📝

Relevant files

Enhancement

base_llm.py Add Debug Logging for LLM Response                                              metagpt/provider/base_llm.py Added debug logging for the response from the language model. +1/-0      engineer.py Enhance Logging and Action Handling in Engineer Role          metagpt/roles/engineer.py Added import for RunCode. Added warning logs for cases where no action is determined or nothing gets done. Included RunCode in the set of actions that trigger writing code. +4/-1      qa_engineer.py Increase QA Engineer Test Rounds Limit                                      metagpt/roles/qa_engineer.py Increased the allowed test rounds from 5 to 20. +1/-1      role.py Improve Logging for Role Actions and Decisions                      metagpt/roles/role.py Added debug logging for actions handling and process flow. Added warning log when no actionable todo is present. +3/-1

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro]

PR Description updated to latest commit (463d2a5)

• Copy walkthrough table to "Files Changed" Tab

[codiumai-pr-agent-pro]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	3, because the PR involves multiple files and changes in the logic of the application, particularly in handling actions and responses. The changes are moderate in complexity, involving updates to logging and action handling which requires a good understanding of the existing system architecture and flow.
🧪 Relevant tests	No
⚡ Possible issues	Possible Bug: The addition of RunCode to write_code_filters without corresponding changes in other related methods or classes might lead to unexpected behavior if not handled properly elsewhere.
	Logging Overhead: The increased use of debug and warning logs could potentially lead to performance issues due to excessive logging, especially in production environments.
🔒 Security concerns	No

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Use structured logging for better performance and flexibility Consider using structured logging instead of formatting strings directly within the logger call. This approach is more flexible and performant, especially when dealing with complex data structures. metagpt/provider/base_llm.py [151] -logger.debug(f"llm {rsp=}") +logger.debug("llm response", extra={"rsp": rsp}) Suggestion importance[1-10]: 8 Why: The suggestion to use structured logging is relevant and improves the logging mechanism by making it more flexible and performant, which is crucial in a logging-heavy module like this one.	8
Maintainability	Handle unexpected todo types explicitly for better error handling Add a specific condition to handle the case when self.rc.todo is an instance of an unexpected class, which could help in debugging and maintaining the code. metagpt/roles/engineer.py [155] +if not isinstance(self.rc.todo, (WriteCodePlanAndChange, SummarizeCode)): + logger.error(f"Unexpected todo type: {type(self.rc.todo)}") logger.warning(f"engineer act got nothing done: {self.rc.todo=}") Suggestion importance[1-10]: 7 Why: Adding explicit handling for unexpected todo types is a good practice for maintainability and debugging. It helps in identifying issues early and makes the code more robust.	7
Enhancement	Validate test rounds to ensure they do not exceed the allowed limit Consider validating the test_round against test_round_allowed to ensure that the number of test rounds does not exceed the allowed limit. This can prevent potential issues in test execution flow. metagpt/roles/qa_engineer.py [35-36] test_round_allowed: int = 20 test_round: int = 0 +def increment_test_round(self): + if self.test_round < self.test_round_allowed: + self.test_round += 1 + else: + logger.error("Exceeded the allowed test rounds") + Suggestion importance[1-10]: 7 Why: Validating test_round against test_round_allowed is a crucial enhancement to prevent potential issues in test execution flow, ensuring the system adheres to defined limits.	7
Possible issue	Add checks for unexpected actions count to improve error detection Instead of logging a debug message for actions count, consider adding a condition to check if the actions count is unexpected and log a warning or error. This can help in identifying issues where the actions count is not as expected. metagpt/roles/role.py [364] -logger.debug("actions number == 1") +if len(self.actions) != 1: + logger.error(f"Unexpected number of actions: {len(self.actions)}") +else: + logger.debug("actions number == 1") Suggestion importance[1-10]: 6 Why: The suggestion to add a check for the actions count is valid and helps in error detection. However, it's a moderate improvement as it mainly adds to the robustness of the logging and error handling.	6

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 77.77778% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 70.20%. Comparing base (f201b2f) to head (463d2a5).

Files	Patch %	Lines
metagpt/roles/engineer.py	50.00%	2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1267      +/-   ##
==========================================
- Coverage   70.22%   70.20%   -0.03%     
==========================================
  Files         316      316              
  Lines       18860    18867       +7     
==========================================
  Hits        13245    13245              
- Misses       5615     5622       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[iorisa]

Engineer并未订阅RunCode消息，因此即使消息发给了Engineer，在_observe阶段也会被忽略掉。
建议参考Product Manager给Engineer转bug单的流程，将需要Engineer重写的代码作为bug单转给Engineer。参见metagpt/actions/write_prd.py的_handle_bugfix：

    async def _handle_bugfix(self, req: Document) -> Message:
        # ... bugfix logic ...
        await self.repo.docs.save(filename=BUGFIX_FILENAME, content=req.content)
        await self.repo.docs.save(filename=REQUIREMENT_FILENAME, content="")
        return AIMessage(
            content=f"A new issue is received: {BUGFIX_FILENAME}",
            cause_by=FixBug,
            send_to="Alex",  # the name of Engineer
        )

metagpt/roles/engineer.py中的_think:

        if self.config.inc and msg.cause_by in write_plan_and_change_filters:
            logger.debug(f"TODO WriteCodePlanAndChange:{msg.model_dump_json()}")
            await self._new_code_plan_and_change_action(cause_by=msg.cause_by)
            return self.rc.todo

[iorisa]

这些调试用的代码在提交前可以删掉了

[bobooscar]

个人觉得llm的回显信息在问题定位(debug级别）还是很重要的，建议保留。（终端显示默认不打印）

[bobooscar]

Engineer并未订阅RunCode消息，因此即使消息发给了Engineer，在_observe阶段也会被忽略掉。 建议参考Product Manager给Engineer转bug单的流程，将需要Engineer重写的代码作为bug单转给Engineer。参见metagpt/actions/write_prd.py的_handle_bugfix：

    async def _handle_bugfix(self, req: Document) -> Message:
        # ... bugfix logic ...
        await self.repo.docs.save(filename=BUGFIX_FILENAME, content=req.content)
        await self.repo.docs.save(filename=REQUIREMENT_FILENAME, content="")
        return AIMessage(
            content=f"A new issue is received: {BUGFIX_FILENAME}",
            cause_by=FixBug,
            send_to="Alex",  # the name of Engineer
        )

metagpt/roles/engineer.py中的_think:

        if self.config.inc and msg.cause_by in write_plan_and_change_filters:
            logger.debug(f"TODO WriteCodePlanAndChange:{msg.model_dump_json()}")
            await self._new_code_plan_and_change_action(cause_by=msg.cause_by)
            return self.rc.todo

好的，感谢！