Fix: state resolution #7110
Merged
Fix: state resolution #7110
Conversation
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
This PR prunes logic in state resolution which eagerly marked cells as stale and enqueued a stale cell request whenever a setter was called; in the main thread, running stale cells in the main cell execution loop. Eager execution of state updates only needs to happen when running state updates in a mo.Thread object (either manually or via the mo.watch APIs). This PR updates conditions how state resolution happens based on whether the current thread is a mo.Thread. Additionally, instead of relying on the state registry to find bound names, which does not work when a state update is called in the cell that defined it, we just scan globals to find cells that have the state object in their refs (similar to what CellRunner does).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Collaborator
|
Nice! I can grab the side-effect issue to take it off your plate if needed |
Contributor
Author
Thanks @dmadisetti -- is the fix relatively simple/easy for you? If so, feel free to push it up to this PR. If not, I can look into it in my downtime. |
dmadisetti
approved these changes
Nov 7, 2025
Collaborator
dmadisetti
left a comment
dmadisetti
left a comment
There was a problem hiding this comment.
Re triggered the tests- flaky windows one started an early failure- but looks good. Much better than mocking out the whole execution queu
| (tmp_path / "test.txt").touch() | ||
| await asyncio.sleep(0.25) | ||
| await k.run([]) | ||
| assert len(control_requests) == 1 |
Collaborator
There was a problem hiding this comment.
Nice, this is a good check too
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR prunes logic in state resolution which eagerly marked cells as stale and enqueued a stale cell request whenever a setter was called; in the main thread, running cells made stale by a state update is handled by the main cell execution loop.
Eager execution of state updates only needs to happen when running state updates in a
mo.Threadobject (either manually or via the mo.watch APIs). This PR updates conditions how state resolution happens based on whether the current thread is amo.Thread.Additionally, instead of relying on the state registry to find bound names, which does not work when a state update is called in the cell that defined it, we just scan globals to find cells that have the state object in their refs (similar to what
CellRunnerdoes).Closes #7060
Closes #6847
Closes #5311
Closes #5139