Skip to content

Commit 46cbcfa

Browse files
committed
getErrorsTool display folder instead of individual child files
1 parent ab39049 commit 46cbcfa

File tree

3 files changed

+56
-18
lines changed

3 files changed

+56
-18
lines changed

src/extension/tools/node/getErrorsTool.tsx

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
5656
* Get diagnostics for the given paths and optional ranges.
5757
* Note - This is made public for testing purposes only.
5858
*/
59-
public getDiagnostics(paths: { uri: URI; range: Range | undefined }[]): Array<{ uri: URI; diagnostics: vscode.Diagnostic[] }> {
60-
const results: Array<{ uri: URI; diagnostics: vscode.Diagnostic[] }> = [];
59+
public getDiagnostics(paths: { uri: URI; range: Range | undefined }[]): Array<{ uri: URI; diagnostics: vscode.Diagnostic[]; inputUri?: URI }> {
60+
const results: Array<{ uri: URI; diagnostics: vscode.Diagnostic[]; inputUri?: URI }> = [];
6161

6262
// for notebooks, we need to find the cell matching the range and get diagnostics for that cell
6363
const nonNotebookPaths = paths.filter(p => {
@@ -89,11 +89,25 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
8989
const ranges: Range[] = [];
9090
let shouldTakeAll = false;
9191
let foundMatch = false;
92+
let inputUri: URI | undefined;
93+
let matchedExactPath = false;
94+
9295
for (const path of nonNotebookPaths) {
9396
// we support file or folder paths
9497
if (isEqualOrParent(resource, path.uri)) {
9598
foundMatch = true;
9699

100+
// Track the input URI that matched - prefer exact matches, otherwise use the folder
101+
const isExactMatch = resource.toString() === path.uri.toString();
102+
if (isExactMatch) {
103+
// Exact match - this is the file itself, no input folder
104+
inputUri = undefined;
105+
matchedExactPath = true;
106+
} else if (!matchedExactPath) {
107+
// Folder match - only set if we haven't found an exact match
108+
inputUri = path.uri;
109+
}
110+
97111
if (pendingMatchPaths.has(path.uri)) {
98112
pendingMatchPaths.delete(path.uri);
99113
}
@@ -103,19 +117,22 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
103117
} else {
104118
// no range, so all diagnostics for this file
105119
shouldTakeAll = true;
106-
break;
120+
// only break for exact matches
121+
if (isExactMatch) {
122+
break;
123+
}
107124
}
108125
}
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 as URI | 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 Set<string>();
194+
for (const d of diagnostics) {
195+
const displayUri = d.inputUri ?? d.uri;
196+
displayUriSet.add(displayUri.toString());
197+
}
198+
const displayUris = Array.from(displayUriSet).map(uriStr => URI.parse(uriStr));
199+
const formattedURIs = this.formatURIs(displayUris);
200+
173201
if (options.input.filePaths?.length) {
174202
result.toolResultMessage = numDiagnostics === 0 ?
175203
new MarkdownString(l10n.t`Checked ${formattedURIs}, no problems found`) :
@@ -276,11 +304,9 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
276304
ToolRegistry.registerTool(GetErrorsTool);
277305

278306
interface IDiagnosticToolOutputProps extends BasePromptElementProps {
279-
diagnosticsGroups: { context: DiagnosticContext; uri: URI; diagnostics: vscode.Diagnostic[] }[];
307+
diagnosticsGroups: { context: DiagnosticContext; uri: URI; diagnostics: vscode.Diagnostic[]; inputUri?: URI }[];
280308
maxDiagnostics?: number;
281-
}
282-
283-
export class DiagnosticToolOutput extends PromptElement<IDiagnosticToolOutputProps> {
309+
}export class DiagnosticToolOutput extends PromptElement<IDiagnosticToolOutputProps> {
284310
constructor(
285311
props: PromptElementProps<IDiagnosticToolOutputProps>,
286312
@IPromptPathRepresentationService private readonly promptPathRepresentationService: IPromptPathRepresentationService,
@@ -326,4 +352,4 @@ export class DiagnosticToolOutput extends PromptElement<IDiagnosticToolOutputPro
326352
)}
327353
</>;
328354
}
329-
}
355+
}

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: 3 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

0 commit comments

Comments
 (0)