Skip to content

feat: append_file incl. all tests [agentskills] #2346

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

Merged
merged 13 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions agenthub/codeact_agent/prompt.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
COMMAND_DOCS = (
'\nApart from the standard Python library, the assistant can also use the following functions (already imported) in <execute_ipython> environment:\n'
f'{_AGENT_SKILLS_DOCS}'
"Please note that THE `edit_file` FUNCTION REQUIRES PROPER INDENTATION. If the assistant would like to add the line ' print(x)', it must fully write that out, with all those spaces before the code! Indentation is important and code that is not indented correctly will fail and require fixing before it can be run."
"Please note that THE `edit_file` and `append_file` FUNCTIONS REQUIRE PROPER INDENTATION. If the assistant would like to add the line ' print(x)', it must fully write that out, with all those spaces before the code! Indentation is important and code that is not indented correctly will fail and require fixing before it can be run."
)

# ======= SYSTEM MESSAGE =======
Expand Down Expand Up @@ -74,7 +74,7 @@ def index():

if __name__ == '__main__':
app.run(port=5000)\"\"\"
edit_file(start=1, end=1, content=EDITED_CODE)
edit_file('app.py', start=1, end=1, content=EDITED_CODE)
</execute_ipython>

USER:
Expand Down Expand Up @@ -213,7 +213,7 @@ def index():
ASSISTANT:
I should edit the file to display the numbers in a table format. I should include correct indentation. Let me update the file:
<execute_ipython>
edit_file(start=7, end=7, content=" return '<table>' + ''.join([f'<tr><td>{i}</td></tr>' for i in numbers]) + '</table>'")
edit_file('app.py', start=7, end=7, content=" return '<table>' + ''.join([f'<tr><td>{i}</td></tr>' for i in numbers]) + '</table>'")
</execute_ipython>

USER:
Expand Down
193 changes: 165 additions & 28 deletions opendevin/runtime/plugins/agent_skills/agentskills.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@
- search_dir(search_term, dir_path='./'): Searches for a term in all files in the specified directory.
- search_file(search_term, file_path=None): Searches for a term in the specified file or the currently open file.
- find_file(file_name, dir_path='./'): Finds all files with the given name in the specified directory.
- edit_file(start, end, content): Replaces lines in a file with the given content.
- edit_file(file_name, start, end, content): Replaces lines in a file with the given content.
- append_file(file_name, content): Appends given content to a file.
"""

import base64
import functools
import os
import subprocess
import tempfile
from inspect import signature
from typing import Optional

Expand Down Expand Up @@ -66,12 +68,13 @@ def wrapper(*args, **kwargs):
return wrapper


def _lint_file(file_path: str) -> Optional[str]:
def _lint_file(file_path: str) -> tuple[Optional[str], Optional[int]]:
"""
Lint the file at the given path.
Lint the file at the given path and return a tuple with a boolean indicating if there are errors,
and the line number of the first error, if any.

Returns:
Optional[str]: A string containing the linting report if the file failed to lint, None otherwise.
tuple[str, Optional[int]]: (lint_error, first_error_line_number)
"""

if file_path.endswith('.py'):
Expand All @@ -91,13 +94,28 @@ def _lint_file(file_path: str) -> Optional[str]:
)
if result.returncode == 0:
# Linting successful. No issues found.
return None
else:
ret = 'ERRORS:\n'
ret += result.stdout.decode().strip()
return ret.rstrip('\n')
return None, None

# Extract the line number from the first error message
error_message = result.stdout.decode().strip()
lint_error = 'ERRORS:\n' + error_message
first_error_line = None
for line in error_message.split('\n'):
if line.strip():
# The format of the error message is: <filename>:<line>:<column>: <error code> <error message>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love this!

parts = line.split(':')
if len(parts) >= 2:
try:
first_error_line = int(parts[1])
break
except ValueError:
# Not a valid line number, continue to the next line
continue

return lint_error, first_error_line

# Not a python file, skip linting
return None
return None, None


def _print_window(CURRENT_FILE, CURRENT_LINE, WINDOW, return_str=False):
Expand Down Expand Up @@ -247,25 +265,26 @@ def create_file(filename: str) -> None:


@update_pwd_decorator
def edit_file(start: int, end: int, content: str) -> None:
def edit_file(file_name: str, start: int, end: int, content: str) -> None:
"""Edit a file.

