Skip to content

fix: Prompt the user to select an environment when running one or more blocks. #310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Artmann merged 11 commits into from
Feb 4, 2026
Merged

fix: Prompt the user to select an environment when running one or more blocks. #310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1253a11
fix: Prompt the user to select an environment when running one or mor…
Artmann Feb 3, 2026
de059fc
CR Feedback
Artmann Feb 3, 2026
506fc53
refactor: Address additional CodeRabbit feedback
Artmann Feb 3, 2026
b8af711
CR Feedback
Artmann Feb 3, 2026
c6eb7fe
CR
Artmann Feb 3, 2026
e9f4047
CR
Artmann Feb 3, 2026
49c9b4d
Remove some comments
Artmann Feb 3, 2026
b64bf9a
refactor
Artmann Feb 3, 2026
30b951d
cr
Artmann Feb 3, 2026
7216421
CR
Artmann Feb 3, 2026
d347cbe
Update claude
Artmann Feb 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 39 additions & 2 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
- Use `Uri.joinPath()` for constructing file paths to ensure platform-correct path separators (e.g., `Uri.joinPath(venvPath, 'share', 'jupyter', 'kernels')` instead of string concatenation with `/`)
- Follow established patterns, especially when importing new packages (e.g. instead of importing uuid directly, use the helper `import { generateUuid } from '../platform/common/uuid';`)


## Code conventions

- Always run `npx prettier` before committing
Expand All @@ -19,7 +18,6 @@
- Tests run against compiled JavaScript files in `out/` directory
- Use `assert.deepStrictEqual()` for object comparisons instead of checking individual properties


## Project Structure

- VSCode extension for Jupyter notebooks
Expand All @@ -40,3 +38,42 @@
- Whitespace is good for readability, add a blank line after const groups and before return statements
- Separate third-party and local file imports
- How the extension works is described in @specs/architecture.md

## Best Practices

### Resource Cleanup

- Always dispose `CancellationTokenSource` - never create inline without storing/disposing
- Use try/finally to ensure cleanup:
```typescript
const cts = new CancellationTokenSource();
try {
await fn(cts.token);
} finally {
cts.dispose();
}
```

### DRY Principle

- Extract duplicate logic into helper methods to prevent drift
- When similar logic appears in multiple places (e.g., placeholder controller setup, interpreter validation), consolidate it

### Magic Numbers

- Extract magic numbers (retry counts, delays, timeouts) as named constants near the top of the module

### Error Handling

- Use per-iteration error handling in loops - wrap each iteration in try/catch so one failure doesn't stop the rest
- Handle `withProgress` cancellation gracefully - it throws when user cancels, so wrap in try/catch and return appropriate value

### State Validation

