fix: disable snippets for LSP completions (#5949)

Upgrade `@marimo-team/codemirror-languageserver` to remove snippet
completions

Author: mscolnick

.github/workflows/playwright.yml

@@ -70,8 +70,7 @@ jobs:
 
       - name: 🐍 Install marimo
         run: |
-          uv pip install .
-          echo "MARIMO_VERSION=$(marimo --version)" >> $GITHUB_ENV
+          echo "MARIMO_VERSION=$(uv run marimo --version)" >> $GITHUB_ENV
 
       - name: 🎭 Get installed Playwright version
         id: playwright-version

frontend/package.json

@@ -50,7 +50,7 @@
     "@lezer/markdown": "^1.4.3",
     "@lezer/python": "^1.1.18",
     "@marimo-team/codemirror-ai": "^0.3.2",
-    "@marimo-team/codemirror-languageserver": "1.15.24",
+    "@marimo-team/codemirror-languageserver": "^1.16.0",
     "@marimo-team/codemirror-mcp": "^0.1.5",
     "@marimo-team/codemirror-sql": "^0.2.3",
     "@marimo-team/llm-info": "workspace:*",

frontend/playwright.config.ts

@@ -104,7 +104,7 @@ export function startServer(app: ApplicationNames): void {
   }
   const port = options.port ?? EDIT_PORT;
   const pathToApp = path.join(pydir, app);
-  const marimoCmd = `marimo -q ${options.command} ${pathToApp} -p ${port} --headless`;
+  const marimoCmd = `uv run marimo -q ${options.command} ${pathToApp} -p ${port} --headless`;
   exec(marimoCmd);
 }
 