It replaces lines `start` through `end` (inclusive) with the given text `content` in the open file. Remember, the file must be open before editing.
Replaces in given file `file_name` the lines `start` through `end` (inclusive) with the given text `content`.

Args:
file_name: str: The name of the file to edit.
start: int: The start line number. Must satisfy start >= 1.
end: int: The end line number. Must satisfy start <= end <= number of lines in the file.
content: str: The content to replace the lines with.
"""
global CURRENT_FILE, CURRENT_LINE, WINDOW
if not CURRENT_FILE or not os.path.isfile(CURRENT_FILE):
raise FileNotFoundError('No file open. Use the open_file function first.')
if not os.path.isfile(file_name):
raise FileNotFoundError(f'File {file_name} not found.')

# Load the file
with open(CURRENT_FILE, 'r') as file:
with open(file_name, 'r') as file:
lines = file.readlines()

ERROR_MSG = f'[Error editing opened file {CURRENT_FILE}. Please confirm the opened file is correct.]'
ERROR_MSG = f'[Error editing file {file_name}. Please confirm the file is correct.]'
ERROR_MSG_SUFFIX = (
'Your changes have NOT been applied. Please fix your edit command and try again.\n'
'You either need to 1) Open the correct file and try again or 2) Specify the correct start/end line arguments.\n'
Expand Down Expand Up @@ -296,24 +315,29 @@ def edit_file(start: int, end: int, content: str) -> None:
return

edited_content = content + '\n'
n_edited_lines = len(edited_content.split('\n'))
new_lines = lines[: start - 1] + [edited_content] + lines[end:]

# directly write edited lines to the file
with open(CURRENT_FILE, 'w') as file:
with open(file_name, 'w') as file:
file.writelines(new_lines)

# set current line to the center of the edited lines
CURRENT_LINE = (start + end) // 2
first_error_line = None

# Handle linting
if ENABLE_AUTO_LINT:
# BACKUP the original file
original_file_backup_path = os.path.join(
os.path.dirname(CURRENT_FILE), f'.backup.{os.path.basename(CURRENT_FILE)}'
os.path.dirname(file_name), f'.backup.{os.path.basename(file_name)}'
)
with open(original_file_backup_path, 'w') as f:
f.writelines(lines)

lint_error = _lint_file(CURRENT_FILE)
if lint_error:
lint_error, first_error_line = _lint_file(file_name)
if lint_error is not None:
if first_error_line is not None:
CURRENT_LINE = int(first_error_line)
# only change any literal strings here in combination with unit tests!
print(
'[Your proposed edit has introduced new syntax error(s). Please understand the errors and retry your edit command.]'
Expand All @@ -322,8 +346,8 @@ def edit_file(start: int, end: int, content: str) -> None:

print('[This is how your edit would have looked if applied]')
print('-------------------------------------------------')
cur_line = (n_edited_lines // 2) + start
_print_window(CURRENT_FILE, cur_line, 10)
cur_line = first_error_line
_print_window(file_name, cur_line, 10)
print('-------------------------------------------------\n')

print('[This is the original code before your edit]')
Expand All @@ -339,25 +363,137 @@ def edit_file(start: int, end: int, content: str) -> None:

# recover the original file
with open(original_file_backup_path, 'r') as fin, open(
CURRENT_FILE, 'w'
file_name, 'w'
) as fout:
fout.write(fin.read())
os.remove(original_file_backup_path)
return

os.remove(original_file_backup_path)

with open(CURRENT_FILE, 'r') as file:
# Update the file information and print the updated content
with open(file_name, 'r') as file:
n_total_lines = len(file.readlines())
# set current line to the center of the edited lines
CURRENT_LINE = (start + end) // 2
if first_error_line is not None and int(first_error_line) > 0:
CURRENT_LINE = first_error_line
else:
CURRENT_LINE = n_total_lines
print(
f'[File: {os.path.abspath(CURRENT_FILE)} ({n_total_lines} lines total after edit)]'
f'[File: {os.path.abspath(file_name)} ({n_total_lines} lines total after edit)]'
)
CURRENT_FILE = file_name
_print_window(CURRENT_FILE, CURRENT_LINE, WINDOW)
print(MSG_FILE_UPDATED)


@update_pwd_decorator
def append_file(file_name: str, content: str) -> None:
"""Append content to the given file.