- Verify state after async setup operations - methods can return early without throwing, so check expected state was created
- Validate cached state before early returns - before returning "already configured", verify the state is still valid (e.g., interpreter paths match, controllers aren't stale)

### Cancellation Tokens

- Use real cancellation tokens tied to lifecycle events instead of fake/never-cancelled tokens
- Create `CancellationTokenSource` tied to relevant events (e.g., notebook close, cell cancel)
40 changes: 25 additions & 15 deletions src/kernels/deepnote/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,24 @@ export interface IDeepnoteServerProvider {

export const IDeepnoteKernelAutoSelector = Symbol('IDeepnoteKernelAutoSelector');
export interface IDeepnoteKernelAutoSelector {
/**
* Clear the controller selection for a notebook using a specific environment.
* This is used when deleting an environment to unselect its controller from any open notebooks.
* @param notebook The notebook document
* @param environmentId The environment ID
*/
clearControllerForEnvironment(notebook: vscode.NotebookDocument, environmentId: string): void;

/**
* Ensure an environment is configured for the notebook before execution.
* If not configured, shows picker and sets up the kernel.
* @returns true if environment is ready, false if user cancelled
*/
ensureEnvironmentConfiguredBeforeExecution(
notebook: vscode.NotebookDocument,
token: vscode.CancellationToken
): Promise<boolean>;

/**
* Automatically selects and starts a Deepnote kernel for the given notebook.
* @param notebook The notebook document
Expand All @@ -221,6 +239,13 @@ export interface IDeepnoteKernelAutoSelector {
token: vscode.CancellationToken
): Promise<boolean>;

/**
* Handle kernel selection errors with user-friendly messages and actions
* @param error The error to handle
* @param notebook The notebook document associated with the error
*/
handleKernelSelectionError(error: unknown, notebook: vscode.NotebookDocument): Promise<void>;

/**
* Force rebuild the controller for a notebook by clearing cached controller and metadata.
* This is used when switching environments to ensure a new controller is created.
Expand All @@ -232,21 +257,6 @@ export interface IDeepnoteKernelAutoSelector {
progress: { report(value: { message?: string; increment?: number }): void },
token: vscode.CancellationToken
): Promise<void>;

/**
* Clear the controller selection for a notebook using a specific environment.
* This is used when deleting an environment to unselect its controller from any open notebooks.
* @param notebook The notebook document
* @param environmentId The environment ID
*/
clearControllerForEnvironment(notebook: vscode.NotebookDocument, environmentId: string): void;

/**
* Handle kernel selection errors with user-friendly messages and actions
* @param error The error to handle
* @param notebook The notebook document associated with the error
*/
handleKernelSelectionError(error: unknown, notebook: vscode.NotebookDocument): Promise<void>;
}

export const IDeepnoteEnvironmentManager = Symbol('IDeepnoteEnvironmentManager');
Expand Down
28 changes: 28 additions & 0 deletions src/notebooks/controllers/vscodeNotebookController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { KernelMessage } from '@jupyterlab/services';
import type { IAnyMessageArgs } from '@jupyterlab/services/lib/kernel/kernel';
import {
CancellationError,
CancellationTokenSource,
commands,
Disposable,
EventEmitter,
Expand Down Expand Up @@ -88,6 +89,7 @@ import { KernelConnector } from './kernelConnector';
import { RemoteKernelReconnectBusyIndicator } from './remoteKernelReconnectBusyIndicator';
import { IConnectionDisplayData, IConnectionDisplayDataProvider, IVSCodeNotebookController } from './types';
import { notebookPathToDeepnoteProjectFilePath } from '../../platform/deepnote/deepnoteProjectUtils';
import { DEEPNOTE_NOTEBOOK_TYPE, IDeepnoteKernelAutoSelector } from '../../kernels/deepnote/types';

/**
* Our implementation of the VSCode Notebook Controller. Called by VS code to execute cells in a notebook. Also displayed
Expand Down Expand Up @@ -435,6 +437,32 @@ export class VSCodeNotebookController implements Disposable, IVSCodeNotebookCont
if (!workspace.isTrusted) {
return;
}

// For Deepnote notebooks, ensure environment is configured before execution
if (notebook.notebookType === DEEPNOTE_NOTEBOOK_TYPE) {
const kernelAutoSelector =
this.serviceContainer.tryGet<IDeepnoteKernelAutoSelector>(IDeepnoteKernelAutoSelector);

if (kernelAutoSelector) {
const cts = new CancellationTokenSource();

try {
const hasEnvironment = await kernelAutoSelector.ensureEnvironmentConfiguredBeforeExecution(
notebook,
cts.token
);

if (!hasEnvironment) {
// User cancelled - do not execute
logger.info(`Execution cancelled: no environment selected for ${getDisplayPath(notebook.uri)}`);
return;
}
} finally {
cts.dispose();
}
}
}

logger.debug(`Handle Execution of Cells ${cells.map((c) => c.index)} for ${getDisplayPath(notebook.uri)}`);
await initializeInteractiveOrNotebookTelemetryBasedOnUserAction(notebook.uri, this.connection);
telemetryTracker?.stop();
Expand Down
Loading
Loading