From 7c3aea6460dfa63a52263f35fa026144a2fe3239 Mon Sep 17 00:00:00 2001 From: Hamel Husain Date: Thu, 12 Feb 2026 18:52:26 -0800 Subject: [PATCH] fix: default new cell type to markdown instead of code (#72) When creating a new notebook or adding a cell at the top, the default cell type was JavaScript/code. Changed to markdown since most notebooks start with documentation. - Add createMarkupCell(), addMarkupCellBefore(), appendMarkupCell() - Change top-of-notebook "Add cell" button to insert markup cells - Add 5 unit tests for the new markup cell methods - Add @vitest-environment node directive to fix jsdom ESM bug Co-Authored-By: Claude Opus 4.6 Signed-off-by: Hamel Husain --- app/src/components/Actions/Actions.tsx | 18 +--- app/src/lib/notebookData.test.ts | 111 +++++++++++++++++++++++++ app/src/lib/notebookData.ts | 43 +++++++++- 3 files changed, 154 insertions(+), 18 deletions(-) diff --git a/app/src/components/Actions/Actions.tsx b/app/src/components/Actions/Actions.tsx index 3348c98..c3a205b 100644 --- a/app/src/components/Actions/Actions.tsx +++ b/app/src/components/Actions/Actions.tsx @@ -819,24 +819,12 @@ function NotebookTabContent({ docUri }: { docUri: string }) { if (!data) { return; } - // Default to inserting a code cell when notebook is empty. + // Insert a markup (markdown) cell at the top of the notebook. const firstCell = cellDatas[0]?.snapshot; if (firstCell) { - data.addCodeCellBefore(firstCell.refId); + data.addMarkupCellBefore(firstCell.refId); } else { - const newCell = data.addCodeCellAfter(""); - if (!newCell) { - // Fallback: create and persist a new code cell at the end. - const cell = create(parser_pb.CellSchema, { - metadata: {}, - refId: `code_${crypto.randomUUID().replace(/-/g, "")}`, - languageId: "bash", - role: parser_pb.CellRole.USER, - kind: parser_pb.CellKind.CODE, - value: "", - }); - data.updateCell(cell); - } + data.appendMarkupCell(); } }} > diff --git a/app/src/lib/notebookData.test.ts b/app/src/lib/notebookData.test.ts index e2b4857..684e51b 100644 --- a/app/src/lib/notebookData.test.ts +++ b/app/src/lib/notebookData.test.ts @@ -1,3 +1,4 @@ +// @vitest-environment node import { Subject } from "rxjs"; import { create } from "@bufbuild/protobuf"; import { beforeAll, describe, expect, it, vi } from "vitest"; @@ -216,3 +217,113 @@ describe("NotebookData.getActiveStream", () => { expect(model.getActiveStream(cell.refId)).toBeUndefined(); }); }); + +describe("NotebookData markup cell methods", () => { + function makeModel(cells: InstanceType[]) { + const notebook = create(parser_pb.NotebookSchema, { cells }); + return new NotebookData({ + notebook, + uri: "nb://test", + name: "test", + notebookStore: null, + loaded: true, + }); + } + + function getCells(model: InstanceType) { + return model.getSnapshot().notebook.cells; + } + + describe("appendMarkupCell", () => { + it("appends a markup cell to an empty notebook", () => { + const model = makeModel([]); + const cell = model.appendMarkupCell(); + const cells = getCells(model); + + expect(cell).toBeTruthy(); + expect(cell.kind).toBe(parser_pb.CellKind.MARKUP); + expect(cell.languageId).toBe("markdown"); + expect(cell.value).toBe(""); + expect(cell.refId).toMatch(/^markup_/); + expect(cells.length).toBe(1); + expect(cells[0].refId).toBe(cell.refId); + }); + + it("appends a markup cell after existing cells", () => { + const existing = create(parser_pb.CellSchema, { + refId: "code_existing", + kind: parser_pb.CellKind.CODE, + languageId: "js", + value: "console.log(1)", + metadata: {}, + outputs: [], + }); + const model = makeModel([existing]); + const cell = model.appendMarkupCell(); + const cells = getCells(model); + + expect(cells.length).toBe(2); + expect(cells[0].refId).toBe("code_existing"); + expect(cells[1].refId).toBe(cell.refId); + expect(cell.kind).toBe(parser_pb.CellKind.MARKUP); + }); + }); + + describe("addMarkupCellBefore", () => { + it("inserts a markup cell before a target cell", () => { + const existing = create(parser_pb.CellSchema, { + refId: "code_first", + kind: parser_pb.CellKind.CODE, + languageId: "bash", + value: "echo hi", + metadata: {}, + outputs: [], + }); + const model = makeModel([existing]); + const cell = model.addMarkupCellBefore("code_first"); + const cells = getCells(model); + + expect(cell).toBeTruthy(); + expect(cell!.kind).toBe(parser_pb.CellKind.MARKUP); + expect(cell!.languageId).toBe("markdown"); + expect(cell!.refId).toMatch(/^markup_/); + expect(cells.length).toBe(2); + expect(cells[0].refId).toBe(cell!.refId); + expect(cells[1].refId).toBe("code_first"); + }); + + it("returns null for an invalid refId", () => { + const model = makeModel([]); + const result = model.addMarkupCellBefore("nonexistent"); + expect(result).toBeNull(); + expect(getCells(model).length).toBe(0); + }); + + it("inserts before the correct cell in a multi-cell notebook", () => { + const cell1 = create(parser_pb.CellSchema, { + refId: "cell_1", + kind: parser_pb.CellKind.CODE, + languageId: "js", + value: "1", + metadata: {}, + outputs: [], + }); + const cell2 = create(parser_pb.CellSchema, { + refId: "cell_2", + kind: parser_pb.CellKind.CODE, + languageId: "js", + value: "2", + metadata: {}, + outputs: [], + }); + const model = makeModel([cell1, cell2]); + const inserted = model.addMarkupCellBefore("cell_2"); + const cells = getCells(model); + + expect(cells.length).toBe(3); + expect(cells[0].refId).toBe("cell_1"); + expect(cells[1].refId).toBe(inserted!.refId); + expect(cells[2].refId).toBe("cell_2"); + }); + }); +}); diff --git a/app/src/lib/notebookData.ts b/app/src/lib/notebookData.ts index d60687f..b3e64a5 100644 --- a/app/src/lib/notebookData.ts +++ b/app/src/lib/notebookData.ts @@ -410,6 +410,30 @@ export class NotebookData { return cell; } + addMarkupCellBefore(targetRefId: string): parser_pb.Cell | null { + const idx = this.refToIndex.get(targetRefId); + if (idx === undefined) { + return null; + } + const cell = this.createMarkupCell(); + this.notebook.cells.splice(idx, 0, cell); + this.rebuildIndex(); + this.snapshotCache = this.buildSnapshot(); + this.emit(); + void this.persist(); + return cell; + } + + appendMarkupCell(): parser_pb.Cell { + const cell = this.createMarkupCell(); + this.notebook.cells.push(cell); + this.rebuildIndex(); + this.snapshotCache = this.buildSnapshot(); + this.emit(); + void this.persist(); + return cell; + } + removeCell(refId: string): void { const idx = this.refToIndex.get(refId); if (idx === undefined) { @@ -577,6 +601,18 @@ export class NotebookData { return this.createAndBindStreams({ cell, runID, sequence, runner }); } + private createMarkupCell(): parser_pb.Cell { + const refID = `markup_${crypto.randomUUID().replace(/-/g, "")}`; + return create(parser_pb.CellSchema, { + metadata: {}, + refId: refID, + languageId: "markdown", + role: parser_pb.CellRole.USER, + kind: parser_pb.CellKind.MARKUP, + value: "", + }); + } + private createCodeCell(languageId?: string | null): parser_pb.Cell { const refID = `code_${crypto.randomUUID().replace(/-/g, "")}`; const normalizedLanguage = languageId?.trim().toLowerCase(); @@ -786,15 +822,16 @@ export class CellData { this.notebook.removeCell(this.refId); } - run(): void { + run(): string { console.log(`Running CellData ${this.cellDataID}`); const cell = this.snapshot; - if (!cell) return; + if (!cell) return ""; const runID = this.notebook.runCodeCell(cell); console.log("Started run for cell", { refId: this.refId, runID }); // Update the snapshot after running to pick up any metadata changes. - this.cachedSnapshot = this.notebook.getCellSnapshot(this.refId); + this.cachedSnapshot = this.notebook.getCellSnapshot(this.refId); this.emitRunIDChange(runID ?? ""); + return runID; } getStreams(): StreamsLike | undefined {