It appends text `content` to the end of the specified file.

Args:
file_name: str: The name of the file to append to.
content: str: The content to append to the file.
"""
global CURRENT_FILE, CURRENT_LINE, WINDOW
if not os.path.isfile(file_name):
raise FileNotFoundError(f'File {file_name} not found.')

# Use a temporary file to write changes
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not simply append?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thought was that it would be a change in behavior, as the other methods error if a file wasn't explicitly opened and/or created first.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. This behavior will be changed soon anyways, right? I mean, you will revert this part in a follow-up PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same as in edit_file. I didn't know this was about to be changed.

Copy link
Collaborator Author

@tobitege tobitege Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @li-boxuan @xingyaoww
Till now the behavior is that only existing files could be edited and the LLM was supposed to use create_file and/or open_file for that.
I'd just like to confirm before I'd remove that exception for non-existing files, because that would require more changes again to prompts etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was already thinking of refactoring the append/edit methods so that both just call one internal method with an extra bool for append, but wanted to do this in followup PR, no problem.

The append is really just to make it easier for the LLM since there were often cases where it tried to start at "end of file + 1" line, got an exception, and had to do it again, thus waisting time+money.

Copy link
Collaborator

@xingyaoww xingyaoww Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense! My inclination is that ideally, we can do the refractor, if possible, merge it - and when I finish the swe-bench eval, I'll re-run the eval from the start to make sure we are not degrading perf.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just prefer to get this merged before I do a refactor and in the meantime another patch comes along with changed unit/integration tests. That takes a lot of time to redo.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve the PR to unblock you :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a 2nd reviewer 🤣

temp_file_path = ''
first_error_line = None
try:
# Create a temporary file
with tempfile.NamedTemporaryFile('w', delete=False) as temp_file:
temp_file_path = temp_file.name

# Read the original file and check if empty and for a trailing newline
with open(file_name, 'r') as original_file:
lines = original_file.readlines()

if lines and not (len(lines) == 1 and lines[0].strip() == ''):
if not lines[-1].endswith('\n'):
lines[-1] += '\n'
content = ''.join(lines) + content
else:
content = content

if not content.endswith('\n'):
content += '\n'

# Append the new content with a trailing newline
temp_file.write(content)

# Replace the original file with the temporary file atomically
os.replace(temp_file_path, file_name)

# Handle linting
if ENABLE_AUTO_LINT:
# BACKUP the original file
original_file_backup_path = os.path.join(
os.path.dirname(file_name),
f'.backup.{os.path.basename(file_name)}',
)
with open(original_file_backup_path, 'w') as f:
f.writelines(lines)

lint_error, first_error_line = _lint_file(file_name)
if lint_error is not None:
if first_error_line is not None:
CURRENT_LINE = int(first_error_line)
print(
'[Your proposed edit has introduced new syntax error(s). Please understand the errors and retry your edit command.]'
)
print(lint_error)

print('[This is how your edit would have looked if applied]')
print('-------------------------------------------------')
_print_window(file_name, CURRENT_LINE, 10)
print('-------------------------------------------------\n')

print('[This is the original code before your edit]')
print('-------------------------------------------------')
_print_window(original_file_backup_path, CURRENT_LINE, 10)
print('-------------------------------------------------')

print(
'Your changes have NOT been applied. Please fix your edit command and try again.\n'
'You need to correct your added code.\n'
'DO NOT re-run the same failed edit command. Running it again will lead to the same error.'
)

# recover the original file
with open(original_file_backup_path, 'r') as fin, open(
file_name, 'w'
) as fout:
fout.write(fin.read())
os.remove(original_file_backup_path)
return

except Exception as e:
# Clean up the temporary file if an error occurs
if temp_file_path and os.path.exists(temp_file_path):
os.remove(temp_file_path)
raise e

# Update the file information and print the updated content
with open(file_name, 'r', encoding='utf-8') as file:
n_total_lines = len(file.readlines())
if first_error_line is not None and int(first_error_line) > 0:
CURRENT_LINE = first_error_line
else:
CURRENT_LINE = n_total_lines
print(
f'[File: {os.path.abspath(file_name)} ({n_total_lines} lines total after edit)]'
)
CURRENT_FILE = file_name
_print_window(CURRENT_FILE, CURRENT_LINE, WINDOW)
print(
'[File updated. Please review the changes and make sure they are correct (correct indentation, no duplicate lines, etc). Edit the file again if necessary.]'
)


@update_pwd_decorator
def search_dir(search_term: str, dir_path: str = './') -> None:
"""Searches for search_term in all files in dir. If dir is not provided, searches in the current directory.
Expand Down Expand Up @@ -672,6 +808,7 @@ def parse_pptx(file_path: str) -> None:
'scroll_down',
'scroll_up',
'create_file',
'append_file',
'edit_file',
'search_dir',
'search_file',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,18 @@ create_file(filename: str) -> None:
Args:
filename: str: The name of the file to create.

