fix: recursive private functions break due to reference order - #10678
fix: recursive private functions break due to reference order#10678dmadisetti wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
All contributors have signed the CLA ✍️ ✅ |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
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="marimo/_ast/visitor.py">
<violation number="1" location="marimo/_ast/visitor.py:606">
P1: When a function is declared directly in a class, this line marks the method name as defined in the class block, hiding unqualified references to that name inside the method. Python method bodies do not resolve bare names through the class namespace, so preserve the reference unless the function is nested in a lexical function scope rather than directly in a class.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| # a nested private function's self-references are not mistaken | ||
| # for cell-local references. Variable metadata is attached after | ||
| # the body has been visited and its references are known. | ||
| self.block_stack[-1].defs.add(node.name) |
There was a problem hiding this comment.
P1: When a function is declared directly in a class, this line marks the method name as defined in the class block, hiding unqualified references to that name inside the method. Python method bodies do not resolve bare names through the class namespace, so preserve the reference unless the function is nested in a lexical function scope rather than directly in a class.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At marimo/_ast/visitor.py, line 606:
<comment>When a function is declared directly in a class, this line marks the method name as defined in the class block, hiding unqualified references to that name inside the method. Python method bodies do not resolve bare names through the class namespace, so preserve the reference unless the function is nested in a lexical function scope rather than directly in a class.</comment>
<file context>
@@ -597,6 +597,13 @@ def _visit_and_get_refs(
+ # a nested private function's self-references are not mistaken
+ # for cell-local references. Variable metadata is attached after
+ # the body has been visited and its references are known.
+ self.block_stack[-1].defs.add(node.name)
self.generic_visit(node)
refs = self.ref_stack.pop()
</file context>
There was a problem hiding this comment.
Pull request overview
Fixes a scoping/mangling edge case where nested underscore-prefixed recursive functions could be treated as unresolved “cell-local” references during AST traversal, leading to runtime NameError (issue #10675).
Changes:
- Update the AST visitor to register a function’s binding in the enclosing scope so nested self-references aren’t misclassified.
- Add runtime regression tests covering nested private recursion and a nested-private-in-public wrapper case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
marimo/_ast/visitor.py |
Adjusts scope-definition tracking for function defs to avoid mangling nested private recursive self-references incorrectly. |
tests/_runtime/test_runtime.py |
Adds regression coverage for nested recursive functions with underscore-prefixed names. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Process the function body | ||
| if isinstance(node, (ast.AsyncFunctionDef, ast.FunctionDef)): | ||
| # A function body can refer to the function's binding in the | ||
| # enclosing scope. Register the name before visiting the body so | ||
| # a nested private function's self-references are not mistaken | ||
| # for cell-local references. Variable metadata is attached after | ||
| # the body has been visited and its references are known. | ||
| self.block_stack[-1].defs.add(node.name) | ||
| self.generic_visit(node) |
📝 Summary
Closes #10675
During visitor traversal scoped functions can be referenced before their definition is complete (in recursion), causing missing variable errors. Our fix is to add the name reference to the block above such that recursive functions can pick up the reference.