mirror of
https://github.com/Pythagora-io/gpt-pilot.git
synced 2026-01-10 13:37:55 -05:00
PR feedback
This commit is contained in:
@@ -700,19 +700,19 @@ REVIEW_CHANGES = {
|
||||
"items": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"number": {
|
||||
"type": "integer",
|
||||
"description": "Index of the hunk in the diff. Starts from 1."
|
||||
},
|
||||
"decision": {
|
||||
"type": "string",
|
||||
"enum": ["apply", "ignore"],
|
||||
"description": "Whether to apply this hunk (if it's a valid change) or ignore it."
|
||||
},
|
||||
"reason": {
|
||||
"type": "string",
|
||||
"desciprion": "Reason for allowing or ignoring this hunk."
|
||||
}
|
||||
"number": {
|
||||
"type": "integer",
|
||||
"description": "Index of the hunk in the diff. Starts from 1."
|
||||
},
|
||||
"decision": {
|
||||
"type": "string",
|
||||
"enum": ["apply", "ignore"],
|
||||
"description": "Whether to apply this hunk (if it's a valid change) or ignore it."
|
||||
},
|
||||
"reason": {
|
||||
"type": "string",
|
||||
"desciprion": "Reason for allowing or ignoring this hunk."
|
||||
}
|
||||
},
|
||||
"required": ["number", "decision", "reason"],
|
||||
"additionalProperties": False
|
||||
@@ -722,7 +722,7 @@ REVIEW_CHANGES = {
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"required": ["hunks"],
|
||||
"required": ["hunks", "review_notes"],
|
||||
"additionalProperties": False
|
||||
}
|
||||
}],
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import os.path
|
||||
import re
|
||||
from typing import Optional
|
||||
from traceback import format_exc
|
||||
from difflib import unified_diff
|
||||
|
||||
from helpers.AgentConvo import AgentConvo
|
||||
@@ -8,6 +9,7 @@ from helpers.Agent import Agent
|
||||
from helpers.files import get_file_contents
|
||||
from const.function_calls import GET_FILE_TO_MODIFY, REVIEW_CHANGES
|
||||
|
||||
from utils.exit import trace_code_event
|
||||
from utils.telemetry import telemetry
|
||||
|
||||
# Constant for indicating missing new line at the end of a file in a unified diff
|
||||
@@ -20,9 +22,8 @@ PATCH_HEADER_PATTERN = re.compile(r"^@@ -(\d+),?(\d+)? \+(\d+),?(\d+)? @@")
|
||||
class CodeMonkey(Agent):
|
||||
save_dev_steps = True
|
||||
|
||||
def __init__(self, project, developer):
|
||||
def __init__(self, project):
|
||||
super().__init__('code_monkey', project)
|
||||
self.developer = developer
|
||||
|
||||
def get_original_file(
|
||||
self,
|
||||
@@ -69,41 +70,41 @@ class CodeMonkey(Agent):
|
||||
def implement_code_changes(
|
||||
self,
|
||||
convo: Optional[AgentConvo],
|
||||
code_changes_description: str,
|
||||
step: dict[str, str],
|
||||
) -> AgentConvo:
|
||||
"""
|
||||
Implement code changes described in `code_changes_description`.
|
||||
|
||||
:param convo: AgentConvo instance (optional)
|
||||
:param task_description: description of the task
|
||||
:param code_changes_description: description of the code changes
|
||||
:param convo: conversation to continue)
|
||||
:param step: information about the step being implemented
|
||||
:param step_index: index of the step to implement
|
||||
"""
|
||||
code_change_description = step['code_change_description']
|
||||
|
||||
standalone = False
|
||||
if not convo:
|
||||
standalone = True
|
||||
convo = AgentConvo(self)
|
||||
|
||||
files = self.project.get_all_coded_files()
|
||||
file_name, file_content = self.get_original_file(code_changes_description, step, files)
|
||||
content = file_content
|
||||
file_name, file_content = self.get_original_file(code_change_description, step, files)
|
||||
|
||||
# Get the new version of the file
|
||||
content = self.replace_complete_file(
|
||||
convo,
|
||||
standalone,
|
||||
code_changes_description,
|
||||
code_change_description,
|
||||
file_content,
|
||||
file_name, files
|
||||
file_name,
|
||||
files,
|
||||
)
|
||||
|
||||
# Review the changes and only apply changes that are useful/approved
|
||||
if content and content != file_content:
|
||||
content = self.review_change(convo, code_changes_description, file_name, file_content, content)
|
||||
content = self.review_change(convo, code_change_description, file_name, file_content, content)
|
||||
|
||||
# If we have changes, update the file
|
||||
# TODO: if we *don't* have changes, we might want to retry the whole process (eg. have the reviewer
|
||||
# explicitly reject the whole PR and use that as feedback in implement_changes.prompt)
|
||||
if content and content != file_content:
|
||||
if not self.project.skip_steps:
|
||||
delta_lines = len(content.splitlines()) - len(file_content.splitlines())
|
||||
@@ -182,7 +183,7 @@ class CodeMonkey(Agent):
|
||||
Review changes that were applied to the file.
|
||||
|
||||
This asks the LLM to act as a PR reviewer and for each part (hunk) of the
|
||||
diff, decide if it shold be applied (kept) or ignored (removed from the PR).
|
||||
diff, decide if it should be applied (kept) or ignored (removed from the PR).
|
||||
|
||||
:param convo: AgentConvo instance
|
||||
:param instructions: instructions for the reviewer
|
||||
@@ -202,23 +203,44 @@ class CodeMonkey(Agent):
|
||||
"old_content": old_content,
|
||||
"hunks": hunks,
|
||||
}, REVIEW_CHANGES)
|
||||
messages_to_remove = 2
|
||||
|
||||
hunk_ids = set()
|
||||
for hunk in llm_response.get("hunks", []):
|
||||
if hunk.get("decision", "").lower() == "apply":
|
||||
hunk_ids.add(hunk["number"] - 1)
|
||||
while True:
|
||||
ids_to_apply = set()
|
||||
ids_to_ignore = set()
|
||||
for hunk in llm_response.get("hunks", []):
|
||||
if hunk.get("decision", "").lower() == "apply":
|
||||
ids_to_apply.add(hunk["number"] - 1)
|
||||
elif hunk.get("decision", "").lower() == "ignore":
|
||||
ids_to_ignore.add(hunk["number"] - 1)
|
||||
|
||||
hunks_to_apply = [ h for i, h in enumerate(hunks) if i in hunk_ids ]
|
||||
diff_log = f"---{file_name}\n+++{file_name}\n" + "\n".join(hunks_to_apply)
|
||||
if len(ids_to_apply | ids_to_ignore) == len(hunks):
|
||||
break
|
||||
|
||||
convo.remove_last_x_messages(2)
|
||||
llm_response = convo.send_message('utils/llm_response_error.prompt', {
|
||||
"error": "Not all hunks have been reviewed. Please review all hunks and add 'apply' or 'ignore' decision for each.",
|
||||
}, REVIEW_CHANGES)
|
||||
messages_to_remove += 2
|
||||
|
||||
convo.remove_last_x_messages(messages_to_remove)
|
||||
|
||||
hunks_to_apply = [ h for i, h in enumerate(hunks) if i in ids_to_apply ]
|
||||
diff_log = f"--- {file_name}\n+++ {file_name}\n" + "\n".join(hunks_to_apply)
|
||||
|
||||
if len(hunks_to_apply) == len(hunks):
|
||||
print("Applying entire change")
|
||||
return new_content
|
||||
elif len(hunks_to_apply) == 0:
|
||||
print("Rejecting all changes, keeping the file as-is")
|
||||
return old_content
|
||||
print("Reviewer has doubts, but applying all proposed changes anyway")
|
||||
trace_code_event(
|
||||
"modify-file-review-reject-all",
|
||||
{
|
||||
"file": file_name,
|
||||
"original": old_content,
|
||||
"diff": f"--- {file_name}\n+++ {file_name}\n" + "\n".join(hunks),
|
||||
}
|
||||
)
|
||||
return new_content
|
||||
else:
|
||||
print("Applying code change:\n" + diff_log)
|
||||
return self.apply_diff(file_name, old_content, hunks_to_apply, new_content)
|
||||
@@ -268,10 +290,7 @@ class CodeMonkey(Agent):
|
||||
approved diff hunks to the original file content.
|
||||
|
||||
If patch apply fails, the fallback is the full new file content
|
||||
with all the changes applied (as if the reviewer approved eveythng).
|
||||
|
||||
FIXME: For verification, it also calls command-line "Patch"
|
||||
utility to check that the internal algorithm works correctly.
|
||||
with all the changes applied (as if the reviewer approved everythng).
|
||||
|
||||
:param file_name: name of the file being modified
|
||||
:param old_content: old file content
|
||||
@@ -289,7 +308,17 @@ class CodeMonkey(Agent):
|
||||
except Exception as e:
|
||||
# This should never happen but if it does, just use the new version from
|
||||
# the LLM and hope for the best
|
||||
print(f"Error applying diff: {e}; assuming all changes are valid")
|
||||
print(f"Error applying diff: {e}; hoping all changes are valid")
|
||||
trace_code_event(
|
||||
"patch-apply-error",
|
||||
{
|
||||
"file": file_name,
|
||||
"error": str(e),
|
||||
"traceback": format_exc(),
|
||||
"original": old_content,
|
||||
"diff": diff
|
||||
}
|
||||
)
|
||||
return fallback
|
||||
|
||||
return fixed_content
|
||||
|
||||
@@ -234,8 +234,8 @@ class Developer(Agent):
|
||||
step = task_steps[i]
|
||||
if 'code_change_description' in step:
|
||||
print(f'Implementing code changes for `{step["code_change_description"]}`')
|
||||
code_monkey = CodeMonkey(self.project, self)
|
||||
updated_convo = code_monkey.implement_code_changes(convo, step['code_change_description'], step)
|
||||
code_monkey = CodeMonkey(self.project)
|
||||
updated_convo = code_monkey.implement_code_changes(convo, step)
|
||||
if test_after_code_changes:
|
||||
return self.test_code_changes(updated_convo, task_steps, i)
|
||||
else:
|
||||
@@ -252,8 +252,8 @@ class Developer(Agent):
|
||||
def step_modify_file(self, convo, step, i, test_after_code_changes):
|
||||
data = step['modify_file']
|
||||
print(f'Updating existing file {data["name"]}: {data["code_change_description"].splitlines()[0]}')
|
||||
code_monkey = CodeMonkey(self.project, self)
|
||||
code_monkey.implement_code_changes(convo, data['code_change_description'], data)
|
||||
code_monkey = CodeMonkey(self.project)
|
||||
code_monkey.implement_code_changes(convo, data)
|
||||
return {"success": True}
|
||||
|
||||
def step_command_run(self, convo, task_steps, i, success_with_cli_response=False):
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
{% if task_steps and step_index is not none -%}
|
||||
The current task has been split into multiple steps, and each step is one of the following:
|
||||
* `command` - command to run
|
||||
* `save_file` or `code_change` - create new or update existing file
|
||||
* `modify_file` - update large existing file
|
||||
* `save_file` - create a NEW file
|
||||
* `modify_file` - update ONE EXISTING file
|
||||
* `human_intervention` - if the human needs to do something
|
||||
|
||||
{% if step_index > 0 %}Here is the list of steps that have been executed:
|
||||
|
||||
@@ -6,159 +6,14 @@ This file needs to be modified by these instructions:
|
||||
---------------------start_of_instructions------------------------------
|
||||
{{ code_changes_description }}
|
||||
----------------------end_of_instructions-----------------------------
|
||||
{% if full_output %}
|
||||
|
||||
I want you to implement the instructions and show me the COMPLETE NEW VERSION of this file in this format:
|
||||
-----------------------format----------------------------
|
||||
```
|
||||
the full contents of the updated file, without skipping over any content
|
||||
```
|
||||
|
||||
{{ logs_and_error_handling }}
|
||||
|
||||
------------------------end_of_format---------------------------
|
||||
**IMPORTANT**: Your reply should not omit any code in the new implementation or substitute anything with comments like `// .. rest of the code goes here ..`, because I will overwrite the existing file with the content you provide. Output ONLY the content for this file, without additional explanation. Your output MUST start with ``` and MUST end with ``` and include only the complete file contents.
|
||||
{% else %}
|
||||
I want you to implement the instructions and show me the exact changes (`diff`) in the file `{{ file_name }}`. Reply only with the modifications (`diff`) in the following format:
|
||||
-----------------------start_of_format----------------------------
|
||||
CURRENT_CODE:
|
||||
```
|
||||
(All lines of code from specific code block in the current file that will be replaced by the code under NEW_CODE.)
|
||||
```
|
||||
NEW_CODE:
|
||||
```
|
||||
(All lines of code that will replace the code under CURRENT_CODE. That includes new lines of code and old lines of code that are not being changed but are part of that code block.)
|
||||
```
|
||||
END
|
||||
------------------------end_of_format---------------------------
|
||||
|
||||
Once you respond in this format, I will find all occurrences of CURRENT_CODE in the file `{{ file_name }}` and replace them with the code under NEW_CODE.
|
||||
**IMPORTANT**: Your reply MUST NOT omit any code in the new implementation or substitute anything with comments like `// .. rest of the code goes here ..` or `# insert existing code here`, because I will overwrite the existing file with the content you provide. Output ONLY the content for this file, without additional explanation. Your output MUST start with ``` and MUST end with ``` and include only the complete file contents.
|
||||
|
||||
{{ logs_and_error_handling }}
|
||||
|
||||
**IMPORTANT**
|
||||
Here are rules how to give good response. You have to strictly follow all rules at all times:
|
||||
|
||||
Rule #1:
|
||||
This is most important rule and there must never be reason to break this rule!
|
||||
When the instructions contain hints such as `# .. insert existing code here ...`, it is imperative to interpret and insert the relevant code from the original. Never omit any code that belongs in the new block, and never replace any code with comments such as `// the rest of the code goes here`, '# existing code from another file', or similar, even if the instructions explicitly request it!
|
||||
If the instruction examples reference existing code to be pasted in place, always use the specified code from the previous messages in this conversation instead of copying the comment, as illustrated in the following example:
|
||||
------------------------start_of_example_1---------------------------
|
||||
Instructions: "Rename function increase() { // ... existing code } to function inc() { // ... existing code } and increase counter by 10 instead of 1."
|
||||
------------------------BAD response for example_1:---------------------------
|
||||
CURRENT_CODE:
|
||||
```
|
||||
function increase() {
|
||||
// ... existing code
|
||||
}
|
||||
```
|
||||
NEW_CODE:
|
||||
```
|
||||
function inc() {
|
||||
// ... existing code
|
||||
return value + 10;
|
||||
}
|
||||
```
|
||||
------------------------GOOD response for example_1:---------------------------
|
||||
|
||||
CURRENT_CODE:
|
||||
```
|
||||
function increase(value) {
|
||||
if (typeof value !== 'number') {
|
||||
throw new Error('Argument must be number');
|
||||
}
|
||||
return value + 1;
|
||||
}
|
||||
```
|
||||
NEW_CODE:
|
||||
```
|
||||
function inc(value) {
|
||||
if (typeof value !== 'number') {
|
||||
throw new Error('Argument must be number');
|
||||
}
|
||||
return value + 10;
|
||||
}
|
||||
END
|
||||
```
|
||||
------------------------end_of_example_1---------------------------
|
||||
|
||||
Rule #2:
|
||||
For each change that needs to be done, you must show exactly one CURRENT_CODE code block and one NEW_CODE code block. You can think of this as difference (`diff`) between the current implementation and the new implementation.
|
||||
If there are no lines of code that need to be replaced by the NEW_CODE (if the NEW_CODE needs to be added into the CURRENT_CODE), show a couple of lines of code in the CURRENT_CODE before the place where NEW_CODE needs to be added.
|
||||
Here is an example of how to add one line `i--;` in the for loop:
|
||||
------------------------start_of_example_2---------------------------
|
||||
CURRENT_CODE:
|
||||
```
|
||||
let i = 0;
|
||||
i++;
|
||||
for (let j = 0; j < 100; j++) {
|
||||
```
|
||||
NEW_CODE:
|
||||
```
|
||||
let i = 0;
|
||||
i++;
|
||||
for (let j = 0; j < 100; j++) {
|
||||
i--;
|
||||
```
|
||||
END
|
||||
------------------------end_of_example_2---------------------------
|
||||
|
||||
Here's an example how to add code to the beginning of the file:
|
||||
------------------------start_of_example_3---------------------------
|
||||
CURRENT_CODE:
|
||||
```
|
||||
const app = express();
|
||||
const bodyParser = require('body-parser');
|
||||
```
|
||||
NEW_CODE:
|
||||
```
|
||||
const express = require('express');
|
||||
const app = express();
|
||||
const bodyParser = require('body-parser');
|
||||
```
|
||||
END
|
||||
------------------------end_of_example_3---------------------------
|
||||
|
||||
Rule #3:
|
||||
Do not show the entire file under CURRENT_CODE and NEW_CODE but only the lines that need to be replaced. If any lines should be left as they are in CURRENT_CODE, do not write them.
|
||||
|
||||
Rule #4:
|
||||
You must output the CURRENT_CODE exactly as it is in the original file, including the indentation from the original code, as it will be used for search-replace, and it should only match the original file in ONE place.
|
||||
In the NEW_CODE, remember to follow the same coding style that is used in the rest of the file. Pay special attention to the indentation of the new code and make sure to include all the required old and new code, without omitting anything.
|
||||
Pay very close attention to parenthesis and make sure that when CURRENT_CODE is replaced with NEW_CODE there are no extra parenthesis or any parenthesis missing.
|
||||
|
||||
Rule #5:
|
||||
Pay attention to the lines of code that you select in the CURRENT_CODE and the lines that you add to NEW_CODE because all lines under CURRENT_CODE will be replaced with the lines under NEW_CODE. Sometimes you might duplicate the currently implemented lines by not putting them under CURRENT_CODE but repeating them under NEW_CODE - you must **NOT** do that.
|
||||
Here is an example. Let's say that you have a code like this:
|
||||
```
|
||||
let someClass = new SomeClass();
|
||||
let methodValue = someClass.someMethod();
|
||||
```
|
||||
And you want to add a line `methodValue += 22;` below the line `let methodValue = someClass.someMethod();`. Here is an example of what you must **NOT** do:
|
||||
------------------------start_of_incorrect_example---------------------------
|
||||
CURRENT_CODE:
|
||||
```
|
||||
let methodValue = someClass.someMethod()
|
||||
```
|
||||
NEW_CODE:
|
||||
```
|
||||
let someClass = new SomeClass();
|
||||
let methodValue = someClass.someMethod();
|
||||
methodValue += 22;
|
||||
```
|
||||
END
|
||||
------------------------end_of_incorrect_example---------------------------
|
||||
|
||||
See how the line `let someClass = new SomeClass();` was added under NEW_CODE but it wasn't mentioned under CURRENT_CODE so the result will be that this line will be duplicated in the end result once the line `let methodValue = someClass.someMethod()` gets replaced with lines under NEW_CODE. Instead, this would be a correct way to do it:
|
||||
------------------------start_of_correct_example---------------------------
|
||||
CURRENT_CODE:
|
||||
```
|
||||
let methodValue = someClass.someMethod()
|
||||
```
|
||||
NEW_CODE:
|
||||
```
|
||||
let methodValue = someClass.someMethod();
|
||||
methodValue += 22;
|
||||
```
|
||||
END
|
||||
------------------------end_of_correct_example---------------------------
|
||||
{% endif %}
|
||||
|
||||
@@ -17,9 +17,9 @@ Here is the diff of the changes:
|
||||
```
|
||||
{% endfor %}
|
||||
|
||||
Think carefully about the instructions and review the proposed changes. For each hunk of change, decide whether it should be applied or should be ignored (for example if it is a code deletion or change that wasn't asked for). Finally, if the changes miss something that was in the instructions, menton that.
|
||||
Think carefully about the instructions and review the proposed changes. For each hunk of change, decide whether it should be applied or should be ignored (for example if it is a code deletion or change that wasn't asked for). Finally, if the changes miss something that was in the instructions, mention that.
|
||||
|
||||
Example output if 3 of 4 hunks in the change should be applied and one of them should be ignored, and no other changes are needed:
|
||||
Here is an example output if 3 of 4 hunks in the change should be applied and one of them should be ignored, and no other changes are needed:
|
||||
```
|
||||
{
|
||||
"hunks": [
|
||||
@@ -36,7 +36,7 @@ Example output if 3 of 4 hunks in the change should be applied and one of them s
|
||||
{
|
||||
"number": 3,
|
||||
"decision": "ignore",
|
||||
"reason": "This hunk accidentially deletes important code",
|
||||
"reason": "This hunk accidentally deletes important code",
|
||||
},
|
||||
{
|
||||
"number": 4,
|
||||
|
||||
@@ -1 +1 @@
|
||||
You are a full stack software developer who works in a software development agency. You write very modular code. Your job is to implement tasks that your tech lead assigns you.
|
||||
You are a full stack software developer that works in a software development agency. You write modular, clean, maintainable, production-ready code. Your job is to implement tasks that your tech lead assigns you.
|
||||
Reference in New Issue
Block a user