edit_file(start: int, end: int, content: str) -> None:
append_file(file_name: str, content: str) -> None:
Append content to the given file.
It appends text `content` to the end of the specified file.
Args:
file_name: str: The name of the file to append to.
content: str: The content to append to the file.

edit_file(file_name: str, start: int, end: int, content: str) -> None:
Edit a file.
It replaces lines `start` through `end` (inclusive) with the given text `content` in the open file. Remember, the file must be open before editing.
Replaces in given file `file_name` the lines `start` through `end` (inclusive) with the given text `content`.
Args:
file_name: str: The name of the file to edit.
start: int: The start line number. Must satisfy start >= 1.
end: int: The end line number. Must satisfy start <= end <= number of lines in the file.
content: str: The content to replace the lines with.
Expand Down Expand Up @@ -97,7 +105,7 @@ parse_pptx(file_path: str) -> None:
Args:
file_path: str: The path to the file to open.

Please note that THE `edit_file` FUNCTION REQUIRES PROPER INDENTATION. If the assistant would like to add the line ' print(x)', it must fully write that out, with all those spaces before the code! Indentation is important and code that is not indented correctly will fail and require fixing before it can be run.
Please note that THE `edit_file` and `append_file` FUNCTIONS REQUIRE PROPER INDENTATION. If the assistant would like to add the line ' print(x)', it must fully write that out, with all those spaces before the code! Indentation is important and code that is not indented correctly will fail and require fixing before it can be run.

Responses should be concise.
The assistant should attempt fewer things at a time instead of putting too much commands OR code in one "execute" block.
Expand Down Expand Up @@ -138,7 +146,7 @@ def index():

if __name__ == '__main__':
app.run(port=5000)"""
edit_file(start=1, end=1, content=EDITED_CODE)
edit_file('app.py', start=1, end=1, content=EDITED_CODE)
</execute_ipython>

USER:
Expand Down Expand Up @@ -277,7 +285,7 @@ USER:
ASSISTANT:
I should edit the file to display the numbers in a table format. I should include correct indentation. Let me update the file:
<execute_ipython>
edit_file(start=7, end=7, content=" return '<table>' + ''.join([f'<tr><td>{i}</td></tr>' for i in numbers]) + '</table>'")
edit_file('app.py', start=7, end=7, content=" return '<table>' + ''.join([f'<tr><td>{i}</td></tr>' for i in numbers]) + '</table>'")
</execute_ipython>

USER:
Expand Down
Loading