feat: `marimo edit` local static notebooks #7474

crivaronicolini · 2025-12-11T18:31:06Z

📝 Summary

This PR adds a way to edit local html notebooks as proposed in #7327. This allows for easier sharing of notebooks: with this feature the notebooks used for presentation (with cell outputs) can be edited without access to the original Python file.

🔍 Description of Changes

The changes involved mainly repurposing the existing StaticNotebookReader as a new file handler that was placed first on the chain of handlers that try to open a file. The new handler now handles every html file regardless of origin, and it raises a Click exception when it's unable to handle the html file. This was required to prevent these files from being read by the LocalFileHandler and breaking the app. The alternative would have been to edit every handler and local reader to ignore html files.

I also modified the regex used to find the actual code inside the <marimo-code> tags so that it can handle whitespace between the tags and the Python code. The previous version was failing on some test files downloaded from Marimo.

Finally, I had also modified the StaticNotebookReader.read function to reuse the file downloaded in the can_read function. This way would save a redundant download. In the end I decided to keep it as it was because it changed the read function signature (name was not needed). Let me know what you think about this and if I should make the change anyways.

📋 Checklist

I have read the contributor guidelines.
For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
I have added tests for the changes made.
I have run the code and verified that it works as expected.

vercel · 2025-12-11T18:31:11Z

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

Project	Deployment	Review	Updated (UTC)
marimo-docs	Ready	Preview, Comment	Dec 15, 2025 11:05pm

mscolnick · 2025-12-11T19:14:48Z

@crivaronicolini this code looks good. the local vs remote was maybe a bad abstraction and makes this a bit difficult.

i think really what local vs remote is: already-a-marimo-file-on-your-computer vs needs-to-be-downloaded-or-written-as-a-tmp-marimo-file

so given that, I wonder if we can change the RemoveFileHandler can_handle to be able to handle local html files. or we let LocalFileHandler also create new tempdirs/files

crivaronicolini · 2025-12-11T19:52:17Z

Thanks for taking a look!
Yeah, that was the first thing I tried, but to handle local html files in RemoteFileHandle I had to bypass html files in LocalFileHandler and LocalFileReader and it seemed messy.

I also tried to let LocalFileHandler handle the html files but changed my mind when I realised it had to use StaticNotebookReader directly and then create temp copies. It was starting to look too similar to RemoteFileHandler.
In the end it seemed neater to handle every html file in a single handler and raise an exception if we can't handle the file.

Let me know if I should try again with the LocalFileHandler anyways.

mscolnick · 2025-12-11T23:58:29Z

@crivaronicolini - it looks like the change fails this test https://github.com/marimo-team/marimo/actions/runs/20143407972/job/57817253583?pr=7474, (the other errors are unrelated)

would you mind taking another crack at the previous way? i think if the can_handle has more logic than is_url, then maybe it will work? not is_url(file) and not file.endswith("html").

dmadisetti · 2025-12-15T19:15:45Z

tests/_cli/test_file_path.py

        temp_dir_obj.cleanup()
-
-    @patch("marimo._utils.requests.get")
-    def test_validate_remote_html_not_notebook(self, mock_get):


Suggested change

def test_validate_remote_html_not_notebook(self, mock_get):

def test_validate_remote_html_is_notebook(self, mock_get):

Thoughts? Looks good though

Copilot

Pull request overview

This PR enables editing of local HTML notebooks (static notebooks with embedded Python code) by extending the StaticNotebookReader to handle local .html files in addition to remote URLs. This allows users to edit presentation notebooks directly without needing the original Python file.

Key Changes

Extended StaticNotebookReader to read local .html files with embedded marimo code
Updated regex patterns to handle whitespace between marimo-code tags and Python content
Modified LocalFileHandler to process .html files using StaticNotebookReader before general validation

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

File	Description
marimo/_cli/file_path.py	Added DEFAULT_FILENAME constant, updated regex patterns for whitespace handling, extended StaticNotebookReader to read local HTML files, added HTML file handling in LocalFileHandler, and refactored temp file creation
tests/_cli/test_file_path.py	Removed old test_static_notebook_reader, added comprehensive TestStaticNotebooks class with tests for local/remote HTML files and validation scenarios

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-15T21:47:32Z

marimo/_cli/file_path.py

    CODE_TAG = r"marimo-code"
-    CODE_REGEX = re.compile(r"<marimo-code\s+hidden(?:=['\"]{2})?\s*>(.*?)<")
+    CODE_REGEX = re.compile(
+        r"<marimo-code\s+hidden(?:=['\"]{2})?\s*>\s*(.*?)\s*<"


The regex pattern may not correctly match multiline Python code within the marimo-code tags. The pattern uses .*? which doesn't match newlines by default. Add the re.DOTALL flag to ensure the pattern matches code spanning multiple lines. This would be: re.compile(r"<marimo-code\s+hidden(?:=['\"]{2})?\s*>\s*(.*?)\s*<", re.DOTALL)

Suggested change

r"<marimo-code\s+hidden(?:=['\"]{2})?\s*>\s*(.*?)\s*<"

