From bcde5171e0ccf86f8a51d4c7bb28ef18093fe255 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E6=A3=92=E6=A3=92?= Date: Mon, 11 Mar 2024 13:57:25 +0800 Subject: [PATCH 1/5] refine parse_outputs in ExecuteNbCode. --- metagpt/actions/mi/execute_nb_code.py | 31 ++++++++++--------- .../actions/mi/test_execute_nb_code.py | 17 ++++++++++ 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/metagpt/actions/mi/execute_nb_code.py b/metagpt/actions/mi/execute_nb_code.py index 3a64a8bec..4644ef5d5 100644 --- a/metagpt/actions/mi/execute_nb_code.py +++ b/metagpt/actions/mi/execute_nb_code.py @@ -9,7 +9,6 @@ from __future__ import annotations import asyncio import base64 import re -import traceback from typing import Literal, Tuple import nbformat @@ -92,17 +91,17 @@ class ExecuteNbCode(Action): else: cell["outputs"].append(new_output(output_type="stream", name="stdout", text=str(output))) - def parse_outputs(self, outputs: list[str]) -> str: + def parse_outputs(self, outputs: list[str], keep_len: int = 2000) -> Tuple[bool, str]: """Parses the outputs received from notebook execution.""" assert isinstance(outputs, list) - parsed_output = "" - + parsed_output, is_success = [], True for i, output in enumerate(outputs): + is_success = "traceback" not in output.keys() if output["output_type"] == "stream" and not any( tag in output["text"] for tag in ["| INFO | metagpt", "| ERROR | metagpt", "| WARNING | metagpt", "DEBUG"] ): - parsed_output += output["text"] + ioutput, is_success = truncate(remove_escape_and_color_codes(output["text"]), keep_len, is_success) elif output["output_type"] == "display_data": if "image/png" in output["data"]: self.show_bytes_figure(output["data"]["image/png"], self.interaction) @@ -110,9 +109,15 @@ class ExecuteNbCode(Action): logger.info( f"{i}th output['data'] from nbclient outputs dont have image/png, continue next output ..." ) + ioutput, is_success = "", True elif output["output_type"] == "execute_result": - parsed_output += output["data"]["text/plain"] - return parsed_output + no_escape_color_output = remove_escape_and_color_codes(output["data"]["text/plain"]) + ioutput, is_success = truncate(no_escape_color_output, keep_len, is_success) + elif output["output_type"] == "error": + no_escape_color_output = remove_escape_and_color_codes("\n".join(output["traceback"])) + ioutput, is_success = truncate(no_escape_color_output, keep_len, is_success) + parsed_output.append(ioutput) + return is_success, ",".join(parsed_output) def show_bytes_figure(self, image_base64: str, interaction_type: Literal["ipython", None]): image_bytes = base64.b64decode(image_base64) @@ -157,7 +162,7 @@ class ExecuteNbCode(Action): await self.reset() return False, "DeadKernelError" except Exception: - return False, f"{traceback.format_exc()}" + return False, "" async def run(self, code: str, language: Literal["python", "markdown"] = "python") -> Tuple[str, bool]: """ @@ -175,13 +180,9 @@ class ExecuteNbCode(Action): # run code cell_index = len(self.nb.cells) - 1 success, error_message = await self.run_cell(self.nb.cells[-1], cell_index) - - if not success: - return truncate(remove_escape_and_color_codes(error_message), is_success=success) - - # code success - outputs = self.parse_outputs(self.nb.cells[-1].outputs) - outputs, success = truncate(remove_escape_and_color_codes(outputs), is_success=success) + success, outputs = self.parse_outputs(self.nb.cells[-1].outputs) + if error_message: + outputs = error_message + outputs if "!pip" in code: success = False diff --git a/tests/metagpt/actions/mi/test_execute_nb_code.py b/tests/metagpt/actions/mi/test_execute_nb_code.py index 3059ad3ae..98c2e5cc3 100644 --- a/tests/metagpt/actions/mi/test_execute_nb_code.py +++ b/tests/metagpt/actions/mi/test_execute_nb_code.py @@ -100,6 +100,7 @@ async def test_terminate(): is_kernel_alive = await executor.nb_client.km.is_alive() assert is_kernel_alive await executor.terminate() + import time time.sleep(2) @@ -123,3 +124,19 @@ async def test_reset(): assert is_kernel_alive await executor.reset() assert executor.nb_client.km is None + + +@pytest.mark.asyncio +async def test_parse_outputs(): + executor = ExecuteNbCode() + code = """ + import pandas as pd + df = pd.DataFrame({'ID': [1,2,3], 'NAME': ['a', 'b', 'c']}) + print(df.columns) + print(df['DUMMPY_ID']) + """ + output, is_success = await executor.run(code) + assert not is_success + assert "Index(['ID', 'NAME'], dtype='object')" in output + assert "Executed code failed," in output + assert "KeyError: 'DUMMPY_ID'" in output From 9db705f20f79de8d77192bbd8adbc7dc37b25174 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E6=A3=92=E6=A3=92?= Date: Mon, 11 Mar 2024 14:43:00 +0800 Subject: [PATCH 2/5] refine: rm truncate. --- metagpt/actions/mi/execute_nb_code.py | 38 +++++++++---------- .../actions/mi/test_execute_nb_code.py | 17 +-------- 2 files changed, 18 insertions(+), 37 deletions(-) diff --git a/metagpt/actions/mi/execute_nb_code.py b/metagpt/actions/mi/execute_nb_code.py index 4644ef5d5..2e22a7d0c 100644 --- a/metagpt/actions/mi/execute_nb_code.py +++ b/metagpt/actions/mi/execute_nb_code.py @@ -101,7 +101,7 @@ class ExecuteNbCode(Action): tag in output["text"] for tag in ["| INFO | metagpt", "| ERROR | metagpt", "| WARNING | metagpt", "DEBUG"] ): - ioutput, is_success = truncate(remove_escape_and_color_codes(output["text"]), keep_len, is_success) + ioutput, is_success = remove_escape_and_color_codes(output["text"]), True elif output["output_type"] == "display_data": if "image/png" in output["data"]: self.show_bytes_figure(output["data"]["image/png"], self.interaction) @@ -112,10 +112,24 @@ class ExecuteNbCode(Action): ioutput, is_success = "", True elif output["output_type"] == "execute_result": no_escape_color_output = remove_escape_and_color_codes(output["data"]["text/plain"]) - ioutput, is_success = truncate(no_escape_color_output, keep_len, is_success) + ioutput, is_success = no_escape_color_output, True elif output["output_type"] == "error": no_escape_color_output = remove_escape_and_color_codes("\n".join(output["traceback"])) - ioutput, is_success = truncate(no_escape_color_output, keep_len, is_success) + ioutput, is_success = no_escape_color_output, False + + # handle coroutines that are not executed asynchronously + if ioutput.strip().startswith(" keep_len and is_success: + prefix = f"Executed code successfully. Truncated to show only first {keep_len} characters\n" + ioutput = prefix + ioutput[:keep_len] + elif len(ioutput) > keep_len and not is_success: + prefix = f"Executed code failed, please reflect the cause of bug and then debug. Truncated to show only last {keep_len} characters\n" + ioutput = prefix + ioutput[-keep_len:] + parsed_output.append(ioutput) return is_success, ",".join(parsed_output) @@ -198,24 +212,6 @@ class ExecuteNbCode(Action): raise ValueError(f"Only support for language: python, markdown, but got {language}, ") -def truncate(result: str, keep_len: int = 2000, is_success: bool = True): - """对于超出keep_len个字符的result: 执行失败的代码, 展示result后keep_len个字符; 执行成功的代码, 展示result前keep_len个字符。""" - if is_success: - desc = f"Executed code successfully. Truncated to show only first {keep_len} characters\n" - else: - desc = f"Executed code failed, please reflect the cause of bug and then debug. Truncated to show only last {keep_len} characters\n" - - if result.strip().startswith(" keep_len: - result = result[-keep_len:] if not is_success else result[:keep_len] - return desc + result, is_success - - return result, is_success - - def remove_escape_and_color_codes(input_str: str): # 使用正则表达式去除jupyter notebook输出结果中的转义字符和颜色代码 # Use regular expressions to get rid of escape characters and color codes in jupyter notebook output. diff --git a/tests/metagpt/actions/mi/test_execute_nb_code.py b/tests/metagpt/actions/mi/test_execute_nb_code.py index 98c2e5cc3..4b90289ea 100644 --- a/tests/metagpt/actions/mi/test_execute_nb_code.py +++ b/tests/metagpt/actions/mi/test_execute_nb_code.py @@ -1,6 +1,6 @@ import pytest -from metagpt.actions.mi.execute_nb_code import ExecuteNbCode, truncate +from metagpt.actions.mi.execute_nb_code import ExecuteNbCode @pytest.mark.asyncio @@ -54,21 +54,6 @@ async def test_plotting_code(): assert is_success -def test_truncate(): - # 代码执行成功 - output, is_success = truncate("hello world", 5, True) - assert "Truncated to show only first 5 characters\nhello" in output - assert is_success - # 代码执行失败 - output, is_success = truncate("hello world", 5, False) - assert "Truncated to show only last 5 characters\nworld" in output - assert not is_success - # 异步 - output, is_success = truncate(" Date: Mon, 11 Mar 2024 14:51:02 +0800 Subject: [PATCH 3/5] refine: rm `is_success = "traceback" not in output.keys()` --- metagpt/actions/mi/execute_nb_code.py | 1 - 1 file changed, 1 deletion(-) diff --git a/metagpt/actions/mi/execute_nb_code.py b/metagpt/actions/mi/execute_nb_code.py index 2e22a7d0c..632f0076c 100644 --- a/metagpt/actions/mi/execute_nb_code.py +++ b/metagpt/actions/mi/execute_nb_code.py @@ -96,7 +96,6 @@ class ExecuteNbCode(Action): assert isinstance(outputs, list) parsed_output, is_success = [], True for i, output in enumerate(outputs): - is_success = "traceback" not in output.keys() if output["output_type"] == "stream" and not any( tag in output["text"] for tag in ["| INFO | metagpt", "| ERROR | metagpt", "| WARNING | metagpt", "DEBUG"] From 09e3a8e1fa98a6d03507f6f4db9ba2a0064d15fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E6=A3=92=E6=A3=92?= Date: Mon, 11 Mar 2024 17:00:44 +0800 Subject: [PATCH 4/5] refine run_cell and parse_outputs. --- metagpt/actions/mi/execute_nb_code.py | 36 ++++++++----------- .../actions/mi/test_execute_nb_code.py | 5 +-- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/metagpt/actions/mi/execute_nb_code.py b/metagpt/actions/mi/execute_nb_code.py index 632f0076c..217fc8ddc 100644 --- a/metagpt/actions/mi/execute_nb_code.py +++ b/metagpt/actions/mi/execute_nb_code.py @@ -96,11 +96,12 @@ class ExecuteNbCode(Action): assert isinstance(outputs, list) parsed_output, is_success = [], True for i, output in enumerate(outputs): + output_text = "" if output["output_type"] == "stream" and not any( tag in output["text"] for tag in ["| INFO | metagpt", "| ERROR | metagpt", "| WARNING | metagpt", "DEBUG"] ): - ioutput, is_success = remove_escape_and_color_codes(output["text"]), True + output_text = output["text"] elif output["output_type"] == "display_data": if "image/png" in output["data"]: self.show_bytes_figure(output["data"]["image/png"], self.interaction) @@ -108,28 +109,22 @@ class ExecuteNbCode(Action): logger.info( f"{i}th output['data'] from nbclient outputs dont have image/png, continue next output ..." ) - ioutput, is_success = "", True elif output["output_type"] == "execute_result": - no_escape_color_output = remove_escape_and_color_codes(output["data"]["text/plain"]) - ioutput, is_success = no_escape_color_output, True + output_text = output["data"]["text/plain"] elif output["output_type"] == "error": - no_escape_color_output = remove_escape_and_color_codes("\n".join(output["traceback"])) - ioutput, is_success = no_escape_color_output, False + output_text, is_success = "\n".join(output["traceback"]), False # handle coroutines that are not executed asynchronously - if ioutput.strip().startswith(" keep_len and is_success: - prefix = f"Executed code successfully. Truncated to show only first {keep_len} characters\n" - ioutput = prefix + ioutput[:keep_len] - elif len(ioutput) > keep_len and not is_success: - prefix = f"Executed code failed, please reflect the cause of bug and then debug. Truncated to show only last {keep_len} characters\n" - ioutput = prefix + ioutput[-keep_len:] + output_text = remove_escape_and_color_codes(output_text) + # The valid information of the exception is at the end, + # the valid information of Normal output is at the begining. + output_text = output_text[:keep_len] if is_success else output_text[-keep_len:] - parsed_output.append(ioutput) + parsed_output.append(output_text) return is_success, ",".join(parsed_output) def show_bytes_figure(self, image_base64: str, interaction_type: Literal["ipython", None]): @@ -164,7 +159,7 @@ class ExecuteNbCode(Action): """ try: await self.nb_client.async_execute_cell(cell, cell_index) - return True, "" + return self.parse_outputs(self.nb.cells[-1].outputs) except CellTimeoutError: assert self.nb_client.km is not None await self.nb_client.km.interrupt_kernel() @@ -175,7 +170,7 @@ class ExecuteNbCode(Action): await self.reset() return False, "DeadKernelError" except Exception: - return False, "" + return self.parse_outputs(self.nb.cells[-1].outputs) async def run(self, code: str, language: Literal["python", "markdown"] = "python") -> Tuple[str, bool]: """ @@ -192,10 +187,7 @@ class ExecuteNbCode(Action): # run code cell_index = len(self.nb.cells) - 1 - success, error_message = await self.run_cell(self.nb.cells[-1], cell_index) - success, outputs = self.parse_outputs(self.nb.cells[-1].outputs) - if error_message: - outputs = error_message + outputs + success, outputs = await self.run_cell(self.nb.cells[-1], cell_index) if "!pip" in code: success = False diff --git a/tests/metagpt/actions/mi/test_execute_nb_code.py b/tests/metagpt/actions/mi/test_execute_nb_code.py index 4b90289ea..2ecfbd2a2 100644 --- a/tests/metagpt/actions/mi/test_execute_nb_code.py +++ b/tests/metagpt/actions/mi/test_execute_nb_code.py @@ -68,7 +68,7 @@ async def test_run_code_text(): executor = ExecuteNbCode() message, success = await executor.run(code='print("This is a code!")', language="python") assert success - assert message == "This is a code!\n" + assert "This is a code!" in message message, success = await executor.run(code="# This is a code!", language="markdown") assert success assert message == "# This is a code!" @@ -118,10 +118,11 @@ async def test_parse_outputs(): import pandas as pd df = pd.DataFrame({'ID': [1,2,3], 'NAME': ['a', 'b', 'c']}) print(df.columns) + print(f"columns num:{len(df.columns)}") print(df['DUMMPY_ID']) """ output, is_success = await executor.run(code) assert not is_success assert "Index(['ID', 'NAME'], dtype='object')" in output - assert "Executed code failed," in output assert "KeyError: 'DUMMPY_ID'" in output + assert "columns num:2" in output From 980851136fb1e63dc6bd04c33046f68f167b4b6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E6=A3=92=E6=A3=92?= Date: Mon, 11 Mar 2024 17:07:54 +0800 Subject: [PATCH 5/5] chore --- metagpt/actions/mi/execute_nb_code.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/metagpt/actions/mi/execute_nb_code.py b/metagpt/actions/mi/execute_nb_code.py index 217fc8ddc..f6a8defbd 100644 --- a/metagpt/actions/mi/execute_nb_code.py +++ b/metagpt/actions/mi/execute_nb_code.py @@ -120,8 +120,8 @@ class ExecuteNbCode(Action): is_success = False output_text = remove_escape_and_color_codes(output_text) - # The valid information of the exception is at the end, - # the valid information of Normal output is at the begining. + # The useful information of the exception is at the end, + # the useful information of normal output is at the begining. output_text = output_text[:keep_len] if is_success else output_text[-keep_len:] parsed_output.append(output_text)