Skip to content

Commit efada45

Browse files
authored
fix(frontend): include unmounted cells in LSP and Copilot context (#10677)
1 parent 5c888b9 commit efada45

3 files changed

Lines changed: 113 additions & 26 deletions

File tree

frontend/src/core/codemirror/copilot/__tests__/getCodes.test.ts

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,18 @@ import { EditorView } from "@codemirror/view";
55
import { describe, expect, it } from "vitest";
66
import { cellId, variableName } from "@/__tests__/branded";
77
import { initialNotebookState, notebookAtom } from "@/core/cells/cells";
8+
import { createCell } from "@/core/cells/types";
89
import { OverridingHotkeyProvider } from "@/core/hotkeys/hotkeys";
910
import { store } from "@/core/state/jotai";
1011
import { variablesAtom } from "@/core/variables/state";
1112
import { MultiColumn } from "@/utils/id-tree";
1213
import { cellConfigExtension } from "../../config/extension";
1314
import { adaptiveLanguageConfiguration } from "../../language/extension";
14-
import { getCodes, getTopologicalCellIds } from "../getCodes";
15+
import {
16+
getCodes,
17+
getTopologicalCellIds,
18+
topologicalCodesAtom,
19+
} from "../getCodes";
1520

1621
const Cells = {
1722
cell1: cellId("cell1"),
@@ -208,4 +213,64 @@ describe("getCodes", () => {
208213
const result = getCodes(otherCode);
209214
expect(result).toEqual("import os\nx = 1\ny = x + 1\nprint('Hello World')");
210215
});
216+
217+
it("should read code from the store when no editor is mounted", () => {
218+
const otherCode = "print('Hello World')";
219+
store.set(notebookAtom, {
220+
...initialNotebookState(),
221+
cellIds: MultiColumn.from([[Cells.cell1, Cells.cell2]]),
222+
cellData: {
223+
[Cells.cell1]: createCell({ id: Cells.cell1, code: "import os" }),
224+
[Cells.cell2]: createCell({ id: Cells.cell2, code: "x = 1" }),
225+
},
226+
});
227+
const result = getCodes(otherCode);
228+
expect(result).toEqual("import os\nx = 1\nprint('Hello World')");
229+
});
230+
231+
it("should prefer the mounted editor code over the store code", () => {
232+
const otherCode = "print('Hello World')";
233+
store.set(notebookAtom, {
234+
...initialNotebookState(),
235+
cellIds: MultiColumn.from([[Cells.cell1, Cells.cell2]]),
236+
cellData: {
237+
[Cells.cell1]: createCell({ id: Cells.cell1, code: "import os" }),
238+
[Cells.cell2]: createCell({ id: Cells.cell2, code: "x = 1" }),
239+
},
240+
cellHandles: {
241+
[Cells.cell2]: { current: createMockEditorView("x = 2") },
242+
},
243+
});
244+
const result = getCodes(otherCode);
245+
expect(result).toEqual("import os\nx = 2\nprint('Hello World')");
246+
});
247+
});
248+
249+
describe("topologicalCodesAtom", () => {
250+
it("should include every cell when no editor is mounted", () => {
251+
store.set(notebookAtom, {
252+
...initialNotebookState(),
253+
cellIds: MultiColumn.from([[Cells.cell1, Cells.cell2, Cells.cell3]]),
254+
cellData: {
255+
[Cells.cell1]: createCell({ id: Cells.cell1, code: "import os" }),
256+
[Cells.cell2]: createCell({ id: Cells.cell2, code: "x = 1" }),
257+
[Cells.cell3]: createCell({ id: Cells.cell3, code: "y = x + 1" }),
258+
},
259+
});
260+
store.set(variablesAtom, {
261+
[Variables.var1]: {
262+
name: Variables.var1,
263+
declaredBy: [Cells.cell1],
264+
usedBy: [Cells.cell2, Cells.cell3],
265+
},
266+
});
267+
268+
const { cellIds, codes } = store.get(topologicalCodesAtom);
269+
expect(cellIds).toEqual([Cells.cell1, Cells.cell2, Cells.cell3]);
270+
expect(codes).toEqual({
271+
[Cells.cell1]: "import os",
272+
[Cells.cell2]: "x = 1",
273+
[Cells.cell3]: "y = x + 1",
274+
});
275+
});
211276
});

frontend/src/core/codemirror/copilot/getCodes.ts

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,32 @@
11
/* Copyright 2026 Marimo. All rights reserved. */
22