r"<marimo-code\s+hidden(?:=['\"]{2})?\s*>\s*(.*?)\s*<",

re.DOTALL

This is fine since we generate the code- but always reminds me of this: https://stackoverflow.com/questions/1732348/regex-match-open-tags-except-xhtml-self-contained-tags

No multiple lines in this cases (all encoded)

marimo/_cli/file_path.py

Copilot · 2025-12-15T21:47:33Z

marimo/_cli/file_path.py

+    @staticmethod
+    def _create_tmp_file_from_content(
+        content: str, name: str, temp_dir: TemporaryDirectory[str]
+    ) -> str:
+        LOGGER.info("Creating temporary file")
+        path_to_app = Path(temp_dir.name) / name
+        path_to_app.write_text(content, encoding="utf-8")
+        LOGGER.info("App saved to %s", path_to_app)
+        return str(path_to_app)


The _create_tmp_file_from_content method is duplicated between LocalFileHandler and RemoteFileHandler with identical implementations. Consider extracting this to a shared utility function or a common base class method to reduce code duplication and improve maintainability.

Copilot · 2025-12-15T21:47:33Z

marimo/_cli/file_path.py

+            # Read a local Static Notebook
+            if url.endswith(".html"):
+                file_contents = Path(url).read_text(encoding="utf-8")
+                return (
+                    StaticNotebookReader.CODE_TAG in file_contents,
+                    file_contents,
+                )


Consider adding path traversal validation for local HTML file paths before reading them. While this is likely handled by the Path object's normal resolution, explicit validation using path normalization and checking for suspicious patterns (e.g., "..") would provide defense in depth against potential security issues.

Copilot · 2025-12-15T21:47:34Z

marimo/_cli/file_path.py

+                content, filename = reader.read(name)
+                path_to_app = self._create_tmp_file_from_content(
+                    content, filename, temp_dir
+                )
+                return path_to_app, temp_dir


The extracted filename is passed directly to create a path without validation. If the filename is an absolute path or contains path separators, the Path / operator may not behave as expected (e.g., Path(temp_dir) / "/absolute/path" returns "/absolute/path"). Consider using only the basename of the extracted filename with os.path.basename() or Path(filename).name to ensure the file is created within the temp directory.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

dmadisetti

Copilot seems rather concerned about path injection, there's a bit of hardening we can do around the general function, but the implementation, as is, is fine (and the code is wrong elsewhere too)

dmadisetti · 2025-12-15T22:57:00Z

marimo/_cli/file_path.py

    CODE_TAG = r"marimo-code"
-    CODE_REGEX = re.compile(r"<marimo-code\s+hidden(?:=['\"]{2})?\s*>(.*?)<")
+    CODE_REGEX = re.compile(
+        r"<marimo-code\s+hidden(?:=['\"]{2})?\s*>\s*(.*?)\s*<"


This is fine since we generate the code- but always reminds me of this: https://stackoverflow.com/questions/1732348/regex-match-open-tags-except-xhtml-self-contained-tags

No multiple lines in this cases (all encoded)

Handle local static notebooks

da4b724

vercel bot deployed to Preview December 11, 2025 18:31 View deployment

Use LocalFileHandler

5c6aa4a

vercel bot deployed to Preview December 12, 2025 20:04 View deployment

dmadisetti reviewed Dec 15, 2025

View reviewed changes

mscolnick requested a review from Copilot December 15, 2025 21:37

Copilot started reviewing on behalf of mscolnick December 15, 2025 21:39 View session

Copilot AI reviewed Dec 15, 2025

View reviewed changes

nit: Marimo -> marimo

1268244

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

vercel bot deployed to Preview December 15, 2025 23:05 View deployment

dmadisetti approved these changes Dec 15, 2025

View reviewed changes

mscolnick merged commit 61c9076 into marimo-team:main Dec 16, 2025
44 of 60 checks passed

crivaronicolini deleted the edit-html branch December 21, 2025 01:02

Light2Dark added the enhancement New feature or request label Jan 8, 2026

	def test_validate_remote_html_not_notebook(self, mock_get):
	def test_validate_remote_html_is_notebook(self, mock_get):

	r"<marimo-code\s+hidden(?:=['\"]{2})?\s>\s(.?)\s<"
	r"<marimo-code\s+hidden(?:=['\"]{2})?\s>\s(.?)\s<",
	re.DOTALL

feat: marimo edit local static notebooks #7474

feat: marimo edit local static notebooks #7474

Uh oh!

Conversation

crivaronicolini commented Dec 11, 2025

📝 Summary

🔍 Description of Changes

📋 Checklist

Uh oh!

vercel bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mscolnick commented Dec 11, 2025

Uh oh!

crivaronicolini commented Dec 11, 2025

Uh oh!

mscolnick commented Dec 11, 2025

Uh oh!

dmadisetti Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

dmadisetti Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

dmadisetti left a comment

Choose a reason for hiding this comment

Uh oh!

dmadisetti Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feat: `marimo edit` local static notebooks #7474

feat: `marimo edit` local static notebooks #7474

vercel bot commented Dec 11, 2025 •

edited

Loading

dmadisetti Dec 15, 2025 •

edited

Loading