Suppress marimo-ui-value-update echo for user-initiated changes#9262
Suppress marimo-ui-value-update echo for user-initiated changes#9262
marimo-ui-value-update echo for user-initiated changes#9262Conversation
The kernel broadcasts a `marimo-ui-value-update` after every UI element value change, including user-initiated ones where the frontend already has the value. The echo is invisible over WebSocket but lands stale on high-latency transports, where `broadcastMessage` applies it and visibly snaps widgets backward mid-drag (the slider lag in marimo-team/marimo-lsp#515). These changes gate the broadcast behind a `notify_frontend` kwarg that defaults off to scope the feature for code-mode only.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR prevents redundant marimo-ui-value-update “echo” messages for user-initiated UI interactions by gating the broadcast behind a new notify_frontend keyword argument (defaulting to False) and enabling it only for kernel-initiated updates (e.g., code-mode set_ui_value).
Changes:
- Add
notify_frontend: bool = False(kw-only) toset_ui_element_valueso UI value broadcasts can be explicitly enabled/disabled. - Gate the
marimo-ui-value-updatebroadcast behindif notify_frontend:. - Thread
notify_frontendthrough theAppKernelRunner/InternalAppcall chain and enable it from code-mode UI update flushing.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
marimo/_runtime/runtime.py |
Adds notify_frontend flag and conditionally broadcasts marimo-ui-value-update. |
marimo/_runtime/app/kernel_runner.py |
Forwards notify_frontend into the kernel runtime call. |
marimo/_code_mode/_context.py |
Enables notify_frontend=True for code-mode batched UI updates on context exit. |
marimo/_ast/app.py |
Extends internal app UI update methods to accept/forward notify_frontend. |
| updated_components.append(component) | ||
| # Broadcast the new value to the frontend so the | ||
| # rendered widget reflects kernel-initiated changes | ||
| # (e.g. from code_mode's set_ui_value). | ||
| broadcast_notification( | ||
| UIElementMessageNotification( | ||
| ui_element=object_id, | ||
| message={ | ||
| "type": "marimo-ui-value-update", | ||
| "value": value, | ||
| }, | ||
| ), | ||
| self.stream, | ||
| ) | ||
| if notify_frontend: | ||
| broadcast_notification( | ||
| UIElementMessageNotification( |
There was a problem hiding this comment.
The new notify_frontend gate changes observable behavior (whether a marimo-ui-value-update UIElementMessageNotification is emitted). There are existing runtime/messaging tests, but none appear to assert the presence/absence of this notification. Please add coverage that verifies: (1) default behavior does not broadcast a value-update message, and (2) notify_frontend=True does broadcast the expected message payload, to prevent regressions on high-latency transports.
Fixes #515 The kernel broadcasts this message after every UI value change. Over LSP it arrives ~one round-trip stale and clobbers the user's in-progress state. Marimo-lsp doesn't surface code_mode yet, so the only legitimate use of these echoes (propagating programmatic value changes) doesn't apply, and dropping them entirely is safe. This is a temporary guard we can remove once the upstream `notify_frontend` gate (marimo-team/marimo#9262) ships and we can upgrade.
Fixes #515 The kernel broadcasts this message after every UI value change. Over LSP it arrives ~one round-trip stale and clobbers the user's in-progress state. Marimo-lsp doesn't surface code_mode yet, so the only legitimate use of these echoes (propagating programmatic value changes) doesn't apply, and dropping them entirely is safe. This is a temporary guard we can remove once the upstream `notify_frontend` gate (marimo-team/marimo#9262) ships and we can upgrade.
| self, | ||
| request: UpdateUIElementCommand, | ||
| *, | ||
| notify_frontend: bool = False, |
There was a problem hiding this comment.
Is it worth removing the default so it's always explicit? seems like you covered all the cases anyways
There was a problem hiding this comment.
Yeah that's great call.
|
I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
|
i tried the copilot agent from my phone to make the naming fix, no CLA lol |
The kernel broadcasts a
marimo-ui-value-updateafter every UI element value change, including user-initiated ones where the frontend already has the value. The echo is invisible over WebSocket but lands stale on high-latency transports, wherebroadcastMessageapplies it and visibly snaps widgets backward mid-drag (the slider lag in marimo-team/marimo-lsp#515).These changes gate the broadcast behind a
notify_frontendkwarg that defaults off to scope the feature for code-mode only.