33
import { atom } from "jotai";
4-
import { getAllEditorViews, notebookAtom } from "@/core/cells/cells";
4+
import { notebookAtom } from "@/core/cells/cells";
55
import type { CellId } from "@/core/cells/ids";
6+
import { store } from "@/core/state/jotai";
67
import { variablesAtom } from "@/core/variables/state";
78
import type { Variables } from "@/core/variables/types";
89
import { Objects } from "@/utils/objects";
910
import { getEditorCodeAsPython } from "../language/utils";
1011

12+
const notebookCellCodes = atom((get) => {
13+
const notebook = get(notebookAtom);
14+
const codes = Objects.fromEntries(
15+
notebook.cellIds.inOrderIds.map((id) => {
16+
// A cell editor mounts progressively, so the notebook store is the only
17+
// complete source of code until every editor is built.
18+
const editorView = notebook.cellHandles[id]?.current?.editorViewOrNull;
19+
return [
20+
id,
21+
editorView
22+
? getEditorCodeAsPython(editorView)
23+
: (notebook.cellData[id]?.code ?? ""),
24+
];
25+
}),
26+
);
27+
return codes;
28+
});
29+
1130
export function getCodes(otherCode: string) {
1231
const codes = getOtherCellsCode(otherCode);
1332

@@ -18,15 +37,8 @@ export function getOtherCellsCode(otherCode: string) {
1837
// Get all other cells' code
1938
// Put `import` statements at the top, as it can help copilot give better suggestions
2039
// TODO: we should sort this topologically
21-
const codes = getAllEditorViews()
22-
.map((editorView) => {
23-
const code = getEditorCodeAsPython(editorView);
24-
if (code === otherCode) {
25-
return null;
26-
}
27-
return code;
28-
})
29-
.filter(Boolean)
40+
const codes = Object.values(store.get(notebookCellCodes))
41+
.filter((code) => code !== "" && code !== otherCode)
3042
.toSorted((a, b) => {
3143
if (a.startsWith("import") && !b.startsWith("import")) {
3244
return -1;
@@ -40,21 +52,6 @@ export function getOtherCellsCode(otherCode: string) {
4052
return codes;
4153
}
4254

43-
const notebookCellCodes = atom((get) => {
44-
const notebook = get(notebookAtom);
45-
const codes = Objects.fromEntries(
46-
notebook.cellIds.inOrderIds.map((id) => {
47-
const handle = notebook.cellHandles[id];
48-
return [
49-
id,
50-
handle?.current?.editorView
51-
? getEditorCodeAsPython(handle.current.editorView)
52-
: "",
53-
];
54-
}),
55-
);
56-
return codes;
57-
});
5855
const inOrderCellIdsAtom = atom((get) => {
5956
const notebook = get(notebookAtom);
6057
return notebook.cellIds.inOrderIds;

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import { beforeEach, describe, expect, it, type Mocked, vi } from "vitest";
88
import * as LSP from "vscode-languageserver-protocol";
99
import { cellId } from "@/__tests__/branded";
1010
import type { CellId } from "@/core/cells/ids";
11+
import { initialNotebookState, notebookAtom } from "@/core/cells/cells";
12+
import { createCell } from "@/core/cells/types";
1113
import { store } from "@/core/state/jotai";
14+
import { MultiColumn } from "@/utils/id-tree";
1215
import { topologicalCodesAtom } from "../../copilot/getCodes";
1316
import { lspWorkspaceAtom } from "@/core/saving/file-state";
1417
import { languageAdapterState } from "../../language/extension";
@@ -62,6 +65,28 @@ function isPublishDiagnosticsNotification(
6265
);
6366
}
6467

68+
describe("merged document with unmounted editors", () => {
69+
it("should include the code of cells whose editors are not mounted", () => {
70+
store.set(notebookAtom, {
71+
...initialNotebookState(),
72+
cellIds: MultiColumn.from([[Cells.cell1, Cells.cell2]]),
73+
cellData: {
74+
[Cells.cell1]: createCell({ id: Cells.cell1, code: "import math" }),
75+
[Cells.cell2]: createCell({
76+
id: Cells.cell2,
77+
code: "print(math.sqrt(4))",
78+
}),
79+
},
80+
});
81+
82+
const { cellIds, codes } = store.get(topologicalCodesAtom);
83+
const lens = createNotebookLens(cellIds, codes);
84+
85+
expect(lens.mergedText).toContain("import math");
86+
expect(lens.mergedText).toContain("print(math.sqrt(4))");
87+
});
88+
});
89+
6590
describe("createNotebookLens", () => {
6691
it("should produce correct lens for same inputs", () => {
6792
// Use unique content for this test

0 commit comments

Comments
 (0)