Use ruff dependency graph for nested autoreloads #7069

mthiboust · 2025-11-04T17:47:35Z

📝 Summary

This PR is a proof-of-concept to make autoreload more robust by ensuring that when a module changes, all modules that depend on it (even indirectly) are also reloaded. This is achieved by leveraging Ruff’s static analysis, which provides a dependency graph. A positive side effect is that ruff analyze graph supports both absolute and relative imports.

As explained in the issue, ruff analyze graph is not working as expected for third-party packages in site-packages. Only current project and packages installed in editable mode are supported for automatic reload on file updates.

Got inspiration from this blog post: https://vincerose.dev/posts/ruff-test-filtering/

🔍 Description of Changes

The code could have been more straightforward but I chose to keep the existing code structure.

If you consider going further, we should decide whether to completely replace the existing, keep the two and use this new implementation as the default one, with the former one as a fallback?

I don't know how to unit test this feature.

📋 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-11-04T17:47:42Z

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

Project	Deployment	Preview	Comments	Updated (UTC)
marimo-docs	Ready	Preview	Comment	Nov 4, 2025 5:48pm

github-actions · 2025-11-04T17:47:49Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

mthiboust · 2025-11-04T17:48:16Z

I have read the CLA Document and I hereby sign the CLA

mthiboust · 2025-11-04T17:51:43Z

marimo/_runtime/reload/module_watcher.py

                        modname_to_cell_id[modname]
                        for modname in stale_modules
                    ),
-                    relatives=dataflow.import_block_relatives,
                )


Without this change, the root cell was successfully reloaded with all necessary modules (even the nested ones) but the dependent cells were not automatically rerun.

Hmm, this may break tests (which is good, will remind us what the intended behavior is)

This is the part I understand the less.

mthiboust · 2025-11-04T17:54:03Z

marimo/_runtime/reload/autoreload.py

+        modules_copy = modules.copy()
+        modules_paths = {
+            get_full_path(v.__file__): v
+            for k, v in modules_copy.items()
+            if hasattr(v, "__file__") and v.__file__ is not None
+        }


Using ruff graph, it is better to use the full absolute path of the module as the dict key (ruff gives module paths, not module names)

Can you leave this as a comment?

mthiboust · 2025-11-04T17:55:08Z

marimo/_runtime/reload/autoreload.py

+def get_full_path(p: str) -> str:
+    p_path = pathlib.Path(p)
+    if p_path.is_absolute():
+        return str(p_path.resolve())
+    else:
+        return str((pathlib.Path.cwd() / p_path).resolve())


Paths returned by ruff are relative paths relative to the current directory where the marimo edit command is run.

mthiboust · 2025-11-04T17:55:28Z

marimo/_runtime/reload/autoreload.py

+def get_downstream_dependents(
+    dependents_map: dict[str, list[str]], changed_files: list[str]
+) -> set[str]:
+    visited = set()
+    queue = deque(changed_files)
+
+    while queue:
+        current = queue.popleft()
+        if current not in visited:
+            visited.add(current)
+            for neighbor in dependents_map.get(current, []):
+                queue.append(neighbor)
+
+    return visited


Credit to https://vincerose.dev/posts/ruff-test-filtering/

dmadisetti

Thanks! First glance looks mostly correct. Missing tests are the most outstanding issue- would you be willing to give them a pass?

Also, I set off our test suite, I'd bet something will fail as is 🤔

dmadisetti · 2025-11-04T18:23:50Z

marimo/_runtime/reload/autoreload.py

+                LOGGER.debug(
+                    f"Reloading '{stale_modname}' and its downstream dependents {downstream_dependents}."
+                )
+                for module in [m] + downstream_dependents_modules:


Chance of loading the same module multiple times (if a->b c->b and a, c, then b is called twice

I think collecting all outside of the loop, and then just traversing once makes the most sense

marimo/_runtime/reload/module_watcher.py

dmadisetti · 2025-11-04T18:28:33Z

marimo/_runtime/reload/module_watcher.py

+
+    # Not sure how this can happen, but guard against it
+    if not (
+        hasattr(src_module, "__file__") and src_module.__file__ is not None


src_file = getattr(src_module, '__file__', None)

dmadisetti · 2025-11-04T18:29:10Z

marimo/_runtime/reload/module_watcher.py

+        else:
+            return False
+
+    # Legacy way


No point keeping dead code

I didn't know if this new implementation would completely replace the current one, or if users could switch from one to the other via settings

dmadisetti · 2025-11-04T18:30:43Z

marimo/_runtime/reload/module_watcher.py

+
+    if GRAPH_MODE:
+        graph = reloader.get_module_dependency_graph(src_module)
+        if src_module_filename in get_downstream_dependents(


is_dependent = any(...) return is_dependent

dmadisetti · 2025-11-04T18:32:11Z

marimo/_runtime/reload/module_watcher.py

                        modname_to_cell_id[modname]
                        for modname in stale_modules
                    ),
-                    relatives=dataflow.import_block_relatives,
                )


Hmm, this may break tests (which is good, will remind us what the intended behavior is)

mscolnick · 2025-11-04T20:11:00Z

hey @mthiboust this may have a bit of churn on the PR/review. it might be easier to make the more incremental fixes/changes (like the fallback mentioned in this comment)

mthiboust · 2025-11-05T10:34:22Z

hey @mthiboust this may have a bit of churn on the PR/review. it might be easier to make the more incremental fixes/changes (like the fallback mentioned in this comment)

This PR was more a proof-of-concept for now. I clarified the current status of the issue in this recent comment. In short, I don't think that the discussed fallback for relative imports will be sufficient to be useful if we are still lacking support for nested imports.

And implementing support for nested imports via ruff will avoid the relative import issue anyway because ruff analyze graph already supports both absolute and relative imports.

Use ruff dep graph for nested autoreloads

399d49f

mthiboust requested a review from dmadisetti as a code owner November 4, 2025 17:47

vercel bot deployed to Preview November 4, 2025 17:48 View deployment

mthiboust commented Nov 4, 2025

View reviewed changes

dmadisetti requested changes Nov 4, 2025

View reviewed changes

Use ruff dependency graph for nested autoreloads #7069

Are you sure you want to change the base?

Use ruff dependency graph for nested autoreloads #7069

Conversation

mthiboust commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Summary

🔍 Description of Changes

📋 Checklist

Uh oh!

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

Uh oh!

Uh oh!

github-actions bot commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mthiboust commented Nov 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mthiboust Nov 4, 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!

Choose a reason for hiding this comment

Uh oh!

mthiboust Nov 4, 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!

Choose a reason for hiding this comment

Uh oh!

dmadisetti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mscolnick commented Nov 4, 2025

Uh oh!

mthiboust commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mthiboust commented Nov 4, 2025 •

edited

Loading

vercel bot commented Nov 4, 2025 •

edited

Loading

github-actions bot commented Nov 4, 2025 •

edited

Loading

mthiboust Nov 4, 2025 •

edited

Loading

mthiboust Nov 4, 2025 •

edited

Loading

mthiboust commented Nov 5, 2025 •

edited

Loading