pr feedback.

Author: Artmann

build/esbuild/build.ts

@@ -320,7 +320,7 @@ async function buildAll() {
         build(
             path.join(extensionFolder, 'src', 'webviews', 'webview-side', 'dataframe-renderer', 'index.ts'),
             path.join(extensionFolder, 'dist', 'webviews', 'webview-side', 'dataframeRenderer', 'dataframeRenderer.js'),
-            { target: 'web', watch: isWatchMode }
+            { target: 'web', watch: watchAll }
         ),
         build(
             path.join(extensionFolder, 'src', 'webviews', 'webview-side', 'variable-view', 'index.tsx'),

src/kernels/execution/cellExecutionMessageHandler.ts

@@ -79,6 +79,14 @@ export function getParentHeaderMsgId(msg: KernelMessage.IMessage): string | unde
     return undefined;
 }
 
+/**
+ * Gets the cell ID, preferring the metadata id from Deepnote blocks,
+ * otherwise falling back to the cell's document URI.
+ */
+function getCellId(cell: NotebookCell): string {
+    return (cell.metadata?.id as string | undefined) || cell.document.uri.toString();
+}
+
 /**
  * Responsible for handling of jupyter messages as a result of execution of individual cells.
  */
@@ -635,7 +643,7 @@ export class CellExecutionMessageHandler implements IDisposable {
             }
         }
         // Use cell metadata id if available (from Deepnote blocks), otherwise fall back to cell's internal id
-        const cellId = (this.cell.metadata?.id as string | undefined) || this.cell.document.uri.toString();
+        const cellId = getCellId(this.cell);
         const cellOutput = cellOutputToVSCCellOutput(output, this.cell.index, cellId);
         const displayId =
             'transient' in output &&
