Skip to content

Commit 981b3e6

Browse files
authored
refactor: consolidate CellManager and NotebookDocument (#9405)
## Summary `CellManager` now wraps a `NotebookDocument` instead of holding its own dict of `CellData`, and `Session.document` is a property that reads through `cell_manager.document`. Removes a class of drift bugs where the two were synced once at session startup and then diverged across save, file-watch reload, and code-mode autosave. ## What changed - **`CellManager` wraps the document.** Removed `_cell_data` dict is gone; `get_cell_data` returns a fresh synthesized view each call. - **`Session.document` is a `@property`** returning `app.cell_manager.document`. No more manual rebinding when the manager or app is swapped. Save-endpoint workaround removed. - **Transitional helpers** preserve identity of `_document` and `_compiled_cells` for callers that still rebuild the cell list as a unit: `CellManager._replace_state_from` and `NotebookDocument._replace_cells`. `InternalApp.with_data` uses the former. A follow-up will route both through `Transaction.apply`. - **File-watch reload bridge.** `AppFileManager.reload()` still swaps in a fresh app, so the coordinator now snapshots `prev_document` before the swap, builds the diff `Transaction`, and passes it to the strategy. A follow-up will rework `reload()` to apply the diff in place. ## Tests Invariants for the document property, `get_cell_data` view freshness, `_replace_state_from` identity preservation, `_compiled_cells` lockstep rekeying (with an **xfail** for the latent inner `Cell._cell.cell_id` mismatch — separate follow-up), `_replace_cells` version bump, and `InternalApp.with_data` identity + compiled-cell carryover.
1 parent 67d2781 commit 981b3e6

11 files changed

Lines changed: 614 additions & 173 deletions

File tree

marimo/_ast/app.py

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ def clone(self) -> App:
294294
strict=False,
295295
):
296296
cell = None
297-
cell_data = self._cell_manager._cell_data.get(cell_id)
297+
cell_data = self._cell_manager.get_cell_data(cell_id)
298298
new_cell_id = app._cell_manager.create_cell_id()
299299
# If the cell exists, the cell data should be set (ie not None).
300300
if cell_data is not None:
@@ -1014,23 +1014,31 @@ def with_data(
10141014
names: Iterable[str],
10151015
configs: Iterable[CellConfig],
10161016
) -> InternalApp:
1017-
new_cell_manager = CellManager()
1017+
"""Rewrite the cell list from textual fields, in place.
1018+
1019+
Mutates ``self._app._cell_manager`` rather than replacing it,
1020+
so any caller holding ``app.cell_manager`` or
1021+
``app.cell_manager.document`` (notably ``Session.document``)
1022+
continues to see live state without rebinding.
1023+
1024+
Cells that survive the rewrite keep their compiled ``Cell``;
1025+
callers from save flows pass the frontend's snapshot, which
1026+
renames/reorders/reconfigures cells but doesn't recompile.
1027+
"""
1028+
cm = self._app._cell_manager
1029+
prev_compiled = dict(cm._compiled_cells)
1030+
rebuilt = CellManager(prefix=cm.prefix)
10181031
for cell_id, code, name, config in zip(
10191032
cell_ids, codes, names, configs, strict=False
10201033
):
1021-
cell = None
1022-
# If the cell exists, the cell data should be set.
1023-
cell_data = self._app._cell_manager._cell_data.get(cell_id)
1024-
if cell_data is not None:
1025-
cell = cell_data.cell
1026-
new_cell_manager.register_cell(
1034+
rebuilt.register_cell(
10271035
cell_id=cell_id,
10281036
code=code,
10291037
name=name,
10301038
config=config,
1031-
cell=cell,
1039+
cell=prev_compiled.get(cell_id),
10321040
)
1033-
self._app._cell_manager = new_cell_manager
1041+
cm._replace_state_from(rebuilt)
10341042
return self
10351043

10361044
async def run_cell_async(
@@ -1073,7 +1081,7 @@ def to_ir(self) -> NotebookSerializationV1:
10731081
name=cell_data.name,
10741082
options=cell_data.config.asdict(),
10751083
)
1076-
for cell_data in self._app._cell_manager._cell_data.values()
1084+
for cell_data in self._app._cell_manager.cell_data()
10771085
],
10781086
app=AppInstantiation(
10791087
options=self._app._config.asdict(),

0 commit comments

Comments
 (0)