fix: edge cases for parsing pasted notebooks#10033
Merged
Merged
Conversation
Pasting a marimo app into a cell used line-based heuristics to strip the
file boilerplate, which mangled several valid constructs. This PR
replaces the line-based heuristics with lezer's Python parser, adding
support for setup cells and app.function's in the process.
Fixes the following, each covered by a test:
- Decorators inside a cell (`@functools.cache`, `@property`, ...) were
dropped and could split the cell in two.
- `async def` cells with multi-line args leaked the argument list into
the body.
- Multi-line returns using `[` or `{` (rather than `(`) leaked their
contents into the cell.
- Cross-cell parser state could corrupt a following cell.
- `with app.setup(...)` blocks and top-level `@app.function`
definitions were not extracted at all.
Because the parser models block structure directly, nested-function
returns, decorators, `async def`, and multi-line returns of any bracket
type are all handled structurally rather than by ad-hoc string matching.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
for more information, see https://pre-commit.ci
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the “paste a marimo app into a cell” workflow by replacing fragile line-based parsing with Lezer’s Python parser, enabling structurally-correct extraction of pasted marimo source (including setup blocks and @app.function).
Changes:
- Replace heuristic cell extraction with syntax-tree-based parsing via
@lezer/python, adding support forwith app.setup...and@app.function. - Route extracted setup code into the notebook’s dedicated setup cell (create it or append to it).
- Expand test coverage for previously-mangled edge cases (nested decorators, async defs, multiline returns, setup/function extraction).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
frontend/src/core/codemirror/misc/paste.ts |
Switches pasted-app parsing to Lezer AST; extracts setup blocks, cells, and @app.function definitions. |
frontend/src/core/codemirror/misc/__tests__/paste.test.ts |
Adds regression tests for parsing edge cases and setup-cell routing behavior. |
frontend/src/core/codemirror/cells/state.ts |
Extends CodemirrorCellActions with addOrAppendSetupCell. |
frontend/src/components/editor/cell/code/cell-editor.tsx |
Implements addOrAppendSetupCell by creating/appending to the setup cell and syncing editor/state. |
Contributor
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/core/codemirror/misc/paste.ts">
<violation number="1" location="frontend/src/core/codemirror/misc/paste.ts:123">
P2: `return false` is unconditional here, so *every* `WithStatement` (not just `app.setup`) stops tree descent into its children. If a decorated `@app.cell` or `@app.function` is nested inside a non-setup `with` block, it will be silently missed. Consider only returning `false` when the node is actually an `app.setup` block, and continuing traversal otherwise. Also, the `.includes("app.setup")` check could match unrelated identifiers (e.g. `my_app.setup_data`) — use a word-boundary–aware check consistent with `looksLikeMarimoApp`'s `\bapp\.setup\b` pattern.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| setupBlocks.push(setup); | ||
| } | ||
| } | ||
| return false; |
Contributor
There was a problem hiding this comment.
P2: return false is unconditional here, so every WithStatement (not just app.setup) stops tree descent into its children. If a decorated @app.cell or @app.function is nested inside a non-setup with block, it will be silently missed. Consider only returning false when the node is actually an app.setup block, and continuing traversal otherwise. Also, the .includes("app.setup") check could match unrelated identifiers (e.g. my_app.setup_data) — use a word-boundary–aware check consistent with looksLikeMarimoApp's \bapp\.setup\b pattern.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/core/codemirror/misc/paste.ts, line 123:
<comment>`return false` is unconditional here, so *every* `WithStatement` (not just `app.setup`) stops tree descent into its children. If a decorated `@app.cell` or `@app.function` is nested inside a non-setup `with` block, it will be silently missed. Consider only returning `false` when the node is actually an `app.setup` block, and continuing traversal otherwise. Also, the `.includes("app.setup")` check could match unrelated identifiers (e.g. `my_app.setup_data`) — use a word-boundary–aware check consistent with `looksLikeMarimoApp`'s `\bapp\.setup\b` pattern.</comment>
<file context>
@@ -1,159 +1,175 @@
+ setupBlocks.push(setup);
+ }
+ }
+ return false;
}
- skipLines--;
</file context>
mscolnick
approved these changes
Jul 1, 2026
Contributor
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.23.13-dev1 |
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.
This PR replaces the line-based heuristics with lezer's Python parser, adding support for setup cells and app.function's in the process.
Previously, pasting a marimo notebook into a cell used line-based heuristics to strip the file boilerplate, which mangled several valid constructs. Because the parser models block structure directly, nested-function returns, decorators,
async def, and multi-line returns of any bracket type are all handled structurally rather than by ad-hoc string matching.This feature remains not very discoverable, but I do really like it ...