Skip to content
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

Refactor navigation commands into its own tool #35

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
3 changes: 2 additions & 1 deletion openhands_aci/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from .editor import file_editor
from .navigator import symbol_navigator

__all__ = ['file_editor']
__all__ = ['file_editor', 'symbol_navigator']
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def __init__(self, message):
self.message = message


class EditorToolParameterMissingError(ToolError):
class MultiCommandToolParameterMissingError(ToolError):
"""Raised when a required parameter is missing for a tool command."""

def __init__(self, command, parameter):
Expand All @@ -14,7 +14,7 @@ def __init__(self, command, parameter):
self.message = f'Parameter `{parameter}` is required for command: {command}.'


class EditorToolParameterInvalidError(ToolError):
class ToolParameterInvalidError(ToolError):
"""Raised when a parameter is invalid for a tool command."""

def __init__(self, parameter, value, hint=None):
Expand Down
13 changes: 11 additions & 2 deletions openhands_aci/editor/results.py → openhands_aci/core/results.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from dataclasses import asdict, dataclass, fields

from .config import MAX_RESPONSE_LEN_CHAR
from .prompts import CONTENT_TRUNCATED_NOTICE
from ..editor.config import MAX_RESPONSE_LEN_CHAR
from ..editor.prompts import CONTENT_TRUNCATED_NOTICE


@dataclass
Expand Down Expand Up @@ -45,3 +45,12 @@ def maybe_truncate(
if not truncate_after or len(content) <= truncate_after
else content[:truncate_after] + CONTENT_TRUNCATED_NOTICE
)


def make_api_tool_result(tool_result: ToolResult) -> str:
"""Convert an agent ToolResult to an API ToolResultBlockParam."""
if tool_result.error:
return f'ERROR:\n{tool_result.error}'

assert tool_result.output, 'Expected output in file_editor.'
return tool_result.output
34 changes: 25 additions & 9 deletions openhands_aci/editor/__init__.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,36 @@
import json
import uuid

from openhands_aci.core.exceptions import ToolError
from openhands_aci.core.results import ToolResult, make_api_tool_result

from .editor import Command, OHEditor
from .exceptions import ToolError
from .results import ToolResult

_GLOBAL_EDITOR = OHEditor()


def _make_api_tool_result(tool_result: ToolResult) -> str:
"""Convert an agent ToolResult to an API ToolResultBlockParam."""
if tool_result.error:
return f'ERROR:\n{tool_result.error}'
TOOL_DESCRIPTION = """Custom editing tool for viewing, creating and editing files
* State is persistent across command calls and discussions with the user
* If `path` is a file, `view` displays the result of applying `cat -n`. If `path` is a directory, `view` lists non-hidden files and directories up to 2 levels deep
* The `create` command cannot be used if the specified `path` already exists as a file
* If a `command` generates a long output, it will be truncated and marked with `<response clipped>`
* The `undo_edit` command will revert the last edit made to the file at `path`

Notes for using the `str_replace` command:
* The `old_str` parameter should match EXACTLY one or more consecutive lines from the original file. Be mindful of whitespaces!
* If the `old_str` parameter is not unique in the file, the replacement will not be performed. Make sure to include enough context in `old_str` to make it unique
* The `new_str` parameter should contain the edited lines that should replace the `old_str`
"""

assert tool_result.output, 'Expected output in file_editor.'
return tool_result.output
PARAMS_DESCRIPTION = {
'command': 'The commands to run. Allowed options are: `view`, `create`, `str_replace`, `insert`, `undo_edit`.',
'path': 'Absolute path to file or directory, e.g. `/workspace/file.py` or `/workspace`.',
'file_text': 'Required parameter of `create` command, with the content of the file to be created.',
'old_str': 'Required parameter of `str_replace` command containing the string in `path` to replace.',
'new_str': 'Optional parameter of `str_replace` command containing the new string (if not given, no string will be added). Required parameter of `insert` command containing the string to insert.',
'insert_line': 'Required parameter of `insert` command. The `new_str` will be inserted AFTER the line `insert_line` of `path`.',
'view_range': 'Optional parameter of `view` command when `path` points to a file. If none is given, the full file is shown. If provided, the file will be shown in the indicated line number range, e.g. [11, 12] will show lines 11 and 12. Indexing at 1 to start. Setting `[start_line, -1]` shows all lines from `start_line` to the end of the file.',
}


