fix: Persist stale cells on session resume

wasimsandhu commented 2 weeks ago

📝 Summary

Fixes #1444, an edge case bug that caused cells to lose "stale" status on session resume.

Important to note that this bug was only for uninstantiated cells, or effectively, the first session resume. Cells that had been run and later became stale were already persisted on subsequent resumes.

🔍 Description of Changes

In createCellRuntimeState(), pass staleInputs: true if lastExecutionTime == null, which will only be null for an uninstantiated cell.
Update cell status tooltip with more relevant "This cell has not yet been run" hint for uninstantiated cells.'

Uninstantiated cells after session resume:

Stale cells after session resume:

📋 Checklist

[x] I have read the contributor guidelines.
[x] For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
[ ] I have added tests for the changes made.
[x] I have run the code and verified that it works as expected.

📜 Reviewers

@mscolnick

vercel[bot] commented 2 weeks ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
marimo-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 17, 2024 0:07am
marimo-storybook	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 17, 2024 0:07am

mscolnick commented 2 weeks ago

awesome! nice fix and improved messaging!

github-actions[bot] commented 2 weeks ago

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.6.20-dev4

akshayka commented 2 weeks ago

@wasimsandhu @mscolnick I think there may be a regression with this PR, with cells incorrectly marked as stale on startup.

I have autorun on startup enabled.
I ran marimo tutorial intro
The notebook correctly ran, but everything is incorrectly marked as stale:

wasimsandhu commented 2 weeks ago

@akshayka After testing the 4 runtime combinations, I removed a line of code that I thought was unnecessary and only tested with autorun on startup disabled. Sorry about that! Fixed in #1626.

marimo-team / marimo