From cb6484d01dba08ec7855e33d28f7bd071707e396 Mon Sep 17 00:00:00 2001 From: garylin2099 Date: Mon, 13 May 2024 15:11:43 +0800 Subject: [PATCH] fix plan & task bugs, add task experience, update editor & browser, test issue fixing ability --- metagpt/actions/di/execute_nb_code.py | 2 + metagpt/roles/di/data_analyst.py | 10 ++++- metagpt/schema.py | 7 ++++ metagpt/strategy/experience_retriever.py | 37 ++++++++++++++++--- metagpt/strategy/thinking_command.py | 8 +++- metagpt/tools/libs/browser.py | 20 +++++++--- metagpt/tools/libs/editor.py | 10 +++-- metagpt/tools/libs/git.py | 2 +- .../environment/mgx_env/run_mgx_env.py | 35 ++++++++++++------ 9 files changed, 102 insertions(+), 29 deletions(-) diff --git a/metagpt/actions/di/execute_nb_code.py b/metagpt/actions/di/execute_nb_code.py index 1df17f2c6..b4fe949fe 100644 --- a/metagpt/actions/di/execute_nb_code.py +++ b/metagpt/actions/di/execute_nb_code.py @@ -246,6 +246,8 @@ class ExecuteNbCode(Action): if "!pip" in code: success = False outputs = outputs[-INSTALL_KEEPLEN:] + elif "git clone" in code: + outputs = outputs[:INSTALL_KEEPLEN] + "..." + outputs[-INSTALL_KEEPLEN:] elif language == "markdown": # add markdown content to markdown cell in a notebook. diff --git a/metagpt/roles/di/data_analyst.py b/metagpt/roles/di/data_analyst.py index 6385bd349..df0b8d288 100644 --- a/metagpt/roles/di/data_analyst.py +++ b/metagpt/roles/di/data_analyst.py @@ -70,8 +70,7 @@ class DataAnalyst(DataInterpreter): example = KeywordExpRetriever().retrieve(self.user_requirement) else: self.working_memory.add_batch(self.rc.news) - context = "\n\n".join([str(mem) for mem in self.working_memory.get()]) - example = KeywordExpRetriever().retrieve(context) + # TODO: implement experience retrieval in multi-round setting plan_status = self.planner.plan.model_dump(include=["goal", "tasks"]) for task in plan_status["tasks"]: @@ -95,6 +94,13 @@ class DataAnalyst(DataInterpreter): async def _act(self) -> Message: """Useful in 'react' mode. Return a Message conforming to Role._act interface.""" logger.info(f"ready to take on task {self.planner.plan.current_task}") + + # TODO: Consider an appropriate location to insert task experience formally + experience = KeywordExpRetriever().retrieve(self.planner.plan.current_task.instruction, exp_type="task") + if experience and experience not in [msg.content for msg in self.rc.working_memory.get()]: + exp_msg = Message(content=experience, role="assistant") + self.rc.working_memory.add(exp_msg) + code, result, is_success = await self._write_and_exec_code() self.planner.plan.current_task.is_success = ( is_success # mark is_success, determine is_finished later in thinking diff --git a/metagpt/schema.py b/metagpt/schema.py index 1b527b594..5af16bc38 100644 --- a/metagpt/schema.py +++ b/metagpt/schema.py @@ -514,6 +514,13 @@ class Plan(BaseModel): if task_id in self.task_map: task = self.task_map[task_id] task.reset() + # reset all downstream tasks that are dependent on the reset task + for dep_task in self.tasks: + if task_id in dep_task.dependent_task_ids: + # FIXME: if LLM generates cyclic tasks, this will result in infinite recursion + self.reset_task(dep_task.task_id) + + self._update_current_task() def replace_task(self, new_task: Task): """ diff --git a/metagpt/strategy/experience_retriever.py b/metagpt/strategy/experience_retriever.py index 7beb1b404..614a26d16 100644 --- a/metagpt/strategy/experience_retriever.py +++ b/metagpt/strategy/experience_retriever.py @@ -1,3 +1,5 @@ +from typing import Literal + from pydantic import BaseModel @@ -159,11 +161,15 @@ class SimpleExpRetriever(ExpRetriever): class KeywordExpRetriever(ExpRetriever): """An experience retriever that returns examples based on keywords in the context.""" - def retrieve(self, context: str) -> str: - if "deploy" in context.lower(): - return DEPLOY_EXAMPLE - elif "issue" in context.lower(): - return FIX_ISSUE_EXAMPLE + def retrieve(self, context: str, exp_type: Literal["plan", "task"] = "plan") -> str: + if exp_type == "plan": + if "deploy" in context.lower(): + return DEPLOY_EXAMPLE + elif "issue" in context.lower(): + return FIX_ISSUE_EXAMPLE + elif exp_type == "task": + if "diagnose" in context.lower(): + return SEARCH_SYMBOL_EXAMPLE return "" @@ -257,3 +263,24 @@ Explanation: The requirement is for software development, focusing on fixing an ] ``` """ + + +SEARCH_SYMBOL_EXAMPLE = """ +## Past Experience +Issue: PaiEasChatEndpoint._call_eas should return bytes type instead of str type +Explanation: To understand the issue, we first need to know the content of the method in the codebase. Therefore, we search the symbol `def _call_eas` in the cloned repo langchain. + +```python +from metagpt.tools.libs.editor import Editor + +# Initialize the Editor tool +editor = Editor() + +# Search for the PaiEasChatEndpoint._call_eas method in the codebase to understand the issue +symbol_to_search = "def _call_eas" +file_block = editor.search_content(symbol=symbol_to_search, root_path="langchain") + +# Output the file block containing the method to diagnose the problem +file_block +``` +""" diff --git a/metagpt/strategy/thinking_command.py b/metagpt/strategy/thinking_command.py index cde16e714..ae689eb1f 100644 --- a/metagpt/strategy/thinking_command.py +++ b/metagpt/strategy/thinking_command.py @@ -96,7 +96,13 @@ def run_plan_command(role: Role, cmd: list[dict]): elif cmd["command_name"] == Command.RESET_TASK.cmd_name: role.planner.plan.reset_task(**cmd["args"]) elif cmd["command_name"] == Command.REPLACE_TASK.cmd_name: - role.planner.plan.replace_task(Task(**cmd["args"])) + new_task = Task( + task_id=cmd["args"]["task_id"], + dependent_task_ids=cmd["args"]["new_dependent_task_ids"], + instruction=cmd["args"]["new_instruction"], + assignee=cmd["args"]["new_assignee"], + ) + role.planner.plan.replace_task(new_task) elif cmd["command_name"] == Command.FINISH_CURRENT_TASK.cmd_name: if role.planner.plan.is_plan_finished(): return diff --git a/metagpt/tools/libs/browser.py b/metagpt/tools/libs/browser.py index 6a7fd8e3b..7fde804fe 100644 --- a/metagpt/tools/libs/browser.py +++ b/metagpt/tools/libs/browser.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from playwright.async_api import async_playwright from metagpt.const import DEFAULT_WORKSPACE_ROOT @@ -5,10 +7,10 @@ from metagpt.tools.tool_registry import register_tool from metagpt.utils.report import BrowserReporter -@register_tool() +@register_tool(tags=["web", "browse", "scrape"]) class Browser: """ - A tool for browsing the web. Don't initialize a new instance of this class if one already exists. + A tool for browsing the web and scraping. Don't initialize a new instance of this class if one already exists. Note: Combine searching and scrolling together to achieve most effective browsing. DON'T stick to one method. """ @@ -31,7 +33,7 @@ class Browser: self.current_page = page self.current_page_url = url print("Now on page ", url) - print(await self._view()) + await self._view() async def open_new_page(self, url: str): """open a new page in the browser and view the page""" @@ -51,6 +53,12 @@ class Browser: else: print(f"Page not found: {url}") + async def _view_page_html(self, keep_len: int = 5000) -> str: + """view the HTML content of current page, return the HTML content as a string. When executed, the content will be printed out""" + html = await self.current_page.content() + html_content = html.strip()[:keep_len] + return html_content + async def search_content_all(self, search_term: str) -> list[dict]: """search all occurences of search term in the current page and return the search results with their position. Useful if you have a keyword or sentence in mind and want to quickly narrow down the content relevant to it. @@ -142,10 +150,12 @@ class Browser: await self.current_page.screenshot(path=path) print(f"Screenshot saved to: {path}") - async def _view(self) -> str: + async def _view(self, keep_len: int = 5000) -> str: """simulate human viewing the current page, return the visible text with links""" visible_text_with_links = await self.current_page.evaluate(VIEW_CONTENT_JS) - return visible_text_with_links + print("The visible text and their links (if any): ", visible_text_with_links[:keep_len]) + # html_content = await self._view_page_html(keep_len=keep_len) + # print("The html content: ", html_content) async def scroll_current_page(self, offset: int = 500): """scroll the current page by offset pixels, negative value means scrolling up, will print out observed content after scrolling""" diff --git a/metagpt/tools/libs/editor.py b/metagpt/tools/libs/editor.py index 9df4acfdc..f973bad91 100644 --- a/metagpt/tools/libs/editor.py +++ b/metagpt/tools/libs/editor.py @@ -16,7 +16,9 @@ class FileBlock(BaseModel): block_start_line: int block_end_line: int 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") + symbol_start_line: int = Field( + default=-1, description="The line number of the symbol in the file, -1 if not applicable" + ) @register_tool() @@ -57,7 +59,7 @@ class Editor: block_start_line: int block_end_line: int 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") + symbol_start_line: int = Field(default=-1, description="The line number of the symbol in the file, -1 if not applicable") """ if not os.path.exists(root_path): print(f"Currently at {os.getcwd()}. Path {root_path} does not exist.") @@ -83,7 +85,7 @@ class Editor: block_start_line=start + 1, block_end_line=end + 1, symbol=symbol, - symbol_line=i + 1, + symbol_start_line=i + 1, ) self.resource.report(result.file_path, "path") return result @@ -98,7 +100,7 @@ class Editor: 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 (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. + This function can sometimes be used given a FileBlock upstream. Think carefully if you want to use block_start_line or symbol_start_line in the FileBlock as your start_line input. Your new_block_content will be placed at the start_line. Args: file_path (str): The file path to write the new block content. diff --git a/metagpt/tools/libs/git.py b/metagpt/tools/libs/git.py index f6e02e40f..ae10e969e 100644 --- a/metagpt/tools/libs/git.py +++ b/metagpt/tools/libs/git.py @@ -57,7 +57,7 @@ async def git_push( return branch -@register_tool(tags=["software development", "git", "create a git pull/merge request"]) +@register_tool(tags=["software development", "git", "create a git pull request or merge request"]) async def git_create_pull( base: str, head: str, diff --git a/tests/metagpt/environment/mgx_env/run_mgx_env.py b/tests/metagpt/environment/mgx_env/run_mgx_env.py index 86678c97c..f0fdbc244 100644 --- a/tests/metagpt/environment/mgx_env/run_mgx_env.py +++ b/tests/metagpt/environment/mgx_env/run_mgx_env.py @@ -1,20 +1,15 @@ import asyncio +import os import threading from metagpt.environment.mgx.mgx_env import MGXEnv -from metagpt.roles import ( - Architect, - Engineer, - ProductManager, - ProjectManager, - QaEngineer, -) +from metagpt.roles import Architect, Engineer, ProductManager, ProjectManager from metagpt.roles.di.data_analyst import DataAnalyst from metagpt.roles.di.team_leader import TeamLeader from metagpt.schema import Message -async def main(requirement, enable_human_input=False): +async def main(requirement="", enable_human_input=False): env = MGXEnv() env.add_roles( [ @@ -23,7 +18,7 @@ async def main(requirement, enable_human_input=False): Architect(), ProjectManager(), Engineer(n_borg=5, use_code_review=False), - QaEngineer(), + # QaEngineer(), DataAnalyst(tools=[""]), ] ) @@ -32,7 +27,9 @@ async def main(requirement, enable_human_input=False): # simulate human sending messages in chatbox send_human_input(env) - env.publish_message(Message(content=requirement)) + if requirement: + env.publish_message(Message(content=requirement)) + # env.publish_message(Message(content=requirement, send_to={"David"}), user_defined_recipient="David") while not env.is_idle: await env.run() @@ -70,9 +67,25 @@ data_path = "data/titanic" train_path = f"{data_path}/split_train.csv" eval_path = f"{data_path}/split_eval.csv" TITANIC_REQ = f"This is a titanic passenger survival dataset, your goal is to predict passenger survival outcome. The target column is Survived. Perform data analysis, data preprocessing, feature engineering, and modeling to predict the target. Report accuracy on the eval data. Train data path: '{train_path}', eval data path: '{eval_path}'." +FIX_ISSUE = """ +Write a fix for this issue: https://github.com/langchain-ai/langchain/issues/20453, +you can fix it on this repo https://github.com/garylin2099/langchain, +checkout a branch named test-fix, commit your changes, push, and create a PR to the master branch of https://github.com/iorisa/langchain +""" +FIX_ISSUE_SIMPLE = """ +Write a fix for this issue: https://github.com/mannaandpoem/simple_calculator/issues/1, +you can fix it on this repo https://github.com/garylin2099/simple_calculator, +checkout a branch named test, commit your changes, push, and create a PR to the original repo. +""" +PUSH_PR_REQ = """ +clone https://github.com/garylin2099/simple_calculator, checkout a new branch named test-branch, add an empty file test_file.py to the repo. +Commit your changes and push, finally, create a PR to the master branch of https://github.com/mannaandpoem/simple_calculator. +""" if __name__ == "__main__": + # NOTE: Add access_token to test github issue fixing + os.environ["access_token"] = "ghp_xxx" # NOTE: Change the requirement to the one you want to test # Set enable_human_input to True if you want to simulate sending messages in chatbox - asyncio.run(main(requirement=SIMPLE_REQ, enable_human_input=False)) + asyncio.run(main(requirement=FIX_ISSUE, enable_human_input=False))