Skip to content

fix: only toast cell logs in app mode#9190

Merged
mscolnick merged 2 commits intomainfrom
ms/cell-toast
Apr 15, 2026
Merged

fix: only toast cell logs in app mode#9190
mscolnick merged 2 commits intomainfrom
ms/cell-toast

Conversation

@mscolnick
Copy link
Copy Markdown
Contributor

@mscolnick mscolnick commented Apr 14, 2026

Previously we were toasting on all internal errors. When only this should be happening in run mode. Also this skips it for the scratch pad (which i saw when used by code_mode)

Copilot AI review requested due to automatic review settings April 14, 2026 13:41
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 14, 2026

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

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment Apr 15, 2026 4:32pm

Request Review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refines frontend error-toasting behavior so cell error toasts only appear in app/run (“read”) mode (avoiding duplicate/noisy error surfacing in edit mode), and adds regression tests around both the toast gating and editor syncing during document transactions.

Changes:

  • Gate internal/exception error toast emission in getCellLogsForMessage to app (“read”) mode only.
  • Add unit tests asserting toast behavior across read/edit/unset initial mode and “toast only once” semantics.
  • Add reducer/editor integration tests ensuring set-code transactions imperatively sync the mounted CodeMirror editor document.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
frontend/src/core/cells/logs.ts Restricts error toast emission to read mode using initial app mode.
frontend/src/core/cells/tests/logs.test.ts Adds targeted tests for toast gating and once-per-session behavior.
frontend/src/core/cells/tests/apply-transaction.test.ts Adds regression coverage for set-code syncing into mounted editor views.

Comment thread frontend/src/core/cells/__tests__/logs.test.ts
Comment thread frontend/src/core/cells/logs.ts Outdated
@mscolnick mscolnick added the bug Something isn't working label Apr 15, 2026
@mscolnick mscolnick requested a review from dmadisetti April 15, 2026 18:24
// the cell UI, so toasting there would be noisy and duplicative. Read the
// atom directly so an unset initial mode (e.g. in tests/islands) simply
// returns undefined instead of throwing and masking real errors.
const isAppMode = store.get(initialModeAtom) === "read";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fair, this was a little annoying

@mscolnick mscolnick merged commit 7ec3efc into main Apr 15, 2026
32 of 33 checks passed
@mscolnick mscolnick deleted the ms/cell-toast branch April 15, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants