-
Notifications
You must be signed in to change notification settings - Fork 424
feat: add diff tool and support for chrome-devtools #797
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,8 +5,9 @@ | |
|
|
||
| from ms_agent.llm.utils import Tool | ||
| from ms_agent.tools.base import ToolBase | ||
| from ms_agent.utils import get_logger | ||
| from ms_agent.utils import MAX_CONTINUE_RUNS, get_logger, retry | ||
| from ms_agent.utils.constants import DEFAULT_OUTPUT_DIR | ||
| from openai import OpenAI | ||
|
|
||
| logger = get_logger() | ||
|
|
||
|
|
@@ -21,6 +22,12 @@ def __init__(self, config, **kwargs): | |
| super(FileSystemTool, self).__init__(config) | ||
| self.exclude_func(getattr(config.tools, 'file_system', None)) | ||
| self.output_dir = getattr(config, 'output_dir', DEFAULT_OUTPUT_DIR) | ||
| if 'edit_file' not in self.exclude_functions: | ||
| self.edit_file_config = getattr(config.tools.file_system, | ||
| 'edit_file_config', None) | ||
| self.client = OpenAI( | ||
| api_key=self.edit_file_config.api_key, | ||
| base_url=self.edit_file_config.base_url) | ||
|
Comment on lines
+26
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code directly accesses file_system_config = getattr(config.tools, 'file_system', None)
self.edit_file_config = getattr(file_system_config, 'edit_file_config', None) if file_system_config else None
if not self.edit_file_config:
raise ValueError("'edit_file_config' is missing in the configuration for FileSystemTool.")
self.client = OpenAI(
api_key=self.edit_file_config.api_key,
base_url=self.edit_file_config.base_url) |
||
| self.trust_remote_code = kwargs.get('trust_remote_code', False) | ||
| self.allow_read_all_files = getattr( | ||
| getattr(config.tools, 'file_system', {}), 'allow_read_all_files', | ||
|
|
@@ -125,6 +132,65 @@ async def get_tools(self): | |
| 'required': ['path'], | ||
| 'additionalProperties': False | ||
| }), | ||
| Tool( | ||
| tool_name='edit_file', | ||
| server_name='file_system', | ||
| description= | ||
| ('Use this tool to make an edit to an existing file.\n\n' | ||
| 'This will be read by a less intelligent model, which will quickly apply the edit. ' | ||
| 'You should make it clear what the edit is, while also minimizing the unchanged code you write.\n' | ||
| 'When writing the edit, you should specify each edit in sequence, with the special comment ' | ||
| '// ... existing code ... to represent unchanged code in between edited lines.\n\n' | ||
| 'For example:\n\n// ... existing code ...\nFIRST_EDIT\n// ... existing code ...\n' | ||
| 'SECOND_EDIT\n// ... existing code ...\nTHIRD_EDIT\n// ... existing code ...\n\n' | ||
| 'You should still bias towards repeating as few lines of the original file ' | ||
| 'as possible to convey the change.\n' | ||
| 'But, each edit should contain minimally sufficient context of unchanged lines ' | ||
| "around the code you're editing to resolve ambiguity.\n" | ||
| 'DO NOT omit spans of pre-existing code (or comments) without using the ' | ||
| '// ... existing code ... comment to indicate its absence. ' | ||
| 'If you omit the existing code comment, the model may inadvertently delete these lines.\n' | ||
| 'If you plan on deleting a section, you must provide context before and after to delete it. ' | ||
| 'If the initial code is ```code \\n Block 1 \\n Block 2 \\n Block 3 \\n code```, ' | ||
| 'and you want to remove Block 2, you would output ' | ||
| '```// ... existing code ... \\n Block 1 \\n Block 3 \\n // ... existing code ...```.\n' | ||
| 'Make sure it is clear what the edit should be, and where it should be applied.\n' | ||
| 'Make edits to a file in a single edit_file call ' | ||
| 'instead of multiple edit_file calls to the same file. ' | ||
| 'The apply model can handle many distinct edits at once.' | ||
| ), | ||
| parameters={ | ||
| 'type': 'object', | ||
| 'properties': { | ||
| 'path': { | ||
| 'type': 'string', | ||
| 'description': | ||
| 'Path of the target file to modify.' | ||
| }, | ||
| 'instructions': { | ||
| 'type': | ||
| 'string', | ||
| 'description': | ||
| ('A single sentence instruction describing ' | ||
| 'what you are going to do for the sketched edit. ' | ||
| 'This is used to assist the less intelligent model in applying the edit. ' | ||
| 'Use the first person to describe what you are going to do. ' | ||
| 'Use it to disambiguate uncertainty in the edit.' | ||
| ) | ||
| }, | ||
| 'code_edit': { | ||
| 'type': | ||
| 'string', | ||
| 'description': | ||
| ('Specify ONLY the precise lines of code that you wish to edit. ' | ||
| 'NEVER specify or write out unchanged code. ' | ||
| 'Instead, represent all unchanged code using the comment of the language ' | ||
| "you're editing in - example: // ... existing code ..." | ||
| ) | ||
| } | ||
| }, | ||
| 'required': ['path', 'instructions', 'code_edit'] | ||
| }), | ||
| ] | ||
| } | ||
| return { | ||
|
|
@@ -267,3 +333,29 @@ async def list_files(self, path: str = None): | |
| except Exception as e: | ||
| return f'List files of <{path or "root path"}> failed, error: ' + str( | ||
| e) | ||
|
|
||
| @retry(max_attempts=MAX_CONTINUE_RUNS, delay=1.0) | ||
| async def edit_file(self, | ||
| path: str = None, | ||
| instructions: str = None, | ||
| code_edit: str = None): | ||
| try: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| with open(os.path.join(self.output_dir, path), 'r') as f: | ||
| initial_code = f.read() | ||
| response = self.client.chat.completions.create( | ||
| model=self.edit_file_config.diff_model, | ||
| messages=[{ | ||
| 'role': | ||
| 'user', | ||
| 'content': | ||
| (f'<instruction>{instructions}</instruction>\n' | ||
| f'<code>{initial_code}</code>\n' | ||
| f'<update>{code_edit}</update>') | ||
| }]) | ||
| merged_code = response.choices[0].message.content | ||
|
|
||
| with open(os.path.join(self.output_dir, path), 'w') as f: | ||
| f.write(merged_code) | ||
| return f'Edit file <{path}> successfully.' | ||
| except Exception as e: | ||
| return f'Edit file <{path}> failed, error: ' + str(e) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,6 @@ | |
| from contextlib import contextmanager | ||
| from typing import List, Optional | ||
|
|
||
| from file_parser import extract_code_blocks | ||
| from ms_agent.agent.runtime import Runtime | ||
| from ms_agent.callbacks import Callback | ||
| from ms_agent.llm.utils import Message | ||
|
|
@@ -26,6 +25,7 @@ def __init__(self, config: DictConfig): | |
| self.compile_round = 300 | ||
| self.cur_round = 0 | ||
| self.last_issue_length = 0 | ||
| self.devtool_prompt = getattr(config.prompt, 'devtool', None) | ||
|
|
||
| async def on_task_begin(self, runtime: Runtime, messages: List[Message]): | ||
| self.omit_intermediate_messages(messages) | ||
|
|
@@ -87,25 +87,17 @@ def check_install(): | |
| @staticmethod | ||
| def check_runtime(): | ||
| try: | ||
| os.system('pkill -f node') | ||
| if os.getcwd().endswith('backend'): | ||
| result = subprocess.run(['npm', 'run', 'dev'], | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=5, | ||
| stdin=subprocess.DEVNULL) | ||
| else: | ||
| result = subprocess.run(['npm', 'run', 'build'], | ||
| capture_output=True, | ||
| text=True, | ||
| check=True) | ||
| result = subprocess.run(['npm', 'run', 'dev'], | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=5, | ||
| stdin=subprocess.DEVNULL) | ||
|
Comment on lines
+90
to
+94
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| except subprocess.CalledProcessError as e: | ||
| output = EvalCallback._parse_e_msg(e) | ||
| except subprocess.TimeoutExpired as e: | ||
| output = EvalCallback._parse_e_msg(e) | ||
| else: | ||
| output = result.stdout + '\n' + result.stderr | ||
| os.system('pkill -f node') | ||
| return output | ||
|
|
||
| def _run_compile(self): | ||
|
|
@@ -139,12 +131,21 @@ async def on_generate_response(self, runtime: Runtime, | |
| self.last_issue_length = len(messages) - 3 - self.last_issue_length | ||
| self.omit_intermediate_messages(messages) | ||
| query = self.get_compile_feedback('frontend').strip() | ||
|
|
||
| # compile -> devtools | ||
| if not query: | ||
| human_feedback = True | ||
| query = self.get_human_feedback().strip() | ||
| feedback_type = 'devtools' | ||
| query = self.devtool_prompt | ||
| self.devtool_prompt = 'Use chrome-devtools to thoroughly test again' | ||
| else: | ||
| human_feedback = False | ||
| feedback_type = 'compling' | ||
| logger.warn(f'[Compile Feedback]: {query}]') | ||
|
|
||
| # devtools -> human | ||
| if not query: | ||
| feedback_type = 'human' | ||
| query = self.get_human_feedback().strip() | ||
|
|
||
| if not query: | ||
| self.feedback_ended = True | ||
| feedback = ( | ||
|
|
@@ -153,22 +154,11 @@ async def on_generate_response(self, runtime: Runtime, | |
| else: | ||
| all_local_files = await self.file_system.list_files() | ||
| feedback = ( | ||
| f'Feedback from {"human" if human_feedback else "compling"}: {query}\n' | ||
| f'Feedback from {feedback_type}: {query}\n' | ||
| f'The files on the local system of this project: {all_local_files}\n' | ||
| f'Now please analyze and fix this issue:\n') | ||
| messages.append(Message(role='user', content=feedback)) | ||
|
|
||
| async def on_tool_call(self, runtime: Runtime, messages: List[Message]): | ||
| design, _ = extract_code_blocks( | ||
| messages[-1].content, target_filename='design.txt') | ||
| if len(design) > 0: | ||
| front, design = messages[-1].content.split( | ||
| '```text: design.txt', maxsplit=1) | ||
| design, end = design.rsplit('```', 1) | ||
| design = design.strip() | ||
| if design: | ||
| messages[2].content = await self.do_arch_update( | ||
| runtime=runtime, messages=messages, updated_arch=design) | ||
| logger.info(messages) | ||
|
|
||
| async def after_tool_call(self, runtime: Runtime, messages: List[Message]): | ||
| runtime.should_stop = runtime.should_stop and self.feedback_ended | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current implementation for handling
Nonearguments replaces them with a dictionary{'__error__': '...'}. This is likely to cause aTypeErrorwhen the tool function is called with an unexpected__error__keyword argument. A safer approach would be to use an empty dictionary. This would result in a more informativeTypeErrorabout missing required arguments if the tool expects any, which is better than an error about an unexpected keyword.