Skip to content

Revert "fix: default new cell type to markdown instead of code" #80

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hamelsmu merged 1 commit into from
Feb 13, 2026
Merged

Revert "fix: default new cell type to markdown instead of code" #80

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions app/src/components/Actions/Actions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -819,12 +819,24 @@ function NotebookTabContent({ docUri }: { docUri: string }) {
if (!data) {
return;
}
// Insert a markup (markdown) cell at the top of the notebook.
// Default to inserting a code cell when notebook is empty.
const firstCell = cellDatas[0]?.snapshot;
if (firstCell) {
data.addMarkupCellBefore(firstCell.refId);
data.addCodeCellBefore(firstCell.refId);
} else {
data.appendMarkupCell();
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);
}
Comment on lines +827 to +839
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fallback logic here is unnecessarily complex and inconsistent with the internal cell creation logic. When addCodeCellAfter("") is called with an empty notebook, it will always return null because there's no cell with an empty refId. The fallback then manually creates a cell with languageId: "bash", but the private createCodeCell method defaults to "js". This inconsistency could lead to unexpected behavior where cells created through different code paths have different default languages. Consider either: 1) Checking if cellDatas is empty first and handling that case directly, or 2) Adding a simpler appendCodeCell() method to NotebookData that appends to the end regardless of existing cells, similar to the removed appendMarkupCell() method.

Suggested change
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);
}
// Notebook is empty; create and persist a new default code cell at the end.
const cell = create(parser_pb.CellSchema, {
metadata: {},
refId: `code_${crypto.randomUUID().replace(/-/g, "")}`,
languageId: "js",
role: parser_pb.CellRole.USER,
kind: parser_pb.CellKind.CODE,
value: "",
});
data.updateCell(cell);

Copilot uses AI. Check for mistakes.
}
}}
>
Expand Down
111 changes: 0 additions & 111 deletions app/src/lib/notebookData.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @vitest-environment node
import { Subject } from "rxjs";
import { create } from "@bufbuild/protobuf";
import { beforeAll, describe, expect, it, vi } from "vitest";
Expand Down Expand Up @@ -217,113 +216,3 @@ describe("NotebookData.getActiveStream", () => {
expect(model.getActiveStream(cell.refId)).toBeUndefined();
});
});

describe("NotebookData markup cell methods", () => {
function makeModel(cells: InstanceType<typeof parser_pb.Cell>[]) {
const notebook = create(parser_pb.NotebookSchema, { cells });
return new NotebookData({
notebook,
uri: "nb://test",
name: "test",
notebookStore: null,
loaded: true,
});
}

function getCells(model: InstanceType<typeof NotebookData>) {
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");
});
});
});
43 changes: 3 additions & 40 deletions app/src/lib/notebookData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,30 +410,6 @@ 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) {
Expand Down Expand Up @@ -601,18 +577,6 @@ 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();
Expand Down Expand Up @@ -822,16 +786,15 @@ export class CellData {
this.notebook.removeCell(this.refId);
}

run(): string {
run(): void {
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);
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing whitespace detected at the end of this line. This should be removed to maintain code cleanliness.

Copilot uses AI. Check for mistakes.
this.emitRunIDChange(runID ?? "");
return runID;
}

getStreams(): StreamsLike | undefined {
Expand Down
Loading