diff --git a/tools/vscode/CHANGELOG.md b/tools/vscode/CHANGELOG.md index c14a82bb5..3ce595daa 100644 --- a/tools/vscode/CHANGELOG.md +++ b/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 diff --git a/tools/vscode/package.json b/tools/vscode/package.json index db2d11530..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": { @@ -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/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..b7025fe6d 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().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_); } } -} \ 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} 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