mirror of
https://github.com/microsoft/vscode.git
synced 2026-04-02 00:09:30 +01:00
PromptsService: remove unnecessary change events (#305662)
* remove unnecessary change events * Update src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
bf4a0eb258
commit
ea959a9027
@@ -101,12 +101,12 @@ export class PromptsService extends Disposable implements IPromptsService {
|
||||
private readonly internalCustomizations: ChatInternalCustomizations;
|
||||
|
||||
/**
|
||||
* Cached custom agents. Caching only happens if the `onDidChangeCustomAgents` event is used.
|
||||
* Cached custom agents.
|
||||
*/
|
||||
private readonly cachedCustomAgents: CachedPromise<readonly ICustomAgent[]>;
|
||||
|
||||
/**
|
||||
* Cached slash commands. Caching only happens if the `onDidChangeSlashCommands` event is used.
|
||||
* Cached slash commands.
|
||||
*/
|
||||
private readonly cachedSlashCommands: CachedPromise<readonly IChatPromptSlashCommand[]>;
|
||||
|
||||
@@ -116,10 +116,15 @@ export class PromptsService extends Disposable implements IPromptsService {
|
||||
private readonly cachedHooks: CachedPromise<IConfiguredHooksInfo | undefined>;
|
||||
|
||||
/**
|
||||
* Cached skills. Caching only happens if the `onDidChangeSkills` event is used.
|
||||
* Cached skills.
|
||||
*/
|
||||
private readonly cachedSkills: CachedPromise<IAgentSkill[]>;
|
||||
|
||||
/**
|
||||
* Cached instructions.
|
||||
*/
|
||||
private readonly cachedInstructions: CachedPromise<readonly IPromptPath[]>;
|
||||
|
||||
/**
|
||||
* Cache for parsed prompt files keyed by URI.
|
||||
* The number in the returned tuple is textModel.getVersionId(), which is an internal VS Code counter that increments every time the text model's content changes.
|
||||
@@ -231,7 +236,11 @@ export class PromptsService extends Disposable implements IPromptsService {
|
||||
|
||||
this.cachedSkills = this._register(new CachedPromise(
|
||||
(token) => this.computeAgentSkills(token),
|
||||
() => Event.any(this.getFileLocatorEvent(PromptsType.skill), Event.filter(modelChangeEvent, e => e.promptType === PromptsType.skill), this._onDidContributedWhenChange.event, this._onDidPluginPromptFilesChange.event)
|
||||
() => Event.any(
|
||||
this.getFileLocatorEvent(PromptsType.skill),
|
||||
Event.filter(modelChangeEvent, e => e.promptType === PromptsType.skill),
|
||||
this._onDidContributedWhenChange.event,
|
||||
this._onDidPluginPromptFilesChange.event)
|
||||
));
|
||||
|
||||
this.cachedHooks = this._register(new CachedPromise(
|
||||
@@ -244,9 +253,15 @@ export class PromptsService extends Disposable implements IPromptsService {
|
||||
)
|
||||
));
|
||||
|
||||
// Hack: Subscribe to activate caching (CachedPromise only caches when onDidChange has listeners)
|
||||
this._register(this.cachedSkills.onDidChange(() => { }));
|
||||
this._register(this.cachedHooks.onDidChange(() => { }));
|
||||
this.cachedInstructions = this._register(new CachedPromise(
|
||||
(token) => this.computeInstructionFiles(token),
|
||||
() => Event.any(
|
||||
this.getFileLocatorEvent(PromptsType.instructions),
|
||||
this._onDidContributedWhenChange.event,
|
||||
this._onDidChangeInstructions.event,
|
||||
this._onDidPluginPromptFilesChange.event,
|
||||
)
|
||||
));
|
||||
|
||||
this._register(this.watchPluginPromptFilesForType(
|
||||
PromptsType.prompt,
|
||||
@@ -587,7 +602,7 @@ export class PromptsService extends Disposable implements IPromptsService {
|
||||
* Emitter for slash commands change events.
|
||||
*/
|
||||
public get onDidChangeSlashCommands(): Event<void> {
|
||||
return this.cachedSlashCommands.onDidChange;
|
||||
return this.cachedSlashCommands.onDidChangePromise;
|
||||
}
|
||||
|
||||
public async getPromptSlashCommands(token: CancellationToken, sessionResource?: URI): Promise<readonly IChatPromptSlashCommand[]> {
|
||||
@@ -684,16 +699,11 @@ export class PromptsService extends Disposable implements IPromptsService {
|
||||
* Emitter for custom agents change events.
|
||||
*/
|
||||
public get onDidChangeCustomAgents(): Event<void> {
|
||||
return this.cachedCustomAgents.onDidChange;
|
||||
return this.cachedCustomAgents.onDidChangePromise;
|
||||
}
|
||||
|
||||
public get onDidChangeInstructions(): Event<void> {
|
||||
return Event.any(
|
||||
this.getFileLocatorEvent(PromptsType.instructions),
|
||||
this._onDidContributedWhenChange.event,
|
||||
this._onDidChangeInstructions.event,
|
||||
this._onDidPluginPromptFilesChange.event,
|
||||
);
|
||||
return this.cachedInstructions.onDidChangePromise;
|
||||
}
|
||||
|
||||
public async getCustomAgents(token: CancellationToken, sessionResource?: URI): Promise<readonly ICustomAgent[]> {
|
||||
@@ -1099,7 +1109,7 @@ export class PromptsService extends Disposable implements IPromptsService {
|
||||
}
|
||||
|
||||
public get onDidChangeSkills(): Event<void> {
|
||||
return this.cachedSkills.onDidChange;
|
||||
return this.cachedSkills.onDidChangePromise;
|
||||
}
|
||||
|
||||
public async findAgentSkills(token: CancellationToken, sessionResource?: URI): Promise<IAgentSkill[] | undefined> {
|
||||
@@ -1259,7 +1269,7 @@ export class PromptsService extends Disposable implements IPromptsService {
|
||||
|
||||
public async getInstructionFiles(token: CancellationToken, sessionResource?: URI): Promise<readonly IPromptPath[]> {
|
||||
const sw = StopWatch.create();
|
||||
const result = await this.listPromptFiles(PromptsType.instructions, token);
|
||||
const result = await this.cachedInstructions.get(token);
|
||||
if (sessionResource) {
|
||||
const elapsed = sw.elapsed();
|
||||
void this.getInstructionsDiscoveryInfo(token).catch(() => undefined).then(discoveryInfo => {
|
||||
@@ -1278,6 +1288,10 @@ export class PromptsService extends Disposable implements IPromptsService {
|
||||
return result;
|
||||
}
|
||||
|
||||
private async computeInstructionFiles(token: CancellationToken): Promise<readonly IPromptPath[]> {
|
||||
return await this.listPromptFiles(PromptsType.instructions, token);
|
||||
}
|
||||
|
||||
private async computeHooks(token: CancellationToken): Promise<IConfiguredHooksInfo | undefined> {
|
||||
const useChatHooks = this.configurationService.getValue(PromptsConfig.USE_CHAT_HOOKS);
|
||||
if (!useChatHooks) {
|
||||
@@ -1715,21 +1729,19 @@ export class PromptsService extends Disposable implements IPromptsService {
|
||||
|
||||
class CachedPromise<T> extends Disposable {
|
||||
private cachedPromise: Promise<T> | undefined = undefined;
|
||||
private onDidUpdatePromiseEmitter: Emitter<void> | undefined = undefined;
|
||||
private readonly onDidUpdatePromiseEmitter: Emitter<void>;
|
||||
|
||||
constructor(private readonly computeFn: (token: CancellationToken) => Promise<T>, private readonly getEvent: () => Event<void>, private readonly delay: number = 0) {
|
||||
super();
|
||||
this.onDidUpdatePromiseEmitter = this._register(new Emitter<void>());
|
||||
const delayer = this._register(new Delayer<void>(this.delay));
|
||||
this._register(this.getEvent()(() => {
|
||||
this.cachedPromise = undefined;
|
||||
delayer.trigger(() => this.onDidUpdatePromiseEmitter.fire());
|
||||
}));
|
||||
}
|
||||
|
||||
public get onDidChange(): Event<void> {
|
||||
if (!this.onDidUpdatePromiseEmitter) {
|
||||
const emitter = this.onDidUpdatePromiseEmitter = this._register(new Emitter<void>());
|
||||
const delayer = this._register(new Delayer<void>(this.delay));
|
||||
this._register(this.getEvent()(() => {
|
||||
this.cachedPromise = undefined;
|
||||
delayer.trigger(() => emitter.fire());
|
||||
}));
|
||||
}
|
||||
public get onDidChangePromise(): Event<void> {
|
||||
return this.onDidUpdatePromiseEmitter.event;
|
||||
}
|
||||
|
||||
@@ -1737,13 +1749,14 @@ class CachedPromise<T> extends Disposable {
|
||||
if (this.cachedPromise !== undefined) {
|
||||
return this.cachedPromise;
|
||||
}
|
||||
const result = this.computeFn(token);
|
||||
if (!this.onDidUpdatePromiseEmitter) {
|
||||
return result; // only cache if there is an event listener
|
||||
}
|
||||
this.cachedPromise = result;
|
||||
this.onDidUpdatePromiseEmitter.fire();
|
||||
return result;
|
||||
const promise = this.computeFn(token).catch(err => {
|
||||
if (this.cachedPromise === promise) {
|
||||
this.cachedPromise = undefined;
|
||||
}
|
||||
throw err;
|
||||
});
|
||||
this.cachedPromise = promise;
|
||||
return promise;
|
||||
}
|
||||
|
||||
public refresh(): void {
|
||||
@@ -1774,19 +1787,28 @@ class ModelChangeTracker extends Disposable {
|
||||
if (promptType !== undefined) {
|
||||
this.listeners.set(model.uri, model.onDidChangeContent(() => this.onDidPromptModelChange.fire({ uri: model.uri, promptType })));
|
||||
}
|
||||
return promptType;
|
||||
};
|
||||
const onRemove = (languageId: string, uri: URI) => {
|
||||
const promptType = getPromptsTypeForLanguageId(languageId);
|
||||
if (promptType !== undefined) {
|
||||
this.listeners.get(uri)?.dispose();
|
||||
this.listeners.delete(uri);
|
||||
this.onDidPromptModelChange.fire({ uri, promptType });
|
||||
}
|
||||
return promptType;
|
||||
};
|
||||
this._register(modelService.onModelAdded(model => onAdd(model)));
|
||||
this._register(modelService.onModelLanguageChanged(e => {
|
||||
onRemove(e.oldLanguageId, e.model.uri);
|
||||
onAdd(e.model);
|
||||
const removedPromptType = onRemove(e.oldLanguageId, e.model.uri);
|
||||
const addedPromptType = onAdd(e.model);
|
||||
if (removedPromptType !== addedPromptType) {
|
||||
if (removedPromptType) {
|
||||
this.onDidPromptModelChange.fire({ uri: e.model.uri, promptType: removedPromptType });
|
||||
}
|
||||
if (addedPromptType) {
|
||||
this.onDidPromptModelChange.fire({ uri: e.model.uri, promptType: addedPromptType });
|
||||
}
|
||||
}
|
||||
}));
|
||||
this._register(modelService.onModelRemoved(model => onRemove(model.getLanguageId(), model.uri)));
|
||||
}
|
||||
|
||||
@@ -194,7 +194,7 @@ export class PromptFilesLocator {
|
||||
}
|
||||
};
|
||||
|
||||
const update = async (emitEvent: boolean) => {
|
||||
const update = async () => {
|
||||
try {
|
||||
const configuredLocations = PromptsConfig.promptSourceFolders(this.configService, type);
|
||||
parentFolders = await this.toAbsoluteLocations(type, configuredLocations, undefined);
|
||||
@@ -203,23 +203,23 @@ export class PromptFilesLocator {
|
||||
return;
|
||||
}
|
||||
updateExternalFolderWatchers();
|
||||
if (emitEvent) {
|
||||
eventEmitter.fire();
|
||||
}
|
||||
} catch (err) {
|
||||
this.logService.error(`Error updating prompt file watchers after config change:`, err);
|
||||
}
|
||||
};
|
||||
disposables.add(this.configService.onDidChangeConfiguration(e => {
|
||||
if (e.affectsConfiguration(key)) {
|
||||
void update(true);
|
||||
if (e.affectsConfiguration(key) || e.affectsConfiguration(PromptsConfig.USE_CUSTOMIZATIONS_IN_PARENT_REPOS)) {
|
||||
void update();
|
||||
eventEmitter.fire();
|
||||
}
|
||||
}));
|
||||
disposables.add(this.onDidChangeWorkspaceFolders()(() => {
|
||||
void update(true);
|
||||
void update();
|
||||
eventEmitter.fire();
|
||||
}));
|
||||
disposables.add(this.workspaceTrustManagementService.onDidChangeTrustedFolders(() => {
|
||||
void update(true);
|
||||
void update();
|
||||
eventEmitter.fire();
|
||||
}));
|
||||
disposables.add(this.fileService.onDidFilesChange(e => {
|
||||
if (e.affects(userDataFolder)) {
|
||||
@@ -233,7 +233,7 @@ export class PromptFilesLocator {
|
||||
}));
|
||||
disposables.add(this.fileService.watch(userDataFolder));
|
||||
|
||||
void update(false);
|
||||
void update();
|
||||
|
||||
return { event: eventEmitter.event, dispose: () => disposables.dispose() };
|
||||
}
|
||||
|
||||
@@ -18,7 +18,7 @@ import { Range } from '../../../../../../../editor/common/core/range.js';
|
||||
import { ILanguageService } from '../../../../../../../editor/common/languages/language.js';
|
||||
import { IModelService } from '../../../../../../../editor/common/services/model.js';
|
||||
import { ModelService } from '../../../../../../../editor/common/services/modelService.js';
|
||||
import { IConfigurationService } from '../../../../../../../platform/configuration/common/configuration.js';
|
||||
import { IConfigurationChangeEvent, IConfigurationService } from '../../../../../../../platform/configuration/common/configuration.js';
|
||||
import { TestConfigurationService } from '../../../../../../../platform/configuration/test/common/testConfigurationService.js';
|
||||
import { IExtensionDescription } from '../../../../../../../platform/extensions/common/extensions.js';
|
||||
import { IFileService } from '../../../../../../../platform/files/common/files.js';
|
||||
@@ -2101,8 +2101,10 @@ suite('PromptsService', () => {
|
||||
},
|
||||
]);
|
||||
|
||||
testConfigService.setUserConfiguration(PromptsConfig.INCLUDE_APPLYING_INSTRUCTIONS, true);
|
||||
testConfigService.setUserConfiguration(PromptsConfig.USE_CUSTOMIZATIONS_IN_PARENT_REPOS, false);
|
||||
|
||||
|
||||
await testConfigService.setUserConfiguration(PromptsConfig.INCLUDE_APPLYING_INSTRUCTIONS, true);
|
||||
await testConfigService.setUserConfiguration(PromptsConfig.USE_CUSTOMIZATIONS_IN_PARENT_REPOS, false);
|
||||
|
||||
// With parent search disabled, should not find parent files
|
||||
let promptFiles = await service.listPromptFiles(PromptsType.prompt, CancellationToken.None);
|
||||
@@ -2115,6 +2117,7 @@ suite('PromptsService', () => {
|
||||
|
||||
// With parent search enabled, should find parent files
|
||||
testConfigService.setUserConfiguration(PromptsConfig.USE_CUSTOMIZATIONS_IN_PARENT_REPOS, true);
|
||||
fireConfigChange(testConfigService, PromptsConfig.USE_CUSTOMIZATIONS_IN_PARENT_REPOS);
|
||||
|
||||
promptFiles = await service.listPromptFiles(PromptsType.prompt, CancellationToken.None);
|
||||
agentFiles = await service.listPromptFiles(PromptsType.agent, CancellationToken.None);
|
||||
@@ -2181,6 +2184,8 @@ suite('PromptsService', () => {
|
||||
|
||||
testConfigService.setUserConfiguration(PromptsConfig.INCLUDE_APPLYING_INSTRUCTIONS, true);
|
||||
testConfigService.setUserConfiguration(PromptsConfig.USE_CUSTOMIZATIONS_IN_PARENT_REPOS, true);
|
||||
fireConfigChange(testConfigService, PromptsConfig.INCLUDE_APPLYING_INSTRUCTIONS, PromptsConfig.USE_CUSTOMIZATIONS_IN_PARENT_REPOS);
|
||||
|
||||
|
||||
// Mark the parent repo root as untrusted
|
||||
workspaceTrustService.getUriTrustInfo = (uri: URI) => {
|
||||
@@ -3992,3 +3997,9 @@ suite('PromptsService', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
function fireConfigChange(configService: TestConfigurationService, ...key: string[]): void {
|
||||
configService.onDidChangeConfigurationEmitter.fire({
|
||||
affectsConfiguration: (k: string) => key.includes(k),
|
||||
} satisfies Partial<IConfigurationChangeEvent> as unknown as IConfigurationChangeEvent);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user