Skip to content

Commit 345a419

Browse files
SamarJyoti496mscolnick
SamarJyoti496
and
mscolnick
authored
feat: Add line highlighting for syntax errors (#7047)
## 📝 Summary Syntax Errors now show red line highlighting in the editor matching the existing behavior for runtime errors. ## Fixes #6979 ## 🔍 Description of Changes **Problem:** Syntax errors displayed error messages but didn't highlight the problematic line in red, unlike runtime errors. **Solution:** Extract line numbers from Python's `SyntaxError.lineno` attribute and feed them into the existing traceback. ## 📋 Checklist - [x] I have read the [contributor guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md). - [x] I have run the code and verified that it works as expected. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Extracts `lineno` for Python syntax/import-star errors in the runtime, exposes it via API, and updates frontend traceback logic to highlight exact error lines, with comprehensive tests. > > - **Backend**: > - Add optional `lineno` to `MarimoSyntaxError` and `ImportStarError`; capture from `SyntaxError.lineno` in runtime. > - **API**: > - Extend OpenAPI schema and generated TS (`packages/openapi`) to include `lineno` on `MarimoSyntaxError` and `ImportStarError`. > - **Frontend**: > - Refactor `createTracebackInfoAtom` to use `cellRuntimeAtom` and parse `syntax`/`import-star` errors with `lineno` into `TracebackInfo` (skip when queued/running). > - Enhance mocks to allow per-cell `cellRuntime` overrides. > - Export `createTracebackInfoAtom` for testing. > - **Tests**: > - Add/expand tests validating `lineno` propagation (including 0, null, multiline) and new traceback behavior; update serialization expectations to include `lineno`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 9da2233. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Myles Scolnick <myles@marimo.io>
1 parent f8d5df9 commit 345a419

File tree

10 files changed

+359
-29
lines changed

10 files changed

+359
-29
lines changed

‎frontend/src/__mocks__/notebook.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@ import { createRef } from "react";
44
import { vi } from "vitest";
55
import type { CellActions, NotebookState } from "@/core/cells/cells";
66
import { CellId } from "@/core/cells/ids";
7-
import { type CellData, createCellRuntimeState } from "@/core/cells/types";
7+
import {
8+
type CellData,
9+
type CellRuntimeState,
10+
createCellRuntimeState,
11+
} from "@/core/cells/types";
812
import type { MarimoError } from "@/core/kernel/messages";
913
import { MultiColumn } from "@/utils/id-tree";
1014
import { Objects } from "@/utils/objects";
@@ -19,8 +23,10 @@ export const MockNotebook = {
1923

2024
notebookState: (opts?: {
2125
cellData: Record<string, Partial<CellData>>;
26+
cellRuntime?: Record<string, Partial<CellRuntimeState>>;
2227
}): NotebookState => {
2328
const cellData = opts?.cellData || {};
29+
const cellRuntime = opts?.cellRuntime || {};
2430
return {
2531
cellData: Objects.mapValues(cellData, (data, cellId) => ({
2632
id: cellId as CellId,
@@ -39,8 +45,8 @@ export const MockNotebook = {
3945
...data,
4046
})),
4147
cellIds: MultiColumn.from([Object.keys(cellData) as CellId[]]),
42-
cellRuntime: Objects.mapValues(cellData, (_data) =>
43-
createCellRuntimeState({}),
48+
cellRuntime: Objects.mapValues(cellData, (_data, cellId) =>
49+
createCellRuntimeState({ ...(cellRuntime[cellId] || {}) }),
4450
),
4551
cellHandles: Objects.mapValues(cellData, (_data) => createRef()),
4652
cellLogs: [],

‎frontend/src/core/cells/__tests__/cells.test.ts‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { python } from "@codemirror/lang-python";
44
import { EditorState } from "@codemirror/state";
55
import { EditorView } from "@codemirror/view";
6+
import { createStore } from "jotai";
67
import {
78
afterAll,
89
beforeAll,
@@ -12,6 +13,7 @@ import {
1213
it,
1314
vi,
1415
} from "vitest";
16+
import { MockNotebook } from "@/__mocks__/notebook";
1517
import type { CellHandle } from "@/components/editor/notebook-cell";
1618
import { CellId, SETUP_CELL_ID } from "@/core/cells/ids";
1719
import { foldAllBulk, unfoldAllBulk } from "@/core/codemirror/editing/commands";
@@ -24,6 +26,7 @@ import {
2426
exportedForTesting,
2527
flattenTopLevelNotebookCells,
2628
type NotebookState,
29+
notebookAtom,
2730
} from "../cells";
2831
import {
2932
focusAndScrollCellIntoView,
@@ -2679,3 +2682,205 @@ describe("isCellCodeHidden", () => {
26792682
);
26802683
});
26812684
});
2685+
2686+
describe("createTracebackInfoAtom", () => {
2687+
const store = createStore();
2688+
2689+
it("returns undefined when cell has no errors", async () => {
2690+
store.set(
2691+
notebookAtom,
2692+
MockNotebook.notebookState({
2693+
cellData: {
2694+
cell1: {
2695+
id: "cell1" as CellId,
2696+
name: "cell1",
2697+
code: "",
2698+
},
2699+
},
2700+
}),
2701+
);
2702+
2703+
const tracebackAtom = exportedForTesting.createTracebackInfoAtom(
2704+
"cell1" as CellId,
2705+
);
2706+
const traceback = store.get(tracebackAtom);
2707+
2708+
expect(traceback).toBeUndefined();
2709+
});
2710+
2711+
it("extracts lineno from syntax errors", async () => {
2712+
store.set(
2713+
notebookAtom,
2714+
MockNotebook.notebookState({
2715+
cellData: {
2716+
cell1: { id: "cell1" as CellId, name: "cell1", code: "x = 1" },
2717+
},
2718+
cellRuntime: {
2719+
cell1: {
2720+
output: {
2721+
channel: "marimo-error",
2722+
data: [{ type: "syntax", msg: "Syntax error", lineno: 5 }],
2723+
mimetype: "application/vnd.marimo+error",
2724+
},
2725+
},
2726+
},
2727+
}),
2728+
);
2729+
2730+
const tracebackAtom = exportedForTesting.createTracebackInfoAtom(
2731+
"cell1" as CellId,
2732+
);
2733+
const traceback = store.get(tracebackAtom);
2734+
2735+
expect(traceback).toBeDefined();
2736+
expect(traceback).toHaveLength(1);
2737+
expect(traceback![0]).toEqual({
2738+
kind: "cell",
2739+
cellId: "cell1",
2740+
lineNumber: 5,
2741+
});
2742+
});
2743+
2744+
it("handles syntax errors with lineno = 0", async () => {
2745+
store.set(
2746+
notebookAtom,
2747+
MockNotebook.notebookState({
2748+
cellData: {
2749+
cell1: { id: "cell1" as CellId, name: "cell1", code: "x = 1" },
2750+
},
2751+
cellRuntime: {
2752+
cell1: {
2753+
output: {
2754+
channel: "marimo-error",
2755+
data: [{ type: "syntax", msg: "Syntax error", lineno: 0 }],
2756+
mimetype: "application/vnd.marimo+error",
2757+
},
2758+
},
2759+
},
2760+
}),
2761+
);
2762+
2763+
const tracebackAtom = exportedForTesting.createTracebackInfoAtom(
2764+
"cell1" as CellId,
2765+
);
2766+
const traceback = store.get(tracebackAtom);
2767+
expect(traceback).toBeDefined();
2768+
expect(traceback).toHaveLength(1);
2769+
expect(traceback![0].lineNumber).toBe(0);
2770+
});
2771+
2772+
it("ignores syntax errors with lineno = null", () => {
2773+
store.set(
2774+
notebookAtom,
2775+
MockNotebook.notebookState({
2776+
cellData: {
2777+
cell1: { id: "cell1" as CellId, name: "cell1", code: "x = 1" },
2778+
},
2779+
cellRuntime: {
2780+
cell1: {
2781+
output: {
2782+
channel: "marimo-error",
2783+
data: [{ type: "syntax", msg: "Syntax error", lineno: null }],
2784+
mimetype: "application/vnd.marimo+error",
2785+
},
2786+
},
2787+
},
2788+
}),
2789+
);
2790+
2791+
const tracebackAtom = exportedForTesting.createTracebackInfoAtom(
2792+
"cell1" as CellId,
2793+
);
2794+
const traceback = store.get(tracebackAtom);
2795+
2796+
expect(traceback).toBeUndefined();
2797+
});
2798+
2799+
it("handles multiple syntax errors", () => {
2800+
store.set(
2801+
notebookAtom,
2802+
MockNotebook.notebookState({
2803+
cellData: {
2804+
cell1: { id: "cell1" as CellId, name: "cell1", code: "x = 1" },
2805+
},
2806+
cellRuntime: {
2807+
cell1: {
2808+
output: {
2809+
channel: "marimo-error",
2810+
data: [
2811+
{ type: "syntax", msg: "Syntax error", lineno: 3 },
2812+
{ type: "syntax", msg: "Syntax error", lineno: 7 },
2813+
],
2814+
mimetype: "application/vnd.marimo+error",
2815+
},
2816+
},
2817+
},
2818+
}),
2819+
);
2820+
2821+
const tracebackAtom = exportedForTesting.createTracebackInfoAtom(
2822+
"cell1" as CellId,
2823+
);
2824+
const traceback = store.get(tracebackAtom);
2825+
expect(traceback).toBeDefined();
2826+
expect(traceback).toHaveLength(2);
2827+
expect(traceback![0].lineNumber).toBe(3);
2828+
expect(traceback![1].lineNumber).toBe(7);
2829+
});
2830+
2831+
it("returns undefined when cell is queued", async () => {
2832+
store.set(
2833+
notebookAtom,
2834+
MockNotebook.notebookState({
2835+
cellData: {
2836+
cell1: { id: "cell1" as CellId, name: "cell1", code: "x = 1" },
2837+
},
2838+
cellRuntime: {
2839+
cell1: {
2840+
output: {
2841+
channel: "marimo-error",
2842+
data: [{ type: "syntax", msg: "Syntax error", lineno: 1 }],
2843+
mimetype: "application/vnd.marimo+error",
2844+
},
2845+
status: "queued",
2846+
},
2847+
},
2848+
}),
2849+
);
2850+
2851+
const tracebackAtom = exportedForTesting.createTracebackInfoAtom(
2852+
"cell1" as CellId,
2853+
);
2854+
const traceback = store.get(tracebackAtom);
2855+
2856+
expect(traceback).toBeUndefined();
2857+
});
2858+
2859+
it("returns undefined when cell is running", () => {
2860+
store.set(
2861+
notebookAtom,
2862+
MockNotebook.notebookState({
2863+
cellData: {
2864+
cell1: { id: "cell1" as CellId, name: "cell1", code: "x = 1" },
2865+
},
2866+
cellRuntime: {
2867+
cell1: {
2868+
output: {
2869+
channel: "marimo-error",
2870+
data: [{ type: "syntax", msg: "Syntax error", lineno: 1 }],
2871+
mimetype: "application/vnd.marimo+error",
2872+
},
2873+
status: "running",
2874+
},
2875+
},
2876+
}),
2877+
);
2878+
2879+
const tracebackAtom = exportedForTesting.createTracebackInfoAtom(
2880+
"cell1" as CellId,
2881+
);
2882+
const traceback = store.get(tracebackAtom);
2883+
2884+
expect(traceback).toBeUndefined();
2885+
});
2886+
});

‎frontend/src/core/cells/cells.ts‎

Lines changed: 44 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1681,40 +1681,59 @@ export function createUntouchedCellAtom(cellId: CellId): Atom<boolean> {
16811681
export function createTracebackInfoAtom(
16821682
cellId: CellId,
16831683
): Atom<TracebackInfo[] | undefined> {
1684-
// We create an intermediate atom that just computes the string
1685-
// so it prevents downstream recomputations.
1686-
const tracebackStringAtom = atom<string | undefined>((get) => {
1687-
const notebook = get(notebookAtom);
1688-
const data = notebook.cellRuntime[cellId];
1684+
//use existing cellRuntimeAtom for intermediate computation
1685+
const cellRuntime = cellRuntimeAtom(cellId);
1686+
1687+
return atom((get) => {
1688+
const data = get(cellRuntime);
1689+
16891690
if (!data) {
16901691
return undefined;
16911692
}
1692-
// Must be errored and idle
1693-
if (data.status !== "idle") {
1693+
1694+
if (data.status === "queued" || data.status === "running") {
16941695
return undefined;
16951696
}
1697+
1698+
const tracebackInfo: TracebackInfo[] = [];
1699+
1700+
// Runtime errors (ZeroDivisionError, etc.)
16961701
const outputs = data.consoleOutputs;
1697-
// console.warn(notebook);
1698-
if (!outputs || outputs.length === 0) {
1699-
return undefined;
1702+
if (outputs && outputs.length > 0) {
1703+
const firstTraceback = outputs.find(
1704+
(output) => output.mimetype === "application/vnd.marimo+traceback",
1705+
);
1706+
if (firstTraceback) {
1707+
const traceback = firstTraceback.data as string;
1708+
tracebackInfo.push(...extractAllTracebackInfo(traceback));
1709+
}
17001710
}
17011711

1702-
const firstTraceback = outputs.find(
1703-
(output) => output.mimetype === "application/vnd.marimo+traceback",
1704-
);
1705-
if (!firstTraceback) {
1706-
return undefined;
1712+
// Syntax errors
1713+
const output = data.output;
1714+
if (output?.mimetype === "application/vnd.marimo+error") {
1715+
const errors = output.data;
1716+
if (Array.isArray(errors)) {
1717+
for (const error of errors) {
1718+
if (error.type === "syntax" && error.lineno != null) {
1719+
tracebackInfo.push({
1720+
kind: "cell",
1721+
cellId: cellId,
1722+
lineNumber: error.lineno,
1723+
});
1724+
}
1725+
if (error.type === "import-star" && error.lineno != null) {
1726+
tracebackInfo.push({
1727+
kind: "cell",
1728+
cellId: cellId,
1729+
lineNumber: error.lineno,
1730+
});
1731+
}
1732+
}
1733+
}
17071734
}
1708-
const traceback = firstTraceback.data;
1709-
return traceback as string;
1710-
});
17111735

1712-
return atom((get) => {
1713-
const traceback = get(tracebackStringAtom);
1714-
if (!traceback) {
1715-
return undefined;
1716-
}
1717-
return extractAllTracebackInfo(traceback);
1736+
return tracebackInfo.length > 0 ? tracebackInfo : undefined;
17181737
});
17191738
}
17201739

@@ -1743,4 +1762,5 @@ export const exportedForTesting = {
17431762
cellDataAtom,
17441763
cellRuntimeAtom,
17451764
cellHandleAtom,
1765+
createTracebackInfoAtom,
17461766
};

‎marimo/_messaging/errors.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ def describe(self) -> str:
3333

3434
class ImportStarError(msgspec.Struct, tag="import-star"):
3535
msg: str
36+
lineno: Optional[int] = None
3637

3738
def describe(self) -> str:
3839
return self.msg
@@ -72,6 +73,7 @@ def describe(self) -> str:
7273

7374
class MarimoSyntaxError(msgspec.Struct, tag="syntax"):
7475
msg: str
76+
lineno: Optional[int] = None
7577

7678
def describe(self) -> str:
7779
return self.msg

‎marimo/_runtime/runtime.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -876,10 +876,14 @@ def _try_compiling_cell(
876876
syntax_error[0] = syntax_error[0][
877877
syntax_error[0].find("line") :
878878
]
879+
880+
lineno = getattr(e, "lineno", None)
879881
if isinstance(e, ImportStarError):
880-
error = MarimoImportStarError(msg=str(e))
882+
error = MarimoImportStarError(msg=str(e), lineno=lineno)
881883
else:
882-
error = MarimoSyntaxError(msg="\n".join(syntax_error))
884+
error = MarimoSyntaxError(
885+
msg="\n".join(syntax_error), lineno=lineno
886+
)
883887
else:
884888
tmpio = io.StringIO()
885889
traceback.print_exc(file=tmpio)

0 commit comments

Comments
 (0)