Skip to content

Conversation

@akshayka
Copy link
Contributor

@akshayka akshayka commented Nov 6, 2025

Right now the runtime eagerly updates states value when a state update
is registered. This should be unnecessary (but maybe was needed
for file watching element, which is when this code was introduced),
and it hits a code path for UI elements that triggers a very loud error.
Instead of fixing the underlying issue, this PR prevents the
code path from being triggered and prints a gentler warning.

Related: #7060

defined

Right now the runtime eagerly updates states value when a state update
is registered. This should be unnecessary (but maybe was needed
for file watching element, which is when this code was introduced),
and it hits a code path for UI elements that triggers a very loud error.
Instead of fixing the underlying issue, this PR prevents the
code path from being triggered and prints a gentler warning.
@akshayka akshayka requested a review from dmadisetti as a code owner November 6, 2025 21:49
@vercel
Copy link

vercel bot commented Nov 6, 2025

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

Project Deployment Preview Comments Updated (UTC)
marimo-docs Ready Ready Preview Comment Nov 6, 2025 9:49pm

cell_id = ctx.execution_context.cell_id
with self._state_lock:
self.state_updates[state] = cell_id
# TODO(akshayka): State should not be updated eagerly like this,
Copy link
Collaborator

@dmadisetti dmadisetti Nov 6, 2025

Choose a reason for hiding this comment

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

Context, this path is hit when you update state from the same cell:

Image

because state does not have a bound name it cannot be looked up, which means that some of this logic in the function doesn't apply- so early exiting is correct.

PR #5143
Also provided an early exit (but this is better since we update state_updates)

There's also an embedded app path where I think state is associated with the parent cell.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Current behavior:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, thank you!

@akshayka akshayka merged commit 0b37362 into main Nov 7, 2025
36 of 41 checks passed
@akshayka akshayka deleted the aka/improve-gentler-warning-state branch November 7, 2025 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants