Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 37 additions & 10 deletions src/extension/tools/node/getErrorsTool.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
* Get diagnostics for the given paths and optional ranges.
* Note - This is made public for testing purposes only.
*/
public getDiagnostics(paths: { uri: URI; range: Range | undefined }[]): Array<{ uri: URI; diagnostics: vscode.Diagnostic[] }> {
const results: Array<{ uri: URI; diagnostics: vscode.Diagnostic[] }> = [];
public getDiagnostics(paths: { uri: URI; range: Range | undefined }[]): Array<{ uri: URI; diagnostics: vscode.Diagnostic[]; inputUri?: URI }> {
const results: Array<{ uri: URI; diagnostics: vscode.Diagnostic[]; inputUri?: URI }> = [];

// for notebooks, we need to find the cell matching the range and get diagnostics for that cell
const nonNotebookPaths = paths.filter(p => {
Expand Down Expand Up @@ -89,11 +89,27 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
const ranges: Range[] = [];
let shouldTakeAll = false;
let foundMatch = false;
let inputUri: URI | undefined;
let matchedExactPath = false;

for (const path of nonNotebookPaths) {
// we support file or folder paths
if (isEqualOrParent(resource, path.uri)) {
foundMatch = true;

// Track the input URI that matched - prefer exact matches, otherwise use the folder
const isExactMatch = resource.toString() === path.uri.toString();
if (isExactMatch) {
// Exact match - this is the file itself, no input folder
inputUri = undefined;
matchedExactPath = true;
} else if (!matchedExactPath) {
// Folder match - only set if we haven't found an exact match or a previous folder match
if (inputUri === undefined) {
inputUri = path.uri;
}
}

if (pendingMatchPaths.has(path.uri)) {
pendingMatchPaths.delete(path.uri);
}
Expand All @@ -109,13 +125,13 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
}

if (shouldTakeAll) {
results.push({ uri: resource, diagnostics: pendingDiagnostics });
results.push({ uri: resource, diagnostics: pendingDiagnostics, inputUri });
continue;
}

if (foundMatch && ranges.length > 0) {
const diagnostics = pendingDiagnostics.filter(d => ranges.some(range => d.range.intersection(range)));
results.push({ uri: resource, diagnostics });
results.push({ uri: resource, diagnostics, inputUri });
}
}

Expand All @@ -129,7 +145,7 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors

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

Expand All @@ -146,14 +162,15 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors

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

const diagnostics = coalesce(await Promise.all(ds.map((async ({ uri, diagnostics }) => {
const diagnostics = coalesce(await Promise.all(ds.map((async ({ uri, diagnostics, inputUri }) => {
try {
const document = await this.workspaceService.openTextDocumentAndSnapshot(uri);
checkCancellation(token);
return {
uri,
diagnostics,
context: { document, language: getLanguage(document) }
context: { document, language: getLanguage(document) },
inputUri
};
} catch (e) {
this.logService.error(e, 'get_errors failed to open doc with diagnostics');
Expand All @@ -169,7 +186,17 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
]);

const numDiagnostics = diagnostics.reduce((acc, { diagnostics }) => acc + diagnostics.length, 0);
const formattedURIs = this.formatURIs(diagnostics.map(d => d.uri));

// For display message, use inputUri if available (indicating file was found via folder input), otherwise use the file uri
// Deduplicate URIs since multiple files may have the same inputUri
const displayUriSet = new Set<string>();
for (const d of diagnostics) {
const displayUri = d.inputUri ?? d.uri;
displayUriSet.add(displayUri.toString());
}
const displayUris = Array.from(displayUriSet).map(uriStr => URI.parse(uriStr));
const formattedURIs = this.formatURIs(displayUris);

if (options.input.filePaths?.length) {
result.toolResultMessage = numDiagnostics === 0 ?
new MarkdownString(l10n.t`Checked ${formattedURIs}, no problems found`) :
Expand Down Expand Up @@ -276,7 +303,7 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
ToolRegistry.registerTool(GetErrorsTool);

interface IDiagnosticToolOutputProps extends BasePromptElementProps {
diagnosticsGroups: { context: DiagnosticContext; uri: URI; diagnostics: vscode.Diagnostic[] }[];
diagnosticsGroups: { context: DiagnosticContext; uri: URI; diagnostics: vscode.Diagnostic[]; inputUri?: URI }[];
maxDiagnostics?: number;
}

Expand Down Expand Up @@ -326,4 +353,4 @@ export class DiagnosticToolOutput extends PromptElement<IDiagnosticToolOutputPro
)}
</>;
}
}
}
17 changes: 13 additions & 4 deletions src/extension/tools/node/test/getErrorsTool.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*--------------------------------------------------------------------------------------------*/

import { afterEach, beforeEach, expect, suite, test } from 'vitest';
import { IFileSystemService } from '../../../../platform/filesystem/common/fileSystemService';
import { MockFileSystemService } from '../../../../platform/filesystem/node/test/mockFileSystemService';
import { ILanguageDiagnosticsService } from '../../../../platform/languages/common/languageDiagnosticsService';
import { TestLanguageDiagnosticsService } from '../../../../platform/languages/common/testLanguageDiagnosticsService';
import { IPromptPathRepresentationService } from '../../../../platform/prompts/common/promptPathRepresentationService';
Expand All @@ -25,9 +27,11 @@ suite('GetErrorsTool - Tool Invocation', () => {
let accessor: ITestingServicesAccessor;
let collection: TestingServiceCollection;
let diagnosticsService: TestLanguageDiagnosticsService;
let fileSystemService: MockFileSystemService;
let tool: GetErrorsTool;

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

// Set up file system service to mock directories
fileSystemService = new MockFileSystemService();
fileSystemService.mockDirectory(srcFolder, []);
collection.define(IFileSystemService, fileSystemService);

accessor = collection.createTestingAccessor();

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

// Should find diagnostics for files in the src folder
expect(results).toEqual([
{ uri: tsFile1, diagnostics: diagnosticsService.getDiagnostics(tsFile1).filter(d => d.severity <= DiagnosticSeverity.Warning) },
{ uri: tsFile2, diagnostics: diagnosticsService.getDiagnostics(tsFile2).filter(d => d.severity <= DiagnosticSeverity.Warning) }
{ uri: tsFile1, diagnostics: diagnosticsService.getDiagnostics(tsFile1).filter(d => d.severity <= DiagnosticSeverity.Warning), inputUri: srcFolder },
{ uri: tsFile2, diagnostics: diagnosticsService.getDiagnostics(tsFile2).filter(d => d.severity <= DiagnosticSeverity.Warning), inputUri: srcFolder }
]);
});

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

// Should only include tsFile1 and tsFile2, not infoHintOnlyFile (which has no Warning/Error)
expect(results).toEqual([
{ uri: tsFile1, diagnostics: diagnosticsService.getDiagnostics(tsFile1).filter(d => d.severity <= DiagnosticSeverity.Warning) },
{ uri: tsFile2, diagnostics: diagnosticsService.getDiagnostics(tsFile2).filter(d => d.severity <= DiagnosticSeverity.Warning) }
{ uri: tsFile1, diagnostics: diagnosticsService.getDiagnostics(tsFile1).filter(d => d.severity <= DiagnosticSeverity.Warning), inputUri: srcFolder },
{ uri: tsFile2, diagnostics: diagnosticsService.getDiagnostics(tsFile2).filter(d => d.severity <= DiagnosticSeverity.Warning), inputUri: srcFolder }
]);
});

Expand Down
15 changes: 15 additions & 0 deletions src/platform/filesystem/node/test/mockFileSystemService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ export class MockFileSystemService implements IFileSystemService {
const mtime = this.mockMtimes.get(uriString) ?? Date.now();
return { type: FileType.File as unknown as FileType, ctime: Date.now() - 1000, mtime, size: contents.length };
}
if (this.mockDirs.has(uriString)) {
return { type: FileType.Directory as unknown as FileType, ctime: Date.now() - 1000, mtime: Date.now(), size: 0 };
}
throw new Error('ENOENT');
}

Expand All @@ -93,6 +96,18 @@ export class MockFileSystemService implements IFileSystemService {
const uriString = uri.toString();
const text = new TextDecoder().decode(content);
this.mockFiles.set(uriString, text);

// add the file to the mock directory listing of its parent directory
const parentUri = uriString.substring(0, uriString.lastIndexOf('/'));
if (this.mockDirs.has(parentUri)) {
const entries = this.mockDirs.get(parentUri)!;
const fileName = uriString.substring(uriString.lastIndexOf('/') + 1);
if (!entries.find(e => e[0] === fileName)) {
entries.push([fileName, FileType.File]);
}
} else {
this.mockDirs.set(parentUri, [[uriString.substring(uriString.lastIndexOf('/') + 1), FileType.File]]);
}
}

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