Skip to content

Commit 4a646df

Browse files
nstrayerseeM
andauthored
fix: debounce output clearing during cell re-execution (#12947)
When a notebook cell is re-run, the runtime immediately clears outputs before sending new ones. This caused the output container to briefly disappear and reappear, producing a distracting flash. https://github.com/user-attachments/assets/9cca2395-8739-46a0-991b-5e489d5f3d8c To address we defer the clear by a small amount when execution is active so fast-completing cells never flash. We also flush the deferred clear immediately when execution ends. **Result:** https://github.com/user-attachments/assets/3ed02241-0e16-4265-9fb9-c8369e26b5ba ### Release Notes #### New Features - N/A #### Bug Fixes - Fixed notebook cell outputs flashing briefly when re-running a cell ### QA Notes @:notebooks @:positron-notebooks 1. Open a Python notebook 2. Run a cell that produces output (e.g. `print("hello")`) 3. Re-run the same cell -- output should update smoothly without a flash/flicker 4. Try with a slow cell (e.g. `import time; time.sleep(2); print("done")`) -- output should clear only after the delay, not flash 5. Clear outputs manually (right-click > Clear Outputs) -- should clear immediately without delay --------- Co-authored-by: seem <mwlorgat@gmail.com>
1 parent 559907c commit 4a646df

4 files changed

Lines changed: 62 additions & 11 deletions

File tree

src/vs/workbench/contrib/positronNotebook/browser/notebookCells/CellLeftActionMenu.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import './CellLeftActionMenu.css';
88

99
// Other dependencies.
10-
import { useObservedValue } from '../useObservedValue.js';
10+
import { useDebouncedObservedValue } from '../useObservedValue.js';
1111
import { PositronNotebookCodeCell } from '../PositronNotebookCells/PositronNotebookCodeCell.js';
1212

1313
interface CellLeftActionMenuProps {
@@ -19,7 +19,7 @@ interface CellLeftActionMenuProps {
1919
*/
2020
export function CellLeftActionMenu({ cell }: CellLeftActionMenuProps) {
2121
// Observed values
22-
const executionOrder = useObservedValue(cell.lastExecutionOrder);
22+
const executionOrder = useDebouncedObservedValue(cell.lastExecutionOrder);
2323

2424
// Determine what to show
2525
const showPending = executionOrder === undefined;

src/vs/workbench/contrib/positronNotebook/browser/notebookCells/CodeCellStatusFooter.tsx

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@ import { useEffect, useRef, useState } from 'react';
1212
// Other dependencies.
1313
import { localize } from '../../../../../nls.js';
1414
import * as DOM from '../../../../../base/browser/dom.js';
15-
import { useObservedValue } from '../useObservedValue.js';
15+
import { useObservedValue, useDebouncedObservedValue } from '../useObservedValue.js';
1616
import { PositronNotebookCodeCell } from '../PositronNotebookCells/PositronNotebookCodeCell.js';
17+
import { ExecutionStatus } from '../PositronNotebookCells/IPositronNotebookCell.js';
1718
import { formatCellDuration, formatTimestamp, getRelativeTime, isMoreThanOneHourAgo } from './cellExecutionUtils.js';
1819
import { Icon } from '../../../../../platform/positronActionBar/browser/components/icon.js';
1920
import { Codicon } from '../../../../../base/common/codicons.js';
@@ -23,17 +24,22 @@ interface CodeCellStatusFooterProps {
2324
hasError: boolean;
2425
}
2526

27+
// Defined outside the component so the reference is stable across renders,
28+
// avoiding memoization invalidation inside useDebouncedObservedValue.
29+
const isRunningOrPending = (s: ExecutionStatus) => s === 'running' || s === 'pending';
30+
2631
/**
2732
* Footer component that displays cell execution status information between
2833
* the editor and outputs sections. Shows execution state, duration, and timestamp.
2934
*/
3035
export function CodeCellStatusFooter({ cell, hasError }: CodeCellStatusFooterProps) {
31-
// Observe cell execution state
32-
const executionStatus = useObservedValue(cell.executionStatus);
36+
// Debounce "clearing" transitions to prevent visual flash during fast re-executions.
37+
// Only delay transitions to running/pending/undefined; new values propagate immediately.
38+
const executionStatus = useDebouncedObservedValue(cell.executionStatus, isRunningOrPending);
3339
const executionOrder = useObservedValue(cell.lastExecutionOrder);
34-
const duration = useObservedValue(cell.lastExecutionDuration);
35-
const lastRunEndTime = useObservedValue(cell.lastRunEndTime);
36-
const lastRunSuccess = useObservedValue(cell.lastRunSuccess);
40+
const duration = useDebouncedObservedValue(cell.lastExecutionDuration);
41+
const lastRunEndTime = useDebouncedObservedValue(cell.lastRunEndTime);
42+
const lastRunSuccess = useDebouncedObservedValue(cell.lastRunSuccess);
3743

3844
/**
3945
* `lastRunEndTime` doesn't change after execution completes, which means the

src/vs/workbench/contrib/positronNotebook/browser/notebookCells/NotebookCodeCell.tsx

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import React, { useMemo } from 'react';
1212
// Other dependencies.
1313
import { NotebookCellOutputs, ParsedTextOutput } from '../PositronNotebookCells/IPositronNotebookCell.js';
1414
import { isParsedTextOutput } from '../getOutputContents.js';
15-
import { useObservedValue } from '../useObservedValue.js';
15+
import { useObservedValue, useDebouncedObservedValue } from '../useObservedValue.js';
1616
import { CellEditorMonacoWidget } from './CellEditorMonacoWidget.js';
1717
import { localize } from '../../../../../nls.js';
1818
import { positronClassNames } from '../../../../../base/common/positronUtilities.js';
@@ -210,7 +210,16 @@ const CellOutputsSection = React.memo(function CellOutputsSection({ cell, output
210210
});
211211

212212
export const NotebookCodeCell = React.memo(function NotebookCodeCell({ cell }: { cell: PositronNotebookCodeCell }) {
213-
const outputContents = useObservedValue(cell.outputs);
213+
// Debounce transitions to empty only while the cell is executing so
214+
// re-execution doesn't flash. Explicit clears (when idle) propagate
215+
// immediately. We read executionStatus synchronously inside the predicate
216+
// so it reflects the state at the moment outputs change.
217+
const shouldDebounceOutputs = React.useCallback(
218+
(outputs: NotebookCellOutputs[]) =>
219+
outputs.length === 0 && cell.executionStatus.get() !== 'idle',
220+
[cell.executionStatus]
221+
);
222+
const outputContents = useDebouncedObservedValue(cell.outputs, shouldDebounceOutputs);
214223
const hasError = outputContents.some(o => o.parsed.type === 'error');
215224

216225
return (

src/vs/workbench/contrib/positronNotebook/browser/useObservedValue.tsx

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
import React from 'react';
88

99
// Other dependencies.
10-
import { IObservable, runOnChange } from '../../../../base/common/observable.js';
10+
import { IObservable, debouncedObservable, runOnChange } from '../../../../base/common/observable.js';
11+
import { isUndefinedOrNull } from '../../../../base/common/types.js';
1112

1213
/**
1314
* Automatically updates the component when the observable changes.
@@ -39,3 +40,38 @@ export function useObservedValue<T>(observable: IObservable<T> | undefined, defa
3940

4041
return value;
4142
}
43+
44+
/**
45+
* Like {@link useObservedValue}, but debounces value transitions where the
46+
* provided `shouldDebounce` predicate returns true. Non-debounced transitions
47+
* propagate immediately.
48+
*
49+
* Useful for suppressing transient UI flashes during cell re-execution without
50+
* delaying meaningful updates.
51+
*
52+
* @param observable The observable to subscribe to.
53+
* @param shouldDebounce Predicate receiving the new value. Return `true` to
54+
* delay propagation by the specified `delayMs`, `false` to propagate
55+
* immediately. Defaults to nullish check which covers `undefined` and `null`
56+
* but not valid falsy values like `0` or `false`. Note that this is used in
57+
* the dependency array of the `useMemo` call that creates the debounced
58+
* observable, so it should be memoized if it is not a stable reference or
59+
* otherwise the memo will be invalidated on every render, defeating the
60+
* purpose of debouncing.
61+
* @param delayMs Optional debounce delay in milliseconds. Defaults to 150ms.
62+
*/
63+
64+
export function useDebouncedObservedValue<T>(
65+
observable: IObservable<T>,
66+
shouldDebounce: (next: T) => boolean = isUndefinedOrNull,
67+
delayMs: number = 150,
68+
): T {
69+
const debounced = React.useMemo(
70+
() => debouncedObservable(observable, (_prev, next) => shouldDebounce(next) ? delayMs : 0),
71+
// We dont need to worry about the delayMs causing invalidation because
72+
// pure number types are compared by value not reference in the
73+
// dependency arrays of hooks.
74+
[observable, shouldDebounce, delayMs]
75+
);
76+
return useObservedValue(debounced);
77+
}

0 commit comments

Comments
 (0)