Skip to content

Commit 1b67a72

Browse files
OrKoNDevtools-frontend LUCI CQ
authored andcommitted
[AI Assistance] fix read file function
Sometimes the files might not load their content. We need to ensure we call requestContentData to get the content if uiSourceCode is not dirty. Bug: 408130689 Change-Id: Ic84eb4f4ad787a8e3e7a21cbc7f6abbbfd01299b Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6448693 Auto-Submit: Alex Rudenko <[email protected]> Commit-Queue: Ergün Erdoğmuş <[email protected]> Reviewed-by: Ergün Erdoğmuş <[email protected]>
1 parent e265def commit 1b67a72

File tree

3 files changed

+47
-40
lines changed

3 files changed

+47
-40
lines changed

front_end/models/ai_assistance/AgentProject.test.ts

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -55,22 +55,22 @@ describeWithEnvironment('AgentProject', () => {
5555
it('can read files', async () => {
5656
const {project} = await mockProject();
5757

58-
assert.deepEqual(project.readFile('index.html'), 'content');
58+
assert.deepEqual(await project.readFile('index.html'), 'content');
5959
});
6060

6161
it('can report processed files', async () => {
6262
const {project} = await mockProject();
6363
assert.deepEqual(project.getProcessedFiles(), []);
64-
project.readFile('index.html');
64+
await project.readFile('index.html');
6565
assert.deepEqual(project.getProcessedFiles(), ['index.html']);
6666
});
6767

6868
describe('write file', () => {
6969
describe('full', () => {
7070
it('can write files files', async () => {
7171
const {project} = await mockProject();
72-
project.writeFile('index.html', 'updated');
73-
assert.deepEqual(project.readFile('index.html'), 'updated');
72+
await project.writeFile('index.html', 'updated');
73+
assert.deepEqual(await project.readFile('index.html'), 'updated');
7474
});
7575
});
7676

@@ -86,8 +86,8 @@ diff
8686
+updated
8787
\`\`\`\`\``;
8888

89-
project.writeFile('index.html', unifiedDiff, AiAssistanceModel.ReplaceStrategy.UNIFIED_DIFF);
90-
assert.deepEqual(project.readFile('index.html'), 'updated');
89+
await project.writeFile('index.html', unifiedDiff, AiAssistanceModel.ReplaceStrategy.UNIFIED_DIFF);
90+
assert.deepEqual(await project.readFile('index.html'), 'updated');
9191
});
9292

9393
it('can write files with multiple changes', async () => {
@@ -115,8 +115,8 @@ diff
115115
+LineUpdated:4
116116
\`\`\`\`\``;
117117

118-
project.writeFile('index.css', unifiedDiff, AiAssistanceModel.ReplaceStrategy.UNIFIED_DIFF);
119-
assert.deepEqual(project.readFile('index.css'), `LineUpdated:1
118+
await project.writeFile('index.css', unifiedDiff, AiAssistanceModel.ReplaceStrategy.UNIFIED_DIFF);
119+
assert.deepEqual(await project.readFile('index.css'), `LineUpdated:1
120120
Line:2
121121
Line:3
122122
LineUpdated:4
@@ -141,8 +141,8 @@ diff
141141
+Line:4
142142
\`\`\`\`\``;
143143

144-
project.writeFile('index.css', unifiedDiff, AiAssistanceModel.ReplaceStrategy.UNIFIED_DIFF);
145-
assert.deepEqual(project.readFile('index.css'), `Line:1
144+
await project.writeFile('index.css', unifiedDiff, AiAssistanceModel.ReplaceStrategy.UNIFIED_DIFF);
145+
assert.deepEqual(await project.readFile('index.css'), `Line:1
146146
Line:4`);
147147
});
148148

@@ -169,8 +169,8 @@ diff
169169
+LineUpdated:1.5
170170
\`\`\`\`\``;
171171

172-
project.writeFile('index.css', unifiedDiff, AiAssistanceModel.ReplaceStrategy.UNIFIED_DIFF);
173-
assert.deepEqual(project.readFile('index.css'), `LineUpdated:1
172+
await project.writeFile('index.css', unifiedDiff, AiAssistanceModel.ReplaceStrategy.UNIFIED_DIFF);
173+
assert.deepEqual(await project.readFile('index.css'), `LineUpdated:1
174174
LineUpdated:1.5
175175
Line:2
176176
Line:3
@@ -201,8 +201,8 @@ diff
201201
Line:3
202202
\`\`\`\`\``;
203203

204-
project.writeFile('index.css', unifiedDiff, AiAssistanceModel.ReplaceStrategy.UNIFIED_DIFF);
205-
assert.deepEqual(project.readFile('index.css'), `Line:1
204+
await project.writeFile('index.css', unifiedDiff, AiAssistanceModel.ReplaceStrategy.UNIFIED_DIFF);
205+
assert.deepEqual(await project.readFile('index.css'), `Line:1
206206
Line:3
207207
Line:4
208208
Line:5`);
@@ -225,8 +225,8 @@ diff
225225
-Line:1
226226
\`\`\`\`\``;
227227

228-
project.writeFile('index.css', unifiedDiff, AiAssistanceModel.ReplaceStrategy.UNIFIED_DIFF);
229-
assert.deepEqual(project.readFile('index.css'), '');
228+
await project.writeFile('index.css', unifiedDiff, AiAssistanceModel.ReplaceStrategy.UNIFIED_DIFF);
229+
assert.deepEqual(await project.readFile('index.css'), '');
230230
});
231231

232232
it('can write files with first line next to @@', async () => {
@@ -251,8 +251,8 @@ diff
251251
Line:3
252252
\`\`\`\`\``;
253253

254-
project.writeFile('index.css', unifiedDiff, AiAssistanceModel.ReplaceStrategy.UNIFIED_DIFF);
255-
assert.deepEqual(project.readFile('index.css'), `Line:3
254+
await project.writeFile('index.css', unifiedDiff, AiAssistanceModel.ReplaceStrategy.UNIFIED_DIFF);
255+
assert.deepEqual(await project.readFile('index.css'), `Line:3
256256
Line:4
257257
Line:5`);
258258
});
@@ -271,12 +271,15 @@ Line:5`);
271271
maxLinesChanged: 10,
272272
});
273273

274-
project.writeFile('index.html', 'updated');
275-
expect(() => {
276-
project.writeFile('example2.js', 'updated2');
277-
}).throws('Too many files changed');
278-
assert.deepEqual(project.readFile('index.html'), 'updated');
279-
assert.deepEqual(project.readFile('example2.js'), 'content');
274+
await project.writeFile('index.html', 'updated');
275+
try {
276+
await project.writeFile('example2.js', 'updated2');
277+
expect.fail('did not throw');
278+
} catch (err) {
279+
expect(err.message).to.eq('Too many files changed');
280+
}
281+
assert.deepEqual(await project.readFile('index.html'), 'updated');
282+
assert.deepEqual(await project.readFile('example2.js'), 'content');
280283
});
281284

282285
it('cannot write same file multiple times', async () => {
@@ -285,9 +288,9 @@ Line:5`);
285288
maxLinesChanged: 10,
286289
});
287290

288-
project.writeFile('index.html', 'updated');
289-
project.writeFile('index.html', 'updated2');
290-
assert.deepEqual(project.readFile('index.html'), 'updated2');
291+
await project.writeFile('index.html', 'updated');
292+
await project.writeFile('index.html', 'updated2');
293+
assert.deepEqual(await project.readFile('index.html'), 'updated2');
291294
});
292295

293296
it('cannot write more lines than allowed', async () => {
@@ -301,10 +304,13 @@ Line:5`);
301304
maxLinesChanged: 1,
302305
});
303306

304-
expect(() => {
305-
project.writeFile('example2.js', 'updated2\nupdated3');
306-
}).throws('Too many lines changed');
307-
assert.deepEqual(project.readFile('example2.js'), 'content');
307+
try {
308+
await project.writeFile('example2.js', 'updated2\nupdated3');
309+
expect.fail('did not throw');
310+
} catch (err) {
311+
expect(err.message).to.eq('Too many lines changed');
312+
}
313+
assert.deepEqual(await project.readFile('example2.js'), 'content');
308314
});
309315
});
310316
});

