From 3af4d05a8480d5e0088252c90eedbe179bcce1a6 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 23 Nov 2023 07:41:57 +1100 Subject: [PATCH] Add setting for kernel completions (#14763) * Update types to reflect version of Node & browsers * Remove completion exp, add setting to enable it --- package-lock.json | 14 +- package.json | 17 +- package.nls.json | 3 +- src/gdpr.ts | 14 ++ src/platform/common/constants.ts | 1 + src/platform/common/types.ts | 3 +- .../nonPythonKernelCompletionProvider.ts | 180 +++++++++++++++--- src/telemetry.ts | 60 +++++- src/tsconfig.extension.node.json | 2 +- src/tsconfig.extension.web.json | 2 +- tsconfig.json | 2 +- 11 files changed, 250 insertions(+), 48 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8562066a8..6614024d0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -113,7 +113,7 @@ "@types/memoize-one": "^4.1.1", "@types/mocha": "^9.1.0", "@types/nock": "^10.0.3", - "@types/node": "^16.18.6", + "@types/node": "^18.15.0", "@types/node-fetch": "^2.5.7", "@types/pdfkit": "^0.11.0", "@types/promisify-node": "^0.4.0", @@ -3573,9 +3573,9 @@ } }, "node_modules/@types/node": { - "version": "16.18.6", - "resolved": "https://registry.npmjs.org/@types/node/-/node-16.18.6.tgz", - "integrity": "sha512-vmYJF0REqDyyU0gviezF/KHq/fYaUbFhkcNbQCuPGFQj6VTbXuHZoxs/Y7mutWe73C8AC6l9fFu8mSYiBAqkGA==", + "version": "18.15.0", + "resolved": "https://registry.npmjs.org/@types/node/-/node-18.15.0.tgz", + "integrity": "sha512-z6nr0TTEOBGkzLGmbypWOGnpSpSIBorEhC4L+4HeQ2iezKCi4f77kyslRwvHeNitymGQ+oFyIWGP96l/DPSV9w==", "dev": true }, "node_modules/@types/node-fetch": { @@ -25095,9 +25095,9 @@ } }, "@types/node": { - "version": "16.18.6", - "resolved": "https://registry.npmjs.org/@types/node/-/node-16.18.6.tgz", - "integrity": "sha512-vmYJF0REqDyyU0gviezF/KHq/fYaUbFhkcNbQCuPGFQj6VTbXuHZoxs/Y7mutWe73C8AC6l9fFu8mSYiBAqkGA==", + "version": "18.15.0", + "resolved": "https://registry.npmjs.org/@types/node/-/node-18.15.0.tgz", + "integrity": "sha512-z6nr0TTEOBGkzLGmbypWOGnpSpSIBorEhC4L+4HeQ2iezKCi4f77kyslRwvHeNitymGQ+oFyIWGP96l/DPSV9w==", "dev": true }, "@types/node-fetch": { diff --git a/package.json b/package.json index 5235800ae..7cd66e970 100644 --- a/package.json +++ b/package.json @@ -1497,9 +1497,7 @@ "type": "array", "default": [], "items": { - "enum": [ - "KernelCompletions" - ] + "enum": [] }, "markdownDescription": "%jupyter.configuration.jupyter.experiments.optInto.markdownDescription%", "scope": "application" @@ -1508,9 +1506,7 @@ "type": "array", "default": [], "items": { - "enum": [ - "KernelCompletions" - ] + "enum": [] }, "markdownDescription": "%jupyter.configuration.jupyter.experiments.optOutFrom.markdownDescription%", "scope": "application" @@ -1973,6 +1969,13 @@ "description": "Experimental feature to enable execution analysis in notebooks", "scope": "application" }, + "jupyter.enableKernelCompletions": { + "type": "boolean", + "default": false, + "description": "Enable Non-Python Kernel Completions", + "markdownDescription": "%jupyter.configuration.jupyter.enableKernelCompletions.markdownDescription%", + "scope": "application" + }, "jupyter.completionTriggerCharacters": { "type": "object", "patternProperties": { @@ -2308,7 +2311,7 @@ "@types/memoize-one": "^4.1.1", "@types/mocha": "^9.1.0", "@types/nock": "^10.0.3", - "@types/node": "^16.18.6", + "@types/node": "^18.15.0", "@types/node-fetch": "^2.5.7", "@types/pdfkit": "^0.11.0", "@types/promisify-node": "^0.4.0", diff --git a/package.nls.json b/package.nls.json index af7cc4f5b..a4ad836db 100644 --- a/package.nls.json +++ b/package.nls.json @@ -236,7 +236,8 @@ "message": "Add PYTHONNOUSERSITE to kernels before starting. This prevents global/user site-packages from being used in the PYTHONPATH of the kernel.", "comment": ["{Locked='PYTHONNOUSERSITE'}", "{Locked='PYTHONPATH'}"] }, - "jupyter.configuration.jupyter.enableExtendedKernelCompletions.markdownDescription": "Enables Jedi support for extended IntelliSense completions in running Jupyter kernels (see this [setting](https://ipython.readthedocs.io/en/stable/config/options/terminal.html?highlight=use_jedi#configtrait-Completer.use_jedi)). This can greatly impact notebook cell execution performance. Use with caution.", + "jupyter.configuration.jupyter.enableExtendedKernelCompletions.markdownDescription": "Enables Jedi support for extended IntelliSense completions in running Python Jupyter kernels for Python (see this [setting](https://ipython.readthedocs.io/en/stable/config/options/terminal.html?highlight=use_jedi#configtrait-Completer.use_jedi)). This can greatly impact notebook cell execution performance. Use with caution.", + "jupyter.configuration.jupyter.enableKernelCompletions.markdownDescription": "Enables support for auto completion in non-Python Notebooks and Interactive Windows using the associated Jupyter Kernel. \n\nWarning: This can greatly impact cell execution performance. Use with caution.", "DataScience.exportDialogTitle": { "message": "Export to Jupyter Notebook", "comment": ["{Locked='Notebook'}"] diff --git a/src/gdpr.ts b/src/gdpr.ts index 2c32dafed..65692bf4e 100644 --- a/src/gdpr.ts +++ b/src/gdpr.ts @@ -403,6 +403,20 @@ "kernelLanguage": {"classification":"PublicNonPersonalData","purpose":"FeatureInsight","comment":"Language of the kernel spec.","owner":"donjayamanne"}, "kernelId": {"classification":"PublicNonPersonalData","purpose":"FeatureInsight","comment":"Hash of the Kernel Connection id.","owner":"donjayamanne"}, "cancelled": {"classification":"PublicNonPersonalData","purpose":"FeatureInsight","comment":"Whether the completion request was cancelled or not.","owner":"donjayamanne"}, + "resolved": {"classification":"PublicNonPersonalData","purpose":"FeatureInsight","comment":"Whether we resolved the documentation or not.","owner":"donjayamanne"}, + "resolveDuration": {"classification":"SystemMetaData","purpose":"PerformanceAndHealth","comment":"Time taken to resolve the documentation.","owner":"donjayamanne","isMeasurement":true}, + "${include}": [ + "${F1}" + + ] + } + */ +//Telemetry.KernelCodeCompletionCannotResolve +/* __GDPR__ + "DATASCIENCE.JUPYTER_KERNEL_CODE_COMPLETION_CANNOT_RESOLVE" : { + "kernelConnectionType": {"classification":"PublicNonPersonalData","purpose":"FeatureInsight","comment":"What kind of kernel spec did we fail to create.","owner":"donjayamanne"}, + "kernelLanguage": {"classification":"PublicNonPersonalData","purpose":"FeatureInsight","comment":"Language of the kernel spec.","owner":"donjayamanne"}, + "kernelId": {"classification":"PublicNonPersonalData","purpose":"FeatureInsight","comment":"Hash of the Kernel Connection id.","owner":"donjayamanne"}, "${include}": [ "${F1}" diff --git a/src/platform/common/constants.ts b/src/platform/common/constants.ts index c0151d358..e1118425c 100644 --- a/src/platform/common/constants.ts +++ b/src/platform/common/constants.ts @@ -386,6 +386,7 @@ export enum Telemetry { CellOutputMimeType = 'DS_INTERNAL.CELL_OUTPUT_MIME_TYPE', JupyterApiUsage = 'DATASCIENCE.JUPYTER_API_USAGE', KernelCodeCompletion = 'DATASCIENCE.JUPYTER_KERNEL_CODE_COMPLETION', + KernelCodeCompletionCannotResolve = 'DATASCIENCE.JUPYTER_KERNEL_CODE_COMPLETION_CANNOT_RESOLVE', JupyterKernelApiUsage = 'DATASCIENCE.JUPYTER_KERNEL_API_USAGE', NewJupyterKernelApiUsage = 'DATASCIENCE.JUPYTER_NEW_KERNEL_API_USAGE', NewJupyterKernelsApiUsage = 'DATASCIENCE.JUPYTER_NEW_KERNELS_API_USAGE', diff --git a/src/platform/common/types.ts b/src/platform/common/types.ts index e6629da7e..561652246 100644 --- a/src/platform/common/types.ts +++ b/src/platform/common/types.ts @@ -296,8 +296,7 @@ export interface IAsyncDisposableRegistry extends IAsyncDisposable { } export enum Experiments { - DataViewerContribution = 'DataViewerContribution', - KernelCompletions = 'KernelCompletions' + DataViewerContribution = 'DataViewerContribution' } /** diff --git a/src/standalone/intellisense/nonPythonKernelCompletionProvider.ts b/src/standalone/intellisense/nonPythonKernelCompletionProvider.ts index 521ba24c2..c8136fc44 100644 --- a/src/standalone/intellisense/nonPythonKernelCompletionProvider.ts +++ b/src/standalone/intellisense/nonPythonKernelCompletionProvider.ts @@ -17,7 +17,7 @@ import { } from 'vscode'; import { raceCancellation } from '../../platform/common/cancellation'; import { traceInfo, traceVerbose, traceWarning } from '../../platform/logging'; -import { Experiments, IDisposable, IDisposableRegistry, IExperimentService } from '../../platform/common/types'; +import { IDisposable, IDisposableRegistry, Resource } from '../../platform/common/types'; import { StopWatch } from '../../platform/common/utils/stopWatch'; import { IKernelProvider, IKernel } from '../../kernels/types'; import { INotebookEditorProvider } from '../../notebooks/types'; @@ -29,13 +29,34 @@ import { IExtensionSyncActivationService } from '../../platform/activation/types import { ServiceContainer } from '../../platform/ioc/container'; import { DisposableBase } from '../../platform/common/utils/lifecycle'; import { generateSortString } from './pythonKernelCompletionProvider'; -import { sleep } from '../../platform/common/utils/async'; +import { raceTimeout, sleep } from '../../platform/common/utils/async'; import { sendTelemetryEvent } from '../../telemetry'; import { getTelemetrySafeHashedString } from '../../platform/telemetry/helpers'; import { getDisplayNameOrNameOfKernelConnection } from '../../kernels/helpers'; +import type { KernelMessage } from '@jupyterlab/services'; +import { stripAnsi } from '../../platform/common/utils/regexp'; + +// Not all kernels support requestInspect method. +// E.g. deno does not support this, hence waiting for this to complete is poinless. +// As that results in a `loading...` method to appear against the completion item. +// If we have n consecutive attempts where the response never comes back in 1s, +// then we'll always ignore `requestInspect` method for this kernel. +const MAX_ATTEMPTS_BEFORE_IGNORING_RESOLVE_COMPLETION = 5; +const MAX_TIMEOUT_WAITING_FOR_RESOLVE_COMPLETION = 1_000; + +const kernelIdsThatToNotSupportCompletionResolve = new Set(); class NotebookCellSpecificKernelCompletionProvider implements CompletionItemProvider { + private totalNumberOfTimeoutsWaitingForResolveCompletion = 0; constructor(private readonly kernel: IKernel) {} + public get canResolveCompletionItem() { + return this.totalNumberOfTimeoutsWaitingForResolveCompletion < MAX_ATTEMPTS_BEFORE_IGNORING_RESOLVE_COMPLETION; + } + + private previousCompletionItems = new WeakMap< + CompletionItem, + { code: string; cursor: { start: number; end: number } } + >(); async provideCompletionItems( document: TextDocument, position: Position, @@ -65,11 +86,13 @@ class NotebookCellSpecificKernelCompletionProvider implements CompletionItemProv if (!this.kernel.session?.kernel) { return []; } + const code = document.getText(); + const cursor_pos = document.offsetAt(position); const kernelCompletions = await raceCancellation( token, this.kernel.session.kernel.requestComplete({ - code: document.getText(), - cursor_pos: document.offsetAt(position) + code, + cursor_pos }) ); traceVerbose(`Jupyter completion time: ${stopWatch.elapsedTime}`); @@ -97,6 +120,7 @@ class NotebookCellSpecificKernelCompletionProvider implements CompletionItemProv // Check if we have more information about the completion items & whether its valid. // This will ensure that we don't regress (as long as all items are valid & we have the same number of completions items // then we should be able to use the experiment matches value) + const dataToStore = { code, cursor: result.cursor }; if ( Array.isArray(experimentMatches) && experimentMatches.length >= result.matches.length && @@ -108,27 +132,84 @@ class NotebookCellSpecificKernelCompletionProvider implements CompletionItemProv typeof item.text === 'string' ) ) { - return experimentMatches.map((item, index) => { - return { - label: item.text, - range: new Range(document.positionAt(item.start), document.positionAt(item.end)), - kind: item.type ? mapJupyterKind.get(item.type) : CompletionItemKind.Field, - sortText: generateSortString(index) - }; + return kernelCompletions.content.matches.map((label, index) => { + const item = experimentMatches[index]; + const type = item.type ? mapJupyterKind.get(item.type) : CompletionItemKind.Field; + + const completionItem = new CompletionItem(label, type); + + completionItem.range = new Range(document.positionAt(item.start), document.positionAt(item.end)); + completionItem.insertText = item.text; + completionItem.sortText = generateSortString(index); + this.previousCompletionItems.set(completionItem, dataToStore); + return completionItem; }); } else { - return result.matches.map((item, index) => { - return { - label: item, - sortText: generateSortString(index) - }; + return result.matches.map((label, index) => { + const completionItem = new CompletionItem(label); + + completionItem.range = new Range( + document.positionAt(result.cursor.start), + document.positionAt(result.cursor.end) + ); + completionItem.sortText = generateSortString(index); + this.previousCompletionItems.set(completionItem, dataToStore); + return completionItem; }); } } + /** + * Kernel provider will use the inspect request to lazy-load the content + * for document panel. + */ + async resolveCompletionItem(item: CompletionItem, token: CancellationToken): Promise { + if (!item.range || !this.kernel.session?.kernel) { + // We always set a range in the completion item we send. + return item; + } + const info = this.previousCompletionItems.get(item); + if (!info) { + return item; + } + const { code, cursor } = info; + const newCode = code.substring(0, cursor.start) + (item.insertText || item.label); + const cursor_pos = + cursor.start + + ( + (typeof item.insertText === 'string' ? item.insertText : item.insertText?.value) || + (typeof item.label === 'string' ? item.label : item.label.label) || + '' + ).length; + const contents: KernelMessage.IInspectRequestMsg['content'] = { + code: newCode, + cursor_pos, + detail_level: 0 + }; + const stopWatch = new StopWatch(); + const msg = await raceTimeout( + MAX_TIMEOUT_WAITING_FOR_RESOLVE_COMPLETION, + raceCancellation(token, this.kernel.session.kernel.requestInspect(contents)) + ); + if (token.isCancellationRequested) { + return item; + } + if (!msg || msg.content.status !== 'ok' || !msg.content.found) { + if (stopWatch.elapsedTime > MAX_TIMEOUT_WAITING_FOR_RESOLVE_COMPLETION) { + this.totalNumberOfTimeoutsWaitingForResolveCompletion += 1; + } + return item; + } + item.documentation = stripAnsi(msg.content.data['text/plain'] as string); + return item; + } } class KernelSpecificCompletionProvider extends DisposableBase implements CompletionItemProvider { private cellCompletionProviders = new WeakMap(); + private completionItemsSent = new WeakMap< + CompletionItem, + { duration: number; provider: NotebookCellSpecificKernelCompletionProvider } + >(); private readonly monacoLanguage: string; private readonly kernelLanguage: string; private completionProvider?: IDisposable; @@ -143,6 +224,12 @@ class KernelSpecificCompletionProvider extends DisposableBase implements Complet this.registerCompletionProvider(); this._register( workspace.onDidChangeConfiguration((e) => { + if (e.affectsConfiguration('jupyter.enableKernelCompletions')) { + if (!isKernelCompletionEnabled(this.kernel.notebook.uri)) { + this.completionProvider?.dispose(); + return; + } + } if (!e.affectsConfiguration('jupyter.completionTriggerCharacters')) { return; } @@ -152,6 +239,10 @@ class KernelSpecificCompletionProvider extends DisposableBase implements Complet ); } private registerCompletionProvider() { + if (!isKernelCompletionEnabled(this.kernel.notebook.uri)) { + return; + } + const triggerCharacters = this.getCompletionTriggerCharacter(); if (triggerCharacters.length === 0) { if (this.kernelLanguage.toLowerCase() === this.monacoLanguage.toLowerCase()) { @@ -228,15 +319,50 @@ class KernelSpecificCompletionProvider extends DisposableBase implements Complet this.cellCompletionProviders.set(document, provider); } const stopWatch = new StopWatch(); - return provider.provideCompletionItems(document, position, token, _context).finally(() => { + const items = await provider.provideCompletionItems(document, position, token, _context); + const duration = stopWatch.elapsedTime; + sendTelemetryEvent( + Telemetry.KernelCodeCompletion, + { duration, resolveDuration: 0 }, + { + kernelId: this.kernelId, + kernelConnectionType: this.kernel.kernelConnectionMetadata.kind, + kernelLanguage: this.monacoLanguage, + cancelled: token.isCancellationRequested + } + ); + const data = { duration, provider }; + items.forEach((item) => this.completionItemsSent.set(item, data)); + return items; + } + async resolveCompletionItem(item: CompletionItem, token: CancellationToken): Promise { + const info = this.completionItemsSent.get(item); + if (!info || kernelIdsThatToNotSupportCompletionResolve.has(this.kernelId)) { + return item; + } + const { duration, provider } = info; + if (!provider.canResolveCompletionItem) { + // Never send the telemetry again and do not try in this session. + kernelIdsThatToNotSupportCompletionResolve.add(this.kernelId); + sendTelemetryEvent(Telemetry.KernelCodeCompletionCannotResolve, undefined, { + kernelId: this.kernelId, + kernelConnectionType: this.kernel.kernelConnectionMetadata.kind, + kernelLanguage: this.monacoLanguage + }); + return item; + } + + const stopWatch = new StopWatch(); + return provider.resolveCompletionItem(item, token).finally(() => { sendTelemetryEvent( Telemetry.KernelCodeCompletion, - { duration: stopWatch.elapsedTime }, + { duration, resolveDuration: stopWatch.elapsedTime }, { kernelId: this.kernelId, kernelConnectionType: this.kernel.kernelConnectionMetadata.kind, kernelLanguage: this.monacoLanguage, - cancelled: token.isCancellationRequested + cancelled: token.isCancellationRequested, + resolved: true } ); }); @@ -255,12 +381,11 @@ export class NonPythonKernelCompletionProvider extends DisposableBase implements } public activate(): void { const kernelProvider = ServiceContainer.instance.get(IKernelProvider); - const experimentService = ServiceContainer.instance.get(IExperimentService); - if (!experimentService.inExperiment(Experiments.KernelCompletions)) { - return; - } this._register( kernelProvider.onDidStartKernel(async (e) => { + if (!isKernelCompletionEnabled(e.notebook.uri)) { + return; + } const kernelId = await getTelemetrySafeHashedString(e.kernelConnectionMetadata.id); const language = getKernelLanguageAsMonacoLanguage(e); if (!language || language.toLowerCase() === PYTHON_LANGUAGE.toLowerCase()) { @@ -304,9 +429,10 @@ function getKernelLanguage(kernel: IKernel) { kernelSpecLanguage = kernel.kernelConnectionMetadata.kernelSpec.language; break; } - if (!kernelSpecLanguage || (kernelSpecLanguage || '').toLowerCase() === PYTHON_LANGUAGE.toLowerCase()) { - return ''; - } - return kernelSpecLanguage.toLowerCase(); + return (kernelSpecLanguage || '').toLowerCase(); +} + +function isKernelCompletionEnabled(resource: Resource) { + return workspace.getConfiguration('jupyter', resource).get('enableKernelCompletions', false); } diff --git a/src/telemetry.ts b/src/telemetry.ts index 339eb005f..01a163e1a 100644 --- a/src/telemetry.ts +++ b/src/telemetry.ts @@ -3254,13 +3254,26 @@ export class IEventNamePropertyMapping { * Whether the completion request was cancelled or not. */ cancelled: boolean; + /** + * Time taken to resolve the documentation. + */ + resolveDuration: number; + /** + * Whether we resolved the documentation or not. + */ + resolved?: boolean; } > = { owner: 'donjayamanne', feature: 'N/A', source: 'N/A', measures: { - ...commonClassificationForDurationProperties() + ...commonClassificationForDurationProperties(), + resolveDuration: { + classification: 'SystemMetaData', + purpose: 'PerformanceAndHealth', + isMeasurement: true + } }, properties: { kernelConnectionType: { @@ -3278,6 +3291,51 @@ export class IEventNamePropertyMapping { cancelled: { classification: 'PublicNonPersonalData', purpose: 'FeatureInsight' + }, + resolved: { + classification: 'PublicNonPersonalData', + purpose: 'FeatureInsight' + } + } + }; + + /** + * Telemetry sent when we the kernel does not reply back with a response for requestInspect message. + * The requestInspect request is used to resolve completion items in auto complete lists. + */ + [Telemetry.KernelCodeCompletionCannotResolve]: TelemetryEventInfo<{ + /** + * Hash of the Kernel Connection id. + */ + kernelId: string; + /** + * What kind of kernel spec did we fail to create. + */ + kernelConnectionType: + | 'startUsingPythonInterpreter' + | 'startUsingLocalKernelSpec' + | 'startUsingRemoteKernelSpec' + | 'connectToLiveRemoteKernel'; + /** + * Language of the kernel spec. + */ + kernelLanguage: string | undefined; + }> = { + owner: 'donjayamanne', + feature: 'N/A', + source: 'N/A', + properties: { + kernelConnectionType: { + classification: 'PublicNonPersonalData', + purpose: 'FeatureInsight' + }, + kernelLanguage: { + classification: 'PublicNonPersonalData', + purpose: 'FeatureInsight' + }, + kernelId: { + classification: 'PublicNonPersonalData', + purpose: 'FeatureInsight' } } }; diff --git a/src/tsconfig.extension.node.json b/src/tsconfig.extension.node.json index 689fa4d81..7612d233f 100644 --- a/src/tsconfig.extension.node.json +++ b/src/tsconfig.extension.node.json @@ -8,7 +8,7 @@ "moduleResolution": "node", // Types - "lib": ["es6", "es2018", "ES2019", "ES2020"], + "lib": ["es6", "es2018", "ES2019", "ES2020", "ES2021", "ES2022"], "types": [] }, "include": [ diff --git a/src/tsconfig.extension.web.json b/src/tsconfig.extension.web.json index e7d597dc2..4fd894d95 100644 --- a/src/tsconfig.extension.web.json +++ b/src/tsconfig.extension.web.json @@ -6,7 +6,7 @@ "outDir": "out", // Types - "lib": ["es6", "es2018", "dom", "ES2019", "ES2020"], + "lib": ["es6", "es2018", "dom", "ES2019", "ES2020", "ES2021", "ES2022"], "paths": { "*": ["types/*"] }, diff --git a/tsconfig.json b/tsconfig.json index 5c1e6ef0b..05b5a173b 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -6,7 +6,7 @@ "outDir": "out", // Types - "lib": ["es6", "es2018", "dom", "ES2019", "ES2020"], + "lib": ["es6", "es2018", "dom", "ES2019", "ES2020", "ES2021", "ES2022"], "paths": { "*": ["types/*"] },