Skip to content

refactor: executor pipeline#9616

Open
dmadisetti wants to merge 5 commits into
mainfrom
dm/executor-impl
Open

refactor: executor pipeline#9616
dmadisetti wants to merge 5 commits into
mainfrom
dm/executor-impl

Conversation

@dmadisetti
Copy link
Copy Markdown
Collaborator

@dmadisetti dmadisetti commented May 19, 2026

📝 Summary

Refactor's marimo internal executor into a pipeline:

  • Scheduling
  • Pending notification hooks
  • ExecutionLifecycle Setup
  • Evaluation/Execution
  • ExecutionLifecycle Teardown
  • Completion notification hooks

Importantly replaces the nested composition of the previous executor implementation into composable Lifecycle components.

This change should have no functionality difference and solely acts as a refactor.


  marimo/_runtime/
  ├── executor.py
  └── runner/
      ├── __init__.py
      ├── cell_runner.py
      └── ...

Tree after change:

  marimo/_runtime/
  ├── executor/
  │   ├── __init__.py
  │   ├── evaluator.py           # composes lifecycles around an Executor
  │   ├── executor.py            # Executor Protocol + DefaultExecutor
  │   └── lifecycles/
  │       ├── __init__.py        # ExecutionLifecycle Protocol + Skip
  │       └── strict.py          # StrictLifecycle (was strict_executor)
  └── runner/
      ├── __init__.py
      ├── cell_runner.py         # rebuilt on Evaluator + SequentialScheduler
      ├── ...
      ├── result.py              # RunResult moved out of cell_runner
      └── scheduler.py           # Scheduler Protocol + SequentialScheduler

@vercel
Copy link
Copy Markdown

vercel Bot commented May 19, 2026

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

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment May 21, 2026 12:04am

Request Review

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 15 files

Architecture diagram
sequenceDiagram
    participant Runner as CellRunner
    participant Sched as SequentialScheduler
    participant Eval as Evaluator
    participant LC as ExecutionLifecycle(s)
    participant Exec as Executor
    participant Result as RunResult

    Note over Runner,Result: NEW: Pipeline Architecture

    Runner->>Sched: init(cells_to_run, graph)
    Sched-->>Runner: owns queue + cancellation state

    loop for each cell
        Runner->>Sched: pop_cell()
        Sched-->>Runner: next CellId_t

        Runner->>Eval: evaluate(cell, glbls)

        Note over Eval,Exec: Setup Phase
        loop for each lifecycle
            Eval->>LC: setup(cell, glbls)
            alt returns Skip
                LC-->>Eval: Skip(value)
                Note over Eval,Exec: short-circuit body execution
            else returns None
                LC-->>Eval: continue
            else raises exception
                LC-->>Eval: exception
                Note over Eval,Exec: halt setup, skip remaining lifecycles
            end
        end

        Note over Eval,Exec: Body Execution Phase
        alt no Skip and no setup exception
            Eval->>Exec: execute_cell_async(cell, glbls) or execute_cell()
            alt success
                Exec-->>Eval: return value
            else exception
                Exec->>Exec: wrap in MarimoRuntimeException
                Exec-->>Eval: exception
            end
        end

        Note over Eval,Result: Teardown Phase (reverse order)
        Eval->>Result: interim RunResult(output, exception)

        loop for completed lifecycles in reverse
            Eval->>LC: teardown(cell, glbls, interim_result)
            alt teardown raises
                LC-->>Eval: override exception
            end
        end

        alt teardown raised
            Eval-->>Result: final RunResult(output, teardown_exception)
        else
            Eval-->>Result: final RunResult(output, body_exception)
        end

        Result-->>Runner: RunResult

        Runner->>Runner: classify exception (MarimoInterrupt, etc.)
        alt MarimoInterrupt
            Runner->>Sched: set interrupted = True
        end
        alt exception present
            Runner->>Runner: store in self.exceptions[cell_id]
        end
    end

    Note over Runner,Sched: Cancellation (cross-cutting)
    Runner->>Sched: cancel(cell_id)
    Sched->>Sched: compute descendants via transitive_closure
    Sched->>Sched: mark cells as cancelled in graph
    Sched-->>Runner: cancelled cells tracked

    Note over Runner,Sched: Completion (cross-cutting)
    Runner->>Runner: run completion hooks
Loading

Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.

Re-trigger cubic

