Merge pull request #330 from UKGovernmentBEIS/feature/vscode-330

Feature/vscode 330

tools/vscode/CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## 0.3.30
+
+- Further refinement to Inspect View behavior when a task is complete. We will now refresh the viewer without bringing it the foreground when it is already displaying and a task completes. This will ensure that focus is handled correctly in the editor. Use `cmd+shift+L` to bring the viewer to the foreground (or show it if it isn't showing).
+- Rename 'Log View Auto' to 'Open Log View' since this better reflects the setting behavior (as it now controls whether the view is initially opened when a task is completed).
+
 ## 0.3.29
 
 - Improve behavior of log viewer when a task completes and the view is already displaying

tools/vscode/package.json

@@ -7,7 +7,7 @@
   "author": {
     "name": "UK AI Safety Institute"
   },
-  "version": "0.3.29",
+  "version": "0.3.30",
   "license": "MIT",
   "homepage": "https://inspect.ai-safety-institute.org.uk/",
   "repository": {
@@ -160,10 +160,10 @@
     "configuration": {
       "title": "Inspect AI",
       "properties": {
-        "inspect_ai.logViewAuto": {
+        "inspect_ai.openLogView": {
           "type": "boolean",
           "default": true,
-          "description": "Automatically show the Inspect log view when evaluations in the workspace complete."
+          "description": "Open the Inspect log view when evaluations in the workspace complete."
         },
         "inspect_ai.logViewType": {
           "type": "string",

tools/vscode/src/components/webview.ts

@@ -73,11 +73,13 @@ export class InspectWebviewManager<T extends InspectWebview<S>, S> {
     }
   }
 
-  public revealWebview() {
+  public revealWebview(preserveEditorFocus: boolean) {
     if (this.activeView_) {
       this.activeView_.reveal();
       this.resolveOnShow();
-      this.preserveEditorFocus();
+      if (preserveEditorFocus) {
+        this.preserveEditorFocus();
+      }
     }
   }
 

tools/vscode/src/providers/logview/commands.ts

@@ -62,7 +62,7 @@ class OpenInInspectViewCommand implements Command {
       const logState = await inspectLogInfo(fileUri);
       if (logState.hasEvalKey && logState.hasStatusKey) {
         if (logState.status !== "started") {
-          await this.manager_.showLogFile(fileUri);
+          await this.manager_.showLogFile(fileUri, "activate");
         } else {
           // This is a running log, we don't yet support viewing these
           const close: MessageItem = { title: "Close" };

tools/vscode/src/providers/logview/logview-file-watcher.ts

@@ -33,30 +33,43 @@ export class LogViewFileWatcher implements Disposable {
           const contents = readFileSync(evalSignalFile, { encoding: "utf-8" });
 
           // Parse the eval signal file result
-          withMinimumInspectVersion(kInspectChangeEvalSignalVersion, () => {
-            // 0.3.10- or later
-            const contentsObj = JSON.parse(contents) as { location: string, workspace_id?: string };
-            evalLogPath = contentsObj.location;
-            workspaceId = contentsObj.workspace_id;
-          }, () => {
-            // 0.3.8 or earlier
-            evalLogPath = contents;
-          });
+          withMinimumInspectVersion(
+            kInspectChangeEvalSignalVersion,
+            () => {
+              // 0.3.10- or later
+              const contentsObj = JSON.parse(contents) as {
+                location: string;
+                workspace_id?: string;
+              };
+              evalLogPath = contentsObj.location;
+              workspaceId = contentsObj.workspace_id;
+            },
+            () => {
+              // 0.3.8 or earlier
+              evalLogPath = contents;
+            }
+          );
 
           if (evalLogPath) {
-            if (!workspaceId || workspaceId === this.workspaceStateManager_.getWorkspaceInstance()) {
-
-              log.appendLine(`New log: ${workspaceRelativePath(toAbsolutePath(evalLogPath))}`);
-              if (settingsMgr_.getSettings().logViewAuto) {
-                // Show the log file
-                this.logviewManager_.showLogFile(Uri.parse(evalLogPath)).catch(async (err: Error) => {
-                  await showError("Unable to preview log file - failed to start Inspect View", err);
-                });
-              } else {
-                this.logviewManager_.showLogFileIfViewerOpen(Uri.parse(evalLogPath)).catch(async (err: Error) => {
-                  await showError("Unable to preview log file - failed to update the current view", err);
+            if (
+              !workspaceId ||
+              workspaceId === this.workspaceStateManager_.getWorkspaceInstance()
+            ) {
+              log.appendLine(
+                `New log: ${workspaceRelativePath(toAbsolutePath(evalLogPath))}`
+              );
+              // Show the log file
+              const openAction = settingsMgr_.getSettings().openLogView
+                ? "open"
+                : undefined;
+              this.logviewManager_
+                .showLogFile(Uri.parse(evalLogPath), openAction)
+                .catch(async (err: Error) => {
+                  await showError(
+                    "Unable to preview log file - failed to start Inspect View",
+                    err
+                  );
                 });
-              }
             }
           }
         }
@@ -72,4 +85,4 @@ export class LogViewFileWatcher implements Disposable {
       clearTimeout(this.watchInterval_);
     }
   }
-}
+}

tools/vscode/src/providers/logview/logview-link-provider.ts

@@ -46,7 +46,7 @@ export const logviewTerminalLinkProvider = (manager: InspectLogviewManager) => {
       // Resolve the clicked link into a complete Uri to the file
       const logUri = await resolveLogFile(link.data);
       if (logUri) {
-        manager.showLogFile(logUri).catch(async (err: Error) => {
+        manager.showLogFile(logUri, "activate").catch(async (err: Error) => {
           await showError("Failed to preview log file - failed to start Inspect View", err);
         });
       } else {

tools/vscode/src/providers/logview/logview-manager.ts

@@ -5,6 +5,7 @@ import { WorkspaceEnvManager } from "../workspace/workspace-env-provider";
 import { activeWorkspaceFolder } from "../../core/workspace";
 import { workspacePath } from "../../core/path";
 import { kInspectEnvValues } from "../inspect/inspect-constants";
+import { join } from "path";
 
 export class InspectLogviewManager {
   constructor(
@@ -13,7 +14,7 @@ export class InspectLogviewManager {
     private readonly envMgr_: WorkspaceEnvManager
   ) { }
 
-  public async showLogFile(logFile: Uri) {
+  public async showLogFile(logFile: Uri, activation?: "open" | "activate") {
     const settings = this.settingsMgr_.getSettings();
     if (settings.logViewType === "text" && logFile.scheme === "file") {
       await workspace.openTextDocument(logFile).then(async (doc) => {
@@ -23,14 +24,10 @@ export class InspectLogviewManager {
         });
       });
     } else {
-      await this.webViewManager_.showLogFile(logFile);
+      await this.webViewManager_.showLogFile(logFile, activation);
     }
   }
 
-  public async showLogFileIfViewerOpen(logfile: Uri) {
-    await this.webViewManager_.showLogFileIfOpen(logfile);
-  }
-
   public async showInspectView() {
     // See if there is a log dir
     const envVals = this.envMgr_.getValues();
@@ -42,12 +39,13 @@ export class InspectLogviewManager {
       log_uri = Uri.parse(env_log, true);
     } catch {
       // This isn't a uri, bud
-      log_uri = Uri.file(workspacePath(env_log).path);
+      const logDir = join(workspacePath(env_log).path, "logs");
+      log_uri = Uri.file(logDir);
     }
 
     // Show the log view for the log dir (or the workspace)
     const log_dir = log_uri || activeWorkspaceFolder().uri;
-    await this.webViewManager_.showLogview({ log_dir });
+    await this.webViewManager_.showLogview({ log_dir }, "activate");
   }
 
   public viewColumn() {

tools/vscode/src/providers/logview/logview-webview.ts

@@ -78,11 +78,11 @@ export class InspectLogviewWebviewManager extends InspectWebviewManager<
   }
   private activeLogDir_: Uri | null = null;
 
-  public async showLogFile(uri: Uri) {
+  public async showLogFile(uri: Uri, activation?: "open" | "activate") {
     // Get the directory name using posix path methods
     const log_dir = dirname(uri);
 
-    await this.showLogview({ log_file: uri, log_dir });
+    await this.showLogview({ log_file: uri, log_dir }, activation);
   }
 
   public async showLogFileIfOpen(uri: Uri) {
@@ -99,24 +99,31 @@ export class InspectLogviewWebviewManager extends InspectWebviewManager<
     }
   }
 
-  public async showLogview(state: LogviewState) {
-    if (state.background_refresh) {
-      // Determine whether we are showing a log viewer for this directory
-      // If we aren't close the log viewer so a fresh one can be opened.
-      if (state.log_file) {
-        // Update the view state
-        this.updateViewState(state);
-
-        // Signal the viewer to either perform a background refresh
-        // or to check whether the view is focused and call us back to
-        // display a log file
-        await this.activeView_?.backgroundUpdate(
-          state.log_file.path,
-          state.log_dir.toString()
-        );
-      }
-    } else {
-      this.displayLogFile(state);
+  public async showLogview(
+    state: LogviewState,
+    activation?: "open" | "activate"
+  ) {
+    switch (activation) {
+      case "open":
+        await this.displayLogFile(state, activation);
+        break;
+      case "activate":
+        await this.displayLogFile(state, activation);
+        break;
+      default:
+        // No activation, just refresh this in the background
+        if (this.isVisible() && state.log_file) {
+          this.updateViewState(state);
+
+          // Signal the viewer to either perform a background refresh
+          // or to check whether the view is focused and call us back to
+          // display a log file
+          await this.activeView_?.backgroundUpdate(
+            state.log_file.path,
+            state.log_dir.toString()
+          );
+        }
+        return;
     }
   }
 
@@ -130,7 +137,10 @@ export class InspectLogviewWebviewManager extends InspectWebviewManager<
     }
   }
 
-  public displayLogFile(state: LogviewState) {
+  public async displayLogFile(
+    state: LogviewState,
+    activation?: "open" | "activate"
+  ) {
     // Determine whether we are showing a log viewer for this directory
     // If we aren't close the log viewer so a fresh one can be opened.
     if (
@@ -150,7 +160,7 @@ export class InspectLogviewWebviewManager extends InspectWebviewManager<
 
     // Ensure that we send the state once the view is loaded
     this.setOnShow(() => {
-      this.updateVisibleView().catch(() => {});
+      this.updateVisibleView().catch(() => { });
     });
 
     // If the view is closed, clear the state
@@ -161,12 +171,21 @@ export class InspectLogviewWebviewManager extends InspectWebviewManager<
 
     // Actually reveal or show the webview
     if (this.activeView_) {
-      this.revealWebview();
+      if (activation === "activate") {
+        this.revealWebview(activation !== "activate");
+      } else if (state.log_file) {
+        await this.activeView_?.backgroundUpdate(
+          state.log_file.path,
+          state.log_dir.toString()
+        );
+      }
     } else {
-      this.showWebview(state, {
-        preserveFocus: true,
-        viewColumn: ViewColumn.Beside,
-      });
+      if (activation) {
+        this.showWebview(state, {
+          preserveFocus: activation !== "activate",
+          viewColumn: ViewColumn.Beside,
+        });
+      }
     }
 
     // TODO: there is probably a better way to handle this
@@ -212,7 +231,7 @@ class InspectLogviewWebview extends InspectWebview<LogviewState> {
     super(context, state, webviewPanel);
     this._register(
       this._webviewPanel.webview.onDidReceiveMessage(
-        async (e: { type: string; url: string; [key: string]: unknown }) => {
+        async (e: { type: string; url: string;[key: string]: unknown }) => {
           switch (e.type) {
             case "openExternal":
               try {
@@ -251,9 +270,8 @@ class InspectLogviewWebview extends InspectWebview<LogviewState> {
                   const state: LogviewState = {
                     log_file: Uri.parse(e.url),
                     log_dir: Uri.parse(e.log_dir as string),
-                    background_refresh: false,
                   };
-                  this._manager.displayLogFile(state);
+                  await this._manager.displayLogFile(state, "open");
                 } else {
                   await showError(
                     "Unable to display log file because of a missing log_dir or manager. This is an unexpected error, please report it."
@@ -339,8 +357,8 @@ class InspectLogviewWebview extends InspectWebview<LogviewState> {
       const stateScript =
         state && state.log_file
           ? `<script id="logview-state" type="application/json">${JSON.stringify(
-              stateMsg
-            )}</script>`
+            stateMsg
+          )}</script>`
           : "";
 
       // decorate the html tag
@@ -351,16 +369,11 @@ class InspectLogviewWebview extends InspectWebview<LogviewState> {
         "<head>\n",
         `<head>
           <meta name="inspect-extension:version" content="${this.getExtensionVersion()}">
-    <meta http-equiv="Content-Security-Policy" content="default-src 'none'; img-src ${
-      this._webviewPanel.webview.cspSource
-    } data:; font-src ${
-          this._webviewPanel.webview.cspSource
-        } data:; style-src ${
-          this._webviewPanel.webview.cspSource
-        } 'unsafe-inline'; worker-src 'self' ${
-          this._webviewPanel.webview.cspSource
-        } blob:; script-src 'nonce-${nonce}' 'unsafe-eval'; connect-src ${
-          this._webviewPanel.webview.cspSource
+    <meta http-equiv="Content-Security-Policy" content="default-src 'none'; img-src ${this._webviewPanel.webview.cspSource
+        } data:; font-src ${this._webviewPanel.webview.cspSource
+        } data:; style-src ${this._webviewPanel.webview.cspSource
+        } 'unsafe-inline'; worker-src 'self' ${this._webviewPanel.webview.cspSource
+        } blob:; script-src 'nonce-${nonce}' 'unsafe-eval'; connect-src ${this._webviewPanel.webview.cspSource
         };">
     ${stateScript}
     ${overrideCssHtml}

tools/vscode/src/providers/settings/inspect-settings.ts

@@ -2,14 +2,14 @@ import { workspace } from "vscode";
 
 // Inspect Settings
 export interface InspectSettings {
-  logViewAuto: boolean;
+  openLogView: boolean;
   logViewType: InspectLogViewStyle;
 }
 export type InspectLogViewStyle = "html" | "text";
 
 // Settings namespace and constants
 const kInspectConfigSection = "inspect_ai";
-const kInspectConfigLogViewAuto = "logViewAuto";
+const kInspectConfigOpenLogView = "openLogView";
 const kInspectConfigLogViewType = "logViewType";
 
 // Manages the settings for the inspect extension
@@ -25,7 +25,7 @@ export class InspectSettingsManager {
       }
     });
   }
-  private settings_ : InspectSettings | undefined;
+  private settings_: InspectSettings | undefined;
 
   // get the current settings values
   getSettings(): InspectSettings {
@@ -40,11 +40,11 @@ export class InspectSettingsManager {
     const configuration = workspace.getConfiguration(kInspectConfigSection);
     const logViewType =
       configuration.get<InspectLogViewStyle>(kInspectConfigLogViewType) || "html";
-    const logViewAuto = configuration.get<boolean>(kInspectConfigLogViewAuto);
+    const openLogView = configuration.get<boolean>(kInspectConfigOpenLogView);
     return {
       logViewType,
-      logViewAuto: logViewAuto !== undefined ? logViewAuto : true,
+      openLogView: openLogView !== undefined ? openLogView : true,
     };
   }
-  
+
 }