All-Hands-AI / OpenHands

🙌 OpenHands: Code Less, Make More
https://all-hands.dev
MIT License
35.27k stars 3.98k forks source link

[Bug]: Editing does not work on lines that contain \n #3650

Closed neubig closed 2 weeks ago

neubig commented 2 months ago

Is there an existing issue for the same bug?

Describe the bug

I have found an easy-to-reproduce error in our editing pipeline that should be a big win if we can fix it: editing seems to break on lines that contain \n.

Here's an example:

Current OpenHands version

c875a5fb777fcdb387df13d78789a5597f3d29e3

Installation and Configuration

make build; make run

Model and Agent

Operating System

Mac OS

Reproduction Steps

No response

Logs, Errors, Screenshots, and Additional Context

Part of #3231

shubhamofbce commented 2 months ago

I tried it using gpt-4o and it worked for me. Intendation was broken after the edit but it fixed that by itself.

image

Output:

image
neubig commented 2 months ago

Interesting! So it may be a claude problem to some extent. But actually @shubhamofbce it didn't really work with gpt-4o either, because gpt-4o deleted the newlines entirely (which was not requested).

shubhamofbce commented 2 months ago

I see the problem now, It removed the newlines(\n) completely. I will take a look at this once i get some time. I am currently working on #3575

neubig commented 2 months ago

Awesome, thanks!

xingyaoww commented 2 months ago

I think this may also be fixed if @RajWorking can implement a EditAction using the Aider Diff format

shubhamofbce commented 2 months ago

Then we can assign this to @RajWorking and I will look into something else.

tobitege commented 2 months ago

This is being worked on by @RajWorking

RajWorking commented 1 month ago

I am working on this. Thanks!

tobitege commented 1 month ago

@neubig - this issue is important to get fixed asap, imo. cc @RajWorking @xingyaoww @enyst

Earlier today I was running ~15 instances of the Aider bench (which previously also failed) and was wondering why the edit_file_by_replace was often not successful in finding the text-to-be-replaced without apparent reason (no text cut off).

As raised in this very issue, the \n is converted incorrectly:

LLM (Sonnet) action:

15:54:00 - openhands:DEBUG: stream.py:134 - Adding IPythonRunCellAction id=7 from AGENT
15:54:00 - openhands:INFO: agent_controller.py:489
ACTION
**IPythonRunCellAction**
THOUGHT: Thank you for the update. Let's continue refactoring the code. We'll now update the `format_entries` function to use the new `Currency` class and improve its structure:
CODE:
edit_file_by_replace(
    'ledger.py',
    to_replace="""def format_entries(currency, locale, entries):
    if locale == 'en_US':
        # Generate Header Row
        table = 'Date'
        for _ in range(7):
            table += ' '
        table += '| Description'
        for _ in range(15):
            table += ' '
        table += '| Change'
        for _ in range(7):
            table += ' '

        while len(entries) > 0:
            table += '\n'

            # Find next entry in order
            min_entry_index = -1

OBSERVATION with error (sorry, but the formatting gets a bit disrupted here):


15:54:00 - openhands:DEBUG: stream.py:134 - Adding IPythonRunCellObservation id=8 from AGENT
15:54:00 - openhands:INFO: agent_controller.py:215
OBSERVATION
**IPythonRunCellObservation**
[No exact match found in ledger.py for
"""
def format_entries(currency, locale, entries):
    if locale == 'en_US':
        # Generate Header Row
        table = 'Date'
        for _ in range(7):
            table += ' '
        table += '| Description'
        for _ in range(15):
            table += ' '
        table += '| Change'
        for _ in range(7):
            table += ' '

        while len(entries) > 0:
            table += '
'

            # Find next entry in order
            min_entry_index = -1
"""

This looks like a common contributor for failed instances just because the step-limits fail and/or loop detection gets triggered?
enyst commented 1 month ago

Ah, I see the issue with '\n'. But the indentation is actually correct?

tobitege commented 1 month ago

It was a formatting issue in the comment, changed some triple-backticks, looks little better now 😄

xingyaoww commented 2 weeks ago

3985 should already fix it