def file_editor(
Expand Down Expand Up @@ -42,7 +58,7 @@ def file_editor(
except ToolError as e:
result = ToolResult(error=e.message)

formatted_output_and_error = _make_api_tool_result(result)
formatted_output_and_error = make_api_tool_result(result)
marker_id = uuid.uuid4().hex
return f"""<oh_aci_output_{marker_id}>
{json.dumps(result.to_dict(extra_field={'formatted_output_and_error': formatted_output_and_error}), indent=2)}
Expand Down
113 changes: 113 additions & 0 deletions openhands_aci/editor/cli.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import argparse
import json
import re
import sys
from pathlib import Path
from typing import Any, NoReturn

from .editor import Command, get_args


def parse_view_range(value: str) -> list[int]:
try:
start, end = map(int, value.split(','))
return [start, end]
except ValueError:
raise argparse.ArgumentTypeError(
'view-range must be two comma-separated integers, e.g. "1,10"'
)


def create_parser() -> argparse.ArgumentParser:
parser = argparse.ArgumentParser(
description='OpenHands Editor CLI - A tool for viewing and editing files'
)
parser.add_argument(
'command',
type=str,
choices=list(get_args(Command)),
help='The command to execute',
)
parser.add_argument(
'path',
type=str,
help='Path to the file or directory to operate on',
)
parser.add_argument(
'--file-text',
type=str,
help='Content for the file when using create command',
)
parser.add_argument(
'--view-range',
type=parse_view_range,
help='Line range to view in format "start,end", e.g. "1,10"',
)
parser.add_argument(
'--old-str',
type=str,
help='String to replace when using str_replace command',
)
parser.add_argument(
'--new-str',
type=str,
help='New string to insert when using str_replace or insert commands',
)
parser.add_argument(
'--insert-line',
type=int,
help='Line number after which to insert when using insert command',
)
parser.add_argument(
'--enable-linting',
action='store_true',
help='Enable linting for file modifications',
)
parser.add_argument(
'--raw',
action='store_true',
help='Output raw JSON response instead of formatted text',
)
return parser


def extract_result(output: str) -> dict[str, Any]:
match = re.search(
r'<oh_aci_output_[0-9a-f]{32}>(.*?)</oh_aci_output_[0-9a-f]{32}>',
output,
re.DOTALL,
)
assert match, f'Output does not contain the expected <oh_aci_output_> tags in the correct format: {output}'
result_dict = json.loads(match.group(1))
return result_dict


def main() -> NoReturn:
parser = create_parser()
args = parser.parse_args()

# Import here to avoid circular imports
from . import file_editor

try:
output = file_editor(
command=args.command,
path=str(Path(args.path).absolute()),
file_text=args.file_text,
view_range=args.view_range,
old_str=args.old_str,
new_str=args.new_str,
insert_line=args.insert_line,
enable_linting=args.enable_linting,
)

result = extract_result(output)
print(result['formatted_output_and_error'])
sys.exit(0)
except Exception as e:
print(f'ERROR: {str(e)}', file=sys.stderr)
sys.exit(1)


if __name__ == '__main__':
main()
44 changes: 21 additions & 23 deletions openhands_aci/editor/editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,23 @@
from pathlib import Path
from typing import Literal, get_args

from openhands_aci.core.exceptions import (
MultiCommandToolParameterMissingError,
ToolError,
ToolParameterInvalidError,
)
from openhands_aci.core.results import CLIResult, maybe_truncate
from openhands_aci.linter import DefaultLinter
from openhands_aci.utils.shell import run_shell_cmd

from .config import SNIPPET_CONTEXT_WINDOW
from .exceptions import (
EditorToolParameterInvalidError,
EditorToolParameterMissingError,
ToolError,
)
from .results import CLIResult, maybe_truncate

Command = Literal[
'view',
'create',
'str_replace',
'insert',
'undo_edit',
# 'jump_to_definition', TODO:

Choose a reason for hiding this comment

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

not sure if there's a formal board and backlog item to link these TODO items to, but in my experience these kinds of things often never get done otherwise

# 'find_references' TODO:
]


Expand Down Expand Up @@ -62,7 +60,7 @@ def __call__(
return self.view(_path, view_range)
elif command == 'create':
if file_text is None:
raise EditorToolParameterMissingError(command, 'file_text')
raise MultiCommandToolParameterMissingError(command, 'file_text')
self.write_file(_path, file_text)
self._file_history[_path].append(file_text)
return CLIResult(
Expand All @@ -73,19 +71,19 @@ def __call__(
)
elif command == 'str_replace':
if old_str is None:
raise EditorToolParameterMissingError(command, 'old_str')
raise MultiCommandToolParameterMissingError(command, 'old_str')
if new_str == old_str:
raise EditorToolParameterInvalidError(
raise ToolParameterInvalidError(
'new_str',
new_str,
'No replacement was performed. `new_str` and `old_str` must be different.',
)
return self.str_replace(_path, old_str, new_str, enable_linting)
elif command == 'insert':
if insert_line is None:
raise EditorToolParameterMissingError(command, 'insert_line')
raise MultiCommandToolParameterMissingError(command, 'insert_line')
if new_str is None:
raise EditorToolParameterMissingError(command, 'new_str')
raise MultiCommandToolParameterMissingError(command, 'new_str')
return self.insert(_path, insert_line, new_str, enable_linting)
elif command == 'undo_edit':
return self.undo_edit(_path)
Expand Down Expand Up @@ -167,7 +165,7 @@ def view(self, path: Path, view_range: list[int] | None = None) -> CLIResult:
"""
if path.is_dir():
if view_range:
raise EditorToolParameterInvalidError(
raise ToolParameterInvalidError(
'view_range',
view_range,
'The `view_range` parameter is not allowed when `path` points to a directory.',
Expand Down Expand Up @@ -195,7 +193,7 @@ def view(self, path: Path, view_range: list[int] | None = None) -> CLIResult:
)

if len(view_range) != 2 or not all(isinstance(i, int) for i in view_range):
raise EditorToolParameterInvalidError(
raise ToolParameterInvalidError(
'view_range',
view_range,
'It should be a list of two integers.',
Expand All @@ -205,21 +203,21 @@ def view(self, path: Path, view_range: list[int] | None = None) -> CLIResult:
num_lines = len(file_content_lines)
start_line, end_line = view_range
if start_line < 1 or start_line > num_lines:
raise EditorToolParameterInvalidError(
raise ToolParameterInvalidError(
'view_range',
view_range,
f'Its first element `{start_line}` should be within the range of lines of the file: {[1, num_lines]}.',
)

if end_line > num_lines:
raise EditorToolParameterInvalidError(
raise ToolParameterInvalidError(
'view_range',
view_range,
f'Its second element `{end_line}` should be smaller than the number of lines in the file: `{num_lines}`.',
)

if end_line != -1 and end_line < start_line:
raise EditorToolParameterInvalidError(
raise ToolParameterInvalidError(
'view_range',
view_range,
f'Its second element `{end_line}` should be greater than or equal to the first element `{start_line}`.',
Expand Down Expand Up @@ -262,7 +260,7 @@ def insert(
num_lines = len(file_text_lines)

if insert_line < 0 or insert_line > num_lines:
raise EditorToolParameterInvalidError(
raise ToolParameterInvalidError(
'insert_line',
insert_line,
f'It should be within the range of lines of the file: {[0, num_lines]}',
Expand Down Expand Up @@ -315,26 +313,26 @@ def validate_path(self, command: Command, path: Path) -> None:
# Check if its an absolute path
if not path.is_absolute():
suggested_path = Path('') / path
raise EditorToolParameterInvalidError(
raise ToolParameterInvalidError(
'path',
path,
f'The path should be an absolute path, starting with `/`. Maybe you meant {suggested_path}?',
)
# Check if path and command are compatible
if command == 'create' and path.exists():
raise EditorToolParameterInvalidError(
raise ToolParameterInvalidError(
'path',
path,
f'File already exists at: {path}. Cannot overwrite files using command `create`.',
)
if command != 'create' and not path.exists():
raise EditorToolParameterInvalidError(
raise ToolParameterInvalidError(
'path',
path,
f'The path {path} does not exist. Please provide a valid path.',
)
if command != 'view' and path.is_dir():
raise EditorToolParameterInvalidError(
raise ToolParameterInvalidError(
'path',
path,
f'The path {path} is a directory and only the `view` command can be used on directories.',
Expand Down
2 changes: 2 additions & 0 deletions openhands_aci/editor/prompts.py
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
CONTENT_TRUNCATED_NOTICE: str = '<response clipped><NOTE>To save on context only part of this file has been shown to you. You should retry this tool after you have searched inside the file with `grep -n` in order to find the line numbers of what you are looking for.</NOTE>'

NAVIGATION_TIPS: str = '<TIP>Use the navigation tool to investigate more about a particular class, function/method and how it is used in the codebase.</TIP>'
Loading
Loading