-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Fix #346
Fix #346
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
This should also fix the phantom save_files
function calls that we've seen LLM try to do.
@@ -16,7 +16,7 @@ This task is broken down into steps. Please tell me the changes needed to implem | |||
{{ step_description }} | |||
``` | |||
|
|||
Within the file modifications, anything needs to be written by the user, add the comment in the same line as the code that starts with `// INPUT_REQUIRED {input_description}` where `input_description` is a description of what needs to be added here by the user. Finally, you can save the modified files on the disk by calling `save_files` function. | |||
Within the file modifications, anything needs to be written by the user, add the comment in the same line as the code that starts with `// INPUT_REQUIRED {input_description}` where `input_description` is a description of what needs to be added here by the user. Just make sure that you put comments only inside files that support comments (e.g. not in JSON files).. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will that actually fix this problem?
/* INPUT_REQUIRED {Download Bootstrap minified CSS from [getbootstrap.com](http://getbootstrap.com/) or use a CDN link to copy its content here} */
I got that in one of my tests. Comments are allowed in .js
files but the point here is to have it filled in, and it wasn't. I don't think this change will solve that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem you're referring to is made by adding the instructions to add INPUT_REQUIRED to parse_task.prompt that didn't have it before and since it's used in the beginning of the project, it added the comment to .json file so this patch makes sense
@@ -1,7 +1,7 @@ | |||
Ok, now, take your previous message and convert it to actionable items. An item might be a code change or a command run. When you need to change code, make sure that you put the entire content of the file in the value of `content` key even though you will likely copy and paste the most of the previous message. The commands must be able to run on a {{ os }} machine. | |||
|
|||
**IMPORTANT** | |||
Within the file modifications, anything needs to be written by the user, add the comment in the same line as the code that starts with `// INPUT_REQUIRED {input_description}` where `input_description` is a description of what needs to be added here by the user. Finally, you can save the modified files on the disk by calling `save_files` function. | |||
Within the file modifications, anything needs to be written by the user, add the comment in the same line as the code that starts with `// INPUT_REQUIRED {input_description}` where `input_description` is a description of what needs to be added here by the user. Just make sure that you put comments only inside files that support comments (e.g. not in JSON files). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this was the culprit behind the phantom save_files
function calls that LLM tried to make.
Finally, you can save the modified files on the disk by calling
save_files
function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
It seems like both prompts have exactly same text. I would put it in /prompts/components so we don't have duplicate code. Besides that looks good |
No description provided.