Fix cells stuck as "needs run" after backend-initiated execution#8794
Fix cells stuck as "needs run" after backend-initiated execution#8794
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Fixes a frontend state-sync gap where cells created as stale (e.g., via code_mode) could remain visually “needs run” after being auto-executed by the backend, by updating CellData when execution completes.
Changes:
- Detects a backend-initiated run completion (cell transitions to
idleafter having arunStartTimestamp) and synchronizesCellData.edited/CellData.lastCodeRun. - Ensures the “needs run” UI state reflects completed execution even when
prepareForRunwas never called.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7708285 to
3aac84c
Compare
| # cells without run_cell leaves them stale/unexecuted. | ||
| _run_set = explicit_run or set() | ||
| if _run_set and self._kernel.reactive_execution_mode == "autorun": | ||
| _run_set = _run_set | cells_to_run |
There was a problem hiding this comment.
this might be from (or related to) a hack a long time ago where we put cells as queued when they were run from the frontend (but are now no longer)
|
The main behavior change here is that execute-code.sh now streams the response of the full execution that is configured by the user (including dependent cells if they have "autorun" enabled). I think this is desired because the agent then "sees" how its executed changes effect other cells. Otherwise if there is a runtime error, the model wouldn't know without figuring out to re-inspect state. |
When code_mode creates cells marked as stale (`code_is_stale=true`), the frontend sets `lastCodeRun=null` and `edited=true`. If the kernel then auto-executes those cells via reactivity (autorun mode), the cell transitions through queued/running/idle in `CellRuntimeState`, but `handleCellMessage` never touched `CellData` — so `edited` stayed true and the cell displayed a permanent yellow "needs run" border despite having correct output. The fix detects when a cell transitions to idle after a completed run (indicated by `runStartTimestamp` being set) while still marked as `edited`, and syncs `CellData.lastCodeRun` and `edited` to reflect the completed execution. This mirrors what `prepareForRun` does for frontend-initiated runs, covering the backend-initiated path that was previously missed.
Guard the edited/lastCodeRun sync with lastCodeRun == null so it only fires for cells created as stale by the backend (where setCellCodes set lastCodeRun to null). This avoids incorrectly clearing edited for cells where the user edits code mid-run — in that case lastCodeRun was already set by prepareForRun, so the guard skips the sync and the cell correctly stays marked as needs-run. Adds two tests: one for the backend-stale path (should clear edited) and one for the mid-run user-edit path (should preserve edited).
The two-step pattern (edit_cell in one flush, run_cell in a separate flush) bypassed _apply_ops entirely via the elif branch in __aexit__, so no UpdateCellCodesNotification was sent and the frontend never cleared its stale state. Send a code_is_stale=false notification before re-running so the frontend syncs edited/lastCodeRun. Also narrow the frontend safety-net in handleCellMessage to only fire when lastCodeRun is null (per review feedback), avoiding interference with mid-run user edits. Add tests for both paths.
Two code_mode paths left cells permanently showing "needs run" in the frontend even after execution. In `_apply_ops`, reactive descendants from `mutate_graph` were never included in the run set, so autorun executed them but the frontend was told they were stale. In `__aexit__`, a run-only flush (no structural ops) bypassed `_apply_ops` entirely and never notified the frontend. Both fixes are backend-only. In autorun mode, `execute-code` now blocks until downstream cells finish so the agent sees their full output. Lazy mode is unchanged -- downstream cells stay stale as expected.
Backend-only fix is sufficient. The frontend safety net in handleCellMessage is not needed since _apply_ops and __aexit__ now correctly send code_is_stale=false before execution.
2f442f8 to
1ef5748
Compare
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.21.2-dev31 |
Two code_mode paths left cells permanently showing "needs run" in the
frontend even after execution. In
_apply_ops, reactive descendantsfrom
mutate_graphwere never included in the run set, so autorunexecuted them but the frontend was told they were stale. In
__aexit__,a run-only flush (no structural ops) bypassed
_apply_opsentirely andnever notified the frontend.
Both fixes are backend-only. In autorun mode,
execute-codenow blocksuntil downstream cells finish so the agent sees their full output. Lazy
mode is unchanged -- downstream cells stay stale as expected.