@@ -1006,7 +1014,7 @@ export class CellExecutionMessageHandler implements IDisposable {
         // If the last output is the desired stream type.
         if (this.lastUsedStreamOutput?.stream === msg.content.name) {
             // Use cell metadata id if available (from Deepnote blocks), otherwise fall back to cell's internal id
-            const cellId = (this.cell.metadata?.id as string | undefined) || this.cell.document.uri.toString();
+            const cellId = getCellId(this.cell);
             const output = cellOutputToVSCCellOutput(
                 {
                     output_type: 'stream',
@@ -1022,7 +1030,7 @@ export class CellExecutionMessageHandler implements IDisposable {
             // Replace the current outputs with a single new output.
             const text = concatMultilineString(msg.content.text);
             // Use cell metadata id if available (from Deepnote blocks), otherwise fall back to cell's internal id
-            const cellId = (this.cell.metadata?.id as string | undefined) || this.cell.document.uri.toString();
+            const cellId = getCellId(this.cell);
             const output = cellOutputToVSCCellOutput(
                 {
                     output_type: 'stream',
@@ -1039,7 +1047,7 @@ export class CellExecutionMessageHandler implements IDisposable {
             // Create a new output
             const text = formatStreamText(concatMultilineString(msg.content.text));
             // Use cell metadata id if available (from Deepnote blocks), otherwise fall back to cell's internal id
-            const cellId = (this.cell.metadata?.id as string | undefined) || this.cell.document.uri.toString();
+            const cellId = getCellId(this.cell);
             const output = cellOutputToVSCCellOutput(
                 {
                     output_type: 'stream',
@@ -1167,9 +1175,7 @@ export class CellExecutionMessageHandler implements IDisposable {
             new NotebookCellOutput(outputToBeUpdated.outputItems, outputToBeUpdated.outputContainer.metadata)
         );
         // Use cell metadata id if available (from Deepnote blocks), otherwise fall back to cell's internal id
-        const cellId =
-            (outputToBeUpdated.cell.metadata?.id as string | undefined) ||
-            outputToBeUpdated.cell.document.uri.toString();
+        const cellId = getCellId(outputToBeUpdated.cell);
         const newOutput = cellOutputToVSCCellOutput(
             {
                 ...output,

src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts

@@ -234,7 +234,7 @@ export class DeepnoteInitNotebookRunner {
                 }
 
                 const block = codeBlocks[i];
-                const percentComplete = Math.floor((i / codeBlocks.length) * 100);
+                const percentComplete = Math.min(100, Math.floor(((i + 1) / codeBlocks.length) * 100));
 
                 // Show more detailed progress with percentage
                 progress(

src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts

@@ -12,7 +12,8 @@ import {
     notebooks,
     NotebookController,
     CancellationTokenSource,
-    Disposable
+    Disposable,
+    l10n
 } from 'vscode';
 import { IExtensionSyncActivationService } from '../../platform/activation/types';
 import { IDisposableRegistry } from '../../platform/common/types';
@@ -127,8 +128,10 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
         // Always try to ensure kernel is selected (this will reuse existing controllers)
         // Don't await - let it happen in background so notebook opens quickly
         void this.ensureKernelSelected(notebook).catch((error) => {
-            logger.error(`Failed to auto-select Deepnote kernel for ${getDisplayPath(notebook.uri)}: ${error}`);
-            void window.showErrorMessage(`Failed to load Deepnote kernel: ${error}`);
+            logger.error(`Failed to auto-select Deepnote kernel for ${getDisplayPath(notebook.uri)}`, error);
+            void window.showErrorMessage(
+                l10n.t('Failed to load Deepnote kernel. Please check the output for details.')
+            );
         });
     }
 
@@ -240,7 +243,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
                 logger.info(`Init notebook cancelled for project ${projectId}`);
                 return;
             }
-            logger.error(`Error running init notebook: ${error}`);
+            logger.error('Error running init notebook', error);
             // Continue anyway - don't block user if init fails
         } finally {
             // Always clean up the CTS and event listeners
@@ -253,7 +256,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
         return window.withProgress(
             {
                 location: ProgressLocation.Notification,
-                title: 'Loading Deepnote Kernel',
+                title: l10n.t('Loading Deepnote Kernel'),
                 cancellable: true
             },
             async (progress, progressToken) => {
@@ -277,7 +280,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
                                 notebook.uri
                             )}`
                         );
-                        progress.report({ message: 'Reusing existing kernel...' });
+                        progress.report({ message: l10n.t('Reusing existing kernel...') });
 
                         // Ensure server is registered with the provider (it might have been unregistered on close)
                         if (connectionMetadata.serverInfo) {
@@ -317,7 +320,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
 
                     // No existing controller, so create a new one
                     logger.info(`Creating new Deepnote kernel for ${getDisplayPath(notebook.uri)}`);
-                    progress.report({ message: 'Setting up Deepnote kernel...' });
+                    progress.report({ message: l10n.t('Setting up Deepnote kernel...') });
 
                     // Check if Python extension is installed
                     if (!this.pythonExtensionChecker.isPythonExtensionInstalled) {
@@ -327,7 +330,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
                     }
 
                     // Get active Python interpreter
-                    progress.report({ message: 'Finding Python interpreter...' });
+                    progress.report({ message: l10n.t('Finding Python interpreter...') });
                     const interpreter = await this.interpreterService.getActiveInterpreter(notebook.uri);
                     if (!interpreter) {
                         logger.warn(
@@ -339,7 +342,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
                     logger.info(`Using base interpreter: ${getDisplayPath(interpreter.uri)}`);
 
                     // Ensure deepnote-toolkit is installed in a venv and get the venv interpreter
-                    progress.report({ message: 'Installing Deepnote toolkit...' });
+                    progress.report({ message: l10n.t('Installing Deepnote toolkit...') });
                     const venvInterpreter = await this.toolkitInstaller.ensureInstalled(
                         interpreter,
                         baseFileUri,
@@ -353,7 +356,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
                     logger.info(`Deepnote toolkit venv ready at: ${getDisplayPath(venvInterpreter.uri)}`);
 
                     // Start the Deepnote server using the venv interpreter
-                    progress.report({ message: 'Starting Deepnote server...' });
+                    progress.report({ message: l10n.t('Starting Deepnote server...') });
                     const serverInfo = await this.serverStarter.getOrStartServer(
                         venvInterpreter,
                         baseFileUri,
@@ -375,7 +378,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
                     this.notebookServerHandles.set(notebookKey, serverProviderHandle.handle);
 
                     // Connect to the server and get available kernel specs
-                    progress.report({ message: 'Connecting to kernel...' });
+                    progress.report({ message: l10n.t('Connecting to kernel...') });
                     const connectionInfo = createJupyterConnectionInfo(
                         serverProviderHandle,
                         {
@@ -409,10 +412,15 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
 
                         if (!kernelSpec) {
                             logger.warn(
-                                `⚠️ Venv kernel spec '${expectedKernelName}' not found! Falling back to generic Python kernel.`
+                                l10n.t(
+                                    "⚠️ Venv kernel spec '{expectedKernelName}' not found! Falling back to generic Python kernel.",
+                                    { expectedKernelName }
+                                )
                             );
                             logger.warn(
-                                `This may cause import errors if packages are installed to the venv but kernel uses system Python.`
+                                l10n.t(
+                                    'This may cause import errors if packages are installed to the venv but kernel uses system Python.'
+                                )
                             );
                             kernelSpec =
                                 kernelSpecs.find((s) => s.language === 'python') ||
@@ -429,7 +437,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
                         await disposeAsync(sessionManager);
                     }
 
-                    progress.report({ message: 'Finalizing kernel setup...' });
+                    progress.report({ message: l10n.t('Finalizing kernel setup...') });
                     const newConnectionMetadata = DeepnoteKernelConnectionMetadata.create({
                         interpreter,
                         kernelSpec,
@@ -467,7 +475,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
 
                     if (project) {
                         // Create requirements.txt first (needs to be ready for init notebook)
-                        progress.report({ message: 'Creating requirements.txt...' });
+                        progress.report({ message: l10n.t('Creating requirements.txt...') });
                         await this.requirementsHelper.createRequirementsFile(project, progressToken);
                         logger.info(`Created requirements.txt for project ${projectId}`);
 
@@ -504,7 +512,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
                     controller.controller.updateNotebookAffinity(notebook, NotebookControllerAffinity.Preferred);
 
                     logger.info(`Successfully auto-selected Deepnote kernel for ${getDisplayPath(notebook.uri)}`);
-                    progress.report({ message: 'Kernel ready!' });
+                    progress.report({ message: l10n.t('Kernel ready!') });
 
                     // Dispose the loading controller once the real one is ready
                     const loadingController = this.loadingControllers.get(notebookKey);
@@ -514,7 +522,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
                         logger.info(`Disposed loading controller for ${notebookKey}`);
                     }
                 } catch (ex) {
-                    logger.error(`Failed to auto-select Deepnote kernel: ${ex}`);
+                    logger.error('Failed to auto-select Deepnote kernel', ex);
                     throw ex;
                 }
             }
@@ -526,7 +534,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
         const loadingController = notebooks.createNotebookController(
             `deepnote-loading-${notebookKey}`,
             DEEPNOTE_NOTEBOOK_TYPE,
-            'Loading Deepnote Kernel...'
+            l10n.t('Loading Deepnote Kernel...')
         );
 
         // Set it as the preferred controller immediately

src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts

@@ -1,18 +1,23 @@
 import { inject, injectable } from 'inversify';
-import { workspace, CancellationToken, window } from 'vscode';
+import { workspace, CancellationToken, window, Uri, l10n } from 'vscode';
 import * as fs from 'fs';
-import * as path from '../../platform/vscode-path/path';
+
 import type { DeepnoteProject } from './deepnoteTypes';
 import { ILogger } from '../../platform/logging/types';
 import { IPersistentStateFactory } from '../../platform/common/types';
 
 const DONT_ASK_OVERWRITE_REQUIREMENTS_KEY = 'DEEPNOTE_DONT_ASK_OVERWRITE_REQUIREMENTS';
 
+export const IDeepnoteRequirementsHelper = Symbol('IDeepnoteRequirementsHelper');
+export interface IDeepnoteRequirementsHelper {
+    createRequirementsFile(project: DeepnoteProject, token: CancellationToken): Promise<void>;
+}
+
 /**
  * Helper class for creating requirements.txt files from Deepnote project settings.
  */
 @injectable()
-export class DeepnoteRequirementsHelper {
+export class DeepnoteRequirementsHelper implements IDeepnoteRequirementsHelper {
     constructor(
         @inject(ILogger) private readonly logger: ILogger,
         @inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory
@@ -36,6 +41,17 @@ export class DeepnoteRequirementsHelper {
                 return;
             }
 
+            // Validate and normalize requirements: ensure they are valid strings, trim them, and remove empty entries
+            const normalizedRequirements = requirements
+                .filter((req) => typeof req === 'string') // Keep only string entries
+                .map((req) => req.trim()) // Trim whitespace
+                .filter((req) => req.length > 0); // Remove empty strings
+
+            if (normalizedRequirements.length === 0) {
+                this.logger.info(`No valid requirements found in project ${project.project.id}`);
+                return;
+            }
+
             // Get the workspace folder to determine where to create the requirements.txt file
             const workspaceFolders = workspace.workspaceFolders;
             if (!workspaceFolders || workspaceFolders.length === 0) {
@@ -48,11 +64,11 @@ export class DeepnoteRequirementsHelper {
                 return;
             }
 
-            const workspaceRoot = workspaceFolders[0].uri.fsPath;
-            const requirementsPath = path.join(workspaceRoot, 'requirements.txt');
+            // Use Uri.joinPath to build the filesystem path using the Uri API
+            const requirementsPath = Uri.joinPath(workspaceFolders[0].uri, 'requirements.txt').fsPath;
 
-            // Convert requirements array to text format first
-            const requirementsText = requirements.join('\n') + '\n';
+            // Convert normalized requirements array to text format
+            const requirementsText = normalizedRequirements.join('\n') + '\n';
 
             // Check if requirements.txt already exists
             const fileExists = await fs.promises
@@ -77,12 +93,14 @@ export class DeepnoteRequirementsHelper {
 
                 if (!dontAskState.value) {
                     // User hasn't chosen "Don't Ask Again", so prompt them
-                    const yes = 'Yes';
-                    const no = 'No';
-                    const dontAskAgain = "Don't Ask Again";
+                    const yes = l10n.t('Yes');
+                    const no = l10n.t('No');
+                    const dontAskAgain = l10n.t("Don't Ask Again");
 
                     const response = await window.showWarningMessage(
-                        `A requirements.txt file already exists in this workspace. Do you want to override it with requirements from your Deepnote project?`,
+                        l10n.t(
+                            'A requirements.txt file already exists in this workspace. Do you want to override it with requirements from your Deepnote project?'
+                        ),
                         { modal: true },
                         yes,
                         no,
@@ -131,15 +149,10 @@ export class DeepnoteRequirementsHelper {
             }
 
             this.logger.info(
-                `Created requirements.txt with ${requirements.length} dependencies at ${requirementsPath}`
+                `Created requirements.txt with ${normalizedRequirements.length} dependencies at ${requirementsPath}`
             );
         } catch (error) {
             this.logger.error(`Error creating requirements.txt:`, error);
         }
     }
 }
-
-export const IDeepnoteRequirementsHelper = Symbol('IDeepnoteRequirementsHelper');
-export interface IDeepnoteRequirementsHelper {
-    createRequirementsFile(project: DeepnoteProject, token: CancellationToken): Promise<void>;
-}