-
Notifications
You must be signed in to change notification settings - Fork 368
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
In-line PR comments sometimes request redundant changes #137
Comments
Do you have a public PR you can share with this behavior? |
I can try to set up a reproduction of this issue but I can see the prompts are clearly trying to avoid this already:
Do you think tweaking that instruction or its placement would make a difference? Something like:
|
Cc @mrT23 |
@KalleV It's tempting to use longer and longer prompts to try and fix cases that didn't work 100% well. You can try your modified prompt on the bad cases you saw. Did it improve ? To try improve the prompting for this specific issue, I would focus on a different aspect -further explain to the model what are the '+' lines. Maybe be a specific example, simulating a hunk in the prompt, would help
(If you prefer, share a public example that gave bad results, and I will try an improved prompt) |
I wasn't able to create a minimum public reproduction yet but I came across a PR consistently requesting a try/catch. Not sure if the PR size makes a big difference but according to https://llmtokencounter.com/, the system prompt + user prompt is about I couldn't consistently prove small tweaks such as: + - It is essential to avoid suggesting changes that mirror modifications already introduced in the new PR code (the '+' lines).
- - Make sure not to provide suggestions repeating modifications already implemented in the new PR code (the '+' lines). Or rearranging the system instructions led to a consistently better response but it was fun to try! |
ok. notice that the '/improve' tool works differently in terms of patch and prompt design, and I think it will happen less there, so you can you it instead |
I am closing this issue. if you encounter this again in the '/improve' command, feel free to open another |
Sounds good, thank you! |
I’m fairly often seeing this type of PR feedback where the change already being applied is added as the PR feedback comment:
Another seemingly consistent way to reproduce this seems to be when adding try / catch around a code block that did not have it before.
The text was updated successfully, but these errors were encountered: