fix: incorrect line number in output snippet for str_replace and insert (#110)

* fix incorrect line num in output snippet for `str_replace` and `insert`

* bump to 0.2.9

* fix test

Author: ryanhoangt

openhands_aci/editor/editor.py

@@ -28,8 +28,6 @@
     'str_replace',
     'insert',
     'undo_edit',
-    # 'jump_to_definition', TODO:
-    # 'find_references' TODO:
 ]
 
 
@@ -60,9 +58,6 @@ def __init__(
             workspace_root: Root directory that serves as the current working directory for relative path
                            suggestions. Must be an absolute path. If None, no path suggestions will be
                            provided for relative paths.
-            workspace_root: Root directory that serves as the current working directory for relative path
-                           suggestions. Must be an absolute path. If None, no path suggestions will be
-                           provided for relative paths.
         """
         self._linter = DefaultLinter()
         self._history_manager = FileHistoryManager(max_history_per_file=10)
@@ -216,7 +211,7 @@ def str_replace(
         end_line = replacement_line + SNIPPET_CONTEXT_WINDOW + new_str.count('\n')
 
         # Read just the snippet range
-        snippet = self.read_file(path, start_line=start_line, end_line=end_line)
+        snippet = self.read_file(path, start_line=start_line + 1, end_line=end_line)
 
         # Prepare the success message
         success_message = f'The file {path} has been edited. '
@@ -428,12 +423,12 @@ def insert(
         shutil.move(temp_file.name, path)
 
         # Read just the snippet range
-        start_line = max(1, insert_line - SNIPPET_CONTEXT_WINDOW)
+        start_line = max(0, insert_line - SNIPPET_CONTEXT_WINDOW)
         end_line = min(
             num_lines + len(new_str_lines),
             insert_line + SNIPPET_CONTEXT_WINDOW + len(new_str_lines),
         )
-        snippet = self.read_file(path, start_line=start_line, end_line=end_line)
+        snippet = self.read_file(path, start_line=start_line + 1, end_line=end_line)
 
         # Save history - we already have the lines in memory
         file_text = ''.join(history_lines)
@@ -467,10 +462,6 @@ def validate_path(self, command: Command, path: Path) -> None:
         """
         Check that the path/command combination is valid.
 
-        Validates:
-        1. Path is absolute
-        2. Path and command are compatible
-
         Validates:
         1. Path is absolute
         2. Path and command are compatible
@@ -627,7 +618,7 @@ def _make_output(
         snippet_content = '\n'.join(
             [
                 f'{i + start_line:6}\t{line}'
-                for i, line in enumerate(snippet_content.split('\n'))
+                for i, line in enumerate(snippet_content.splitlines())
             ]
         )
         return (

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "openhands-aci"
-version = "0.2.8"
+version = "0.2.9"
 description = "An Agent-Computer Interface (ACI) designed for software development agents OpenHands."
 authors = ["OpenHands"]
 license = "MIT"

tests/integration/editor/test_basic_operations.py

@@ -102,7 +102,6 @@ def test_file_editor_with_xml_tag_parsing(temp_file):
      5\t    re.DOTALL,
      6\t)
      7\t...More text here.
-     8\t
 """
     )
 

tests/integration/test_oh_editor.py

@@ -100,6 +100,7 @@ def test_str_replace_no_linting(editor):
      2\tThis file is for testing purposes.
 Review the changes and make sure they are as expected. Edit the file again if necessary."""
     )
+    print(result.output)
 
     # Test that the file content has been updated
     assert 'This is a sample file.' in test_file.read_text()
@@ -605,3 +606,49 @@ def test_validate_path_suggests_absolute_path(editor, tmp_path):
     suggested_path = error_message.split('Maybe you meant ')[1].strip('?')
     assert Path(suggested_path).is_absolute()
     assert str(test_file.parent) in suggested_path
+
+
+def test_str_replace_and_insert_snippet_output_on_a_large_file(editor):
+    editor, test_file = editor
+
+    # Replace the current content with content: Line {line_number}
+    _ = editor(
+        command='str_replace',
+        path=str(test_file),
+        old_str='This is a test file.\nThis file is for testing purposes.',
+        new_str='',
+    )
+    for i in range(0, 700):
+        _ = editor(
+            command='insert', path=str(test_file), insert_line=i, new_str=f'Line {i+1}'
+        )
+
+    # View file
+    result = editor(command='view', path=str(test_file))
+    assert '     1\tLine 1' in result.output
+    assert '   500\tLine 500' in result.output
+
+    # Replace line 500's content with '500 new'
+    result = editor(
+        command='str_replace',
+        path=str(test_file),
+        old_str='Line 500',
+        new_str='500 new',
+    )
+    assert '   500\t500 new' in result.output
+
+    # Delete the line '500 new'
+    result = editor(
+        command='str_replace', path=str(test_file), old_str='500 new\n', new_str=''
+    )
+    assert '   499\tLine 499' in result.output
+    assert '   500\tLine 501' in result.output
+
+    # Insert content at line 500
+    result = editor(
+        command='insert',
+        path=str(test_file),
+        insert_line=499,
+        new_str='Inserted line at 500',
+    )
+    assert '   500\tInserted line at 500' in result.output