Skip to content

Commit 3b6a290

Browse files
RomneyDaRomneyDa
committed
fix: address bot feedback
1 parent 393c06f commit 3b6a290

File tree

5 files changed

+148
-106
lines changed

5 files changed

+148
-106
lines changed

‎core/tools/definitions/multiEdit.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ parameter is useful if you want to rename a variable for instance.
7575
If you want to create a new file, use:
7676
- A new file path
7777
- First edit: empty old_string and the new file's contents as new_string
78-
- Subsequent edits: normal edit operations on the created content`,
78+
- Subsequent edits are not allowed - there is no need since you are creating`,
7979
parameters: {
8080
type: "object",
8181
required: ["filepath", "edits"],

‎gui/src/pages/gui/ToolCallDiv/FunctionSpecificToolCallDiv.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ function FunctionSpecificToolCallDiv({
3333
/>
3434
);
3535
case BuiltInToolNames.SearchAndReplaceInFile:
36-
const changes = args.diffs
36+
const changes = args?.diffs
3737
? Array.isArray(args.diffs)
38-
? args.diffs.join("\n\n---\n\n")
38+
? args.diffs?.join("\n\n---\n\n")
3939
: args.diffs
4040
: "";
4141

@@ -76,7 +76,7 @@ ${edit?.old_string ?? ""}
7676
${edit?.new_string ?? ""}
7777
===== REPLACE`,
7878
)
79-
.join("\n\n---\n\n") ?? "";
79+
?.join("\n\n---\n\n") ?? "";
8080

8181
return (
8282
<EditFile

‎gui/src/util/clientTools/findAndReplaceUtils.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/**
2+
* Performs a find and replace operation on text content with proper handling of special characters
3+
*/
4+
export function performFindAndReplace(
5+
content: string,
6+
oldString: string,
7+
newString: string,
8+
replaceAll: boolean = false,
9+
index?: number, // For error messages
10+
): string {
11+
const errorContext = index !== undefined ? `Edit #${index + 1}: ` : "";
12+
// Check if old_string exists in current content
13+
if (!content.includes(oldString)) {
14+
throw new Error(`${errorContext}String not found in file: "${oldString}"`);
15+
}
16+
17+
if (replaceAll) {
18+
// Replace all occurrences using replaceAll for proper handling of special characters
19+
return content.replaceAll(oldString, newString);
20+
} else {
21+
// Count occurrences using indexOf for proper handling of special characters
22+
let count = 0;
23+
let index = content.indexOf(oldString);
24+
while (index !== -1) {
25+
count++;
26+
index = content.indexOf(oldString, index + 1);
27+
}
28+
29+
if (count > 1) {
30+
throw new Error(
31+
`${errorContext}String "${oldString}" appears ${count} times in the file. Either provide a more specific string with surrounding context to make it unique, or use replace_all=true to replace all occurrences.`,
32+
);
33+
}
34+
35+
// Replace only the first occurrence
36+
const firstIndex = content.indexOf(oldString);
37+
return (
38+
content.substring(0, firstIndex) +
39+
newString +
40+
content.substring(firstIndex + oldString.length)
41+
);
42+
}
43+
}
44+
45+
/**
46+
* Validates a single edit operation
47+
*/
48+
export function validateSingleEdit(
49+
oldString: string,
50+
newString: string,
51+
index?: number,
52+
): void {
53+
const context = index !== undefined ? `Edit #${index + 1}: ` : "";
54+
55+
if (!oldString && oldString !== "") {
56+
throw new Error(`${context}old_string is required`);
57+
}
58+
if (newString === undefined) {
59+
throw new Error(`${context}new_string is required`);
60+
}
61+
if (oldString === newString) {
62+
throw new Error(`${context}old_string and new_string must be different`);
63+
}
64+
}

‎gui/src/util/clientTools/multiEditImpl.ts‎

Lines changed: 68 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,66 +1,41 @@
1-
import { resolveRelativePathInDir } from "core/util/ideUtils";
1+
import {
2+
inferResolvedUriFromRelativePath,
3+
resolveRelativePathInDir,
4+
} from "core/util/ideUtils";
25
import { v4 as uuid } from "uuid";
36
import { applyForEditTool } from "../../redux/thunks/handleApplyStateUpdate";
47
import { ClientToolImpl } from "./callClientTool";
8+
import {
9+
performFindAndReplace,
10+
validateSingleEdit,
11+
} from "./findAndReplaceUtils";
512

613
interface EditOperation {
714
old_string: string;
815
new_string: string;
916
replace_all?: boolean;
1017
}
1118

12-
function validateEdits(edits: EditOperation[]): void {
13-
for (let i = 0; i < edits.length; i++) {
14-
const edit = edits[i];
15-
if (!edit.old_string && edit.old_string !== "") {
16-
throw new Error(`Edit ${i + 1}: old_string is required`);
17-
}
18-
if (edit.new_string === undefined) {
19-
throw new Error(`Edit ${i + 1}: new_string is required`);
20-
}
21-
if (edit.old_string === edit.new_string) {
19+
export function validateCreating(edits: EditOperation[]) {
20+
const isCreating = edits[0].old_string === "";
21+
if (edits.length > 1) {
22+
if (isCreating) {
2223
throw new Error(
23-
`Edit ${i + 1}: old_string and new_string must be different`,
24+
"cannot make subsequent edits on a file you are creating",
2425
);
26+
} else {
27+
for (let i = 1; i < edits.length; i++) {
28+
if (edits[i].old_string === "") {
29+
throw new Error(
30+
`edit #${i + 1}: only the first edit can contain an empty old_string, which is only used for file creation.`,
31+
);
32+
}
33+
}
2534
}
2635
}
27-
}
28-
29-
function applyEdit(
30-
content: string,
31-
edit: EditOperation,
32-
editIndex: number,
33-
isFirstEditOfNewFile: boolean,
34-
): string {
35-
const { old_string, new_string, replace_all = false } = edit;
3636

37-
// For new file creation, the first edit can have empty old_string
38-
if (isFirstEditOfNewFile && old_string === "") {
39-
return new_string;
40-
}
41-
42-
// Check if old_string exists in current content
43-
if (!content.includes(old_string)) {
44-
throw new Error(
45-
`Edit ${editIndex + 1}: String not found in file: "${old_string}"`,
46-
);
47-
}
48-
49-
if (replace_all) {
50-
// Replace all occurrences
51-
return content.split(old_string).join(new_string);
52-
} else {
53-
// Replace only the first occurrence, but check for uniqueness
54-
const occurrences = content.split(old_string).length - 1;
55-
if (occurrences > 1) {
56-
throw new Error(
57-
`Edit ${editIndex + 1}: String "${old_string}" appears ${occurrences} times in the file. Either provide a more specific string with surrounding context to make it unique, or use replace_all=true to replace all occurrences.`,
58-
);
59-
}
60-
return content.replace(old_string, new_string);
61-
}
37+
return isCreating;
6238
}
63-
6439
export const multiEditImpl: ClientToolImpl = async (
6540
args,
6641
toolCallId,
@@ -81,49 +56,69 @@ export const multiEditImpl: ClientToolImpl = async (
8156
}
8257

8358
// Validate each edit operation
84-
validateEdits(edits);
59+
for (let i = 0; i < edits.length; i++) {
60+
const edit = edits[i];
61+
validateSingleEdit(edit.old_string, edit.new_string, i);
62+
}
8563

8664
// Check if this is creating a new file (first edit has empty old_string)
87-
const isCreatingNewFile = edits[0].old_string === "";
88-
89-
// Resolve the file path
90-
const resolvedFilepath = await resolveRelativePathInDir(
65+
const isCreatingNewFile = validateCreating(edits);
66+
const resolvedUri = await resolveRelativePathInDir(
9167
filepath,
9268
extras.ideMessenger.ide,
9369
);
9470

95-
// For new files, resolvedFilepath might be null, so we construct the path
96-
const targetFilepath = resolvedFilepath || filepath;
97-
98-
if (!isCreatingNewFile) {
99-
// For existing files, check if file exists
100-
if (!resolvedFilepath) {
101-
throw new Error(`File ${filepath} does not exist`);
71+
let newContent: string;
72+
let fileUri: string;
73+
if (isCreatingNewFile) {
74+
if (resolvedUri) {
75+
throw new Error(
76+
`file ${filepath} already exists, cannot create new file`,
77+
);
10278
}
103-
}
104-
105-
try {
106-
// Read current file content (or start with empty for new files)
107-
let currentContent = "";
108-
if (!isCreatingNewFile) {
109-
currentContent = await extras.ideMessenger.ide.readFile(targetFilepath);
79+
newContent = edits[0].new_string;
80+
const response = await extras.ideMessenger.request(
81+
"getWorkspaceDirs",
82+
undefined,
83+
);
84+
if (response.status === "error") {
85+
throw new Error(
86+
"Error getting workspace directories to infer file creation path",
87+
);
11088
}
111-
112-
let newContent = currentContent;
113-
114-
// Apply all edits sequentially
89+
fileUri = await inferResolvedUriFromRelativePath(
90+
filepath,
91+
extras.ideMessenger.ide,
92+
response.content,
93+
);
94+
} else {
95+
if (!resolvedUri) {
96+
throw new Error(
97+
`file ${filepath} does not exist. If you are trying to edit it, correct the filepath. If you are trying to create it, you must pass old_string=""`,
98+
);
99+
}
100+
newContent = await extras.ideMessenger.ide.readFile(resolvedUri);
101+
fileUri = resolvedUri;
115102
for (let i = 0; i < edits.length; i++) {
116-
const isFirstEditOfNewFile = i === 0 && isCreatingNewFile;
117-
newContent = applyEdit(newContent, edits[i], i, isFirstEditOfNewFile);
103+
const { old_string, new_string, replace_all } = edits[i];
104+
newContent = performFindAndReplace(
105+
old_string,
106+
edits[0],
107+
new_string,
108+
replace_all,
109+
i,
110+
);
118111
}
112+
}
119113

114+
try {
120115
// Apply the changes to the file
121116
void extras.dispatch(
122117
applyForEditTool({
123118
streamId,
124119
toolCallId,
125120
text: newContent,
126-
filepath: targetFilepath,
121+
filepath: fileUri,
127122
isSearchAndReplace: true,
128123
}),
129124
);

‎gui/src/util/clientTools/singleFindAndReplaceImpl.ts‎

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ import { resolveRelativePathInDir } from "core/util/ideUtils";
22
import { v4 as uuid } from "uuid";
33
import { applyForEditTool } from "../../redux/thunks/handleApplyStateUpdate";
44
import { ClientToolImpl } from "./callClientTool";
5+
import {
6+
performFindAndReplace,
7+
validateSingleEdit,
8+
} from "./findAndReplaceUtils";
59

610
export const singleFindAndReplaceImpl: ClientToolImpl = async (
711
args,
@@ -16,15 +20,7 @@ export const singleFindAndReplaceImpl: ClientToolImpl = async (
1620
if (!filepath) {
1721
throw new Error("filepath is required");
1822
}
19-
if (!old_string) {
20-
throw new Error("old_string is required");
21-
}
22-
if (new_string === undefined) {
23-
throw new Error("new_string is required");
24-
}
25-
if (old_string === new_string) {
26-
throw new Error("old_string and new_string must be different");
27-
}
23+
validateSingleEdit(old_string, new_string);
2824

2925
// Resolve the file path
3026
const resolvedFilepath = await resolveRelativePathInDir(
@@ -40,26 +36,13 @@ export const singleFindAndReplaceImpl: ClientToolImpl = async (
4036
const originalContent =
4137
await extras.ideMessenger.ide.readFile(resolvedFilepath);
4238

43-
// Check if old_string exists in the file
44-
if (!originalContent.includes(old_string)) {
45-
throw new Error(`String not found in file: ${old_string}`);
46-
}
47-
48-
let newContent: string;
49-
50-
if (replace_all) {
51-
// Replace all occurrences
52-
newContent = originalContent.split(old_string).join(new_string);
53-
} else {
54-
// Replace only the first occurrence
55-
const occurrences = originalContent.split(old_string).length - 1;
56-
if (occurrences > 1) {
57-
throw new Error(
58-
`String "${old_string}" appears ${occurrences} times in the file. Either provide a more specific string with surrounding context to make it unique, or use replace_all=true to replace all occurrences.`,
59-
);
60-
}
61-
newContent = originalContent.replace(old_string, new_string);
62-
}
39+
// Perform the find and replace operation
40+
const newContent = performFindAndReplace(
41+
originalContent,
42+
old_string,
43+
new_string,
44+
replace_all,
45+
);
6346

6447
// Apply the changes to the file
6548
void extras.dispatch(

0 commit comments

Comments
 (0)