Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (5)
📝 WalkthroughWalkthroughFive small edits across Marimo: a CSS rule hides the "Back to gallery" anchor; runtime TOML adds markdown and ipynb to auto-downloads and enables serving cached sessions; dev template adds Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
template/assets/dataset_marimo/pyproject.toml.hbs (1)
12-12: Consider pinning versions for dev dependencies.Adding
ruffandnbconvertto dev tooling is appropriate—ruffprovides modern linting/formatting, andnbconvertsupports the new ipynb functionality. However, unpinned dev dependencies may lead to inconsistent development environments across team members or over time.📌 Optional: Add minimum version constraints
-dev = ["ty", "watchdog", "ruff", "nbconvert"] +dev = ["ty", "watchdog", "ruff>=0.1.0", "nbconvert>=7.0.0"]Note: Adjust version numbers based on your minimum requirements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/assets/dataset_marimo/pyproject.toml.hbs` at line 12, The dev dependency list (dev = ["ty", "watchdog", "ruff", "nbconvert"]) should include pinned or minimum version constraints to ensure reproducible dev environments; update the template line that defines the dev array to replace each package entry with a versioned specifier (e.g., "ruff>=x.y.z" or "nbconvert==x.y.z") according to your minimum supported versions, and ensure the template generator preserves those constraints when rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/kubimo/marimo/marimo.toml`:
- Around line 30-35: The config includes an unknown option
serve_cached_sessions_in_apps; verify whether the Marimo runtime version
actually supports this key or remove it: check Marimo docs or code for
serve_cached_sessions_in_apps and if unsupported, delete that entry from
marimo.toml (leaving default_auto_download, on_cell_change, auto_instantiate,
default_sql_output, dotenv intact) or replace it with the correct supported
setting.
---
Nitpick comments:
In `@template/assets/dataset_marimo/pyproject.toml.hbs`:
- Line 12: The dev dependency list (dev = ["ty", "watchdog", "ruff",
"nbconvert"]) should include pinned or minimum version constraints to ensure
reproducible dev environments; update the template line that defines the dev
array to replace each package entry with a versioned specifier (e.g.,
"ruff>=x.y.z" or "nbconvert==x.y.z") according to your minimum supported
versions, and ensure the template generator preserves those constraints when
rendering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 37dbd830-2eef-4858-984b-cf8cb9d77a77
📒 Files selected for processing (3)
docker/kubimo/marimo/marimo.cssdocker/kubimo/marimo/marimo.tomltemplate/assets/dataset_marimo/pyproject.toml.hbs
| default_auto_download = ["html", "markdown", "ipynb"] | ||
| on_cell_change = "autorun" | ||
| auto_instantiate = false | ||
| default_sql_output = "auto" | ||
| dotenv = [".env"] | ||
| serve_cached_sessions_in_apps = true |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What configuration options are available in marimo runtime settings, specifically default_auto_download and serve_cached_sessions_in_apps?
💡 Result:
Marimo runtime settings are configured via the UI settings menu, .marimo.toml user config file, pyproject.toml overrides under [tool.marimo.runtime], or script metadata. Known runtime options include on_cell_change (autorun/lazy), auto_reload, auto_instantiate, pythonpath (list of dirs), dotenv (list of .env files), and default_sql_output. default_auto_download is an app-specific configuration option (moved from UI to per-notebook app config in Sep 2024 via PR #2309), likely controlling automatic HTML/IPYNB exports/downloads. It appears in user configs as default_auto_download = [] (possibly list of formats like ["html"]). No sources explicitly document or confirm serve_cached_sessions_in_apps as a runtime setting; it may not exist or be undocumented/internal.
Citations:
- 1: https://docs.marimo.io/guides/configuration/runtime_configuration/
- 2: https://docs.marimo.io/guides/configuration/
- 3: improvement: move auto-download to app config marimo-team/marimo#2309
- 4: https://gist.github.com/pythoninthegrass/1e3c6823e353c2d4b2555c7ea303f933
- 5: https://docs.marimo.io/guides/configuration
- 6: "marimo config show" wording marimo-team/marimo#2600
- 7: feat: override configuration with pyproject.toml marimo-team/marimo#3071
- 8: Add documentation for how to set up the
.marimo.tomlfile marimo-team/marimo#2922 - 9: https://www.marimo.io
- 10: https://www.marimoeng.com
Verify or remove the serve_cached_sessions_in_apps configuration option.
The serve_cached_sessions_in_apps option cannot be confirmed in marimo's official runtime configuration documentation and may not be a valid setting. Verify this option exists in the target marimo version or remove it. The other options (default_auto_download, on_cell_change, auto_instantiate, default_sql_output, dotenv) are documented and valid.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker/kubimo/marimo/marimo.toml` around lines 30 - 35, The config includes
an unknown option serve_cached_sessions_in_apps; verify whether the Marimo
runtime version actually supports this key or remove it: check Marimo docs or
code for serve_cached_sessions_in_apps and if unsupported, delete that entry
from marimo.toml (leaving default_auto_download, on_cell_change,
auto_instantiate, default_sql_output, dotenv intact) or replace it with the
correct supported setting.
jpopesculian
force-pushed
the
jpop/upgrade-marimo
branch
from
8d50bb3 to
9cbb892
Compare
1 participant
requires aqora-io/kubimo#15 and the version in the docker image to be updated after merging and releasing
Summary by CodeRabbit
UI Updates
Improvements
Chores