Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion src/extension/tools/node/findTestsFilesTool.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class FindTestFilesTool extends Disposable implements ICopilotTool<IFindTestFile
}

private formatURIs(uris: URI[]): string {
return uris.map(formatUriForFileWidget).join(', ');
return uris.map(uri => formatUriForFileWidget(uri)).join(', ');
}

async provideInput(promptContext: IBuildPromptContext): Promise<IFindTestFilesToolsParams | undefined> {
Expand Down
2 changes: 1 addition & 1 deletion src/extension/tools/node/getErrorsTool.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ export class GetErrorsTool extends Disposable implements ICopilotTool<IGetErrors
}

private formatURIs(uris: URI[]): string {
return uris.map(formatUriForFileWidget).join(', ');
return uris.map(uri => formatUriForFileWidget(uri)).join(', ');
}

private getNotebookCellDiagnostics(uri: URI) {
Expand Down
13 changes: 13 additions & 0 deletions src/extension/tools/node/readFileTool.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,14 @@ export class ReadFileTool implements ICopilotTool<ReadFileParams> {
}

const { start, end } = getParamRanges(input, documentSnapshot);

if (start === 1 && end === documentSnapshot.lineCount) {
if (this.customInstructionsService.isSkillFile(uri)) {
return {
invocationMessage: new MarkdownString(l10n.t`Loading skill ${formatUriForFileWidget(uri)}`),
pastTenseMessage: new MarkdownString(l10n.t`Loaded skill ${formatUriForFileWidget(uri)}`),
};
}
return {
invocationMessage: new MarkdownString(l10n.t`Reading ${formatUriForFileWidget(uri)}`),
pastTenseMessage: new MarkdownString(l10n.t`Read ${formatUriForFileWidget(uri)}`),
Expand All @@ -180,6 +187,12 @@ export class ReadFileTool implements ICopilotTool<ReadFileParams> {

// Jump to the start of the range, don't select the whole range
const readLocation = new Location(uri, new Range(start - 1, 0, start - 1, 0));
if (this.customInstructionsService.isSkillFile(uri)) {
return {
invocationMessage: new MarkdownString(l10n.t`Reading skill ${formatUriForFileWidget(readLocation)}, lines ${start} to ${end}`),
pastTenseMessage: new MarkdownString(l10n.t`Read skill ${formatUriForFileWidget(readLocation)}, lines ${start} to ${end}`),
};
}
return {
invocationMessage: new MarkdownString(l10n.t`Reading ${formatUriForFileWidget(readLocation)}, lines ${start} to ${end}`),
pastTenseMessage: new MarkdownString(l10n.t`Read ${formatUriForFileWidget(readLocation)}, lines ${start} to ${end}`),
Expand Down
149 changes: 148 additions & 1 deletion src/extension/tools/node/test/readFile.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,20 @@
*--------------------------------------------------------------------------------------------*/

import { afterAll, beforeAll, expect, suite, test } from 'vitest';
import { ICustomInstructionsService } from '../../../../platform/customInstructions/common/customInstructionsService';
import { ITestingServicesAccessor } from '../../../../platform/test/node/services';
import { TestWorkspaceService } from '../../../../platform/test/node/testWorkspaceService';
import { IWorkspaceService } from '../../../../platform/workspace/common/workspaceService';
import { createTextDocumentData } from '../../../../util/common/test/shims/textDocument';
import { CancellationToken } from '../../../../util/vs/base/common/cancellation';
import { URI } from '../../../../util/vs/base/common/uri';
import { SyncDescriptor } from '../../../../util/vs/platform/instantiation/common/descriptors';
import { IInstantiationService } from '../../../../util/vs/platform/instantiation/common/instantiation';
import { MarkdownString } from '../../../../vscodeTypes';
import { createExtensionUnitTestingServices } from '../../../test/node/services';
import { ToolName } from '../../common/toolNames';
import { IToolsService } from '../../common/toolsService';
import { IReadFileParamsV1, IReadFileParamsV2 } from '../readFileTool';
import { IReadFileParamsV1, IReadFileParamsV2, ReadFileTool } from '../readFileTool';
import { toolResultToString } from './toolTestUtils';

suite('ReadFile', () => {
Expand Down Expand Up @@ -243,4 +246,148 @@ suite('ReadFile', () => {
expect(resultString).toContain('line 2');
});
});

suite('prepareInvocation', () => {
class MockCustomInstructionsService implements ICustomInstructionsService {
declare readonly _serviceBrand: undefined;
private skillFiles = new Set<string>();

setSkillFiles(uris: URI[]) {
this.skillFiles.clear();
uris.forEach(uri => this.skillFiles.add(uri.toString()));
}

isSkillFile(uri: URI): boolean {
return this.skillFiles.has(uri.toString());
}

isExternalInstructionsFile(): boolean {
return false;
}

isExternalInstructionsFolder(): boolean {
return false;
}

fetchInstructionsFromSetting(): Promise<any[]> {
return Promise.resolve([]);
}

fetchInstructionsFromFile(): Promise<any> {
return Promise.resolve(undefined);
}

getAgentInstructions(): Promise<URI[]> {
return Promise.resolve([]);
}
}

test('should return "Loading/Loaded skill" message for skill files', async () => {
const testDoc = createTextDocumentData(URI.file('/workspace/test.skill.md'), 'skill content', 'markdown').document;

const services = createExtensionUnitTestingServices();
services.define(IWorkspaceService, new SyncDescriptor(
TestWorkspaceService,
[
[URI.file('/workspace')],
[testDoc],
]
));

const mockCustomInstructions = new MockCustomInstructionsService();
mockCustomInstructions.setSkillFiles([URI.file('/workspace/test.skill.md')]);
services.define(ICustomInstructionsService, mockCustomInstructions);

const testAccessor = services.createTestingAccessor();
const readFileTool = testAccessor.get(IInstantiationService).createInstance(ReadFileTool);

const input: IReadFileParamsV2 = {
filePath: '/workspace/test.skill.md'
};

const result = await readFileTool.prepareInvocation(
{ input },
CancellationToken.None
);

expect(result).toBeDefined();
expect((result!.invocationMessage as MarkdownString).value).toBe('Loading skill [](file:///workspace/test.skill.md)');
expect((result!.pastTenseMessage as MarkdownString).value).toBe('Loaded skill [](file:///workspace/test.skill.md)');

testAccessor.dispose();
});

test('should return "Reading/Read" message for non-skill files', async () => {
const testDoc = createTextDocumentData(URI.file('/workspace/test.ts'), 'code content', 'typescript').document;

const services = createExtensionUnitTestingServices();
services.define(IWorkspaceService, new SyncDescriptor(
TestWorkspaceService,
[
[URI.file('/workspace')],
[testDoc],
]
));

const mockCustomInstructions = new MockCustomInstructionsService();
// Don't mark this file as a skill file
services.define(ICustomInstructionsService, mockCustomInstructions);

const testAccessor = services.createTestingAccessor();
const readFileTool = testAccessor.get(IInstantiationService).createInstance(ReadFileTool);

const input: IReadFileParamsV2 = {
filePath: '/workspace/test.ts'
};

const result = await readFileTool.prepareInvocation(
{ input },
CancellationToken.None
);

expect(result).toBeDefined();
expect((result!.invocationMessage as MarkdownString).value).toBe('Reading [](file:///workspace/test.ts)');
expect((result!.pastTenseMessage as MarkdownString).value).toBe('Read [](file:///workspace/test.ts)');

testAccessor.dispose();
});

test('should return "Reading skill/Read skill" message for skill files with line range', async () => {
const testDoc = createTextDocumentData(URI.file('/workspace/test.skill.md'), 'line 1\nline 2\nline 3\nline 4\nline 5', 'markdown').document;

const services = createExtensionUnitTestingServices();
services.define(IWorkspaceService, new SyncDescriptor(
TestWorkspaceService,
[
[URI.file('/workspace')],
[testDoc],
]
));

const mockCustomInstructions = new MockCustomInstructionsService();
mockCustomInstructions.setSkillFiles([URI.file('/workspace/test.skill.md')]);
services.define(ICustomInstructionsService, mockCustomInstructions);

const testAccessor = services.createTestingAccessor();
const readFileTool = testAccessor.get(IInstantiationService).createInstance(ReadFileTool);

const input: IReadFileParamsV2 = {
filePath: '/workspace/test.skill.md',
offset: 2,
limit: 2
};

const result = await readFileTool.prepareInvocation(
{ input },
CancellationToken.None
);

expect(result).toBeDefined();
// When reading a partial range of a skill file, it should say "Reading skill"
expect((result!.invocationMessage as MarkdownString).value).toBe('Reading skill [](file:///workspace/test.skill.md#2-2), lines 2 to 4');
expect((result!.pastTenseMessage as MarkdownString).value).toBe('Read skill [](file:///workspace/test.skill.md#2-2), lines 2 to 4');

testAccessor.dispose();
});
});
});