Add error field and animation in inline completion (#15344)

* Add error field and animation in inline completion * Correct name of the indicator variable * Clear error timeout when not in error anymore * Replace binary `isError` with error object to allow passing `message` In future this could include additional fields like `type` or `traceback` * Add shape to error indicator to avoid reliance on color alone * Add tests for error indicator when streaming * Fix type annotation to run on node * Styling adjustments * Comment out unused test variable --------- Co-authored-by: krassowski <5832902+krassowski@users.noreply.github.com>
jupyterlab · May 14, 2024 · 0d1bb84 · 0d1bb84
1 parent ce9976b
commit 0d1bb84
Show file tree

Hide file tree

Showing 6 changed files with 217 additions and 4 deletions.
diff --git a/packages/completer/src/ghost.ts b/packages/completer/src/ghost.ts
@@ -14,6 +14,7 @@ const TRANSIENT_LETTER_SPACER_CLASS = 'jp-GhostText-letterSpacer';
 const GHOST_TEXT_CLASS = 'jp-GhostText';
 const STREAMED_TOKEN_CLASS = 'jp-GhostText-streamedToken';
 const STREAMING_INDICATOR_CLASS = 'jp-GhostText-streamingIndicator';
+const ERROR_INDICATOR_CLASS = 'jp-GhostText-errorIndicator';
 
 /**
  * Ghost text content and placement.
@@ -39,6 +40,15 @@ export interface IGhostText {
    * Whether streaming is in progress.
    */
   streaming?: boolean;
+  /**
+   * An error occurred in the request.
+   */
+  error?: {
+    /**
+     * A message explaining the error.
+     */
+    message?: string;
+  };
   /**
    * Callback to execute when pointer enters the boundary of the ghost text.
    */
@@ -107,7 +117,8 @@ class GhostTextWidget extends WidgetType {
   eq(other: GhostTextWidget) {
     return (
       other.content == this.content &&
-      other.options.streaming === this.options.streaming
+      other.options.streaming === this.options.streaming &&
+      other.options.error === this.options.error
     );
   }
 
@@ -139,7 +150,54 @@ class GhostTextWidget extends WidgetType {
     return wrap;
   }
 
+  private _removeErrorAnimation(dom: HTMLElement) {
+    const elementsToRemove = dom.querySelectorAll(`.${ERROR_INDICATOR_CLASS}`);
+
+    elementsToRemove.forEach(element => {
+      element.remove();
+    });
+  }
+
+  /**
+   * Mount the error animation DOM and remove the streaming indicator if any.
+   */
+  private _mountErrorAnimation(dom: HTMLElement) {
+    const errorIndicator = document.createElement('span');
+    errorIndicator.className = ERROR_INDICATOR_CLASS;
+    const error = this.options.error;
+    if (error?.message) {
+      errorIndicator.title = error?.message;
+    }
+
+    // Delete stream and previous error animation
+    const elementsToRemove = dom.querySelectorAll(
+      `.${STREAMING_INDICATOR_CLASS}, .${ERROR_INDICATOR_CLASS}`
+    );
+
+    elementsToRemove.forEach(element => {
+      element.remove();
+    });
+
+    dom.appendChild(errorIndicator);
+  }
+
   private _updateDOM(dom: HTMLElement) {
+    if (this.options.error) {
+      this._mountErrorAnimation(dom);
+
+      this._clearErrorTimeout = setTimeout(() => {
+        this._removeErrorAnimation(dom);
+        this._clearErrorTimeout = null;
+      }, 5000);
+      return;
+    }
+    // If not in an error anymore, clear the error indicator
+    if (this._clearErrorTimeout !== null) {
+      clearTimeout(this._clearErrorTimeout);
+      this._removeErrorAnimation(dom);
+      this._clearErrorTimeout = null;
+    }
+
     const content = this.content;
     let addition = this.options.addedPart;
     if (addition && !this.isSpacer) {
@@ -172,6 +230,8 @@ class GhostTextWidget extends WidgetType {
     }
     super.destroy(dom);
   }
+
+  private _clearErrorTimeout: ReturnType<typeof setTimeout> | null = null;
 }
 
 export namespace GhostTextManager {

diff --git a/packages/completer/src/inline.ts b/packages/completer/src/inline.ts
@@ -415,7 +415,8 @@ export class InlineCompleter extends Widget {
       addedPart: item.lastStreamed,
       streaming: item.streaming,
       onPointerOver: this._onPointerOverGhost.bind(this),
-      onPointerLeave: this._onPointerLeaveGhost.bind(this)
+      onPointerLeave: this._onPointerLeaveGhost.bind(this),
+      error: item.error
     });
     editor.host.classList.add(INLINE_COMPLETER_ACTIVE_CLASS);
   }
@@ -452,7 +453,11 @@ export class InlineCompleter extends Widget {
 
     let anchor: DOMRect;
     try {
-      anchor = editor.getCoordinateForPosition(model.cursor) as DOMRect;
+      const maybeAnchor = editor.getCoordinateForPosition(model.cursor);
+      if (!maybeAnchor) {
+        throw Error('No coordinates for cursor position');
+      }
+      anchor = maybeAnchor as DOMRect;
     } catch {
       // if coordinate is no longer in editor (e.g. after deleting a line), hide widget
       this.hide();

diff --git a/packages/completer/src/reconciliator.ts b/packages/completer/src/reconciliator.ts
@@ -128,6 +128,7 @@ export class ProviderReconciliator implements IProviderReconciliator {
       // Stream an update.
       item.insertText = updated.insertText;
       item.lastStreamed = addition;
+      item.error = reply.response.error;
       streamed.emit(CompletionHandler.StraemEvent.update);
     }
 

diff --git a/packages/completer/src/tokens.ts b/packages/completer/src/tokens.ts
@@ -199,6 +199,17 @@ interface IInlineCompletionItemLSP {
   filterText?: string;
 }
 
+/**
+ * A representation of an error that occurred in completion generation.
+ * @alpha
+ */
+export interface IInlineCompletionError {
+  /**
+   * The optional message which may be shown in the user interface.
+   */
+  message?: string;
+}
+
 /**
  * An inline completion item represents a text snippet that is proposed inline
  * to complete text that is being typed.
@@ -212,6 +223,11 @@ export interface IInlineCompletionItem extends IInlineCompletionItemLSP {
    * with `token` which has to be set for incomplete completions.
    */
   isIncomplete?: boolean;
+
+  /**
+   * This field is marked when an error occurs during a stream or fetch request.
+   */
+  error?: IInlineCompletionError;
 }
 
 export interface IInlineCompletionList<

diff --git a/packages/completer/style/base.css b/packages/completer/style/base.css
@@ -281,6 +281,30 @@
   }
 }
 
