Skip to content

Commit cd35125

Browse files
seeMsharon-wang
andauthored
Basic inline completions in notebooks (#9115)
This PR continues @sharon-wang's work in #8498 to add basic inline completions in notebooks (#8061), which also works by default in Positron notebooks (#8734). The Copilot language server advertises that it supports notebooks (in the initialize result) which causes vscode-languageclient to send notebookDocument/did* notifications instead of textDocument/did* notifications. Servers are expected to create the text documents referenced in the notebook document, however, the Copilot server doesn't seem to do that, causing "document not found" errors. This PR adds language client middleware that intercepts notebookDocument/did* notifications and sends textDocument/did* notifications for each affected cell. The current implementation treats each cell independently, so the server will be aware of all cells in a notebook, but not their structure, kind, outputs, etc. ### Release Notes #### New Features - Assistant: Copilot inline completions are now available in Jupyter notebooks #### Bug Fixes - N/A ### QA Notes via @sharon-wang: @:assistant * inline completions should be available for ipynb files when authenticated with GitHub Copilot * the `positron.assistant.inlineCompletionExcludes` setting should continue to work --------- Co-authored-by: sharon wang <[email protected]>
1 parent f5808a3 commit cd35125

File tree

6 files changed

+96
-45
lines changed

6 files changed

+96
-45
lines changed

extensions/positron-assistant/src/completion.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ class OpenAILegacyCompletion extends CompletionModel {
188188
token: vscode.CancellationToken
189189
): Promise<vscode.InlineCompletionItem[] | vscode.InlineCompletionList> {
190190
// Check if the file should be excluded from AI features
191-
if (await positron.ai.areCompletionsEnabled(document.uri)) {
191+
if (!await positron.ai.areCompletionsEnabled(document.uri)) {
192192
return [];
193193
}
194194

@@ -384,7 +384,7 @@ abstract class FimPromptCompletion extends CompletionModel {
384384
token: vscode.CancellationToken
385385
): Promise<vscode.InlineCompletionItem[] | vscode.InlineCompletionList> {
386386
// Check if the file should be excluded from AI features
387-
if (await positron.ai.areCompletionsEnabled(document.uri)) {
387+
if (!await positron.ai.areCompletionsEnabled(document.uri)) {
388388
return [];
389389
}
390390

@@ -646,7 +646,7 @@ export class CopilotCompletion implements vscode.InlineCompletionItemProvider {
646646
token: vscode.CancellationToken
647647
): Promise<vscode.InlineCompletionItem[] | vscode.InlineCompletionList | undefined> {
648648
// Check if the file should be excluded from AI features
649-
if (await positron.ai.areCompletionsEnabled(document.uri)) {
649+
if (!await positron.ai.areCompletionsEnabled(document.uri)) {
650650
return [];
651651
}
652652
return await this._copilotService.inlineCompletion(document, position, context, token);

extensions/positron-assistant/src/copilot.ts

Lines changed: 81 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import * as positron from 'positron';
88
import * as path from 'path';
99

1010
import { ExtensionContext } from 'vscode';
11-
import { Command, Executable, ExecuteCommandRequest, InlineCompletionItem, InlineCompletionRequest, LanguageClient, LanguageClientOptions, NotificationType, RequestType, ServerOptions, TransportKind } from 'vscode-languageclient/node';
11+
import { Command, DidChangeTextDocumentNotification, DidChangeTextDocumentParams, DidCloseTextDocumentNotification, DidOpenTextDocumentNotification, Executable, ExecuteCommandRequest, InlineCompletionItem, InlineCompletionRequest, LanguageClient, LanguageClientOptions, Middleware, NotebookDocumentMiddleware, NotificationType, RequestType, ServerOptions, TextDocumentItem, TransportKind } from 'vscode-languageclient/node';
1212
import { arch, platform } from 'os';
1313
import { ALL_DOCUMENTS_SELECTOR } from './constants.js';
1414

@@ -98,7 +98,7 @@ export class CopilotService implements vscode.Disposable {
9898
/** The CopilotService singleton instance. */
9999
private static _instance?: CopilotService;
100100

101-
private _client?: CopilotLanguageClient;
101+
private _clientManager?: CopilotLanguageClientManager;
102102

103103
/** The cancellation token for the current operation. */
104104
private _cancellationToken: vscode.CancellationTokenSource | null = null;
@@ -125,9 +125,9 @@ export class CopilotService implements vscode.Disposable {
125125
) { }
126126

127127
/** Get the Copilot language client. */
128-
private client(): CopilotLanguageClient {
129-
if (!this._client) {
130-
// The client does not exist, create it.
128+
private client(): LanguageClient {
129+
if (!this._clientManager) {
130+
// The client manager does not exist, create it.
131131
const serverName = platform() === 'win32' ? 'copilot-language-server.exe' : 'copilot-language-server';
132132
let serverPath = path.join(this._context.extensionPath, 'resources', 'copilot');
133133

@@ -148,9 +148,9 @@ export class CopilotService implements vscode.Disposable {
148148
name: packageJSON.name,
149149
version: packageJSON.version,
150150
};
151-
this._client = new CopilotLanguageClient(executable, editorPluginInfo);
151+
this._clientManager = new CopilotLanguageClientManager(executable, editorPluginInfo);
152152
}
153-
return this._client;
153+
return this._clientManager.client;
154154
}
155155

156156
/**
@@ -234,8 +234,14 @@ export class CopilotService implements vscode.Disposable {
234234
): Promise<vscode.InlineCompletionItem[] | vscode.InlineCompletionList | undefined> {
235235
const client = this.client();
236236
const params = client.code2ProtocolConverter.asInlineCompletionParams(textDocument, position, context);
237-
const result = await client.sendRequest(InlineCompletionRequest.type, params, token);
238-
return client.protocol2CodeConverter.asInlineCompletionResult(result);
237+
client.debug(`Sending inline completion request: ${JSON.stringify(params)}`);
238+
try {
239+
const result = await client.sendRequest(InlineCompletionRequest.type, params, token);
240+
return client.protocol2CodeConverter.asInlineCompletionResult(result);
241+
} catch (error) {
242+
client.debug(`Error getting inline completions: ${error}`);
243+
throw error;
244+
}
239245
}
240246

241247
private asCopilotInlineCompletionItem(completionItem: vscode.InlineCompletionItem, updatedInsertText?: string): InlineCompletionItem {
@@ -277,19 +283,11 @@ export class CopilotService implements vscode.Disposable {
277283
}
278284
}
279285

280-
export class CopilotLanguageClient implements vscode.Disposable {
286+
export class CopilotLanguageClientManager implements vscode.Disposable {
281287
private readonly _disposables: vscode.Disposable[] = [];
282288

283289
/** The wrapped language client. */
284-
private readonly _client: LanguageClient;
285-
286-
// Expose wrapped properties from the language client.
287-
public code2ProtocolConverter: typeof this._client.code2ProtocolConverter;
288-
public onNotification: typeof this._client.onNotification;
289-
public protocol2CodeConverter: typeof this._client.protocol2CodeConverter;
290-
public sendNotification: typeof this._client.sendNotification;
291-
public sendRequest: typeof this._client.sendRequest;
292-
public start: typeof this._client.start;
290+
public readonly client: LanguageClient;
293291

294292
/**
295293
* @param executable The language server executable.
@@ -318,31 +316,84 @@ export class CopilotLanguageClient implements vscode.Disposable {
318316
},
319317
editorPluginInfo,
320318
},
319+
middleware: {
320+
notebooks: this.createNotebookMiddleware(),
321+
},
321322
};
322323

323324
// Create the client.
324-
this._client = new LanguageClient(
325+
this.client = new LanguageClient(
325326
'githubCopilotLanguageServer',
326327
'GitHub Copilot Language Server',
327328
serverOptions,
328329
clientOptions,
329330
);
330-
this._disposables.push(this._client);
331+
this._disposables.push(this.client);
331332

332333
// Log status changes for debugging.
333334
this._disposables.push(
334-
this._client.onNotification(DidChangeStatusNotification.type, (params: DidChangeStatusParams) => {
335-
outputChannel.debug(`DidChangeStatusNotification: ${JSON.stringify(params)}`);
335+
this.client.onNotification(DidChangeStatusNotification.type, (params: DidChangeStatusParams) => {
336+
this.client.debug(`DidChangeStatusNotification: ${JSON.stringify(params)}`);
336337
})
337338
);
339+
}
338340

339-
// Expose wrapped properties from the language client.
340-
this.code2ProtocolConverter = this._client.code2ProtocolConverter;
341-
this.onNotification = this._client.onNotification.bind(this._client);
342-
this.protocol2CodeConverter = this._client.protocol2CodeConverter;
343-
this.sendNotification = this._client.sendNotification.bind(this._client);
344-
this.sendRequest = this._client.sendRequest.bind(this._client);
345-
this.start = this._client.start.bind(this._client);
341+
private createNotebookMiddleware(): NotebookDocumentMiddleware['notebooks'] {
342+
// The Copilot language server advertises that it supports notebooks
343+
// (in the initialize result) which causes vscode-languageclient to
344+
// send notebookDocument/did* notifications instead of textDocument/did*
345+
// notifications. Servers are expected to create the text documents
346+
// referenced in the notebook document, however, the Copilot server
347+
// doesn't seem to do that, causing "document not found" errors.
348+
// See: https://github.com/posit-dev/positron/issues/8061.
349+
//
350+
// This middleware intercepts notebookDocument/did* notifications and sends
351+
// textDocument/did* notifications for each affected cell.
352+
//
353+
// TODO: The current implementation treats each cell independently,
354+
// so the server will be aware of all cells in a notebook, but not
355+
// their structure, kind, outputs, etc.
356+
357+
const manager = this;
358+
return {
359+
async didOpen(notebookDocument, cells, next) {
360+
for (const cell of cells) {
361+
const params = manager.client.code2ProtocolConverter.asOpenTextDocumentParams(cell.document);
362+
await manager.client.sendNotification(DidOpenTextDocumentNotification.type, params);
363+
}
364+
return next(notebookDocument, cells);
365+
},
366+
async didChange(event, next) {
367+
for (const cell of event.cells?.structure?.didOpen ?? []) {
368+
const params = manager.client.code2ProtocolConverter.asOpenTextDocumentParams(cell.document);
369+
await manager.client.sendNotification(DidOpenTextDocumentNotification.type, params);
370+
}
371+
372+
for (const cell of event.cells?.structure?.didClose ?? []) {
373+
const params = manager.client.code2ProtocolConverter.asCloseTextDocumentParams(cell.document);
374+
await manager.client.sendNotification(DidCloseTextDocumentNotification.type, params);
375+
}
376+
377+
for (const change of event.cells?.textContent ?? []) {
378+
const params: DidChangeTextDocumentParams = {
379+
textDocument: manager.client.code2ProtocolConverter.asVersionedTextDocumentIdentifier(change.document),
380+
contentChanges: change.contentChanges.map(change => ({
381+
range: manager.client.code2ProtocolConverter.asRange(change.range),
382+
text: change.text,
383+
})),
384+
};
385+
await manager.client.sendNotification(DidChangeTextDocumentNotification.type, params);
386+
}
387+
return await next(event);
388+
},
389+
async didClose(notebookDocument, cells, next) {
390+
for (const cell of cells) {
391+
const params = manager.client.code2ProtocolConverter.asCloseTextDocumentParams(cell.document);
392+
await manager.client.sendNotification(DidCloseTextDocumentNotification.type, params);
393+
}
394+
return await next(notebookDocument, cells);
395+
},
396+
};
346397
}
347398

348399
dispose(): void {

src/positron-dts/positron.d.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2208,10 +2208,10 @@ declare module 'positron' {
22082208
}
22092209

22102210
/**
2211-
* Checks the file for exclusion from AI completion.
2212-
* @param file The file to check for exclusion.
2213-
* @returns A Thenable that resolves to true if the file should be excluded, false otherwise.
2211+
* Checks if completions are enabled for the given file.
2212+
* @param uri The file URI to check if completions are enabled.
2213+
* @returns A Thenable that resolves to true if completions should be enabled for the file, false otherwise.
22142214
*/
2215-
export function areCompletionsEnabled(file: vscode.Uri): Thenable<boolean>;
2215+
export function areCompletionsEnabled(uri: vscode.Uri): Thenable<boolean>;
22162216
}
22172217
}

src/vs/workbench/api/browser/positron/mainThreadAiFeatures.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ export class MainThreadAiFeatures extends Disposable implements MainThreadAiFeat
116116
}
117117

118118
/**
119-
* Check if a file should be excluded from AI completions based on configuration settings.
119+
* Check if a file should be enabled for AI completions based on configuration settings.
120120
*/
121121
async $areCompletionsEnabled(file: UriComponents): Promise<boolean> {
122122
const uri = URI.revive(file);

src/vs/workbench/contrib/positronAssistant/browser/positronAssistantService.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,17 +106,17 @@ export class PositronAssistantService extends Disposable implements IPositronAss
106106
const globPattern = this._configurationService.getValue<string[]>('positron.assistant.inlineCompletionExcludes');
107107

108108
if (!globPattern || globPattern.length === 0) {
109-
return false; // No glob patterns configured, so no files are excluded
109+
return true; // No glob patterns configured, so completions are enabled
110110
}
111111

112-
// Check all of the glob patterns and return true if any match
112+
// Check all of the glob patterns and return false if any match
113113
for (const pattern of globPattern) {
114114
if (glob.match(pattern, uri.path)) {
115-
return true; // File matches an exclusion pattern
115+
return false; // File matches an exclusion pattern, so it is excluded from completions
116116
}
117117
}
118118

119-
return false; // No patterns matched, so the file is not excluded
119+
return true; // No patterns matched, so completions are enabled
120120
}
121121

122122
//#endregion

src/vs/workbench/contrib/positronAssistant/common/interfaces/positronAssistantService.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,9 @@ export interface IPositronAssistantService {
124124
removeLanguageModelConfig(source: IPositronLanguageModelSource): void;
125125

126126
/**
127-
* Check if a file should be excluded from AI completions.
128-
* @param uri The URI of the file to check.
129-
* @returns True if the file should be excluded from the Positron Assistant, false otherwise.
127+
* Checks if completions are enabled for the given file.
128+
* @param uri The file URI to check if completions are enabled.
129+
* @returns true if completions should be enabled for the file, false otherwise.
130130
*/
131131
areCompletionsEnabled(uri: URI): boolean;
132132

0 commit comments

Comments
 (0)