Code review changes for SQL Notebooks

extensions/mssql/l10n/bundle.l10n.json

@@ -1979,9 +1979,9 @@
     "Keep in query pane": "Keep in query pane",
     "Max row count for filtering/sorting has been exceeded. To update it, navigate to User Settings and change the setting: mssql.resultsGrid.inMemoryDataProcessingThreshold": "Max row count for filtering/sorting has been exceeded. To update it, navigate to User Settings and change the setting: mssql.resultsGrid.inMemoryDataProcessingThreshold",
     "New Deployment": "New Deployment",
-    "SQL: Not connected": "SQL: Not connected",
-    "SQL Notebooks: click to change database": "SQL Notebooks: click to change database",
-    "SQL Notebooks: click to connect": "SQL Notebooks: click to connect",
+    "MSSQL: Not connected": "MSSQL: Not connected",
+    "MSSQL: Click to change database": "MSSQL: Click to change database",
+    "MSSQL: Click to connect": "MSSQL: Click to connect",
     "Connection failed": "Connection failed",
     "Query execution failed": "Query execution failed",
     "No active notebook.": "No active notebook.",
@@ -2020,15 +2020,15 @@
     "(current)": "(current)",
     "Click to change database": "Click to change database",
     "Connect to SQL Server": "Connect to SQL Server",
