gpt-engineer icon indicating copy to clipboard operation
gpt-engineer copied to clipboard

potential fix for some/dir/ bug #928

Open ATheorell opened this issue 1 year ago • 2 comments

The problem in bug #928 is that "some/dir/" is used as a placeholder to indicate that a filename should be represented by a relative path. The placeholder is introduced in the preprompt improve. Occasionally, the LLM misunderstands this intention and appends "some/dir/" explicitly to file names. This fix adds a clarification to the LLM the "some/dir/" is a placeholder.

ATheorell avatar Dec 23 '23 14:12 ATheorell

I think the some/dir/ could perhaps be omitted entirely, and instead mention that it should output a file path, not file name.

Also not sure if this is the right place/time, but I've had great success by writing save codeblocks like this:

```hello.py
print("hello world")
```

And similarly for patch, simply:

```patch hello.py
<<<<<<< ORIGINAL
print("Hello world")
=======
name = input("What is your name? ")
print(f"hello {name}")
>>>>>>> UPDATED
```

I think this might be a superior format overall, but not sure what your experiences with the current one is.

ErikBjare avatar Dec 23 '23 15:12 ErikBjare

Thanks for input @ErikBjare ! I'll have test removing the placeholder entirely. Regarding the syntax you propose, the risk I see is that the ORIGINAL part of the block may occur multiple times in the file, and we will by default replace all, even if we only want to modify one. The current (cumbersome) syntax tries to avoid this by always including a function name (unique by design).

I actually made feature request for a third idea https://github.com/gpt-engineer-org/gpt-engineer/issues/869 ...and really, I have no idea what is the best way to go -> we need a proper benchmarking flow -> which is under construction!

For now, as I said, I will try just removing the placeholder.

ATheorell avatar Dec 24 '23 10:12 ATheorell

This should at least partially address the concern in #928, by removing the part of the prompt that misleads the LLM to append some/dir to paths

I have also added an additional test for the case that we are editing a new file (works) and overwriting an existing file (works). This, as I understand should address #952 , which I will close. Correct me if I'm misunderstanding something here @Wheaties466 .

ATheorell avatar Jan 05 '24 12:01 ATheorell

This should at least partially address the concern in #928, by removing the part of the prompt that misleads the LLM to append some/dir to paths

I have also added an additional test for the case that we are editing a new file (works) and overwriting an existing file (works). This, as I understand should address #952 , which I will close. Correct me if I'm misunderstanding something here @Wheaties466 .

@ATheorell Thank you for the response! Yes, as I understand it. It should address that concern I raised in #952.

If you need or want a test lmk which branch has the fix and I can pull/build/test.

miqueet avatar Jan 05 '24 13:01 miqueet

Great! I merged to main

ATheorell avatar Jan 05 '24 19:01 ATheorell