diff --git a/examples/di/fix_github_issue.py b/examples/di/fix_github_issue.py index 8e9685e5e..726cc1ace 100644 --- a/examples/di/fix_github_issue.py +++ b/examples/di/fix_github_issue.py @@ -1,8 +1,7 @@ -# This is a real issue from MetaGPT: https://github.com/geekan/MetaGPT/issues/1067 -# with corresponding bugfix as https://github.com/geekan/MetaGPT/pull/1069 -# We demonstrate that DataInterpreter has the capability to fix such issues. -# Prerequisite: You need to manually add back the bug in your local file metagpt/utils/repair_llm_raw_output.py -# to test the DataInterpreter's issue solving ability. +"""This example is from a real issue from MetaGPT: https://github.com/geekan/MetaGPT/issues/1067 with corresponding bugfix as https://github.com/geekan/MetaGPT/pull/1069 +We demonstrate that DataInterpreter has the capability to fix such issues. +Prerequisite: You need to manually add the bug back to your local file metagpt/utils/repair_llm_raw_output.py to test DataInterpreter's debugging ability. For detail, please check the issue and PR link above. +""" import asyncio diff --git a/metagpt/tools/libs/file_manager.py b/metagpt/tools/libs/file_manager.py index c70f5ec72..aae5a6875 100644 --- a/metagpt/tools/libs/file_manager.py +++ b/metagpt/tools/libs/file_manager.py @@ -1,22 +1,26 @@ import os +import shutil +import subprocess -from pydantic import BaseModel +from pydantic import BaseModel, Field from metagpt.tools.tool_registry import register_tool class FileBlock(BaseModel): + """A block of content in a file""" + file_path: str block_content: str block_start_line: int block_end_line: int - symbol: str = "" - symbol_line: int = -1 + symbol: str = Field(default="", description="The symbol of interest in the block, empty if not applicable.") + symbol_line: int = Field(default=-1, description="The line number of the symbol in the file, -1 if not applicable") @register_tool() class FileManager: - """A tool for handling file io, read or write into files""" + """A tool for reading, understanding, writing, and editing files""" def write(self, path: str, content: str): """Write the whole content to a file.""" @@ -36,7 +40,7 @@ class FileManager: Args: symbol (str): The symbol to search. root_path (str, optional): The root path to search in. If not provided, search in the current directory. Defaults to "". - window (int, optional): The window size to return. + window (int, optional): The window size to return. Defaults to 20. Returns: FileBlock: The block containing the symbol, a pydantic BaseModel with the schema below. @@ -45,8 +49,8 @@ class FileManager: block_content: str block_start_line: int block_end_line: int - symbol: str = "" - symbol_line: int = -1 + symbol: str = Field(default="", description="The symbol of interest in the block, empty if not applicable.") + symbol_line: int = Field(default=-1, description="The line number of the symbol in the file, -1 if not applicable") """ for root, _, files in os.walk(root_path or "."): for file in files: @@ -56,7 +60,7 @@ class FileManager: with open(file_path, "r", encoding="utf-8") as f: try: lines = f.readlines() - except UnicodeDecodeError: + except Exception: continue for i, line in enumerate(lines): if symbol in line: @@ -73,19 +77,46 @@ class FileManager: ) return None - def write_content(self, file_path: str, start_line: int, end_line: int, new_block_content: str = ""): + def write_content(self, file_path: str, start_line: int, end_line: int, new_block_content: str = "") -> str: """ - Write a new block of content into a file. Use this method to update a block of code in a file. There are several cases: + Write a new block of content into a file. Use this method to update a block of code in a file. There are three cases: 1. If the new block content is empty, the original block will be deleted. - 2. If the new block content is not empty and end_line >= start_line, the original block from start_line to end_line (both inclusively) will be replaced by the new block content. - 3. If the new block content is not empty and end_line < start_line (e.g. set end_line = -1) the new block content will be inserted at start_line. + 2. If the new block content is not empty and end_line < start_line (e.g. set end_line = -1) the new block content will be inserted at start_line. + 3. If the new block content is not empty and end_line >= start_line, the original block from start_line to end_line (both inclusively) will be replaced by the new block content. + This function can sometimes be used given a FileBlock upstream. Think carefully if you want to use block_start_line or symbol_line in the FileBlock as your start_line input. Args: file_path (str): The file path to write the new block content. start_line (int): start line of the original block to be updated. end_line (int): end line of the original block to be updated. new_block_content (str): The new block content to write. + + Returns: + str: A message indicating the status of the write operation. """ + # Create a temporary copy of the file + temp_file_path = file_path + ".temp" + shutil.copy(file_path, temp_file_path) + + try: + # Modify the temporary file with the new content + self._write_content(temp_file_path, start_line, end_line, new_block_content) + + # Lint the modified temporary file + lint_passed, lint_message = self._lint_file(temp_file_path) + if not lint_passed: + return f"Linting the content at a temp file, failed with:\n{lint_message}" + + # If linting passes, overwrite the original file with the temporary file + shutil.move(temp_file_path, file_path) + return f"Content written successfully to {file_path}" + + finally: + # Clean up: Ensure the temporary file is removed if it still exists + if os.path.exists(temp_file_path): + os.remove(temp_file_path) + + def _write_content(self, file_path: str, start_line: int, end_line: int, new_block_content: str = ""): with open(file_path, "r") as file: lines = file.readlines() @@ -106,3 +137,16 @@ class FileManager: with open(file_path, "w") as file: file.writelines(lines) + + @classmethod + def _lint_file(cls, file_path: str) -> (bool, str): + """Lints an entire Python file using pylint, returns True if linting passes, along with pylint's output.""" + result = subprocess.run( + ["pylint", file_path, "--disable=all", "--enable=E"], + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + ) + lint_passed = result.returncode == 0 + lint_message = result.stdout + return lint_passed, lint_message diff --git a/tests/metagpt/tools/libs/test_file_manager.py b/tests/metagpt/tools/libs/test_file_manager.py index 4e9cdbecd..8fad587b8 100644 --- a/tests/metagpt/tools/libs/test_file_manager.py +++ b/tests/metagpt/tools/libs/test_file_manager.py @@ -43,8 +43,8 @@ def test_search_content(test_file): EXPECTED_CONTENT_AFTER_REPLACE = """ # this is line one def test_function_for_fm(): - This is the new line A replacing lines 3 to 5. - This is the new line B. + # This is the new line A replacing lines 3 to 5. + # This is the new line B. c = 3 # this is the 7th line """.strip() @@ -55,11 +55,10 @@ def test_replace_content(test_file): file_path=str(TEST_FILE_PATH), start_line=3, end_line=5, - new_block_content=" This is the new line A replacing lines 3 to 5.\n This is the new line B.", + new_block_content=" # This is the new line A replacing lines 3 to 5.\n # This is the new line B.", ) with open(TEST_FILE_PATH, "r") as f: new_content = f.read() - print(new_content) assert new_content == EXPECTED_CONTENT_AFTER_REPLACE @@ -81,7 +80,7 @@ def test_delete_content(test_file): EXPECTED_CONTENT_AFTER_INSERT = """ # this is line one def test_function_for_fm(): - This is the new line to be inserted, at line 3 + # This is the new line to be inserted, at line 3 "some docstring" a = 1 b = 2 @@ -95,8 +94,28 @@ def test_insert_content(test_file): file_path=str(TEST_FILE_PATH), start_line=3, end_line=-1, - new_block_content=" This is the new line to be inserted, at line 3", + new_block_content=" # This is the new line to be inserted, at line 3", ) with open(TEST_FILE_PATH, "r") as f: new_content = f.read() assert new_content == EXPECTED_CONTENT_AFTER_INSERT + + +def test_new_content_wrong_indentation(test_file): + msg = FileManager().write_content( + file_path=str(TEST_FILE_PATH), + start_line=3, + end_line=-1, + new_block_content=" This is the new line to be inserted, at line 3", # omit # should throw a syntax error + ) + assert "failed" in msg + + +def test_new_content_format_issue(test_file): + msg = FileManager().write_content( + file_path=str(TEST_FILE_PATH), + start_line=3, + end_line=-1, + new_block_content=" # This is the new line to be inserted, at line 3 ", # trailing spaces are format issue only, and should not throw an error + ) + assert "failed" not in msg