From c12eb0eee9271b1316cc6820b180ed6625c36a34 Mon Sep 17 00:00:00 2001 From: Charles Teague Date: Thu, 5 Sep 2024 11:56:30 -0400 Subject: [PATCH 1/3] Improve Task Complete Workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a task completes we will now no longer bring a viewer to the foreground (unless we’re opening it for the first time). In the case that a viewer is already open, we will just get it into the correct state (respecting its focus to determine whether to fully display the new log or whether to just reload the log list so the log is available in the sidebar). This is designed to reduce focus stealing. --- tools/vscode/CHANGELOG.md | 4 + tools/vscode/src/components/webview.ts | 6 +- .../vscode/src/providers/logview/commands.ts | 2 +- .../providers/logview/logview-file-watcher.ts | 57 ++++++----- .../logview/logview-link-provider.ts | 2 +- .../src/providers/logview/logview-manager.ts | 14 ++- .../src/providers/logview/logview-webview.ts | 97 +++++++++++-------- 7 files changed, 106 insertions(+), 76 deletions(-) diff --git a/tools/vscode/CHANGELOG.md b/tools/vscode/CHANGELOG.md index c14a82bb5..abd298f6c 100644 --- a/tools/vscode/CHANGELOG.md +++ b/tools/vscode/CHANGELOG.md @@ -1,5 +1,9 @@ # 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). + ## 0.3.29 - Improve behavior of log viewer when a task completes and the view is already displaying diff --git a/tools/vscode/src/components/webview.ts b/tools/vscode/src/components/webview.ts index 621c72a16..7e2c195a0 100644 --- a/tools/vscode/src/components/webview.ts +++ b/tools/vscode/src/components/webview.ts @@ -73,11 +73,13 @@ export class InspectWebviewManager, S> { } } - public revealWebview() { + public revealWebview(preserveEditorFocus: boolean) { if (this.activeView_) { this.activeView_.reveal(); this.resolveOnShow(); - this.preserveEditorFocus(); + if (preserveEditorFocus) { + this.preserveEditorFocus(); + } } } diff --git a/tools/vscode/src/providers/logview/commands.ts b/tools/vscode/src/providers/logview/commands.ts index fe5c50975..d11359113 100644 --- a/tools/vscode/src/providers/logview/commands.ts +++ b/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" }; diff --git a/tools/vscode/src/providers/logview/logview-file-watcher.ts b/tools/vscode/src/providers/logview/logview-file-watcher.ts index 86f18c9ff..4abb4765a 100644 --- a/tools/vscode/src/providers/logview/logview-file-watcher.ts +++ b/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().logViewAuto + ? "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_); } } -} \ No newline at end of file +} diff --git a/tools/vscode/src/providers/logview/logview-link-provider.ts b/tools/vscode/src/providers/logview/logview-link-provider.ts index 9eb1760a3..9a66d7455 100644 --- a/tools/vscode/src/providers/logview/logview-link-provider.ts +++ b/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 { diff --git a/tools/vscode/src/providers/logview/logview-manager.ts b/tools/vscode/src/providers/logview/logview-manager.ts index 480abbcc5..c20194c44 100644 --- a/tools/vscode/src/providers/logview/logview-manager.ts +++ b/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() { diff --git a/tools/vscode/src/providers/logview/logview-webview.ts b/tools/vscode/src/providers/logview/logview-webview.ts index b09589aa1..08af4a689 100644 --- a/tools/vscode/src/providers/logview/logview-webview.ts +++ b/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 { 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 { 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 { const stateScript = state && state.log_file ? `` + stateMsg + )}` : ""; // decorate the html tag @@ -351,16 +369,11 @@ class InspectLogviewWebview extends InspectWebview { "\n", ` - ${stateScript} ${overrideCssHtml} From 10a2d535c8006182b7de75441e8c801c39aaafcf Mon Sep 17 00:00:00 2001 From: Charles Teague Date: Thu, 5 Sep 2024 11:57:09 -0400 Subject: [PATCH 2/3] Rename logViewAuto setting to openLogView This setting really now only controls whether the view is opened the first time upon task execution completion. --- tools/vscode/CHANGELOG.md | 1 + tools/vscode/package.json | 4 ++-- .../src/providers/logview/logview-file-watcher.ts | 2 +- .../src/providers/settings/inspect-settings.ts | 12 ++++++------ 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/tools/vscode/CHANGELOG.md b/tools/vscode/CHANGELOG.md index abd298f6c..3ce595daa 100644 --- a/tools/vscode/CHANGELOG.md +++ b/tools/vscode/CHANGELOG.md @@ -3,6 +3,7 @@ ## 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 diff --git a/tools/vscode/package.json b/tools/vscode/package.json index db2d11530..aeb4d9f4f 100644 --- a/tools/vscode/package.json +++ b/tools/vscode/package.json @@ -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", diff --git a/tools/vscode/src/providers/logview/logview-file-watcher.ts b/tools/vscode/src/providers/logview/logview-file-watcher.ts index 4abb4765a..b7025fe6d 100644 --- a/tools/vscode/src/providers/logview/logview-file-watcher.ts +++ b/tools/vscode/src/providers/logview/logview-file-watcher.ts @@ -59,7 +59,7 @@ export class LogViewFileWatcher implements Disposable { `New log: ${workspaceRelativePath(toAbsolutePath(evalLogPath))}` ); // Show the log file - const openAction = settingsMgr_.getSettings().logViewAuto + const openAction = settingsMgr_.getSettings().openLogView ? "open" : undefined; this.logviewManager_ diff --git a/tools/vscode/src/providers/settings/inspect-settings.ts b/tools/vscode/src/providers/settings/inspect-settings.ts index 87487d9fd..77eed3a46 100644 --- a/tools/vscode/src/providers/settings/inspect-settings.ts +++ b/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(kInspectConfigLogViewType) || "html"; - const logViewAuto = configuration.get(kInspectConfigLogViewAuto); + const openLogView = configuration.get(kInspectConfigOpenLogView); return { logViewType, - logViewAuto: logViewAuto !== undefined ? logViewAuto : true, + openLogView: openLogView !== undefined ? openLogView : true, }; } - + } \ No newline at end of file From 7a5af2dd0d83aaacc3f600763f875241460f9019 Mon Sep 17 00:00:00 2001 From: Charles Teague Date: Thu, 5 Sep 2024 11:59:17 -0400 Subject: [PATCH 3/3] Update the vscode version --- tools/vscode/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/vscode/package.json b/tools/vscode/package.json index aeb4d9f4f..643ea916d 100644 --- a/tools/vscode/package.json +++ b/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": {