From e0a104dd05b0cfc13859848176ec598c1b40eeff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=BB=84=E4=BC=9F=E9=9F=AC?= Date: Thu, 12 Sep 2024 15:52:14 +0800 Subject: [PATCH 1/3] write_new_code_fail --- metagpt/prompts/di/engineer2.py | 3 +++ metagpt/roles/di/engineer2.py | 11 +++++++++-- metagpt/roles/di/role_zero.py | 2 +- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/metagpt/prompts/di/engineer2.py b/metagpt/prompts/di/engineer2.py index 73127a2be..1345f6f6b 100644 --- a/metagpt/prompts/di/engineer2.py +++ b/metagpt/prompts/di/engineer2.py @@ -103,6 +103,9 @@ WRITE_CODE_PROMPT = """ # Plan Status {plan_status} +# Current Coding File +{file_path} + # Further Instruction {instruction} diff --git a/metagpt/roles/di/engineer2.py b/metagpt/roles/di/engineer2.py index 1a224623b..6d5abfe2d 100644 --- a/metagpt/roles/di/engineer2.py +++ b/metagpt/roles/di/engineer2.py @@ -1,5 +1,6 @@ from __future__ import annotations +import re from pathlib import Path from pydantic import Field @@ -14,7 +15,7 @@ from metagpt.prompts.di.engineer2 import ( WRITE_CODE_SYSTEM_PROMPT, ) from metagpt.roles.di.role_zero import RoleZero -from metagpt.schema import UserMessage +from metagpt.schema import AIMessage, UserMessage from metagpt.strategy.experience_retriever import ENGINEER_EXAMPLE from metagpt.tools.libs.cr import CodeReview from metagpt.tools.libs.git import git_create_pull @@ -117,10 +118,16 @@ class Engineer2(RoleZero): plan_status, _ = self._get_plan_status() prompt = WRITE_CODE_PROMPT.format( user_requirement=self.planner.plan.goal, + file_path=path, plan_status=plan_status, instruction=instruction, ) - context = self.llm.format_msg(self.rc.memory.get(self.memory_k) + [UserMessage(content=prompt)]) + # Remove the json command in last message. + memory = self.rc.memory.get(self.memory_k) + pattern = r"```json.*?\s+(.*)\n```" + last_memory_content = re.sub(pattern, "", memory[-1].content, flags=re.DOTALL) + memory = memory[:-1] + [AIMessage(content=last_memory_content)] + context = self.llm.format_msg(memory + [UserMessage(content=prompt)]) async with EditorReporter(enable_llm_stream=True) as reporter: await reporter.async_report({"type": "code", "filename": Path(path).name, "src_path": path}, "meta") diff --git a/metagpt/roles/di/role_zero.py b/metagpt/roles/di/role_zero.py index ab8179618..2d1fb556a 100644 --- a/metagpt/roles/di/role_zero.py +++ b/metagpt/roles/di/role_zero.py @@ -430,7 +430,7 @@ class RoleZero(Role): for index, cmd in enumerate(commands) if index == index_of_first_exclusive or cmd["command_name"] not in self.exclusive_tool_commands ] - command_rsp = "```json\n" + json.dumps(commands, indent=4, ensure_ascii=False) + "\n```json" + command_rsp = "```json\n" + json.dumps(commands, indent=4, ensure_ascii=False) + "\n```" logger.info( "exclusive command more than one in current command list. change the command list.\n" + command_rsp ) From 09f25460be440512a8b61810010692c328d8f9bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=BB=84=E4=BC=9F=E9=9F=AC?= Date: Thu, 12 Sep 2024 16:49:32 +0800 Subject: [PATCH 2/3] ignore the lastest command in write new code. --- metagpt/roles/di/engineer2.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/metagpt/roles/di/engineer2.py b/metagpt/roles/di/engineer2.py index 6d5abfe2d..1110b61c9 100644 --- a/metagpt/roles/di/engineer2.py +++ b/metagpt/roles/di/engineer2.py @@ -1,6 +1,5 @@ from __future__ import annotations -import re from pathlib import Path from pydantic import Field @@ -15,7 +14,7 @@ from metagpt.prompts.di.engineer2 import ( WRITE_CODE_SYSTEM_PROMPT, ) from metagpt.roles.di.role_zero import RoleZero -from metagpt.schema import AIMessage, UserMessage +from metagpt.schema import UserMessage from metagpt.strategy.experience_retriever import ENGINEER_EXAMPLE from metagpt.tools.libs.cr import CodeReview from metagpt.tools.libs.git import git_create_pull @@ -122,11 +121,9 @@ class Engineer2(RoleZero): plan_status=plan_status, instruction=instruction, ) - # Remove the json command in last message. - memory = self.rc.memory.get(self.memory_k) - pattern = r"```json.*?\s+(.*)\n```" - last_memory_content = re.sub(pattern, "", memory[-1].content, flags=re.DOTALL) - memory = memory[:-1] + [AIMessage(content=last_memory_content)] + # Sometimes the Engineer repeats the last command to respond. + # Replace the last command with a manual prompt to guide the Engineer to write new code. + memory = self.rc.memory.get(self.memory_k)[:-1] context = self.llm.format_msg(memory + [UserMessage(content=prompt)]) async with EditorReporter(enable_llm_stream=True) as reporter: From 190405ac3cc68ffc74f721ee81afe7e113f54dcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=BB=84=E4=BC=9F=E9=9F=AC?= Date: Thu, 12 Sep 2024 20:40:34 +0800 Subject: [PATCH 3/3] editor use tmpfile to backup --- metagpt/prompts/di/engineer2.py | 6 ++---- metagpt/roles/di/engineer2.py | 7 +++++-- metagpt/tools/libs/editor.py | 24 ++++++++++-------------- metagpt/tools/libs/linter.py | 1 + tests/metagpt/tools/libs/test_editor.py | 23 ++++++++++++++++++++++- 5 files changed, 40 insertions(+), 21 deletions(-) diff --git a/metagpt/prompts/di/engineer2.py b/metagpt/prompts/di/engineer2.py index 1345f6f6b..f9b814a43 100644 --- a/metagpt/prompts/di/engineer2.py +++ b/metagpt/prompts/di/engineer2.py @@ -80,10 +80,8 @@ Note: """ CURRENT_STATE = """ The current editor state is: -(Editor current directory: {editor_current_directory}) -(Editor open file: {editor_open_file}) -The current terminal state is: -(Terminal current directory: {terminal_current_directory}) +(Current directory: {current_directory}) +(Open file: {editor_open_file}) """ ENGINEER2_INSTRUCTION = ROLE_INSTRUCTION + EXTRA_INSTRUCTION.strip() diff --git a/metagpt/roles/di/engineer2.py b/metagpt/roles/di/engineer2.py index 1110b61c9..2fb1b2184 100644 --- a/metagpt/roles/di/engineer2.py +++ b/metagpt/roles/di/engineer2.py @@ -58,10 +58,13 @@ class Engineer2(RoleZero): Display the current terminal and editor state. This information will be dynamically added to the command prompt. """ + current_directory = (await self.terminal.run_command("pwd")).strip() + # Synchronize Terminal and Editor Working Directories + if str(self.editor.working_dir.absolute()).strip() != current_directory: + self.editor._set_workdir(current_directory) state = { "editor_open_file": self.editor.current_file, - "editor_current_directory": self.editor.working_dir.absolute(), - "terminal_current_directory": (await self.terminal.run_command("pwd")).strip(), + "current_directory": current_directory, } self.cmd_prompt_current_state = CURRENT_STATE.format(**state).strip() diff --git a/metagpt/tools/libs/editor.py b/metagpt/tools/libs/editor.py index 63d7f9fd9..2463c563a 100644 --- a/metagpt/tools/libs/editor.py +++ b/metagpt/tools/libs/editor.py @@ -514,6 +514,8 @@ class Editor(BaseModel): temp_file_path = "" src_abs_path = file_name.resolve() first_error_line = None + # The file to store previous content and will be removed automatically. + temp_backup_file = tempfile.NamedTemporaryFile("w", delete=True) try: # lint the original file @@ -556,10 +558,8 @@ class Editor(BaseModel): # because the env var will be set AFTER the agentskills is imported if self.enable_auto_lint: # BACKUP the original file - original_file_backup_path = file_name.parent / f".backup.{file_name.name}" - with original_file_backup_path.open("w") as f: - f.writelines(lines) - + temp_backup_file.writelines(lines) + temp_backup_file.flush() lint_error, first_error_line = self._lint_file(file_name) # Select the errors caused by the modification @@ -609,15 +609,13 @@ class Editor(BaseModel): linter_error_msg=LINTER_ERROR_MSG + lint_error, window_after_applied=self._print_window(file_name, show_line, n_added_lines + 20), window_before_applied=self._print_window( - original_file_backup_path, show_line, n_added_lines + 20 + Path(temp_backup_file.name), show_line, n_added_lines + 20 ), guidance_message=guidance_message, ).strip() # recover the original file - with original_file_backup_path.open() as fin, file_name.open("w") as fout: - fout.write(fin.read()) - original_file_backup_path.unlink() + shutil.move(temp_backup_file.name, src_abs_path) return lint_error_info except FileNotFoundError as e: @@ -635,18 +633,16 @@ class Editor(BaseModel): error_info = ERROR_GUIDANCE.format( linter_error_msg=LINTER_ERROR_MSG + str(e), window_after_applied=self._print_window(file_name, start or len(lines), 40), - window_before_applied=self._print_window(original_file_backup_path, start or len(lines), 40), + window_before_applied=self._print_window(Path(temp_backup_file.name), start or len(lines), 40), guidance_message=guidance_message, ).strip() # Clean up the temporary file if an error occurs - with original_file_backup_path.open() as fin, file_name.open("w") as fout: - fout.write(fin.read()) + shutil.move(temp_backup_file.name, src_abs_path) if temp_file_path and Path(temp_file_path).exists(): Path(temp_file_path).unlink() # logger.warning(f"An unexpected error occurred: {e}") raise Exception(f"{error_info}") from e - # Update the file information and print the updated content with file_name.open("r", encoding="utf-8") as file: n_total_lines = max(1, len(file.readlines())) @@ -798,7 +794,6 @@ class Editor(BaseModel): If you need to use it multiple times, wait for the next turn. """ file_name = self._try_fix_path(file_name) - ret_str = self._edit_file_impl( file_name, start=line_number, @@ -807,6 +802,7 @@ class Editor(BaseModel): is_insert=True, is_append=False, ) + self.resource.report(file_name, "path") return ret_str def append_file(self, file_name: str, content: str) -> str: @@ -821,7 +817,6 @@ class Editor(BaseModel): If you need to use it multiple times, wait for the next turn. """ file_name = self._try_fix_path(file_name) - ret_str = self._edit_file_impl( file_name, start=None, @@ -830,6 +825,7 @@ class Editor(BaseModel): is_insert=False, is_append=True, ) + self.resource.report(file_name, "path") return ret_str def search_dir(self, search_term: str, dir_path: str = "./") -> str: diff --git a/metagpt/tools/libs/linter.py b/metagpt/tools/libs/linter.py index 7720d6962..61de7acce 100644 --- a/metagpt/tools/libs/linter.py +++ b/metagpt/tools/libs/linter.py @@ -33,6 +33,7 @@ class Linter: self.languages = dict( python=self.py_lint, sql=self.fake_lint, # base_lint lacks support for full SQL syntax. Use fake_lint to bypass the validation. + css=self.fake_lint, # base_lint lacks support for css syntax. Use fake_lint to bypass the validation. ) self.all_lint_cmd = None diff --git a/tests/metagpt/tools/libs/test_editor.py b/tests/metagpt/tools/libs/test_editor.py index 0c5d64a9a..a716b3de4 100644 --- a/tests/metagpt/tools/libs/test_editor.py +++ b/tests/metagpt/tools/libs/test_editor.py @@ -545,7 +545,7 @@ def test_edit_file_by_replace_to_palce_empty(empty_file): editor.edit_file_by_replace( file_name=str(empty_file), to_replace="", new_content=EXPECTED_CONTENT_AFTER_REPLACE_TEXT.strip() ) - assert "is empty. Use the append method to add content." in str(exc_info.value) + assert "is empty. Use the append method to add content." in str(exc_info.value) def test_append_file(temp_file_path): @@ -604,6 +604,27 @@ def test_search_dir(tmp_path): assert "Another file with different content." not in result +def test_search_dir_in_default_dir(tmp_path): + editor = Editor() + dir_path = editor.working_dir / "test_dir" + dir_path.mkdir(exist_ok=True) + + # Create some files with specific content + (dir_path / "file1.txt").write_text("This is a test file with some content.") + (dir_path / "file2.txt").write_text("Another file with different content.") + sub_dir = dir_path / "sub_dir" + sub_dir.mkdir(exist_ok=True) + (sub_dir / "file3.txt").write_text("This file is inside a sub directory with some content.") + + search_term = "some content" + + result = editor.search_dir(search_term) + + assert "file1.txt" in result + assert "file3.txt" in result + assert "Another file with different content." not in result + + def test_search_file(temp_file_path): editor = Editor() file_path = temp_file_path