+.jp-GhostText-errorIndicator::after {
+  animation: jp-GhostText-error 500ms 1;
+  animation-delay: 3500ms;
+  color: var(--jp-error-color1);
+  font-size: 150%;
+  line-height: 10px;
+  margin-left: 2px;
+  padding: 0 4px;
+  content: '⚠';
+  cursor: help;
+  position: relative;
+  top: 2px;
+}
+
+@keyframes jp-GhostText-error {
+  0% {
+    opacity: 1;
+  }
+
+  100% {
+    opacity: 0;
+  }
+}
+
 .jp-InlineCompleter {
   box-shadow: var(--jp-elevation-z2);
   background: var(--jp-layout-color1);

diff --git a/packages/completer/test/inline.spec.ts b/packages/completer/test/inline.spec.ts
@@ -7,13 +7,17 @@ import {
 } from '@jupyterlab/completer';
 import { CodeEditorWrapper } from '@jupyterlab/codeeditor';
 import { nullTranslator } from '@jupyterlab/translation';
-import { simulate } from '@jupyterlab/testing';
+import { framePromise, simulate } from '@jupyterlab/testing';
 import { Signal } from '@lumino/signaling';
 import { createEditorWidget } from '@jupyterlab/completer/lib/testutils';
 import { Widget } from '@lumino/widgets';
 import { MessageLoop } from '@lumino/messaging';
 import { Doc, Text } from 'yjs';
 
+const GHOST_TEXT_CLASS = 'jp-GhostText';
+const STREAMING_INDICATOR_CLASS = 'jp-GhostText-streamingIndicator';
+const ERROR_INDICATOR_CLASS = 'jp-GhostText-errorIndicator';
+
 describe('completer/inline', () => {
   const exampleProvider: IInlineCompletionProvider = {
     name: 'An inline provider',
@@ -39,6 +43,10 @@ describe('completer/inline', () => {
 
     beforeEach(() => {
       editorWidget = createEditorWidget();
+      // Mock coordinate for position as jest does not implement layout
+      editorWidget.editor.getCoordinateForPosition = () => {
+        return { left: 0, top: 0, right: 0, bottom: 0 };
+      };
       model = new InlineCompleter.Model();
       model.cursor = {
         line: 0,
@@ -78,6 +86,105 @@ describe('completer/inline', () => {
       });
     });
 
+    describe('#_setText()', () => {
+      const findInHost = (selector: string) =>
+        editorWidget.editor.host.querySelector(selector);
+
+      it('should render the completion as it is streamed', async () => {
+        Widget.attach(editorWidget, document.body);
+        Widget.attach(completer, document.body);
+        const stream = new Signal<any, CompletionHandler.StraemEvent>(
+          {} as any
+        );
+        const item: CompletionHandler.IInlineItem = {
+          insertText: 'this',
+          streaming: true,
+          provider: exampleProvider,
+          isIncomplete: true,
+          stream
+        };
+        model.setCompletions({ items: [item] });
+        await framePromise();
+
+        stream.emit(CompletionHandler.StraemEvent.opened);
+
+        let ghost = findInHost(`.${GHOST_TEXT_CLASS}`) as HTMLElement;
+        let streamingIndicator = findInHost(`.${STREAMING_INDICATOR_CLASS}`);
+
+        expect(ghost.innerText).toBe('this');
+        expect(streamingIndicator).toBeDefined();
+
+        item.insertText = 'this is';
+        stream.emit(CompletionHandler.StraemEvent.update);
+        expect(ghost.innerText).toBe('this is');
+
+        item.insertText = 'this is a';
+        stream.emit(CompletionHandler.StraemEvent.update);
+        expect(ghost.innerText).toBe('this is a');
+
+        item.insertText = 'this is a test';
+        item.streaming = false;
+        item.isIncomplete = false;
+        stream.emit(CompletionHandler.StraemEvent.closed);
+
+        const errorIndicator = findInHost(`.${ERROR_INDICATOR_CLASS}`);
+        expect(ghost.innerText).toBe('this is a test');
+        expect(errorIndicator).toBeNull();
+        // On runtime streaming indicator gets removed because innerText gets called,
+        // but behold jsdom which does not implement innerText as of 2024 and thus
+        // setting innerText on an element does not clear its children in tests, see
+        // https://github.com/jsdom/jsdom/issues/1245
+        // streamingIndicator = findInHost(`.${STREAMING_INDICATOR_CLASS}`);
+        // expect(streamingIndicator).toBeNull();
+      });
+
+      it('should render the error if stream errors out', async () => {
+        Widget.attach(editorWidget, document.body);
+        Widget.attach(completer, document.body);
+        const stream = new Signal<any, CompletionHandler.StraemEvent>(
+          {} as any
+        );
+        const item: CompletionHandler.IInlineItem = {
+          insertText: 'this',
+          streaming: true,
+          provider: exampleProvider,
+          isIncomplete: true,
+          stream
+        };
+        model.setCompletions({ items: [item] });
+        await framePromise();
+
+        stream.emit(CompletionHandler.StraemEvent.opened);
+
+        let ghost = findInHost(`.${GHOST_TEXT_CLASS}`) as HTMLElement;
+        let streamingIndicator = findInHost(`.${STREAMING_INDICATOR_CLASS}`);
+        let errorIndicator = findInHost(`.${ERROR_INDICATOR_CLASS}`) as
+          | HTMLElement
+          | undefined;
+
+        expect(ghost.innerText).toBe('this');
+        expect(streamingIndicator).toBeDefined();
+        expect(errorIndicator).toBeNull();
+
+        item.insertText = 'this is';
+        stream.emit(CompletionHandler.StraemEvent.update);
+
+        item.error = { message: 'Completion generation failed' };
+        stream.emit(CompletionHandler.StraemEvent.update);
+        errorIndicator = findInHost(`.${ERROR_INDICATOR_CLASS}`) as HTMLElement;
+        streamingIndicator = findInHost(`.${STREAMING_INDICATOR_CLASS}`);
+        // should not remove suggestion
+        expect(ghost.innerText).toBe('this is');
+        // should show error indicator
+        expect(errorIndicator).toBeDefined();
+        // should hide streaming indicator
+        expect(streamingIndicator).toBeNull();
+
+        // the error indicator should include the title
+        expect(errorIndicator.title).toBe('Completion generation failed');
+      });
+    });
+
     describe('#current', () => {
       it('preserves active completion when results change order', () => {
         Widget.attach(completer, document.body);