Skip to content

Commit 0537364

Browse files
committed
simplify execution flow.
1 parent 9fec8d4 commit 0537364

File tree

6 files changed

+338
-112
lines changed

6 files changed

+338
-112
lines changed

src/kernels/deepnote/deepnoteController.ts

Lines changed: 0 additions & 84 deletions
This file was deleted.

src/kernels/execution/cellExecutionCreator.ts

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -61,16 +61,17 @@ export class NotebookCellExecutionWrapper implements NotebookCellExecution {
6161
if (!this.started) {
6262
this._started = true;
6363

64-
// Clear outputs immediately on first start if configured to do so
64+
// Must call start() before clearOutput() per VS Code API requirements
65+
this._impl.start(startTime);
66+
this._startTime = startTime;
67+
68+
// Clear outputs immediately after start if configured to do so
6569
// This ensures old outputs are removed before any new outputs arrive from the kernel
6670
if (this.clearOutputOnStartWithTime) {
6771
logger.trace(`Start cell ${this.cell.index} execution (clear output)`);
6872
this._impl.clearOutput().then(noop, noop);
6973
}
7074

71-
this._impl.start(startTime);
72-
this._startTime = startTime;
73-
7475
if (startTime) {
7576
logger.trace(`Start cell ${this.cell.index} execution @ ${startTime}`);
7677
}
@@ -125,27 +126,31 @@ export class NotebookCellExecutionWrapper implements NotebookCellExecution {
125126
export class CellExecutionCreator {
126127
private static _map = new WeakMap<NotebookCell, NotebookCellExecutionWrapper>();
127128
static getOrCreate(cell: NotebookCell, controller: IKernelController, clearOutputOnStartWithTime = false) {
128-
let cellExecution: NotebookCellExecutionWrapper | undefined;
129-
cellExecution = this.get(cell);
130-
if (!cellExecution) {
131-
cellExecution = CellExecutionCreator.create(cell, controller, clearOutputOnStartWithTime);
132-
} else {
133-
// Cell execution may already exist, but its controller may be different
134-
if (cellExecution.controllerId !== controller.id) {
135-
// Stop the old execution so we don't have more than one for a cell at a time.
136-
const oldExecution = cellExecution;
137-
oldExecution.end(undefined);
129+
const existingExecution = this.get(cell);
130+
131+
if (existingExecution) {
132+
// Always end and replace existing executions.
133+
// VS Code's NotebookCellExecution API doesn't support reuse - once end() is called,
134+
// you cannot call start(), clearOutput(), or any other methods on it again.
135+
// This handles both controller changes and re-executions.
136+
const wasStarted = existingExecution.started;
137+
existingExecution.end(undefined);
138+
// Note: end() callback automatically removes it from the map
138139

139-
// Create a new one with the new controller
140-
cellExecution = CellExecutionCreator.create(cell, controller, clearOutputOnStartWithTime);
140+
// Create a fresh execution wrapper
141+
const cellExecution = CellExecutionCreator.create(cell, controller, clearOutputOnStartWithTime);
141142

142-
// Start the new one off now if the old one was already started
143-
if (oldExecution.started) {
144-
cellExecution.start(new Date().getTime());
145-
}
143+
// If the old execution was started, start the new one immediately
144+
// This handles the case where we're switching controllers mid-execution
145+
if (wasStarted) {
146+
cellExecution.start(new Date().getTime());
146147
}
148+
149+
return cellExecution;
147150
}
148-
return cellExecution;
151+
152+
// No existing execution, create a fresh one
153+
return CellExecutionCreator.create(cell, controller, clearOutputOnStartWithTime);
149154
}
150155
static get(cell: NotebookCell) {
151156
return CellExecutionCreator._map.get(cell);

0 commit comments

Comments
 (0)