feat: auto-save in code-mode and marimo-pair#9191
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds backend-driven auto-save so kernel-sourced notebook document transactions (from code_mode / marimo-pair) persist to disk, aligning agent-driven edits with the existing frontend save behavior.
Changes:
- Intercept kernel-sourced
NotebookDocumentTransactionNotifications inNotificationListenerExtensionand trigger best-effort autosave (skip run mode + unnamed notebooks; surface failures as anAlertNotification). - Introduce
SerialTaskRunnerto FIFO-dispatch blocking saves off the asyncio event loop and routeon_errorback to the loop thread. - Add
AppFileManager.save_from_cells()plus a re-entrant write lock; normalize empty cell names during codegen to avoid generating invaliddef ():.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
marimo/_session/extensions/extensions.py |
Intercepts kernel document transactions and schedules autosave + failure toast. |
marimo/_utils/serial_task_runner.py |
New single-worker FIFO executor helper with loop-thread error routing. |
marimo/_session/notebook/file_manager.py |
Adds save_from_cells() and serializes write paths under an RLock. |
marimo/_ast/codegen.py |
Normalizes empty cell names to DEFAULT_CELL_NAME during file generation. |
tests/_utils/test_serial_task_runner.py |
Unit tests for sync/async runner behavior, FIFO ordering, and shutdown semantics. |
tests/_session/extensions/test_notification_listener_autosave.py |
Tests autosave behavior, skip conditions, and failure-to-toast behavior (incl. HTML escaping). |
tests/_code_mode/test_context_autosave.py |
End-to-end tests from real code_mode mutations through interceptor to disk writes. |
tests/_server/test_file_manager.py |
Tests save_from_cells() behavior and lock serialization under concurrent writes. |
manzt
approved these changes
Apr 15, 2026
Comment on lines
+62
to
+67
| Offloads to the executor when called from the event loop; | ||
| otherwise runs inline on the caller thread (e.g. the | ||
| ``QueueDistributor`` worker thread). ``on_error`` is invoked | ||
| with any exception raised by ``work`` — posted back to the event | ||
| loop when off-loop, inline otherwise. A failing ``on_error`` is | ||
| logged and swallowed. |
Comment on lines
+352
to
+359
| assert not t1.is_alive(), ( | ||
| "frontend save thread did not terminate within 10s " | ||
| "(likely deadlock in AppFileManager write lock)" | ||
| ) | ||
| assert not t2.is_alive(), ( | ||
| "autosave thread did not terminate within 10s " | ||
| "(likely deadlock in AppFileManager write lock)" | ||
| ) |
Collaborator
There was a problem hiding this comment.
wierd copilot didn't see this?
Contributor
Author
There was a problem hiding this comment.
those are old comments
| persist=request.persist, | ||
| ) | ||
|
|
||
| def save_from_cells(self, cells: Sequence[NotebookCell]) -> str: |
Collaborator
There was a problem hiding this comment.
I'm guessing we can improve our "save" paths overall in the future to derive from our NotebookDocument, for a followup.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Agent mutations via
code_mode/marimo-pairemit kernel document transactions but never hit disk.NotificationListenerExtensionnow intercepts kernel-sourced transactions and triggers a best-effort save; run mode and unnamed notebooks are skipped, and failures surface as anAlertNotificationtoast instead of raising.SerialTaskRunnerFIFO-dispatches the blocking save off the event loop so a slow older save can't clobber a newer one, and routeson_errorback to the loop thread.AppFileManagergainssave_from_cells(cells)plus a reentrant_save_lockserializing every public writer.generate_filecontentsnormalizes empty cell names toDEFAULT_CELL_NAME(empty names emitted invaliddef ():).Note: the frontend still thinks the UI is not saved (because it was saved in the backend which is a new concept). I think this is ok for now, but something we can track in a follow.