-
Notifications
You must be signed in to change notification settings - Fork 1k
fix: update apply_patch format instructions to be consistent and include required space #531
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -400,30 +400,34 @@ export class ApplyPatchFormatInstructions extends PromptElement { | |||
render() { | ||||
return <> | ||||
*** Update File: [file_path]<br /> | ||||
[context_before] -> See below for further instructions on context.<br /> | ||||
[context_before] -> See below for further instructions on context.<br /> | ||||
-[old_code] -> Precede each line in the old code with a minus sign.<br /> | ||||
+[new_code] -> Precede each line in the new, replacement code with a plus sign.<br /> | ||||
[context_after] -> See below for further instructions on context.<br /> | ||||
[context_after] -> See below for further instructions on context.<br /> | ||||
<br /> | ||||
Prefix each line of old code with exactly one minus (-) before its original first character; keep the original indentation after it.<br /> | ||||
Prefix each line of new code with exactly one plus (+), the remainder of the line after the plus is the full new line content.<br /> | ||||
For instructions on [context_before] and [context_after]:<br /> | ||||
- Prefix each unchanged context line with exactly one ASCII space (U+0020) before its original first character; keep the original indentation after it.<br /> | ||||
- By default, show 3 lines of code immediately above and 3 lines immediately below each change. If a change is within 3 lines of a previous change, do NOT duplicate the first change's [context_after] lines in the second change's [context_before] lines.<br /> | ||||
- If 3 lines of context is insufficient to uniquely identify the snippet of code within the file, use the @@ operator to indicate the class or function to which the snippet belongs.<br /> | ||||
- If a code block is repeated so many times in a class or function such that even a single @@ statement and 3 lines of context cannot uniquely identify the snippet of code, you can use multiple `@@` statements to jump to the right context. | ||||
<br /> | ||||
You must use the same indentation style as the original code. If the original code uses tabs, you must use tabs. If the original code uses spaces, you must use spaces. Be sure to use a proper UNESCAPED tab character.<br /> | ||||
<br /> | ||||
See below for an example of the patch format. If you propose changes to multiple regions in the same file, you should repeat the *** Update File header for each snippet of code to change:<br /> | ||||
<br /> | ||||
*** Begin Patch<br /> | ||||
*** Update File: /Users/someone/pygorithm/searching/binary_search.py<br /> | ||||
@@ class BaseClass<br /> | ||||
@@ def method():<br /> | ||||
[3 lines of pre-context]<br /> | ||||
-[old_code]<br /> | ||||
+[new_code]<br /> | ||||
+[new_code]<br /> | ||||
[3 lines of post-context]<br /> | ||||
*** End Patch<br /> | ||||
<Tag name='applyPatchExample'> | ||||
*** Begin Patch<br /> | ||||
*** Update File: /Users/someone/pygorithm/searching/binary_search.py<br /> | ||||
@@ class BaseClass<br /> | ||||
@@ def method():<br /> | ||||
[3 lines of pre-context]<br /> | ||||
-[old_code]<br /> | ||||
+[new_code]<br /> | ||||
+[new_code]<br /> | ||||
[3 lines of post-context]<br /> | ||||
*** End Patch<br /> | ||||
</Tag> | ||||
</>; | ||||
} | ||||
} | ||||
|
@@ -433,7 +437,9 @@ class ApplyPatchInstructions extends PromptElement<DefaultAgentPromptProps> { | |||
const isGpt5 = this.props.modelFamily === 'gpt-5'; | ||||
return <Tag name='applyPatchInstructions'> | ||||
To edit files in the workspace, use the {ToolName.ApplyPatch} tool. If you have issues with it, you should first try to fix your patch and continue using {ToolName.ApplyPatch}. If you are stuck, you can fall back on the {ToolName.EditFile} tool. But {ToolName.ApplyPatch} is much faster and is the preferred tool.<br /> | ||||
{isGpt5 && <>Prefer the smallest set of changes needed to satisfy the task. Avoid reformatting unrelated code; preserve existing style and public APIs unless the task requires changes. When practical, complete all edits for a file within a single message.<br /></>} | ||||
{isGpt5 && <> | ||||
Never use {ToolName.ApplyPatch} for creating files.<br /> | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Creating a file with apply patch is fine, we should handle it
If we don't, that is a bug we should fix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can do some additional testing today as I'm working on things and report back regarding apply_patch failing on creating a file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't explicitly instruct it to use Add File afaik in our instructions, but GPT is fine-tuned on apply_patch so it might include it nevertheless |
||||
Prefer the smallest set of changes needed to satisfy the task. Avoid reformatting unrelated code; preserve existing style and public APIs unless the task requires changes. When practical, complete all edits for a file within a single message.<br /></>} | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, do we need these instructions? Would they limit GPT-5 at all and have GPT-5 focus on only making small simple changes?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure what the origin of this line is. cc @bhavyaus |
||||
The input for this tool is a string representing the patch to apply, following a special format. For each snippet of code that needs to be changed, repeat the following:<br /> | ||||
<ApplyPatchFormatInstructions /><br /> | ||||
NEVER print this out to the user, instead call the tool and the edits will be applied and shown to the user.<br /> | ||||
|
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.
These prefix instructions don't appear in Codex's apply patch instructions, so I would tend to say we should not include them here unless we have strong reason to believe they improve behavior https://github.com/openai/codex/blob/main/codex-rs/apply-patch/apply_patch_tool_instructions.md
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.
Good point, I can look at Codex's code first and see where this is a problem. Also I'll try testing this out further and produce smaller set of sample data to reproduce the issue.
Uh oh!
There was an error while loading. Please reload this page.
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 made a repo with contrived examples, capturing conversation, example file before conversation, results, and all logs.
https://github.com/agreaves-ms/vscode-copilot-chat-example-issues
The one example I have in there right now is specifically around what these instructions were meant to fix.
@connor4312 do you think it makes sense to share this and address these instruction changes in Codex's repo first?
I don't believe it's possible to work around this issue in apply_patch's logic as it would always see something like
- list item
differently than- list item
.Captured screenshot of the problem:
### 🔐 SSL and Kubernetes Integration (SSE-Only)
should be### 🔐 SSL and Kubernetes Integration (SSE-Only)