Skip to content

[nd 3/5] Frontend transaction sync#8844

Merged
manzt merged 3 commits intomanzt/nd-sessionfrom
manzt/nd-frontend
Mar 25, 2026
Merged

[nd 3/5] Frontend transaction sync#8844
manzt merged 3 commits intomanzt/nd-sessionfrom
manzt/nd-frontend

Conversation

@manzt
Copy link
Copy Markdown
Collaborator

@manzt manzt commented Mar 24, 2026

Prev #8842, #8843. This closes the frontend ↔ session loop in three steps:

Frontend → session. A reducer middleware intercepts cell actions (create, delete, move, rename, set-config) as the user performs them, batches them into transaction ops, debounces (400ms), and flushes to POST /api/document/transaction. A suppress guard prevents echo loops when applying server-originated ops.

Session → frontend. applyTransactionOps maps each op to the corresponding cell reducer action.

Queued-cell staleness. When the kernel queues a cell for execution, the frontend now snapshots lastCodeRun. Previously only the user-initiated "Run" button did this, so kernel-initiated runs (e.g. via code_mode) left cells visually stale even after successful execution.

@manzt manzt requested a review from Light2Dark as a code owner March 24, 2026 01:37
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment Mar 25, 2026 9:04pm

Request Review

@manzt manzt changed the title [nd 3/5] Frontend transaction sync [nd 3/5] Frontend transaction sync Mar 24, 2026
@manzt manzt added the enhancement New feature or request label Mar 24, 2026
@manzt manzt force-pushed the manzt/nd-session branch from 510769c to 8f1aab9 Compare March 24, 2026 01:45
@manzt manzt force-pushed the manzt/nd-frontend branch from ee2fc9f to 552dbb6 Compare March 24, 2026 01:45
@manzt manzt force-pushed the manzt/nd-frontend branch from 552dbb6 to 2bb42ff Compare March 24, 2026 02:08
@github-actions github-actions bot added the bash-focus Area to focus on during release bug bash label Mar 24, 2026
@manzt manzt force-pushed the manzt/nd-session branch from 8f1aab9 to 527fa05 Compare March 24, 2026 02:16
@manzt manzt force-pushed the manzt/nd-frontend branch from 2bb42ff to 468dd42 Compare March 24, 2026 02:16
NotificationMessageData<"notebook-document-transaction">["transaction"];
type TransactionOp = Transaction["ops"][number];

interface CellActions {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

might be able to Pick<CellActions, ...> here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think i ran into some circular reference (because the type is inferred). We could probably have an explicit type somewhere that we reuse as a type-only import.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah yes it is inferred. maybe we can make middleware additive

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe creates other problems/smells, but something like this

image

Comment on lines +22 to +36
/**
* Actions the middleware intercepts. Payload types are hardcoded from
* the notebook reducer in cells.ts. A follow-up can infer these from
* the reducer directly once it's extracted to a named variable.
*/
type DocumentAction =
| { type: "createNewCell"; payload: { cellId: CellId } }
| { type: "deleteCell"; payload: { cellId: CellId } }
| { type: "moveCell"; payload: { cellId: CellId } }
| { type: "sendToTop"; payload: { cellId: CellId } }
| { type: "sendToBottom"; payload: { cellId: CellId } }
| { type: "dropCellOverCell"; payload: unknown }
| { type: "dropCellOverColumn"; payload: unknown }
| { type: "updateCellCode"; payload: { cellId: CellId; code: string } }
| { type: "updateCellName"; payload: { cellId: CellId; name: string } };
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@mscolnick similar here... our middleware interface isn't strictly typed on the reducer. I hard-coded types to try to reduce the diff.

let cellId: CellId | "__end__" = "__end__";
let before = false;
if (op.after) {
cellId = op.after as CellId;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think these as could be cleaned up now from your other PRs

manzt added 2 commits March 25, 2026 16:58
The document transaction middleware and apply-transaction logic were
spread across two files with duplicated type definitions and several
gaps in the op mapping. This PR consolidates them into `document-ops.ts`
as two pure functions — `toDocumentOps` (action → ops) and
`fromDocumentOps` (ops → actions) — with thin impure wrappers for
debouncing and dispatch.

The middleware was missing the `updateCellConfig` handler entirely
(config changes were silently dropped), sent an empty config object for
`createNewCell` instead of reading the actual `CellConfig` from
post-reducer state, and produced `after: null` when a cell moved to the
first position. Position derivation now uses an `anchorOf` helper that
reads from `newState`, eliminating type casts and correctly handling
`__end__`, column objects, and left/right moves without special-casing.

Column structure actions (`addColumnBreakpoint`, `dropOverNewColumn`,
etc.) previously only emitted `reorder-cells`, losing the per-cell
column index. They now diff column assignments between prev and new
state and emit `set-config` ops for cells whose column changed, keeping
the Python `NotebookDocument` in sync.

The middleware switch is exhaustive with `assertNever` — adding a new
reducer action without handling it here is a compile error. A new
round-trip test suite proves the two directions are correct inverses by
performing actions on a primary notebook, applying the emitted ops to a
replica, and asserting both converge to identical document state.
@manzt manzt force-pushed the manzt/nd-session branch from 1be085c to d0c7932 Compare March 25, 2026 20:59
@manzt manzt force-pushed the manzt/nd-frontend branch from 671e569 to 8002a4a Compare March 25, 2026 20:59
Prev #8842, #8843, #8844. This PR closes the kernel → session loop.

**Transaction emission.** `_apply_ops` previously assembled three
separate legacy notifications (`UpdateCellCodesNotification` grouped by
stale status, plus `UpdateCellIdsNotification`). Now it builds typed
`Op`s via `_plan_to_document_ops()` and broadcasts a **single**
`NotebookDocumentTransactionNotification`.

**Document context.** The execution endpoint snapshots
`session.document.cells` into `ExecuteScratchpadCommand.notebook_cells`.
The kernel handler sets a `ContextVar` from it so `AsyncCodeModeContext`
can read cell state from the document.`_CellsView` now delegates
entirely to the document and returns `NotebookCell` directly, removing
the `NotebookCellData` wrapper and the module-level `_cell_names` dict.
@manzt manzt requested a review from dmadisetti as a code owner March 25, 2026 20:59
@manzt manzt merged commit 5a3191b into manzt/nd-session Mar 25, 2026
9 of 11 checks passed
@manzt manzt deleted the manzt/nd-frontend branch March 25, 2026 21:00
manzt added a commit that referenced this pull request Mar 26, 2026
This PR introduces the core data model, with wiring to session (#8843),
frontend (#8844), and kernel (#8845).

`NotebookDocument` is an ordered list of `NotebookCell` entries that
applies `Transaction`s atomically. A transaction is a tuple of typed ops
(`CreateCell`, `DeleteCell`, `MoveCell`, `ReorderCells`, `SetCode`,
`SetName`, `SetConfig`) with a `source` tag identifying the writer (e.g.
`"frontend"`, `"kernel"`, `"file-watch"`).

The document validates ops for conflicts (e.g. delete + update on the
same cell) before applying. Version is `None` on creation and stamped by
`apply()`.

```python
doc = NotebookDocument([NotebookCell(id="a", code="x = 1", ...)])
tx = Transaction(ops=(SetCode(cell_id="a", code="x = 2"),), source="kernel")
applied = doc.apply(tx)  # stamps version → Transaction(version=1)
```

---------

Co-authored-by: Shahmir Varqha <Sham9871@gmail.com>
Co-authored-by: Myles Scolnick <myles@marimo.io>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bash-focus Area to focus on during release bug bash enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants