diff --git a/extension/src/lib/__tests__/extractExecuteCodeRequest.test.ts b/extension/src/lib/__tests__/extractExecuteCodeRequest.test.ts new file mode 100644 index 00000000..26c42dcf --- /dev/null +++ b/extension/src/lib/__tests__/extractExecuteCodeRequest.test.ts @@ -0,0 +1,118 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect, Option } from "effect"; +import type * as vscode from "vscode"; + +import { + createNotebookCell, + createNotebookUri, + createTestNotebookDocument, +} from "../../__mocks__/TestVsCode.ts"; +import { Constants } from "../../platform/Constants.ts"; +import type { CellMetadata } from "../../schemas/CellMetadata.ts"; +import { extractExecuteCodeRequest } from "../extractExecuteCodeRequest.ts"; + +const notebookUri = createNotebookUri("file:///test/notebook_mo.py"); + +// Helper to create a raw vscode.NotebookCell (extractExecuteCodeRequest +// consumes raw cells, not MarimoNotebookCell) +function createRawCell( + value: string, + metadata: Partial, + index: number, +): vscode.NotebookCell { + return createNotebookCell( + createTestNotebookDocument(notebookUri), + { + kind: 2, // Code + value, + languageId: "python", + metadata, + }, + index, + ); +} + +describe("extractExecuteCodeRequest", () => { + it.effect("includes enabled cells with stable ids", () => + Effect.gen(function* () { + const { LanguageId } = yield* Constants; + + const cellA = createRawCell("x = 1", { stableId: "cell-a" }, 0); + const cellB = createRawCell("y = x + 1", { stableId: "cell-b" }, 1); + + const request = extractExecuteCodeRequest([cellA, cellB], LanguageId); + + expect(Option.isSome(request)).toBe(true); + const { codes, cellIds } = Option.getOrThrow(request); + expect(cellIds).toEqual(["cell-a", "cell-b"]); + expect(codes).toEqual(["x = 1", "y = x + 1"]); + }).pipe(Effect.provide(Constants.Default)), + ); + + it.effect("skips cells without a stable id", () => + Effect.gen(function* () { + const { LanguageId } = yield* Constants; + + const withId = createRawCell("x = 1", { stableId: "cell-a" }, 0); + const withoutId = createRawCell("y = 2", {}, 1); + + const request = extractExecuteCodeRequest( + [withId, withoutId], + LanguageId, + ); + + expect(Option.isSome(request)).toBe(true); + const { cellIds } = Option.getOrThrow(request); + expect(cellIds).toEqual(["cell-a"]); + }).pipe(Effect.provide(Constants.Default)), + ); + + // Regression test for https://github.com/marimo-team/marimo-lsp/issues/154 + // A cell authored as `@app.cell(disabled=True)` reaches the extension with + // metadata.options = { disabled: true } (set by NotebookSerializer from the + // LSP server's deserialize response). It must NOT be sent for execution. + it.effect( + "excludes cells disabled via @app.cell(disabled=True) (issue #154)", + () => + Effect.gen(function* () { + const { LanguageId } = yield* Constants; + + const enabled = createRawCell("x = 1", { stableId: "cell-enabled" }, 0); + const disabled = createRawCell( + 'print("RAN")', + { stableId: "cell-disabled", options: { disabled: true } }, + 1, + ); + + const request = extractExecuteCodeRequest( + [enabled, disabled], + LanguageId, + ); + + expect(Option.isSome(request)).toBe(true); + const { codes, cellIds } = Option.getOrThrow(request); + expect(cellIds).toContain("cell-enabled"); + expect(cellIds).not.toContain("cell-disabled"); + expect(codes).not.toContain('print("RAN")'); + }).pipe(Effect.provide(Constants.Default)), + ); + + // Once disabled cells are filtered, a selection of only disabled cells + // produces an empty request, which must be Option.none() so the controller + // stops instead of sending an empty execute-cells request (issue #154). + it.effect("returns none when every selected cell is disabled", () => + Effect.gen(function* () { + const { LanguageId } = yield* Constants; + + const disabled = createRawCell( + 'print("RAN")', + { stableId: "cell-disabled", options: { disabled: true } }, + 0, + ); + + const request = extractExecuteCodeRequest([disabled], LanguageId); + + expect(Option.isNone(request)).toBe(true); + }).pipe(Effect.provide(Constants.Default)), + ); +}); diff --git a/extension/src/lib/extractExecuteCodeRequest.ts b/extension/src/lib/extractExecuteCodeRequest.ts index af51401d..b41d8f8e 100644 --- a/extension/src/lib/extractExecuteCodeRequest.ts +++ b/extension/src/lib/extractExecuteCodeRequest.ts @@ -22,6 +22,11 @@ export function extractExecuteCodeRequest( continue; } + // Disabled cells (`@app.cell(disabled=True)`) must not run (issue #154). + if (cell.isDisabled) { + continue; + } + const code = getCellExecutableCode(cell, LanguageId); const cellId = cell.id.value; @@ -30,8 +35,8 @@ export function extractExecuteCodeRequest( } if (codes.length === 0) { - // no request - Option.none(); + // no request — e.g. a selection of only disabled cells (issue #154) + return Option.none(); } return Option.some({ codes, cellIds }); diff --git a/extension/src/schemas/MarimoNotebookDocument.ts b/extension/src/schemas/MarimoNotebookDocument.ts index 5b20d70d..1cc33b13 100644 --- a/extension/src/schemas/MarimoNotebookDocument.ts +++ b/extension/src/schemas/MarimoNotebookDocument.ts @@ -116,6 +116,20 @@ export class MarimoNotebookCell { ); } + /** + * Whether the cell is disabled via `@app.cell(disabled=True)`. + * + * marimo stores the decorator's `disabled` flag in the cell's config; the + * LSP deserialize path surfaces it as `metadata.options.disabled`. Disabled + * cells must not be sent for execution (issue #154). + */ + get isDisabled() { + return this.metadata.pipe( + Option.map((meta) => meta.options?.disabled === true), + Option.getOrElse(() => false), + ); + } + /** * The cell's language metadata, if present. */