Comment thread marimo/_runtime/executor/executor.py
Comment thread marimo/_runtime/app/script_runner.py Outdated
Comment thread marimo/_runtime/runner/result.py
Comment thread marimo/_runtime/executor/__init__.py Outdated
@dmadisetti dmadisetti added the internal A refactor or improvement that is not user facing label May 19, 2026
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 issues found across 7 files (changes from recent commits).

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/_runtime/runner/cell_runner.py">

<violation number="1" location="marimo/_runtime/runner/cell_runner.py:169">
P2: Resolving the executor here makes every `Runner` construction load all installed executor entry points. Because `resolve_executor()` uses `get_all()` before picking the first factory, a broken or slow third-party executor can now break or delay ordinary notebook runs even when it would not be selected.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread marimo/_runtime/executor/evaluator.py Outdated
Comment thread marimo/_runtime/executor/lifecycles/__init__.py
Comment thread marimo/_runtime/runner/cell_runner.py Outdated
Comment thread marimo/_runtime/runner/cell_runner.py Outdated
Comment thread marimo/_runtime/executor/evaluator.py Outdated
Comment thread marimo/_runtime/executor/lifecycles/strict.py Outdated
Comment thread marimo/_runtime/executor/evaluator.py Outdated
Comment thread marimo/_runtime/executor/executor.py
Comment thread marimo/_runtime/runner/cell_runner.py
Comment thread marimo/_runtime/runner/cell_runner.py
Comment thread marimo/_runtime/runner/cell_runner.py Outdated
Comment thread tests/_runtime/test_executor_evaluator.py Outdated
@dmadisetti dmadisetti marked this pull request as ready for review May 20, 2026 16:37
Copilot AI review requested due to automatic review settings May 20, 2026 16:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors marimo’s internal cell execution machinery into a more composable pipeline by separating execution concerns into an Evaluator (lifecycle composition), Executor (body execution strategy), and runner-side Scheduler + RunResult value type.

Changes:

  • Replace the previous nested executor composition with Evaluator + ExecutionLifecycle components (including StrictLifecycle).
  • Introduce SequentialScheduler (queue + cancellation ownership) and move RunResult into its own module.
  • Update runners/tests to use the new executor registry model and new module layout.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
tests/test_entrypoints.py Updates executor entrypoint tests to reflect the new executor registry approach.
tests/_runtime/test_scheduler.py Adds unit tests for FIFO + cancellation behavior of SequentialScheduler.
tests/_runtime/test_executor_evaluator.py Adds tests for evaluator + lifecycle ordering, skip semantics, and teardown behavior.
marimo/_runtime/runner/scheduler.py Introduces Scheduler protocol and SequentialScheduler implementation.
marimo/_runtime/runner/result.py Moves RunResult into a dedicated module.
marimo/_runtime/runner/cell_runner.py Rebuilds the runner around Evaluator + SequentialScheduler and adds _finalize_run_result.
marimo/_runtime/executor/lifecycles/strict.py Extracts strict-mode globals sanitization as a lifecycle.
marimo/_runtime/executor/lifecycles/init.py Defines lifecycle protocol and Skip, and provides built-in lifecycle lookup.
marimo/_runtime/executor/executor.py Defines Executor protocol and DefaultExecutor.
marimo/_runtime/executor/evaluator.py Implements Evaluator, config builder, and executor entrypoint resolution.
marimo/_runtime/executor/init.py Re-exports the executor/evaluator/lifecycle public surface from the new package.
marimo/_runtime/executor.py Removes the legacy monolithic executor implementation.
marimo/_runtime/exceptions.py Adds unwrap_user_exception helper for downstream error classification.
marimo/_runtime/dataflow/runner.py Switches the dataflow runner to the new executor API (no graph parameter).
marimo/_runtime/app/script_runner.py Switches script runner to resolve_executor() and new executor API.

Comment thread tests/test_entrypoints.py
Comment thread tests/test_entrypoints.py
Comment thread tests/test_entrypoints.py
Comment thread tests/_runtime/test_scheduler.py Outdated
Comment thread marimo/_runtime/app/script_runner.py Outdated
Comment thread marimo/_runtime/dataflow/runner.py
Comment thread marimo/_runtime/runner/cell_runner.py Outdated
Comment thread marimo/_runtime/runner/cell_runner.py Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 7 files (changes from recent commits).

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/_runtime/executor/lifecycles/strict.py">