-    "SQL Notebook connected to {0}/{0} is the connection label": {
-        "message": "SQL Notebook connected to {0}",
+    "MSSQL Notebook connected to {0}/{0} is the connection label": {
+        "message": "MSSQL Notebook connected to {0}",
         "comment": ["{0} is the connection label"]
     },
     "Error: {0}/{0} is the error message": {
         "message": "Error: {0}",
         "comment": ["{0} is the error message"]
     },
-    "Query execution was cancelled.": "Query execution was cancelled.",
+    "Query execution was canceled.": "Query execution was canceled.",
     "Execute SQL against SQL Server / Azure SQL": "Execute SQL against SQL Server / Azure SQL",
     "Not connected": "Not connected",
     "Error loading; refresh to try again": "Error loading; refresh to try again",

extensions/mssql/scripts/bundle-notebook-renderer.js

@@ -3,7 +3,20 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+// This is a separate build script from bundle-reactviews.js because the notebook
+// renderer has fundamentally incompatible esbuild requirements:
+//
+// 1. CSS inlining: Notebook renderers run in an isolated iframe that only loads the
+//    JS entrypoint — extracted CSS files are never loaded. We use a custom plugin to
+//    inline CSS as <style> elements, whereas webview panels use esbuild's native .css
+//    loader with separate files loaded via <link> tags in HTML templates.
+//
+// 2. No code splitting: The renderer must be a single self-contained file
+//    (splitting: false + outfile), whereas webviews use splitting: true + outdir
+//    to share chunks. These options are mutually exclusive in esbuild.
+
 const fs = require("fs");
+const path = require("path");
 const logger = require("../../../scripts/terminal-logger");
 const { esbuildProblemMatcherPlugin, build, watch } = require("./esbuild-utils");
 
@@ -14,12 +27,35 @@ const isWatch = args.includes("--watch") || args.includes("-w");
 
 // Plugin to inline CSS into the JS bundle as <style> elements.
 // Notebook renderers run in an isolated iframe that only loads the JS entrypoint,
-// so extracted CSS files would never be loaded.
+// so extracted CSS files would never be loaded. Asset url() references (e.g. SVG
+// icons) are resolved to data URIs so they work inside the iframe.
 const inlineCssPlugin = {
     name: "inline-css",
     setup(build) {
         build.onLoad({ filter: /\.css$/ }, async (args) => {
-            const css = await fs.promises.readFile(args.path, "utf-8");
+            let css = await fs.promises.readFile(args.path, "utf-8");
+            const cssDir = path.dirname(args.path);
+
+            // Resolve url() references to local assets as inline data URIs.
+            const urlPattern = /url\(["']?([^"')]+\.(svg|png|gif))["']?\)/g;
+            const replacements = [];
+            let match;
+            while ((match = urlPattern.exec(css)) !== null) {
+                const assetPath = path.resolve(cssDir, match[1]);
+                try {
+                    const content = await fs.promises.readFile(assetPath);
+                    const ext = match[2];
+                    const mime = ext === "svg" ? "image/svg+xml" : `image/${ext}`;
+                    const dataUrl = `data:${mime};base64,${content.toString("base64")}`;
+                    replacements.push({ original: match[0], replacement: `url("${dataUrl}")` });
+                } catch {
+                    // Asset not found — leave the url() as-is
+                }
+            }
+            for (const { original, replacement } of replacements) {
+                css = css.split(original).join(replacement);
+            }
+
             return {
                 contents: `
                     (function() {

extensions/mssql/src/constants/locConstants.ts

@@ -640,11 +640,9 @@ export let newDeployment = l10n.t("New Deployment");
 
 export class Notebooks {
     // Status bar
-    public static statusBarNotConnected = l10n.t("SQL: Not connected");
-    public static statusBarClickToChangeDatabase = l10n.t(
-        "SQL Notebooks: click to change database",
-    );
-    public static statusBarClickToConnect = l10n.t("SQL Notebooks: click to connect");
+    public static statusBarNotConnected = l10n.t("MSSQL: Not connected");
+    public static statusBarClickToChangeDatabase = l10n.t("MSSQL: Click to change database");
+    public static statusBarClickToConnect = l10n.t("MSSQL: Click to connect");
 
     // Errors
     public static connectionFailed = l10n.t("Connection failed");
@@ -715,7 +713,7 @@ export class Notebooks {
     // Info
     public static notebookConnectedTo(label: string) {
         return l10n.t({
-            message: "SQL Notebook connected to {0}",
+            message: "MSSQL Notebook connected to {0}",
             args: [label],
             comment: ["{0} is the connection label"],
         });
@@ -729,7 +727,7 @@ export class Notebooks {
     }
 
     // Cancellation
-    public static executionCancelled = l10n.t("Query execution was cancelled.");
+    public static executionCanceled = l10n.t("Query execution was canceled.");
 
     // Controller
     public static controllerDescription = l10n.t("Execute SQL against SQL Server / Azure SQL");

extensions/mssql/src/controllers/queryNotificationHandler.ts

@@ -7,7 +7,6 @@
  *  Class for handler and distributing notification coming from the
  *  service layer
  */
-import QueryRunner from "./queryRunner";
 import SqlToolsServiceClient from "../languageservice/serviceclient";
 import {
     QueryExecuteCompleteNotification,
@@ -26,21 +25,31 @@ import {
 } from "../models/contracts/queryExecute";
 import { NotificationHandler } from "vscode-languageclient";
 
+export interface IQueryEventHandler {
+    handleQueryComplete(result: QueryExecuteCompleteNotificationResult): void;
+    handleBatchStart(result: QueryExecuteBatchNotificationParams): void;
+    handleBatchComplete(result: QueryExecuteBatchNotificationParams): void;
+    handleResultSetAvailable(result: QueryExecuteResultSetAvailableNotificationParams): void;
+    handleResultSetUpdated(result: QueryExecuteResultSetUpdatedNotificationParams): void;
+    handleResultSetComplete(result: QueryExecuteResultSetCompleteNotificationParams): void;
+    handleMessage(result: QueryExecuteMessageParams): void;
+}
+
 export class QueryNotificationHandler {
     private static _instance: QueryNotificationHandler;
     static get instance() {
         return (this._instance ??= new QueryNotificationHandler());
     }
 
     // public for testing only
-    public _queryRunners = new Map<string, QueryRunner>();
+    public _queryRunners = new Map<string, IQueryEventHandler>();
     constructor() {
         this.initialize();
     }
 
     // Registers queryRunners with their uris to distribute notifications.
     // public for testing only
-    public registerRunner(runner: QueryRunner, uri: string): void {
+    public registerRunner(runner: IQueryEventHandler, uri: string): void {
         this._queryRunners.set(uri, runner);
     }
 
@@ -82,7 +91,7 @@ export class QueryNotificationHandler {
     }
 
     private makeHandler<T extends { ownerUri: string }>(
-        invoke: (r: QueryRunner, e: T) => void,
+        invoke: (r: IQueryEventHandler, e: T) => void,
         onComplete = false,
     ): NotificationHandler<T> {
         return (e: T) => {

extensions/mssql/src/models/contracts/queryExecute.ts

@@ -131,6 +131,23 @@ export class QueryExecuteStatementParams {
     executionPlanOptions?: ExecutionPlanOptions;
 }
 
+/**
+ * Unlike "query/simpleexecute", which returns only the first result set as a flat response,
+ * "query/executeString" uses the full query execution pipeline and supports multiple batches.
+ * Use this when the query may contain multiple statements (e.g. GO-separated batches) and
+ * all result sets need to be retrieved.
+ */
+export namespace QueryExecuteStringRequest {
+    export const type = new RequestType<QueryExecuteStringParams, QueryExecuteResult, void, void>(
+        "query/executeString",
+    );
+}
+
+export class QueryExecuteStringParams {
+    ownerUri: string;
+    query: string;
+}
+
 export class QueryExecuteResult {}
 
 export class ExecutionPlanOptions {

extensions/mssql/src/notebooks/notebookCodeLensProvider.ts

@@ -15,8 +15,23 @@ import { NotebookConnectionManager } from "./notebookConnectionManager";
  * The MSSQL extension provides its own code lens via SqlCodeLensProvider
  * registered with { language: "sql" }. That provider auto-connects cells to
  * _lastActiveConnectionInfo (which may be stale/wrong for notebooks).
+ *
+ * Why "last connection" doesn't work for notebooks: SqlDocumentService updates
+ * _lastActiveConnectionInfo in onDidChangeActiveTextEditor, which fires from
+ * vscode.window.activeTextEditor. When a notebook is focused, activeTextEditor
+ * is undefined (VS Code uses activeNotebookEditor instead), so the "last
+ * connection" never reflects the notebook's connection. When a notebook cell
+ * document opens, SqlDocumentService auto-connects it to whatever the last
+ * *text editor* connection was — which is unrelated to the notebook.
+ *
  * SqlCodeLensProvider defers to this provider for notebook cells by checking
  * the document URI scheme.
+ *
+ * This is intentionally separate from SqlCodeLensProvider rather than sharing
+ * a base class. The two have different data sources (ConnectionManager vs
+ * NotebookConnectionManager), different lookup logic (document URI vs
+ * cell→notebook mapping), different states (connecting/error vs simple
+ * connected/not-connected), and trigger different commands.
  */
 export class NotebookCodeLensProvider implements vscode.CodeLensProvider, vscode.Disposable {
     private readonly _onDidChangeCodeLenses = new vscode.EventEmitter<void>();