chore: merge main into dm/micropip-streaming - #10529
Closed
dmadisetti wants to merge 9 commits into
Closed
Conversation
for more information, see https://pre-commit.ci
- Track requested packages by parsed PEP 508 base name (via packaging.requirements.Requirement), so versioned / URL specs like `foo==1.0` and `foo @ git+…@ref` correctly match the wheel names micropip yields back instead of getting reported as failed. - Replace `# type: ignore` micropip imports with importlib.import_module so the module loads cleanly outside Pyodide and mypy doesn't need to see the optional dep at all. - Isolate loadPackage failures so one pyodide-batch error yields per-package False rather than terminating the generator. - Fallback path now retries only packages the engine didn't yield, avoiding double-yield + (pkg, False) reports for packages that already succeeded. - Defensive isinstance(url, str) checks when reading PEP 723 index config from the notebook (guards malformed pyproject entries). - Regenerate packages/openapi/api.yaml for the new index_urls field.
…sts) The WASM streaming PR batched all packages into a single addPackage call but then looped, emitting one toast per package keyed on a single aggregate success/error. PackageOperationResponse only carries an aggregate success+error (no per-package detail), so this implied a per-package outcome that doesn't exist: one failure showed N error toasts, one success showed N success toasts even if some silently failed. Show a single aggregate toast covering all requested packages instead, with truthful plural/singular wording. Batching behavior is preserved. Also adds tests for MicropipPackageManager.stream_install covering the fallback path: when the engine raises after partial yields, already-yielded packages are not re-installed and each package is yielded exactly once.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
Contributor
Coverage Report for ./frontend
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
9 issues found across 12 files
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="tests/_runtime/packages/test_micropip_streaming.py">
<violation number="1" location="tests/_runtime/packages/test_micropip_streaming.py:444">
P3: test_loadpackage_failure_yields_false_no_terminate repeats the fake-micropip module setup (constructing micropip, micropip._utils, micropip.transaction modules and monkeypatching them into sys.modules) that `_install_fake_micropip` already encapsulates, differing only in the custom compat layer. This duplicates ~20 lines of boilerplate that can drift from the helper. Give `_install_fake_micropip` an optional `compat_layer` parameter and reuse it here so the fake installation lives in one place.</violation>
</file>
<file name="marimo/_runtime/packages/_micropip_streaming.py">
<violation number="1" location="marimo/_runtime/packages/_micropip_streaming.py:18">
P3: `_append_version` and `_split_packages` are verbatim duplicates of `append_version` and `split_packages` in `marimo/_runtime/packages/utils.py`. Because the module intentionally avoids marimo imports, the copies are already the single source of this logic in two places, so they can silently drift (for example a fix to the marker/editable handling in one will miss the other). Prefer moving these pure helpers into a marimo-agnostic module and re-exporting from `utils.py` so both consumers share one implementation.</violation>
<violation number="2" location="marimo/_runtime/packages/_micropip_streaming.py:54">
P1: Compound PEP 508 markers are split into invalid requirements here, so installs such as `foo; python_version > '3.6' and os_name == 'posix'` fail during resolution. Keep the marker together until the complete marker expression has been consumed.</violation>
<violation number="3" location="marimo/_runtime/packages/_micropip_streaming.py:147">
P2: When a requested package fails because one of its transitive dependencies is unresolvable, `transaction.failed` typically contains both the root and the transitive name. This loop does not filter `failed_name` by whether it is in `requested`, so it yields `(failed_name, False)` for dependency names the caller never asked to install. `MicropipPackageManager.stream_install` forwards every yielded `(pkg, success)` to the caller, and `PackagesCallbacks.install_missing_packages` treats each yielded name as a real requested package: it sets `package_statuses[transitive] = "failed"` and calls `package_to_module(transitive)` and `module_registry.excluded_modules.add(...)` on a module the user never requested, spamming the UI and excluding an unrelated module.</violation>
<violation number="4" location="marimo/_runtime/packages/_micropip_streaming.py:150">
P2: A valid non-applicable environment marker is reported as an installation failure when the package is not already installed. Treat requirements whose markers evaluate false as successful skips before the metadata reconciliation pass.</violation>
</file>
<file name="packages/openapi/src/api.ts">
<violation number="1" location="packages/openapi/src/api.ts:5174">
P2: The public install-packages HTTP endpoint now advertises `indexUrls` on `InstallPackagesRequest`, but the server silently drops it: `InstallPackagesRequest.as_command()` (marimo/_server/models/models.py) builds `InstallPackagesCommand(manager=..., versions=..., source=...)` without forwarding `index_urls`. Clients that send `indexUrls` (the whole point of this field) find them ignored on the HTTP path, so the new API contract is non-functional. Forward `index_urls=self.index_urls` in `as_command()` so the kernel install honors the passed index URLs.</violation>
</file>
<file name="marimo/_runtime/callbacks/packages.py">
<violation number="1" location="marimo/_runtime/callbacks/packages.py:314">
P1: When micropip resolution raises an installation error instead of yielding a result, this `stream_install` loop lets the exception escape. The old path converted `ValueError` failures into a failed package status; catch resolution failures or make the streaming manager yield failures so the callback completes and reports the affected packages.</violation>
</file>
<file name="frontend/src/core/packages/toast-components.tsx">
<violation number="1" location="frontend/src/core/packages/toast-components.tsx:28">
P3: Using the package name as the React key breaks when the same package appears more than once in the installed list. Callers such as `InstallPackageButton` pass arbitrary arrays of package names that can contain duplicates, so a duplicate would trigger a React "Encountered two children with the same key" warning and cause spans to render incorrectly (one entry dropped/mis-ordered). Key by index instead, since these package names are not guaranteed unique in the array.</violation>
</file>
<file name="marimo/_runtime/packages/pypi_package_manager.py">
<violation number="1" location="marimo/_runtime/packages/pypi_package_manager.py:277">
P2: When `stream_install` receives a package already in `_attempted_packages`, this override bypasses the base guard and submits it to micropip again. Filter attempted packages before batching and preserve the base result for them.</violation>
</file>
Architecture diagram
sequenceDiagram
participant UI as Package Install UI
participant Hook as useInstallPackages Hook
participant Toast as Toast Components
participant API as Kernel API
participant Callbacks as Packages Callbacks
participant PM as Package Manager
participant MuPip as Micropip Streaming Engine
participant Transaction as Micropip Transaction API
participant Notebook as Notebook Metadata
Note over UI,Notebook: Package Installation Flow with Streaming Support
UI->>Hook: handleInstallPackages(["numpy", "pandas"])
Hook->>Hook: Batch packages into single string "numpy pandas"
Hook->>API: addPackage({ package: "numpy pandas" })
API->>Callbacks: InstallPackagesCommand(versions, index_urls)
Note over Callbacks,PM: Server-side package resolution
Callbacks->>PM: stream_install(packages, versions, index_urls)
PM->>MuPip: stream_transaction_install(packages, versions, index_urls)
MuPip->>Transaction: gather_requirements(["numpy", "pandas"])
Transaction-->>MuPip: Resolved wheels + pyodide packages
alt Fresh packages (not attempted)
MuPip->>Transaction: Install wheels in parallel
Note over MuPip,Transaction: asyncio.as_completed for streaming yield
Transaction-->>MuPip: Yields (pkg_name, success) as each completes
MuPip-->>PM: Streams (pkg, success) results
PM->>Callbacks: Yield (pkg, success) one at a time
Callbacks->>Callbacks: Update package status to "installing"/"installed"/"failed"
Callbacks->>Callbacks: Broadcast InstallingPackageAlertNotification with logs
else Already attempted packages
PM->>Callbacks: Yield (pkg, false) immediately
end
Note over PM,MuPip: Fallback handling for API changes
alt Micropip Transaction API unavailable (AttributeError/ImportError/TypeError)
MuPip-->>PM: Throw API error
PM->>PM: Fall back to base sequential stream_install
PM->>Transaction: Install packages one at a time via install()
end
Callbacks-->>API: Aggregate success/error result
Hook-->>UI: Response (single aggregate result)
alt Success (all packages in batch)
Hook->>Toast: showAddPackageToast(["numpy", "pandas"])
Toast->>Toast: Display "Packages added" (plural) toast
else Failure (any package failed)
Hook->>Toast: showAddPackageToast(["numpy", "pandas"], error)
Toast->>Toast: Display "Failed to add packages" error toast
end
Note over Notebook,Callbacks: Index URL resolution
alt Notebook file exists with PEP 723 config
Callbacks->>Notebook: _notebook_index_urls()
Notebook->>Notebook: Read index-url, extra-index-url, [[tool.uv.index]] entries
Notebook-->>Callbacks: Primary index URL + extras (deduplicated)
Callbacks->>PM: Pass index_urls to stream_install
PM->>MuPip: Override default index URLs
MuPip->>Transaction: Use custom index URLs (falls back to singleton if none)
end
opt Pyodide packages detected
MuPip->>Transaction: loadPackage(pyodide_packages)
Transaction-->>MuPip: Load native distribution
MuPip-->>PM: Yield (pkg, success) for each loaded package
end
Note over MuPip: Reconciliation fallback
opt Package resolved transitively under different name
MuPip->>MuPip: Check importlib.metadata.version(base_name)
MuPip-->>PM: Yield (original_spec, true) if importable
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| current.append(part) | ||
| elif in_marker: | ||
| current.append(part) | ||
| if part.endswith(("'", '"')): |
Contributor
There was a problem hiding this comment.
P1: Compound PEP 508 markers are split into invalid requirements here, so installs such as foo; python_version > '3.6' and os_name == 'posix' fail during resolution. Keep the marker together until the complete marker expression has been consumed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At marimo/_runtime/packages/_micropip_streaming.py, line 54:
<comment>Compound PEP 508 markers are split into invalid requirements here, so installs such as `foo; python_version > '3.6' and os_name == 'posix'` fail during resolution. Keep the marker together until the complete marker expression has been consumed.</comment>
<file context>
@@ -0,0 +1,196 @@
+ current.append(part)
+ elif in_marker:
+ current.append(part)
+ if part.endswith(("'", '"')):
+ in_marker = False
+ packages.append(" ".join(current))
</file context>
| versions: dict[str, str | None] = { | ||
| pkg: request.versions.get(pkg) for pkg in installable | ||
| } | ||
| async for pkg, success in self.package_manager.stream_install( |
Contributor
There was a problem hiding this comment.
P1: When micropip resolution raises an installation error instead of yielding a result, this stream_install loop lets the exception escape. The old path converted ValueError failures into a failed package status; catch resolution failures or make the streaming manager yield failures so the callback completes and reports the affected packages.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At marimo/_runtime/callbacks/packages.py, line 314:
<comment>When micropip resolution raises an installation error instead of yielding a result, this `stream_install` loop lets the exception escape. The old path converted `ValueError` failures into a failed package status; catch resolution failures or make the streaming manager yield failures so the callback completes and reports the affected packages.</comment>
<file context>
@@ -249,34 +282,43 @@ def log_callback(log_line: str) -> None:
+ versions: dict[str, str | None] = {
+ pkg: request.versions.get(pkg) for pkg in installable
+ }
+ async for pkg, success in self.package_manager.stream_install(
+ installable,
+ versions=versions,
</file context>
| for failed_name in transaction.failed: | ||
| normalized = canonicalize_name(failed_name) | ||
| original, _ = requested.pop(normalized, (failed_name, failed_name)) | ||
| yield (original, False) |
Contributor
There was a problem hiding this comment.
P2: A valid non-applicable environment marker is reported as an installation failure when the package is not already installed. Treat requirements whose markers evaluate false as successful skips before the metadata reconciliation pass.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At marimo/_runtime/packages/_micropip_streaming.py, line 150:
<comment>A valid non-applicable environment marker is reported as an installation failure when the package is not already installed. Treat requirements whose markers evaluate false as successful skips before the metadata reconciliation pass.</comment>
<file context>
@@ -0,0 +1,196 @@
+ for failed_name in transaction.failed:
+ normalized = canonicalize_name(failed_name)
+ original, _ = requested.pop(normalized, (failed_name, failed_name))
+ yield (original, False)
+
+ async def _install_wheel(wheel: Any) -> tuple[str, Exception | None]:
</file context>
| */ | ||
| InstallPackagesCommand: { | ||
| /** @default [] */ | ||
| indexUrls?: string[]; |
Contributor
There was a problem hiding this comment.
P2: The public install-packages HTTP endpoint now advertises indexUrls on InstallPackagesRequest, but the server silently drops it: InstallPackagesRequest.as_command() (marimo/_server/models/models.py) builds InstallPackagesCommand(manager=..., versions=..., source=...) without forwarding index_urls. Clients that send indexUrls (the whole point of this field) find them ignored on the HTTP path, so the new API contract is non-functional. Forward index_urls=self.index_urls in as_command() so the kernel install honors the passed index URLs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/openapi/src/api.ts, line 5174:
<comment>The public install-packages HTTP endpoint now advertises `indexUrls` on `InstallPackagesRequest`, but the server silently drops it: `InstallPackagesRequest.as_command()` (marimo/_server/models/models.py) builds `InstallPackagesCommand(manager=..., versions=..., source=...)` without forwarding `index_urls`. Clients that send `indexUrls` (the whole point of this field) find them ignored on the HTTP path, so the new API contract is non-functional. Forward `index_urls=self.index_urls` in `as_command()` so the kernel install honors the passed index URLs.</comment>
<file context>
@@ -5161,12 +5161,17 @@ export interface components {
*/
InstallPackagesCommand: {
+ /** @default [] */
+ indexUrls?: string[];
manager: string;
/**
</file context>
|
|
||
| yielded: set[str] = set() | ||
| try: | ||
| async for pkg, success in stream_transaction_install( |
Contributor
There was a problem hiding this comment.
P2: When stream_install receives a package already in _attempted_packages, this override bypasses the base guard and submits it to micropip again. Filter attempted packages before batching and preserve the base result for them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At marimo/_runtime/packages/pypi_package_manager.py, line 277:
<comment>When `stream_install` receives a package already in `_attempted_packages`, this override bypasses the base guard and submits it to micropip again. Filter attempted packages before batching and preserve the base result for them.</comment>
<file context>
@@ -245,6 +252,63 @@ async def _install(
+
+ yielded: set[str] = set()
+ try:
+ async for pkg, success in stream_transaction_install(
+ packages,
+ versions=versions,
</file context>
Comment on lines
+147
to
+148
| for failed_name in transaction.failed: | ||
| normalized = canonicalize_name(failed_name) |
Contributor
There was a problem hiding this comment.
P2: When a requested package fails because one of its transitive dependencies is unresolvable, transaction.failed typically contains both the root and the transitive name. This loop does not filter failed_name by whether it is in requested, so it yields (failed_name, False) for dependency names the caller never asked to install. MicropipPackageManager.stream_install forwards every yielded (pkg, success) to the caller, and PackagesCallbacks.install_missing_packages treats each yielded name as a real requested package: it sets package_statuses[transitive] = "failed" and calls package_to_module(transitive) and module_registry.excluded_modules.add(...) on a module the user never requested, spamming the UI and excluding an unrelated module.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At marimo/_runtime/packages/_micropip_streaming.py, line 147:
<comment>When a requested package fails because one of its transitive dependencies is unresolvable, `transaction.failed` typically contains both the root and the transitive name. This loop does not filter `failed_name` by whether it is in `requested`, so it yields `(failed_name, False)` for dependency names the caller never asked to install. `MicropipPackageManager.stream_install` forwards every yielded `(pkg, success)` to the caller, and `PackagesCallbacks.install_missing_packages` treats each yielded name as a real requested package: it sets `package_statuses[transitive] = "failed"` and calls `package_to_module(transitive)` and `module_registry.excluded_modules.add(...)` on a module the user never requested, spamming the UI and excluding an unrelated module.</comment>
<file context>
@@ -0,0 +1,196 @@
+ base_name = spec
+ requested[canonicalize_name(base_name)] = (spec, base_name)
+
+ for failed_name in transaction.failed:
+ normalized = canonicalize_name(failed_name)
+ original, _ = requested.pop(normalized, (failed_name, failed_name))
</file context>
Suggested change
| for failed_name in transaction.failed: | |
| normalized = canonicalize_name(failed_name) | |
| for failed_name in transaction.failed: | |
| normalized = canonicalize_name(failed_name) | |
| if normalized not in requested: | |
| # Only surface failures for packages the caller actually requested; | |
| # transitive dependencies resolve under the root's failed entry. | |
| continue | |
| original, _ = requested.pop(normalized) | |
| yield (original, False) |
| del names | ||
| raise RuntimeError("pyodide load failed") | ||
|
|
||
| fake_mgr = _FakeMicropipManager(compat_layer=_BadCompatLayer()) |
Contributor
There was a problem hiding this comment.
P3: test_loadpackage_failure_yields_false_no_terminate repeats the fake-micropip module setup (constructing micropip, micropip._utils, micropip.transaction modules and monkeypatching them into sys.modules) that _install_fake_micropip already encapsulates, differing only in the custom compat layer. This duplicates ~20 lines of boilerplate that can drift from the helper. Give _install_fake_micropip an optional compat_layer parameter and reuse it here so the fake installation lives in one place.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/_runtime/packages/test_micropip_streaming.py, line 444:
<comment>test_loadpackage_failure_yields_false_no_terminate repeats the fake-micropip module setup (constructing micropip, micropip._utils, micropip.transaction modules and monkeypatching them into sys.modules) that `_install_fake_micropip` already encapsulates, differing only in the custom compat layer. This duplicates ~20 lines of boilerplate that can drift from the helper. Give `_install_fake_micropip` an optional `compat_layer` parameter and reuse it here so the fake installation lives in one place.</comment>
<file context>
@@ -0,0 +1,469 @@
+ del names
+ raise RuntimeError("pyodide load failed")
+
+ fake_mgr = _FakeMicropipManager(compat_layer=_BadCompatLayer())
+ tx = _FakeTransaction(
+ wheels=[_FakeWheel("foo")],
</file context>
| dependencies has been added to your environment. | ||
| {packageNames.length > 1 ? "The packages " : "The package "} | ||
| {packageNames.map((name, index) => ( | ||
| <span key={name}> |
Contributor
There was a problem hiding this comment.
P3: Using the package name as the React key breaks when the same package appears more than once in the installed list. Callers such as InstallPackageButton pass arbitrary arrays of package names that can contain duplicates, so a duplicate would trigger a React "Encountered two children with the same key" warning and cause spans to render incorrectly (one entry dropped/mis-ordered). Key by index instead, since these package names are not guaranteed unique in the array.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/core/packages/toast-components.tsx, line 28:
<comment>Using the package name as the React key breaks when the same package appears more than once in the installed list. Callers such as `InstallPackageButton` pass arbitrary arrays of package names that can contain duplicates, so a duplicate would trigger a React "Encountered two children with the same key" warning and cause spans to render incorrectly (one entry dropped/mis-ordered). Key by index instead, since these package names are not guaranteed unique in the array.</comment>
<file context>
@@ -4,23 +4,34 @@ import { Kbd } from "@/components/ui/kbd";
- dependencies has been added to your environment.
+ {packageNames.length > 1 ? "The packages " : "The package "}
+ {packageNames.map((name, index) => (
+ <span key={name}>
+ {index > 0 && ", "}
+ <Kbd className="inline">{name}</Kbd>
</file context>
Suggested change
| <span key={name}> | |
| <span key={`${name}-${index}`}> |
| from collections.abc import AsyncIterator | ||
|
|
||
|
|
||
| def _append_version(pkg_name: str, version: str | None) -> str: |
Contributor
There was a problem hiding this comment.
P3: _append_version and _split_packages are verbatim duplicates of append_version and split_packages in marimo/_runtime/packages/utils.py. Because the module intentionally avoids marimo imports, the copies are already the single source of this logic in two places, so they can silently drift (for example a fix to the marker/editable handling in one will miss the other). Prefer moving these pure helpers into a marimo-agnostic module and re-exporting from utils.py so both consumers share one implementation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At marimo/_runtime/packages/_micropip_streaming.py, line 18:
<comment>`_append_version` and `_split_packages` are verbatim duplicates of `append_version` and `split_packages` in `marimo/_runtime/packages/utils.py`. Because the module intentionally avoids marimo imports, the copies are already the single source of this logic in two places, so they can silently drift (for example a fix to the marker/editable handling in one will miss the other). Prefer moving these pure helpers into a marimo-agnostic module and re-exporting from `utils.py` so both consumers share one implementation.</comment>
<file context>
@@ -0,0 +1,196 @@
+ from collections.abc import AsyncIterator
+
+
+def _append_version(pkg_name: str, version: str | None) -> str:
+ """Qualify a version string with a leading '==' if it doesn't have one."""
+ if version is None or version in ("", "latest"):
</file context>
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
1 participant
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.
📝 Summary
Merges
origin/mainintodm/micropip-streamingto fix CI on #9702 and #10522.The branch forked at
7df9434b3(2026-07-01) and CI resolves dependencies withUV_EXCLUDE_NEWER: 7 days, so it now installs releases the branch predates:mcp2.x, wheremcp.server.fastmcpno longer exists, failing everycore,optional depsandminimal depsjob. Main pinsmcp>=1.0.0,<2.ruff, whoseimplicit-string-concatenation-in-collection-literaland
sorted-min-maxrules flag 8 pre-existing lines. Main already fixed them.The merge is clean, with no conflicts. After it,
uv run ruff checkpasses andtests/_mcppasses 25 tests locally.📋 Pre-Review Checklist
✅ Merge Checklist