<violation number="1" location="marimo/_runtime/executor/lifecycles/strict.py:101">
P2: This blamed-cell lookup duplicates `Runner._get_blamed_cell`; extracting one shared helper would keep strict-mode error resolution from drifting.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread marimo/_runtime/executor/lifecycles/strict.py
Comment thread marimo/_runtime/app/script_runner.py
dmadisetti added a commit that referenced this pull request May 20, 2026
## 📝 Summary

This fix is a followup to #9617 and #9616 and restores the truncated
stacktrace to the user's notebook.

---

Before fix:

<img width="1086" height="405" alt="image"
src="https://github.com/user-attachments/assets/39918a95-a4ec-4ca5-b701-d77fef42169f"
/>

After fix:

<img width="1088" height="397" alt="image"
src="https://github.com/user-attachments/assets/f73e98ba-7bfd-4559-bd34-ef0ea1515bbd"
/>
@mscolnick mscolnick requested a review from Copilot May 20, 2026 22:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

result = RunResult(output=None, exception=None)
else:
try:
value = await self.executor.execute_cell_async(cell, glbls)
Comment on lines +111 to +125
def resolve_executor() -> Executor:
"""Return the registered executor factory's product, or ``DefaultExecutor``.

Used by both the kernel runner and script runner so a plugin
registered against ``marimo.cell.executor`` takes effect for both.
Only the first registered factory is loaded; others are noted via
``LOGGER.warning`` but never imported, so a broken third-party
plugin doesn't take down notebook execution.
"""
names = _EXECUTOR_REGISTRY.names()
if not names:
return DefaultExecutor()
name, *additional = names
if additional:
LOGGER.warning(
Comment thread marimo/_runtime/executor/evaluator.py Outdated
Comment thread marimo/_runtime/executor/lifecycles/__init__.py Outdated
Comment thread marimo/_runtime/executor/evaluator.py Outdated
- ``resolve_executor`` docstring: clarify that selection is the
  alphabetically-first name across registered plugins and installed
  entry points (per ``EntryPointRegistry.names()``'s sorted union),
  not "first registered" (which suggests order-of-registration).
- Wrap the factory invocation in a try/except so a third-party
  plugin that raises on construction falls back to ``DefaultExecutor``
  with a logged warning instead of taking down the kernel.
- Drop ``get_lifecycle_class`` from ``lifecycles/__init__.py`` and the
  ``executor`` re-export — no callers, dead since the lifecycle
  hookup moved into ``Runner.__init__``.
- Remove the ``EvaluatorConfig`` dataclass and ``build_evaluator``
  helper — the helper was a one-liner over ``Evaluator(...)`` and
  the lone caller (``Runner.__init__``) now constructs ``Evaluator``
  directly. Slimmer public surface.
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 5 files (changes from recent commits).

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/_runtime/executor/evaluator.py">

<violation number="1" location="marimo/_runtime/executor/evaluator.py:124">
P2: Don't silently downgrade to `DefaultExecutor` when the selected executor plugin fails. A broken or misconfigured plugin should still surface as an error; otherwise notebooks/scripts can run with different executor semantics than the user selected.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment on lines +124 to +133
try:
return _EXECUTOR_REGISTRY.get(name)()
except Exception as e:
LOGGER.warning(
"marimo.cell.executor factory %r failed to construct: %s; "
"falling back to ``DefaultExecutor``.",
name,
e,
)
return DefaultExecutor()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Don't silently downgrade to DefaultExecutor when the selected executor plugin fails. A broken or misconfigured plugin should still surface as an error; otherwise notebooks/scripts can run with different executor semantics than the user selected.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At marimo/_runtime/executor/evaluator.py, line 124:

<comment>Don't silently downgrade to `DefaultExecutor` when the selected executor plugin fails. A broken or misconfigured plugin should still surface as an error; otherwise notebooks/scripts can run with different executor semantics than the user selected.</comment>

<file context>
@@ -128,4 +121,13 @@ def resolve_executor() -> Executor:
             len(additional),
         )
-    return _EXECUTOR_REGISTRY.get(name)()
+    try:
+        return _EXECUTOR_REGISTRY.get(name)()
+    except Exception as e:
</file context>
Suggested change
try:
return _EXECUTOR_REGISTRY.get(name)()
except Exception as e:
LOGGER.warning(
"marimo.cell.executor factory %r failed to construct: %s; "
"falling back to ``DefaultExecutor``.",
name,
e,
)
return DefaultExecutor()
try:
return _EXECUTOR_REGISTRY.get(name)()
except Exception:
LOGGER.exception(
"marimo.cell.executor factory %r failed to construct.",
name,
)
raise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal A refactor or improvement that is not user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants