diff --git a/.changeset/quiet-session-warnings.md b/.changeset/quiet-session-warnings.md index f64ae07..434e7a2 100644 --- a/.changeset/quiet-session-warnings.md +++ b/.changeset/quiet-session-warnings.md @@ -2,4 +2,4 @@ "@jmfederico/pi-web": patch --- -Let users minimise session warnings into an accessible status-bar count, and replace warning emoji with SVG icons. +Let users minimise session warnings into an accessible status-bar count that stays minimised when they revisit the session, and replace warning emoji with SVG icons. diff --git a/src/client/src/components/PiWebApp.ts b/src/client/src/components/PiWebApp.ts index f1e11d9..97a199e 100644 --- a/src/client/src/components/PiWebApp.ts +++ b/src/client/src/components/PiWebApp.ts @@ -22,6 +22,7 @@ import { SessionStorageTerminalSelectionMemory } from "../controllers/terminalSe import { SessionStorageWorkspaceSelectionMemory } from "../controllers/workspaceSelection"; import { KeyboardShortcutDispatcher } from "../keyboardShortcuts"; import { selectedMachineId } from "../controllers/types"; +import { machineSessionKey } from "../machineKeys"; import { sessionCleanupRequestKey, sessionCleanupUnavailableMessage } from "../sessionCleanupUi"; import { selectedNotificationView } from "../sessionNotifications"; import { hasAuthoritativeSessionPersistence as runtimeHasAuthoritativeSessionPersistence } from "../sessionPersistence"; @@ -255,10 +256,11 @@ export class PiWebApp extends LitElement { } private syncSessionWarningVisibility(): void { + const session = this.state.selectedSession; this.sessionWarningVisibility = reconcileSessionWarningVisibility( this.sessionWarningVisibility, - this.state.selectedSession?.id, - this.state.status?.warnings, + session === undefined ? undefined : machineSessionKey(selectedMachineId(this.state), session.id), + this.state.status === undefined ? undefined : this.state.status.warnings ?? [], ); } diff --git a/src/client/src/components/PiWebApp.warningVisibility.test.ts b/src/client/src/components/PiWebApp.warningVisibility.test.ts index 7670872..b10d119 100644 --- a/src/client/src/components/PiWebApp.warningVisibility.test.ts +++ b/src/client/src/components/PiWebApp.warningVisibility.test.ts @@ -26,11 +26,26 @@ describe("PiWebApp session-warning visibility wiring", () => { collapse(); const collapsedChat = renderChatView(app, state); - const collapsedStatusBar = renderStatusBar(app, state); expect(templateValueAfterMarker(collapsedChat, ".warningsVisible=")).toBe(false); - expect(templateValueAfterMarker(collapsedStatusBar, ".collapsedWarningCount=")).toBe(2); + expect(templateValueAfterMarker(renderStatusBar(app, state), ".collapsedWarningCount=")).toBe(2); - const restore = templateCallbackAfterMarker(collapsedStatusBar, ".onRestoreWarnings="); + const otherState = stateWithWarnings("session-2"); + setAppState(app, otherState); + syncWarningVisibility(app); + expect(templateValueAfterMarker(renderChatView(app, otherState), ".warningsVisible=")).toBe(true); + + const returningState = { ...state, status: undefined }; + setAppState(app, returningState); + syncWarningVisibility(app); + expect(templateValueAfterMarker(renderChatView(app, returningState), ".warningsVisible=")).toBe(true); + + setAppState(app, state); + syncWarningVisibility(app); + const returnedStatusBar = renderStatusBar(app, state); + expect(templateValueAfterMarker(renderChatView(app, state), ".warningsVisible=")).toBe(false); + expect(templateValueAfterMarker(returnedStatusBar, ".collapsedWarningCount=")).toBe(2); + + const restore = templateCallbackAfterMarker(returnedStatusBar, ".onRestoreWarnings="); restore(); expect(templateValueAfterMarker(renderChatView(app, state), ".warningsVisible=")).toBe(true); @@ -53,11 +68,11 @@ function createApp(): PiWebApp { return new PiWebApp(); } -function stateWithWarnings(): AppState { +function stateWithWarnings(sessionId = "session-1"): AppState { const selectedSession: SessionInfo = { - id: "session-1", + id: sessionId, cwd: "/repo", - path: "/repo/session-1.jsonl", + path: `/repo/${sessionId}.jsonl`, created: "2026-07-14T00:00:00.000Z", modified: "2026-07-14T00:00:00.000Z", messageCount: 1, @@ -66,13 +81,13 @@ function stateWithWarnings(): AppState { return { ...initialAppState(), selectedSession, - status: warningStatus(), + status: warningStatus(sessionId), }; } -function warningStatus(): SessionStatus { +function warningStatus(sessionId: string): SessionStatus { return { - sessionId: "session-1", + sessionId, isStreaming: true, isCompacting: false, isBashRunning: false, diff --git a/src/client/src/sessionWarningVisibility.test.ts b/src/client/src/sessionWarningVisibility.test.ts index 8673bcc..5c55101 100644 --- a/src/client/src/sessionWarningVisibility.test.ts +++ b/src/client/src/sessionWarningVisibility.test.ts @@ -46,32 +46,53 @@ describe("session warning visibility transitions", () => { expect(reconcileSessionWarningVisibility(collapsed, "session-1", replacement)).toBe(collapsed); }); - it("reopens for changed warnings and when the same warnings return after clearing", () => { + it("retains collapse while status is unavailable but reopens after a known warning change", () => { const visible = reconcileSessionWarningVisibility(initialSessionWarningVisibilityState(), "session-1", warnings); const collapsed = collapseSessionWarnings(visible); + const unavailable = reconcileSessionWarningVisibility(collapsed, "session-1", undefined); + const refreshed = reconcileSessionWarningVisibility(unavailable, "session-1", warnings); const changed = reconcileSessionWarningVisibility(collapsed, "session-1", [{ ...subscriptionWarning, message: "changed" }, skillWarning]); const cleared = reconcileSessionWarningVisibility(collapsed, "session-1", []); const returned = reconcileSessionWarningVisibility(cleared, "session-1", warnings); + expect(unavailable.collapsed).toBe(false); + expect(refreshed.collapsed).toBe(true); expect(changed.collapsed).toBe(false); expect(cleared.collapsed).toBe(false); expect(returned.collapsed).toBe(false); }); - it("reopens when session selection changes even if the warning set is equal", () => { - const visible = reconcileSessionWarningVisibility(initialSessionWarningVisibilityState(), "session-1", warnings); - const collapsed = collapseSessionWarnings(visible); + it("remembers collapse per session while navigating between unchanged warning sets", () => { + const firstVisible = reconcileSessionWarningVisibility(initialSessionWarningVisibilityState(), "session-1", warnings); + const firstCollapsed = collapseSessionWarnings(firstVisible); + const secondVisible = reconcileSessionWarningVisibility(firstCollapsed, "session-2", warnings); + const secondCollapsed = collapseSessionWarnings(secondVisible); + const returnedToFirst = reconcileSessionWarningVisibility(secondCollapsed, "session-1", warnings.map((warning) => ({ ...warning }))); - expect(reconcileSessionWarningVisibility(collapsed, "session-2", warnings).collapsed).toBe(false); + expect(secondVisible.collapsed).toBe(false); + expect(returnedToFirst.collapsed).toBe(true); + expect(reconcileSessionWarningVisibility(returnedToFirst, "session-2", warnings).collapsed).toBe(true); }); - it("only collapses a non-empty warning set and restores it explicitly", () => { + it("keeps an explicitly restored warning set visible after navigating away and back", () => { + const visible = reconcileSessionWarningVisibility(initialSessionWarningVisibilityState(), "session-1", warnings); + const restored = restoreSessionWarnings(collapseSessionWarnings(visible)); + const away = reconcileSessionWarningVisibility(restored, "session-2", warnings); + + expect(reconcileSessionWarningVisibility(away, "session-1", warnings).collapsed).toBe(false); + }); + + it("only collapses a selected, non-empty warning set and restores it explicitly", () => { const empty = initialSessionWarningVisibilityState(); + const selectedEmpty = reconcileSessionWarningVisibility(empty, "session-1", []); const visible = reconcileSessionWarningVisibility(empty, "session-1", warnings); const collapsed = collapseSessionWarnings(visible); + const restored = restoreSessionWarnings(collapsed); expect(collapseSessionWarnings(empty)).toBe(empty); + expect(collapseSessionWarnings(selectedEmpty)).toBe(selectedEmpty); expect(collapsed.collapsed).toBe(true); - expect(restoreSessionWarnings(collapsed)).toEqual({ ...collapsed, collapsed: false }); + expect(restored.collapsed).toBe(false); + expect(restored.collapsedWarningSets.size).toBe(0); }); }); diff --git a/src/client/src/sessionWarningVisibility.ts b/src/client/src/sessionWarningVisibility.ts index c31a097..d550860 100644 --- a/src/client/src/sessionWarningVisibility.ts +++ b/src/client/src/sessionWarningVisibility.ts @@ -1,18 +1,21 @@ import type { SessionWarning } from "./api"; export interface SessionWarningVisibilityState { - sessionId: string | undefined; + selectedSessionKey: string | undefined; warningSetSignature: string; warningCount: number; collapsed: boolean; + /** Warning-set signatures the user collapsed, keyed by machine/session identity. */ + collapsedWarningSets: ReadonlyMap; } export function initialSessionWarningVisibilityState(): SessionWarningVisibilityState { return { - sessionId: undefined, + selectedSessionKey: undefined, warningSetSignature: sessionWarningSetSignature(undefined), warningCount: 0, collapsed: false, + collapsedWarningSets: new Map(), }; } @@ -30,28 +33,49 @@ export function sessionWarningSetSignature(warnings: readonly SessionWarning[] | return JSON.stringify(warningIdentities); } -/** Preserve collapse only while both the selected session and warning set are unchanged. */ +/** Preserve collapse per session while reopening whenever that session's warning set changes. */ export function reconcileSessionWarningVisibility( current: SessionWarningVisibilityState, - sessionId: string | undefined, + sessionKey: string | undefined, warnings: readonly SessionWarning[] | undefined, ): SessionWarningVisibilityState { const warningSetSignature = sessionWarningSetSignature(warnings); - if (current.sessionId === sessionId && current.warningSetSignature === warningSetSignature) return current; + const warningCount = warnings?.length ?? 0; + const warningSetKnown = warnings !== undefined; + const collapsedWarningSet = sessionKey === undefined ? undefined : current.collapsedWarningSets.get(sessionKey); + let collapsedWarningSets = current.collapsedWarningSets; + if (warningSetKnown && sessionKey !== undefined && collapsedWarningSet !== undefined && collapsedWarningSet !== warningSetSignature) { + const nextCollapsedWarningSets = new Map(current.collapsedWarningSets); + nextCollapsedWarningSets.delete(sessionKey); + collapsedWarningSets = nextCollapsedWarningSets; + } + const collapsed = warningSetKnown && warningCount > 0 && collapsedWarningSet === warningSetSignature; + if ( + current.selectedSessionKey === sessionKey + && current.warningSetSignature === warningSetSignature + && current.warningCount === warningCount + && current.collapsed === collapsed + && current.collapsedWarningSets === collapsedWarningSets + ) return current; return { - sessionId, + selectedSessionKey: sessionKey, warningSetSignature, - warningCount: warnings?.length ?? 0, - collapsed: false, + warningCount, + collapsed, + collapsedWarningSets, }; } export function collapseSessionWarnings(current: SessionWarningVisibilityState): SessionWarningVisibilityState { - if (current.collapsed || current.warningCount === 0) return current; - return { ...current, collapsed: true }; + if (current.collapsed || current.warningCount === 0 || current.selectedSessionKey === undefined) return current; + const collapsedWarningSets = new Map(current.collapsedWarningSets); + collapsedWarningSets.set(current.selectedSessionKey, current.warningSetSignature); + return { ...current, collapsed: true, collapsedWarningSets }; } export function restoreSessionWarnings(current: SessionWarningVisibilityState): SessionWarningVisibilityState { if (!current.collapsed) return current; - return { ...current, collapsed: false }; + const collapsedWarningSets = new Map(current.collapsedWarningSets); + if (current.selectedSessionKey !== undefined) collapsedWarningSets.delete(current.selectedSessionKey); + return { ...current, collapsed: false, collapsedWarningSets }; }