@@ -196,7 +196,7 @@ const config: PlaywrightTestConfig = {
       const baseUrl = command === "run" ? options.baseUrl : undefined;
 
       const pathToApp = path.join(pydir, app);
-      let marimoCmd = `marimo -q ${command} ${pathToApp} -p ${port} --headless --no-token`;
+      let marimoCmd = `uv run marimo -q ${command} ${pathToApp} -p ${port} --headless --no-token`;
       if (baseUrl) {
         marimoCmd += ` --base-url=${baseUrl}`;
       }
@@ -213,7 +213,7 @@ const config: PlaywrightTestConfig = {
       };
     }),
     {
-      command: `marimo -q edit -p ${EDIT_PORT} --headless --no-token`,
+      command: `uv run marimo -q edit -p ${EDIT_PORT} --headless --no-token`,
       url: getUrl(EDIT_PORT),
       reuseExistingServer: true,
       timeout: 30 * 1000,

frontend/src/components/editor/navigation/__tests__/clipboard.test.ts

@@ -178,11 +178,6 @@ describe("useCellClipboard", () => {
         "Failed to copy cells to clipboard",
         expect.any(Error),
       );
-      expect(toast).toHaveBeenCalledWith({
-        title: "Copy failed",
-        description: "Failed to copy cells to clipboard.",
-        variant: "danger",
-      });
     });
 
     it("should filter out non-existent cells", async () => {

frontend/src/components/editor/navigation/clipboard.ts

@@ -5,6 +5,7 @@ import { z } from "zod";
 import { toast } from "@/components/ui/use-toast";
 import { getNotebook, useCellActions } from "@/core/cells/cells";
 import type { CellId } from "@/core/cells/ids";
+import { copyToClipboard } from "@/utils/copy";
 import { Logger } from "@/utils/Logger";
 
 // According to MDN, custom mimetypes should start with "web "
@@ -70,7 +71,7 @@ export function useCellClipboard() {
       // Fallback to simple text copy
       try {
         const plainText = cells.map((cell) => cell.code).join("\n\n");
-        await navigator.clipboard.writeText(plainText);
+        await copyToClipboard(plainText);
         toastSuccess(cells.length);
       } catch {
         toastError();

frontend/src/core/codemirror/language/languages/python.ts

@@ -128,7 +128,6 @@ const pylspClient = once((lspConfig: LSPConfig) => {
   return new NotebookLanguageServerClient(
     new LanguageServerClient({
       ...lspClientOpts,
-      autoClose: false,
     }),
     settings,
   );
@@ -146,7 +145,6 @@ const tyLspClient = once((_: LSPConfig) => {
   return new NotebookLanguageServerClient(
     new LanguageServerClient({
       ...lspClientOpts,
-      autoClose: false,
       getWorkspaceConfiguration: (_) => [{ disableLanguageServices: true }],
     }),
     {},
@@ -165,7 +163,6 @@ const pyrightClient = once((_: LSPConfig) => {
   return new NotebookLanguageServerClient(
     new LanguageServerClient({
       ...lspClientOpts,
-      autoClose: false,
     }),
     {},
   );
@@ -242,6 +239,7 @@ export class PythonLanguageAdapter implements LanguageAdapter<{}> {
             client: client as unknown as LanguageServerClient,
             languageId: "python",
             allowHTMLContent: true,
+            useSnippetOnCompletion: false,
             hoverConfig: hoverOptions,
             completionConfig: autocompleteOptions,
             // Default to false

frontend/src/core/codemirror/lsp/__tests__/notebook-lsp.test.ts

@@ -195,35 +195,36 @@ describe("createNotebookLens", () => {
 });
 
 describe("NotebookLanguageServerClient", () => {
-  let plugins: any[] = [];
   let mockClient: Mocked<ILanguageServerClient>;
   let notebookClient: NotebookLanguageServerClient;
 
   beforeEach(() => {
-    plugins = [];
     mockClient = {
-      plugins,
       ready: true,
       capabilities: {},
       initializePromise: Promise.resolve(),
+      clientCapabilities: {},
+      completionItemResolve: vi.fn(),
       initialize: vi.fn(),
       close: vi.fn(),
-      detachPlugin: vi.fn().mockImplementation((plugin) => {
-        plugins = plugins.filter((p) => p !== plugin);
-      }),
-      attachPlugin: vi.fn().mockImplementation((plugin) => {
-        plugins.push(plugin);
-      }),
+      onNotification: vi.fn(),
       textDocumentDidOpen: vi.fn(),
       textDocumentDidChange: vi.fn(),
       textDocumentHover: vi.fn(),
       textDocumentCompletion: vi.fn(),
+      textDocumentDefinition: vi.fn(),
+      textDocumentPrepareRename: vi.fn(),
+      textDocumentCodeAction: vi.fn(),
+      textDocumentSignatureHelp: vi.fn(),
       textDocumentRename: vi.fn(),
-      processNotification: vi.fn(),
-      notify: vi.fn(),
-      request: vi.fn(),
-    } as unknown as Mocked<ILanguageServerClient>;
-    notebookClient = new NotebookLanguageServerClient(mockClient, {});
+    };
+    (mockClient as any).processNotification = vi.fn();
+    (mockClient as any).notify = vi.fn();
+    notebookClient = new NotebookLanguageServerClient(mockClient, {}, () => ({
+      [Cells.cell1]: new EditorView({ doc: "# this is a comment" }),
+      [Cells.cell2]: new EditorView({ doc: "import math\nimport numpy" }),
+      [Cells.cell3]: new EditorView({ doc: "print(math.sqrt(4))" }),
+    }));
 
     // Mock the atom instead of the instance method
     vi.spyOn(store, "get").mockImplementation((atom) => {
@@ -404,6 +405,12 @@ describe("NotebookLanguageServerClient", () => {
       });
       expect(mockView3.state.doc.toString()).toBe("print(math.sqrt(4))");
 
+      (notebookClient as any).getNotebookEditors = () => ({
+        [Cells.cell1]: mockView1,
+        [Cells.cell2]: mockView2,
+        [Cells.cell3]: mockView3,
+      });
+
       // Setup rename params
       const renameParams: LSP.RenameParams = {
         textDocument: { uri: CellDocumentUri.of(Cells.cell2) },

frontend/src/core/codemirror/lsp/federated-lsp.ts

@@ -1,6 +1,5 @@
 /* Copyright 2024 Marimo. All rights reserved. */
 
-import type { LanguageServerPlugin } from "@marimo-team/codemirror-languageserver";
 import type * as LSP from "vscode-languageserver-protocol";
 import { Objects } from "@/utils/objects";
 import type { ILanguageServerClient } from "./types";
@@ -24,6 +23,26 @@ export class FederatedLanguageServerClient implements ILanguageServerClient {
     this.documentUri = getLSPDocument();
   }
 
+  onNotification(
+    listener: (n: {
+      jsonrpc: "2.0";
+      id?: null | undefined;
+      method: "textDocument/publishDiagnostics";
+      params: LSP.PublishDiagnosticsParams;
+    }) => void,
+  ): () => boolean {
+    const callbacks: Array<() => boolean> = [];
+    for (const client of this.clients) {
+      callbacks.push(client.onNotification(listener));
+    }
+    return () => {
+      for (const cb of callbacks) {
+        cb();
+      }
+      return true;
+    };
+  }
+
   get clientCapabilities(): LSP.ClientCapabilities | undefined {
     const capabilities = this.clients
       .map((client) => {
@@ -153,14 +172,6 @@ export class FederatedLanguageServerClient implements ILanguageServerClient {
     return null;
   }
 
-  attachPlugin(plugin: LanguageServerPlugin): void {
-    this.clients.forEach((client) => client.attachPlugin(plugin));
-  }
-
-  detachPlugin(plugin: LanguageServerPlugin): void {
-    this.clients.forEach((client) => client.detachPlugin(plugin));
-  }
-
   // Merge completions from all clients
   async textDocumentCompletion(
     params: LSP.CompletionParams,

frontend/src/core/codemirror/lsp/notebook-lsp.ts

@@ -1,18 +1,20 @@
 /* Copyright 2024 Marimo. All rights reserved. */
 
+import type { EditorView } from "@codemirror/view";
 import type * as LSP from "vscode-languageserver-protocol";
+import { getNotebook } from "@/core/cells/cells";
 import type { CellId } from "@/core/cells/ids";
 import { store } from "@/core/state/jotai";
 import { invariant } from "@/utils/invariant";
 import { Logger } from "@/utils/Logger";
 import { LRUCache } from "@/utils/lru";
+import { Objects } from "@/utils/objects";
 import { topologicalCodesAtom } from "../copilot/getCodes";
 import { createNotebookLens, type NotebookLens } from "./lens";
 import {
   CellDocumentUri,
   type ILanguageServerClient,
   isClientWithNotify,
-  isClientWithPlugins,
 } from "./types";
 import { getLSPDocument } from "./utils";
 
@@ -84,18 +86,32 @@ class Snapshotter {
   }
 }
 
+const defaultGetNotebookEditors = () => {
+  const evs = getNotebook().cellHandles;
+  return Objects.mapValues(evs, (r) => r.current?.editorViewOrNull);
+};
+
 export class NotebookLanguageServerClient implements ILanguageServerClient {
   public readonly documentUri: LSP.DocumentUri;
   private readonly client: ILanguageServerClient;
   private readonly snapshotter: Snapshotter;
+  private readonly getNotebookEditors: () => Record<
+    CellId,
+    EditorView | null | undefined
+  >;
 
   private static readonly SEEN_CELL_DOCUMENT_URIS = new Set<CellDocumentUri>();
 
   constructor(
     client: ILanguageServerClient,
     initialSettings: Record<string, unknown>,
+    getNotebookEditors: () => Record<
+      CellId,
+      EditorView | null | undefined
+    > = defaultGetNotebookEditors,
   ) {
     this.documentUri = getLSPDocument();
+    this.getNotebookEditors = getNotebookEditors;
 
     this.client = client;
     this.patchProcessNotification();
@@ -114,6 +130,17 @@ export class NotebookLanguageServerClient implements ILanguageServerClient {
     this.snapshotter = new Snapshotter(this.getNotebookCode.bind(this));
   }
 
+  onNotification(
+    listener: (n: {
+      jsonrpc: "2.0";
+      id?: null | undefined;
+      method: "textDocument/publishDiagnostics";
+      params: LSP.PublishDiagnosticsParams;
+    }) => void,
+  ): () => boolean {
+    return this.client.onNotification(listener);
+  }
+
   get ready(): boolean {
     return this.client.ready;
   }
@@ -150,16 +177,6 @@ export class NotebookLanguageServerClient implements ILanguageServerClient {
     this.client.close();
   }
 
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  detachPlugin(plugin: any): void {
-    this.client.detachPlugin(plugin);
-  }
-
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  attachPlugin(plugin: any): void {
-    this.client.attachPlugin(plugin);
-  }
-
   private getNotebookCode() {
     return store.get(topologicalCodesAtom);
   }
@@ -371,37 +388,26 @@ export class NotebookLanguageServerClient implements ILanguageServerClient {
     const newEdits = lens.getEditsForNewText(edit.newText);
     const editsToNewCode = new Map(newEdits.map((e) => [e.cellId, e.text]));
 
-    invariant(
-      isClientWithPlugins(this.client),
-      "Expected client with plugins.",
-    );
-
     // Update the code in the plugins manually
-    for (const plugin of this.client.plugins) {
-      const documentUri: string = plugin.documentUri;
-      if (!CellDocumentUri.is(documentUri)) {
-        Logger.warn("Invalid cell document URI", documentUri);
-        continue;
-      }
-
-      const cellId = CellDocumentUri.parse(documentUri);
+    const editors = this.getNotebookEditors();
+    for (const [cellId, ev] of Objects.entries(editors)) {
       const newCode = editsToNewCode.get(cellId);
       if (newCode == null) {
         Logger.warn("No new code for cell", cellId);
         continue;
       }
 
-      if (!plugin.view) {
-        Logger.warn("No view for plugin", plugin);
+      if (!ev) {
+        Logger.warn("No view for plugin", cellId);
         continue;
       }
 
       // Only update if it has changed
-      if (plugin.view.state.doc.toString() !== newCode) {
-        plugin.view.dispatch({
+      if (ev.state.doc.toString() !== newCode) {
+        ev.dispatch({
           changes: {
             from: 0,
-            to: plugin.view.state.doc.length,
+            to: ev.state.doc.length,
             insert: newCode,
           },
         });

frontend/src/core/codemirror/lsp/types.ts

@@ -1,6 +1,5 @@
 /* Copyright 2024 Marimo. All rights reserved. */
 
-import type { EditorView } from "@codemirror/view";
 import type { LanguageServerClient } from "@marimo-team/codemirror-languageserver";
 import type { DocumentUri } from "vscode-languageserver-protocol";
 import type { CellId } from "@/core/cells/ids";
@@ -41,15 +40,3 @@ export function isClientWithNotify(
 } {
   return "notify" in client;
 }
-
-/**
- * Plugins are a @private on `LanguageServerClient`,
- * hiding public use with TypeScript.
- */
-export function isClientWithPlugins(
-  client: ILanguageServerClient,
-): client is ILanguageServerClient & {
-  plugins: { documentUri: string; view?: EditorView }[];
-} {
-  return "plugins" in client;
-}