feat: make notebook cell state clear without visual noise - #10624
Draft
Light2Dark wants to merge 1 commit into
Draft
feat: make notebook cell state clear without visual noise#10624Light2Dark wants to merge 1 commit into
Light2Dark wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
Light2Dark
added
enhancement
Contributor
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
11 issues found across 43 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/editor/cell/cell-status.css">
<violation number="1" location="frontend/src/components/editor/cell/cell-status.css:62">
P3: The compact-mode spinner-hide rule only matches a direct `<svg>` child of `.contents`. When a cell is running in combination with an error or outdated modifier, `CellStatusIcon` renders a `MultiIcon` (whose first icon is the `Loader2Icon` spinner) instead of a bare svg, so the running spinner is not hidden in compact mode. If the intent is to always replace the loader with the live timer in compact state, the selector needs to also account for the MultiIcon case.</violation>
</file>
<file name="frontend/src/components/editor/cell/RunButton.tsx">
<violation number="1" location="frontend/src/components/editor/cell/RunButton.tsx:71">
P2: When a blocked cell has edited code, the semantic model marks it actionable and this button remains enabled with the active variant, but this unconditional branch tells users it cannot be run. Restrict the blocked-only presentation to unedited blocked cells so edited cells use the normal runnable button and shortcut tooltip.</violation>
</file>
<file name="frontend/src/css/app/Cell.css">
<violation number="1" location="frontend/src/css/app/Cell.css:327">
P3: In dark mode, paused and blocked cells match these generic selectors, overriding their state border colors and weakening the intended state treatment. Exclude `.cell-paused` and `.cell-blocked` from both dark-mode selectors.</violation>
</file>
<file name="frontend/src/components/editor/cell/CellStatus.tsx">
<violation number="1" location="frontend/src/components/editor/cell/CellStatus.tsx:139">
P2: When a paused or blocked cell has both an error and outdated inputs, the compact badge renders only the error modifier, so the outdated state disappears. Render both modifiers, or otherwise encode both states in the compact icon.</violation>
<violation number="2" location="frontend/src/components/editor/cell/CellStatus.tsx:178">
P2: For statuses whose reason is in `status.description`, this element exposes only the terse `label` to assistive technology and relies on a hover-only tooltip for the explanation. Expose `status.description` through an accessible description or persistent text.</violation>
</file>
<file name="frontend/src/components/editor/controls/Controls.tsx">
<violation number="1" location="frontend/src/components/editor/controls/Controls.tsx:188">
P3: Setting `disabled` on the run button makes the "Nothing to run" tooltip never appear. A natively disabled `<button>` stops dispatching pointer events, so the Radix TooltipTrigger wrapping it never shows its content. The same applies to the stop button when `showInterrupt` is false ("No cells running" tooltip). Previously the run button was only disabled while the app was not interactive, so the tooltip showed in the common case; it now regresses to always-hidden. To keep the tooltip, keep the button non-disabled and drive interactivity via `onClick`/`aria-disabled`, or wrap the disabled button in a span that receives the hover events.</violation>
</file>
<file name="frontend/src/core/cells/semantic-state.ts">
<violation number="1" location="frontend/src/core/cells/semantic-state.ts:343">
P2: When a cell is outdated while running, queued, interrupted, stopped, or not yet run, the visual refresh modifier is omitted from its accessible label and description. Include the outdated modifier whenever `primary !== "outdated"` so assistive technology receives the same combined state shown visually.</violation>
</file>
<file name="frontend/src/components/editor/cell/collapse.tsx">
<violation number="1" location="frontend/src/components/editor/cell/collapse.tsx:95">
P2: When a descendant was interrupted without edited code or changed inputs, this summary reports it only as `Has cells to run` and hides the interrupted outcome. Add a separate interrupted summary/icon or use an interrupted-specific label here.</violation>
<violation number="2" location="frontend/src/components/editor/cell/collapse.tsx:100">
P2: When a collapsed range contains a directly disabled cell, this banner does not show that paused state because it only renders the upstream-blocked flag. Add a paused descendant summary and render its status so collapsed cells do not hide direct availability state.</violation>
</file>
<file name="frontend/src/components/editor/renderers/cell-array.tsx">
<violation number="1" location="frontend/src/components/editor/renderers/cell-array.tsx:198">
P2: In single-column layouts, this hook computes and subscribes to a full column summary even though `Column` discards `statusSummary`. Restrict the subscription to multi-column layouts, such as by moving it into the columns-only rendering path or adding an enabled variant to the hook.</violation>
</file>
<file name="frontend/src/components/editor/cell/toolbar.tsx">
<violation number="1" location="frontend/src/components/editor/cell/toolbar.tsx:16">
P3: The new `active` variant declares an explicit border color (`border-[var(--blue-6)]`) but the equally-new filled `stop` variant does not, so its 1px border falls back to the base `border` (currentColor). The two filled states render with visibly different borders, which undercuts the PR's goal of consistent semantic styling. Add a matching border color to `stop` (e.g. `border-[var(--red-7)]`) or drop the explicit border on `active`.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User
participant UI as Notebook Cell UI
participant Store as Jotai Store (Notebook State)
participant Logic as NEW: Semantic State Logic
participant Kernel as Python Kernel
Note over User, Kernel: Runtime Request & State Derivation Flow
User->>UI: Edits code or clicks "Run"
alt User Edited Code
UI->>Store: Update cell code & set edited=true
else User Clicked Run
UI->>Kernel: Request cell execution
Kernel-->>Store: Update status (queued/running)
end
Store->>UI: Trigger re-render with raw data
Note over UI, Logic: Centralized Derivation
UI->>Logic: NEW: deriveCellSemanticState(data, runtime)
Logic->>Logic: Calculate availability (paused/blocked)
Logic->>Logic: Calculate freshness (outdated/not-run)
Logic->>Logic: Calculate execution phase (idle/queued/running)
Logic-->>UI: Return CellSemanticState object
Note over UI: UI Components consume semantic state
UI->>UI: CHANGED: Render semantic rails (CSS classes)
UI->>UI: NEW: Render CellStatusComponent (Badges/Timers)
opt Output Handling
UI->>UI: CHANGED: OutputArea(state: updating|previous|current)
Note right of UI: Output dims or shows loader<br/>based on semantic freshness
end
opt Action Controls
UI->>UI: CHANGED: RunButton (evaluates semantic needsRun)
UI->>UI: CHANGED: StopButton (evaluates semantic phase)
end
alt Multi-column Layout
UI->>UI: NEW: presentCellStatus(state, "compact")
Note right of UI: Stable layout for drag handles<br/>while showing live timer
end
Note over UI, Store: Global State Summary
Store->>Logic: NEW: summarizeCellStates(allCells)
Logic-->>UI: Update Header/Column Status Badges
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ); | ||
| } | ||
| if (!config.disabled && blockedStatus && !edited) { | ||
| if (blocked) { |
Contributor
There was a problem hiding this comment.
P2: When a blocked cell has edited code, the semantic model marks it actionable and this button remains enabled with the active variant, but this unconditional branch tells users it cannot be run. Restrict the blocked-only presentation to unedited blocked cells so edited cells use the normal runnable button and shortcut tooltip.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/cell/RunButton.tsx, line 71:
<comment>When a blocked cell has edited code, the semantic model marks it actionable and this button remains enabled with the active variant, but this unconditional branch tells users it cannot be run. Restrict the blocked-only presentation to unedited blocked cells so edited cells use the normal runnable button and shortcut tooltip.</comment>
<file context>
@@ -69,7 +68,7 @@ export const RunButton = (props: {
);
}
- if (!config.disabled && blockedStatus && !edited) {
+ if (blocked) {
return (
<ToolbarItem
</file context>
Suggested change
| if (blocked) { | |
| if (blocked && !state.codeEdited) { |
| data-testid="cell-status" | ||
| data-status={state} | ||
| data-display={display} | ||
| aria-label={label} |
Contributor
There was a problem hiding this comment.
P2: For statuses whose reason is in status.description, this element exposes only the terse label to assistive technology and relies on a hover-only tooltip for the explanation. Expose status.description through an accessible description or persistent text.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/cell/CellStatus.tsx, line 178:
<comment>For statuses whose reason is in `status.description`, this element exposes only the terse `label` to assistive technology and relies on a hover-only tooltip for the explanation. Expose `status.description` through an accessible description or persistent text.</comment>
<file context>
@@ -1,325 +1,241 @@
+ data-testid="cell-status"
+ data-status={state}
+ data-display={display}
+ aria-label={label}
+ role="status"
+ aria-atomic={true}
</file context>
| const CellStatusIcon = ({ status }: { status: CellStatusPresentation }) => { | ||
| const primary = primaryStatusIcon(status.primary); | ||
| const modifier = | ||
| status.modifiers.hasError && status.primary !== "error" ? ( |
Contributor
There was a problem hiding this comment.
P2: When a paused or blocked cell has both an error and outdated inputs, the compact badge renders only the error modifier, so the outdated state disappears. Render both modifiers, or otherwise encode both states in the compact icon.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/cell/CellStatus.tsx, line 139:
<comment>When a paused or blocked cell has both an error and outdated inputs, the compact badge renders only the error modifier, so the outdated state disappears. Render both modifiers, or otherwise encode both states in the compact icon.</comment>
<file context>
@@ -1,325 +1,241 @@
+const CellStatusIcon = ({ status }: { status: CellStatusPresentation }) => {
+ const primary = primaryStatusIcon(status.primary);
+ const modifier =
+ status.modifiers.hasError && status.primary !== "error" ? (
+ <AlertCircleIcon className="size-2.5 bg-inherit" />
+ ) : status.modifiers.outdated && status.primary !== "outdated" ? (
</file context>
| labels.push("Error"); | ||
| descriptions.push("This cell also has an error."); | ||
| } | ||
| if ((showModifiers || primary === "error") && outdated) { |
Contributor
There was a problem hiding this comment.
P2: When a cell is outdated while running, queued, interrupted, stopped, or not yet run, the visual refresh modifier is omitted from its accessible label and description. Include the outdated modifier whenever primary !== "outdated" so assistive technology receives the same combined state shown visually.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/core/cells/semantic-state.ts, line 343:
<comment>When a cell is outdated while running, queued, interrupted, stopped, or not yet run, the visual refresh modifier is omitted from its accessible label and description. Include the outdated modifier whenever `primary !== "outdated"` so assistive technology receives the same combined state shown visually.</comment>
<file context>
@@ -0,0 +1,456 @@
+ labels.push("Error");
+ descriptions.push("This cell also has an error.");
+ }
+ if ((showModifiers || primary === "error") && outdated) {
+ labels.push("Outdated");
+ descriptions.push(
</file context>
| {states.stale && ( | ||
| <Tooltip content="Has stale cells" delayDuration={100}> | ||
| <RefreshCcw className="w-4 h-4 shrink-0 text-(--yellow-11)" /> | ||
| {states.outdated && ( |
Contributor
There was a problem hiding this comment.
P2: When a descendant was interrupted without edited code or changed inputs, this summary reports it only as Has cells to run and hides the interrupted outcome. Add a separate interrupted summary/icon or use an interrupted-specific label here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/cell/collapse.tsx, line 95:
<comment>When a descendant was interrupted without edited code or changed inputs, this summary reports it only as `Has cells to run` and hides the interrupted outcome. Add a separate interrupted summary/icon or use an interrupted-specific label here.</comment>
<file context>
@@ -84,19 +87,34 @@ export const CollapsedCellBanner: React.FC<{
- {states.stale && (
- <Tooltip content="Has stale cells" delayDuration={100}>
- <RefreshCcw className="w-4 h-4 shrink-0 text-(--yellow-11)" />
+ {states.outdated && (
+ <Tooltip content="Has cells to run" delayDuration={100}>
+ <RefreshCcw className="w-4 h-4 shrink-0 text-(--slate-11)" />
</file context>
| const cellIds = useCellIds(); | ||
| const column = cellIds.get(columnId); | ||
| invariant(column, `Expected column for: ${columnId}`); | ||
| const statusSummary = useColumnStateSummary(columnId); |
Contributor
There was a problem hiding this comment.
P2: In single-column layouts, this hook computes and subscribes to a full column summary even though Column discards statusSummary. Restrict the subscription to multi-column layouts, such as by moving it into the columns-only rendering path or adding an enabled variant to the hook.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/renderers/cell-array.tsx, line 198:
<comment>In single-column layouts, this hook computes and subscribes to a full column summary even though `Column` discards `statusSummary`. Restrict the subscription to multi-column layouts, such as by moving it into the columns-only rendering path or adding an enabled variant to the hook.</comment>
<file context>
@@ -194,6 +195,7 @@ const CellColumn: React.FC<{
const cellIds = useCellIds();
const column = cellIds.get(columnId);
invariant(column, `Expected column for: ${columnId}`);
+ const statusSummary = useColumnStateSummary(columnId);
const hasOnlyOneCell = cellIds.hasOnlyOneId();
</file context>
| display: none; | ||
| } | ||
|
|
||
| .cell-status-indicator[data-display="compact"][data-status^="running"] |
Contributor
There was a problem hiding this comment.
P3: The compact-mode spinner-hide rule only matches a direct <svg> child of .contents. When a cell is running in combination with an error or outdated modifier, CellStatusIcon renders a MultiIcon (whose first icon is the Loader2Icon spinner) instead of a bare svg, so the running spinner is not hidden in compact mode. If the intent is to always replace the loader with the live timer in compact state, the selector needs to also account for the MultiIcon case.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/cell/cell-status.css, line 62:
<comment>The compact-mode spinner-hide rule only matches a direct `<svg>` child of `.contents`. When a cell is running in combination with an error or outdated modifier, `CellStatusIcon` renders a `MultiIcon` (whose first icon is the `Loader2Icon` spinner) instead of a bare svg, so the running spinner is not hidden in compact mode. If the intent is to always replace the loader with the live timer in compact state, the selector needs to also account for the MultiIcon case.</comment>
<file context>
@@ -1,21 +1,78 @@
+ display: none;
+}
+
+.cell-status-indicator[data-display="compact"][data-status^="running"]
+ > .contents
+ > svg {
</file context>
|
|
||
| &:hover { | ||
| border: 1px solid var(--gray-9); | ||
| &:not(.cell-active, .cell-failed, [data-cell-freshness="outdated"]) { |
Contributor
There was a problem hiding this comment.
P3: In dark mode, paused and blocked cells match these generic selectors, overriding their state border colors and weakening the intended state treatment. Exclude .cell-paused and .cell-blocked from both dark-mode selectors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/css/app/Cell.css, line 327:
<comment>In dark mode, paused and blocked cells match these generic selectors, overriding their state border colors and weakening the intended state treatment. Exclude `.cell-paused` and `.cell-blocked` from both dark-mode selectors.</comment>
<file context>
@@ -327,14 +324,12 @@
-
- &:hover {
- border: 1px solid var(--gray-9);
+ &:not(.cell-active, .cell-failed, [data-cell-freshness="outdated"]) {
+ border-color: var(--border);
}
</file context>
| size="medium" | ||
| shape="circle" | ||
| disabled={!canInteractWithApp} | ||
| disabled={true} |
Contributor
There was a problem hiding this comment.
P3: Setting disabled on the run button makes the "Nothing to run" tooltip never appear. A natively disabled <button> stops dispatching pointer events, so the Radix TooltipTrigger wrapping it never shows its content. The same applies to the stop button when showInterrupt is false ("No cells running" tooltip). Previously the run button was only disabled while the app was not interactive, so the tooltip showed in the common case; it now regresses to always-hidden. To keep the tooltip, keep the button non-disabled and drive interactivity via onClick/aria-disabled, or wrap the disabled button in a span that receives the hover events.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/controls/Controls.tsx, line 188:
<comment>Setting `disabled` on the run button makes the "Nothing to run" tooltip never appear. A natively disabled `<button>` stops dispatching pointer events, so the Radix TooltipTrigger wrapping it never shows its content. The same applies to the stop button when `showInterrupt` is false ("No cells running" tooltip). Previously the run button was only disabled while the app was not interactive, so the tooltip showed in the common case; it now regresses to always-hidden. To keep the tooltip, keep the button non-disabled and drive interactivity via `onClick`/`aria-disabled`, or wrap the disabled button in a span that receives the hover events.</comment>
<file context>
@@ -179,11 +180,12 @@ const RunControlButton = ({
size="medium"
shape="circle"
- disabled={!canInteractWithApp}
+ disabled={true}
>
<PlayIcon strokeWidth={1.5} size={16} />
</file context>
Comment on lines
+16
to
19
| active: | ||
| "bg-(--blue-3) hover:bg-(--blue-4) text-(--blue-11) border-[var(--blue-6)]", | ||
| green: | ||
| "hover:bg-(--grass-2) hover:text-(--grass-11) hover:border-[var(--grass-7)],", |
Contributor
There was a problem hiding this comment.
P3: The new active variant declares an explicit border color (border-[var(--blue-6)]) but the equally-new filled stop variant does not, so its 1px border falls back to the base border (currentColor). The two filled states render with visibly different borders, which undercuts the PR's goal of consistent semantic styling. Add a matching border color to stop (e.g. border-[var(--red-7)]) or drop the explicit border on active.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/cell/toolbar.tsx, line 16:
<comment>The new `active` variant declares an explicit border color (`border-[var(--blue-6)]`) but the equally-new filled `stop` variant does not, so its 1px border falls back to the base `border` (currentColor). The two filled states render with visibly different borders, which undercuts the PR's goal of consistent semantic styling. Add a matching border color to `stop` (e.g. `border-[var(--red-7)]`) or drop the explicit border on `active`.</comment>
<file context>
@@ -13,11 +13,13 @@ const toolbarItemVariants = cva(
variant: {
default: "hover:bg-accent hover:text-accent-foreground",
- stale: "bg-(--yellow-3) hover:bg-(--yellow-4) text-(--yellow-11)",
+ active:
+ "bg-(--blue-3) hover:bg-(--blue-4) text-(--blue-11) border-[var(--blue-6)]",
green:
</file context>
Suggested change
| active: | |
| "bg-(--blue-3) hover:bg-(--blue-4) text-(--blue-11) border-[var(--blue-6)]", | |
| green: | |
| "hover:bg-(--grass-2) hover:text-(--grass-11) hover:border-[var(--grass-7)],", | |
| active: | |
| "bg-(--blue-3) hover:bg-(--blue-4) text-(--blue-11) border-[var(--blue-6)]", | |
| green: | |
| "hover:bg-(--grass-2) hover:text-(--grass-11) hover:border-[var(--grass-7)],", | |
| disabled: "opacity-50 cursor-not-allowed", | |
| danger: "hover:bg-(--red-3) hover:text-(--red-11)", | |
| stop: "bg-(--red-3) hover:bg-(--red-4) text-(--red-11) border-[var(--red-6)]", |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 pull request was authored by a coding agent.
📝 Summary
Cell state was communicated through several independent styling paths, which made combinations such as disabled plus outdated or failed plus outdated ambiguous. Broad yellow cell treatments also made notebooks visually noisy, while running status and selection could be easy to miss.
This centralizes runtime state into a semantic model and derives badges, actions, accessible descriptions, output freshness, column summaries, and cell emphasis from it. State is now communicated through compact labels and non-layout semantic rails instead of full-cell tinting. Execution, availability, freshness, prior outcome, and command-mode selection remain independent, so combined states stay visible without shifting cell geometry. Multi-column notebooks use compact status treatments with stable space for the drag handle and live timer.
Screen.Recording.2026-08-21.at.11.21.26.PM.mov
📋 Pre-Review Checklist
✅ Merge Checklist