Skip to content

Commit 1c494de

Browse files
bryanchen-dbryanchen-d
authored
display folder instead of individual child files in get-errors result (microsoft#1866)
* getErrorsTool display folder instead of individual child files * address copilot comments. * Fix test failure * updated per comments
1 parent 39d4619 commit 1c494de

File tree

3 files changed

+65
-13
lines changed

3 files changed

+65
-13
lines changed

‎src/extension/tools/node/getErrorsTool.tsx‎

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { isLocation } from '../../../util/common/types';
1717
import { coalesce } from '../../../util/vs/base/common/arrays';
1818
import { CancellationToken } from '../../../util/vs/base/common/cancellation';
1919
import { Disposable } from '../../../util/vs/base/common/lifecycle';
20+
import { ResourceSet } from '../../../util/vs/base/common/map';
2021
import { isEqualOrParent } from '../../../util/vs/base/common/resources';
2122
import { URI } from '../../../util/vs/base/common/uri';
2223
import { IInstantiationService } from '../../../util/vs/platform/instantiation/common/instantiation';
@@ -56,8 +57,8 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
5657
* Get diagnostics for the given paths and optional ranges.
5758
* Note - This is made public for testing purposes only.
5859
*/
59-
public getDiagnostics(paths: { uri: URI; range: Range | undefined }[]): Array<{ uri: URI; diagnostics: vscode.Diagnostic[] }> {
60-
const results: Array<{ uri: URI; diagnostics: vscode.Diagnostic[] }> = [];
60+
public getDiagnostics(paths: { uri: URI; range: Range | undefined }[]): Array<{ uri: URI; diagnostics: vscode.Diagnostic[]; inputUri?: URI }> {
61+
const results: Array<{ uri: URI; diagnostics: vscode.Diagnostic[]; inputUri?: URI }> = [];
6162

6263
// for notebooks, we need to find the cell matching the range and get diagnostics for that cell
6364
const nonNotebookPaths = paths.filter(p => {
@@ -89,11 +90,27 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
8990
const ranges: Range[] = [];
9091
let shouldTakeAll = false;
9192
let foundMatch = false;
93+
let inputUri: URI | undefined;
94+
let matchedExactPath = false;
95+
9296
for (const path of nonNotebookPaths) {
9397
// we support file or folder paths
9498
if (isEqualOrParent(resource, path.uri)) {
9599
foundMatch = true;
96100

101+
// Track the input URI that matched - prefer exact matches, otherwise use the folder
102+
const isExactMatch = resource.toString() === path.uri.toString();
103+
if (isExactMatch) {
104+
// Exact match - this is the file itself, no input folder
105+
inputUri = undefined;
106+
matchedExactPath = true;
107+
} else if (!matchedExactPath) {
108+
// Folder match - only set if we haven't found an exact match or a previous folder match
109+
if (inputUri === undefined) {
110+
inputUri = path.uri;
111+
}
112+
}
113+
97114
if (pendingMatchPaths.has(path.uri)) {
98115
pendingMatchPaths.delete(path.uri);
99116
}
@@ -109,13 +126,13 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
109126
}
110127

111128
if (shouldTakeAll) {
112-
results.push({ uri: resource, diagnostics: pendingDiagnostics });
129+
results.push({ uri: resource, diagnostics: pendingDiagnostics, inputUri });
113130
continue;
114131
}
115132

116133
if (foundMatch && ranges.length > 0) {
117134
const diagnostics = pendingDiagnostics.filter(d => ranges.some(range => d.range.intersection(range)));
118-
results.push({ uri: resource, diagnostics });
135+
results.push({ uri: resource, diagnostics, inputUri });
119136
}
120137
}
121138

@@ -129,7 +146,7 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
129146

130147
async invoke(options: vscode.LanguageModelToolInvocationOptions<IGetErrorsParams>, token: CancellationToken) {
131148
const getAll = () => this.languageDiagnosticsService.getAllDiagnostics()
132-
.map(d => ({ uri: d[0], diagnostics: d[1].filter(e => e.severity <= DiagnosticSeverity.Warning) }))
149+
.map(d => ({ uri: d[0], diagnostics: d[1].filter(e => e.severity <= DiagnosticSeverity.Warning), inputUri: undefined }))
133150
// filter any documents w/o warnings or errors
134151
.filter(d => d.diagnostics.length > 0);
135152

@@ -146,14 +163,15 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
146163

147164
const ds = options.input.filePaths?.length ? getSome(options.input.filePaths) : getAll();
148165

149-
const diagnostics = coalesce(await Promise.all(ds.map((async ({ uri, diagnostics }) => {
166+
const diagnostics = coalesce(await Promise.all(ds.map((async ({ uri, diagnostics, inputUri }) => {
150167
try {
151168
const document = await this.workspaceService.openTextDocumentAndSnapshot(uri);
152169
checkCancellation(token);
153170
return {
154171
uri,
155172
diagnostics,
156-
context: { document, language: getLanguage(document) }
173+
context: { document, language: getLanguage(document) },
174+
inputUri
157175
};
158176
} catch (e) {
159177
this.logService.error(e, 'get_errors failed to open doc with diagnostics');
@@ -169,7 +187,17 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
169187
]);
170188

171189
const numDiagnostics = diagnostics.reduce((acc, { diagnostics }) => acc + diagnostics.length, 0);
172-
const formattedURIs = this.formatURIs(diagnostics.map(d => d.uri));
190+
191+
// For display message, use inputUri if available (indicating file was found via folder input), otherwise use the file uri
192+
// Deduplicate URIs since multiple files may have the same inputUri
193+
const displayUriSet = new ResourceSet();
194+
for (const d of diagnostics) {
195+
const displayUri = d.inputUri ?? d.uri;
196+
displayUriSet.add(displayUri);
197+
}
198+
199+
const formattedURIs = this.formatURIs(Array.from(displayUriSet));
200+
173201
if (options.input.filePaths?.length) {
174202
result.toolResultMessage = numDiagnostics === 0 ?
175203
new MarkdownString(l10n.t`Checked ${formattedURIs}, no problems found`) :
@@ -326,4 +354,4 @@ export class DiagnosticToolOutput extends PromptElement<IDiagnosticToolOutputPro
326354
)}
327355
</>;
328356
}
329-
}
357+
}

‎src/extension/tools/node/test/getErrorsTool.spec.tsx‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { afterEach, beforeEach, expect, suite, test } from 'vitest';
7+
import { IFileSystemService } from '../../../../platform/filesystem/common/fileSystemService';
8+
import { MockFileSystemService } from '../../../../platform/filesystem/node/test/mockFileSystemService';
79
import { ILanguageDiagnosticsService } from '../../../../platform/languages/common/languageDiagnosticsService';
810
import { TestLanguageDiagnosticsService } from '../../../../platform/languages/common/testLanguageDiagnosticsService';
911
import { IPromptPathRepresentationService } from '../../../../platform/prompts/common/promptPathRepresentationService';
@@ -25,9 +27,11 @@ suite('GetErrorsTool - Tool Invocation', () => {
2527
let accessor: ITestingServicesAccessor;
2628
let collection: TestingServiceCollection;
2729
let diagnosticsService: TestLanguageDiagnosticsService;
30+
let fileSystemService: MockFileSystemService;
2831
let tool: GetErrorsTool;
2932

3033
const workspaceFolder = URI.file('/test/workspace');
34+
const srcFolder = URI.file('/test/workspace/src');
3135
const tsFile1 = URI.file('/test/workspace/src/file1.ts');
3236
const tsFile2 = URI.file('/test/workspace/src/file2.ts');
3337
const jsFile = URI.file('/test/workspace/lib/file.js');
@@ -50,6 +54,11 @@ suite('GetErrorsTool - Tool Invocation', () => {
5054
diagnosticsService = new TestLanguageDiagnosticsService();
5155
collection.define(ILanguageDiagnosticsService, diagnosticsService);
5256

57+
// Set up file system service to mock directories
58+
fileSystemService = new MockFileSystemService();
59+
fileSystemService.mockDirectory(srcFolder, []);
60+
collection.define(IFileSystemService, fileSystemService);
61+
5362
accessor = collection.createTestingAccessor();
5463

5564
// Create the tool instance
@@ -125,8 +134,8 @@ suite('GetErrorsTool - Tool Invocation', () => {
125134

126135
// Should find diagnostics for files in the src folder
127136
expect(results).toEqual([
128-
{ uri: tsFile1, diagnostics: diagnosticsService.getDiagnostics(tsFile1).filter(d => d.severity <= DiagnosticSeverity.Warning) },
129-
{ uri: tsFile2, diagnostics: diagnosticsService.getDiagnostics(tsFile2).filter(d => d.severity <= DiagnosticSeverity.Warning) }
137+
{ uri: tsFile1, diagnostics: diagnosticsService.getDiagnostics(tsFile1).filter(d => d.severity <= DiagnosticSeverity.Warning), inputUri: srcFolder },
138+
{ uri: tsFile2, diagnostics: diagnosticsService.getDiagnostics(tsFile2).filter(d => d.severity <= DiagnosticSeverity.Warning), inputUri: srcFolder }
130139
]);
131140
});
132141

@@ -171,8 +180,8 @@ suite('GetErrorsTool - Tool Invocation', () => {
171180

172181
// Should only include tsFile1 and tsFile2, not infoHintOnlyFile (which has no Warning/Error)
173182
expect(results).toEqual([
174-
{ uri: tsFile1, diagnostics: diagnosticsService.getDiagnostics(tsFile1).filter(d => d.severity <= DiagnosticSeverity.Warning) },
175-
{ uri: tsFile2, diagnostics: diagnosticsService.getDiagnostics(tsFile2).filter(d => d.severity <= DiagnosticSeverity.Warning) }
183+
{ uri: tsFile1, diagnostics: diagnosticsService.getDiagnostics(tsFile1).filter(d => d.severity <= DiagnosticSeverity.Warning), inputUri: srcFolder },
184+
{ uri: tsFile2, diagnostics: diagnosticsService.getDiagnostics(tsFile2).filter(d => d.severity <= DiagnosticSeverity.Warning), inputUri: srcFolder }
176185
]);
177186
});
178187

‎src/platform/filesystem/node/test/mockFileSystemService.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ export class MockFileSystemService implements IFileSystemService {
7474
const mtime = this.mockMtimes.get(uriString) ?? Date.now();
7575
return { type: FileType.File as unknown as FileType, ctime: Date.now() - 1000, mtime, size: contents.length };
7676
}
77+
if (this.mockDirs.has(uriString)) {
78+
return { type: FileType.Directory as unknown as FileType, ctime: Date.now() - 1000, mtime: Date.now(), size: 0 };
79+
}
7780
throw new Error('ENOENT');
7881
}
7982

@@ -93,6 +96,18 @@ export class MockFileSystemService implements IFileSystemService {
9396
const uriString = uri.toString();
9497
const text = new TextDecoder().decode(content);
9598
this.mockFiles.set(uriString, text);
99+
100+
// add the file to the mock directory listing of its parent directory
101+
const parentUri = uriString.substring(0, uriString.lastIndexOf('/'));
102+
if (this.mockDirs.has(parentUri)) {
103+
const entries = this.mockDirs.get(parentUri)!;
104+
const fileName = uriString.substring(uriString.lastIndexOf('/') + 1);
105+
if (!entries.find(e => e[0] === fileName)) {
106+
entries.push([fileName, FileType.File]);
107+
}
108+
} else {
109+
this.mockDirs.set(parentUri, [[uriString.substring(uriString.lastIndexOf('/') + 1), FileType.File]]);
110+
}
96111
}
97112

98113
async delete(uri: URI, options?: { recursive?: boolean; useTrash?: boolean }): Promise<void> {

0 commit comments

Comments
 (0)