front_end/models/ai_assistance/AgentProject.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,18 @@ export class AgentProject {
6262
* Provides access to the file content in the working copy
6363
* of the matching UiSourceCode.
6464
*/
65-
readFile(filepath: string): string|undefined {
65+
async readFile(filepath: string): Promise<string|undefined> {
6666
const {map} = this.#indexFiles();
6767
const uiSourceCode = map.get(filepath);
6868
if (!uiSourceCode) {
6969
return;
7070
}
71+
const content =
72+
uiSourceCode.isDirty() ? uiSourceCode.workingCopyContentData() : await uiSourceCode.requestContentData();
73+
7174
this.#processedFiles.add(filepath);
72-
// TODO: needs additional handling for binary files.
73-
const content = uiSourceCode.workingCopyContentData();
74-
if (!content.isTextContent) {
75+
76+
if (TextUtils.ContentData.ContentData.isError(content) || !content.isTextContent) {
7577
return;
7678
}
7779

@@ -82,13 +84,13 @@ export class AgentProject {
8284
* This method updates the file content in the working copy of the
8385
* UiSourceCode identified by the filepath.
8486
*/
85-
writeFile(filepath: string, update: string, mode = ReplaceStrategy.FULL_FILE): void {
87+
async writeFile(filepath: string, update: string, mode = ReplaceStrategy.FULL_FILE): Promise<void> {
8688
const {map} = this.#indexFiles();
8789
const uiSourceCode = map.get(filepath);
8890
if (!uiSourceCode) {
8991
throw new Error(`UISourceCode ${filepath} not found`);
9092
}
91-
const currentContent = this.readFile(filepath);
93+
const currentContent = await this.readFile(filepath);
9294
let content: string;
9395
switch (mode) {
9496
case ReplaceStrategy.FULL_FILE:
@@ -214,7 +216,6 @@ export class AgentProject {
214216
break;
215217
}
216218

217-
await file.requestContentData();
218219
debugLog('searching in', filepath, 'for', query);
219220
const content = file.isDirty() ? file.workingCopyContentData() : await file.requestContentData();
220221
const results =

front_end/models/ai_assistance/agents/PatchAgent.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ export class PatchAgent extends AiAgent<Workspace.Workspace.Project> {
163163
debugLog('updateFiles', args.files);
164164
for (const file of args.files) {
165165
debugLog('updating', file);
166-
const content = this.#project.readFile(file);
166+
const content = await this.#project.readFile(file);
167167
if (content === undefined) {
168168
debugLog(file, 'not found');
169169
return {
@@ -204,7 +204,7 @@ ${content}
204204
};
205205
}
206206
const updated = response.text;
207-
this.#project.writeFile(file, updated, strategy);
207+
await this.#project.writeFile(file, updated, strategy);
208208
debugLog('updated', updated);
209209
}
210210
return {

0 commit comments

Comments
 (0)