fix: preserve minimised warnings across session navigation

This commit is contained in:
Federico Jaramillo Martinez
2026-07-20 19:35:45 +02:00
parent 15c12aa1b8
commit a20a8c8c09
5 changed files with 92 additions and 30 deletions
+1 -1
View File
@@ -2,4 +2,4 @@
"@jmfederico/pi-web": patch "@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.
+4 -2
View File
@@ -22,6 +22,7 @@ import { SessionStorageTerminalSelectionMemory } from "../controllers/terminalSe
import { SessionStorageWorkspaceSelectionMemory } from "../controllers/workspaceSelection"; import { SessionStorageWorkspaceSelectionMemory } from "../controllers/workspaceSelection";
import { KeyboardShortcutDispatcher } from "../keyboardShortcuts"; import { KeyboardShortcutDispatcher } from "../keyboardShortcuts";
import { selectedMachineId } from "../controllers/types"; import { selectedMachineId } from "../controllers/types";
import { machineSessionKey } from "../machineKeys";
import { sessionCleanupRequestKey, sessionCleanupUnavailableMessage } from "../sessionCleanupUi"; import { sessionCleanupRequestKey, sessionCleanupUnavailableMessage } from "../sessionCleanupUi";
import { selectedNotificationView } from "../sessionNotifications"; import { selectedNotificationView } from "../sessionNotifications";
import { hasAuthoritativeSessionPersistence as runtimeHasAuthoritativeSessionPersistence } from "../sessionPersistence"; import { hasAuthoritativeSessionPersistence as runtimeHasAuthoritativeSessionPersistence } from "../sessionPersistence";
@@ -255,10 +256,11 @@ export class PiWebApp extends LitElement {
} }
private syncSessionWarningVisibility(): void { private syncSessionWarningVisibility(): void {
const session = this.state.selectedSession;
this.sessionWarningVisibility = reconcileSessionWarningVisibility( this.sessionWarningVisibility = reconcileSessionWarningVisibility(
this.sessionWarningVisibility, this.sessionWarningVisibility,
this.state.selectedSession?.id, session === undefined ? undefined : machineSessionKey(selectedMachineId(this.state), session.id),
this.state.status?.warnings, this.state.status === undefined ? undefined : this.state.status.warnings ?? [],
); );
} }
@@ -26,11 +26,26 @@ describe("PiWebApp session-warning visibility wiring", () => {
collapse(); collapse();
const collapsedChat = renderChatView(app, state); const collapsedChat = renderChatView(app, state);
const collapsedStatusBar = renderStatusBar(app, state);
expect(templateValueAfterMarker(collapsedChat, ".warningsVisible=")).toBe(false); 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(); restore();
expect(templateValueAfterMarker(renderChatView(app, state), ".warningsVisible=")).toBe(true); expect(templateValueAfterMarker(renderChatView(app, state), ".warningsVisible=")).toBe(true);
@@ -53,11 +68,11 @@ function createApp(): PiWebApp {
return new PiWebApp(); return new PiWebApp();
} }
function stateWithWarnings(): AppState { function stateWithWarnings(sessionId = "session-1"): AppState {
const selectedSession: SessionInfo = { const selectedSession: SessionInfo = {
id: "session-1", id: sessionId,
cwd: "/repo", cwd: "/repo",
path: "/repo/session-1.jsonl", path: `/repo/${sessionId}.jsonl`,
created: "2026-07-14T00:00:00.000Z", created: "2026-07-14T00:00:00.000Z",
modified: "2026-07-14T00:00:00.000Z", modified: "2026-07-14T00:00:00.000Z",
messageCount: 1, messageCount: 1,
@@ -66,13 +81,13 @@ function stateWithWarnings(): AppState {
return { return {
...initialAppState(), ...initialAppState(),
selectedSession, selectedSession,
status: warningStatus(), status: warningStatus(sessionId),
}; };
} }
function warningStatus(): SessionStatus { function warningStatus(sessionId: string): SessionStatus {
return { return {
sessionId: "session-1", sessionId,
isStreaming: true, isStreaming: true,
isCompacting: false, isCompacting: false,
isBashRunning: false, isBashRunning: false,
@@ -46,32 +46,53 @@ describe("session warning visibility transitions", () => {
expect(reconcileSessionWarningVisibility(collapsed, "session-1", replacement)).toBe(collapsed); 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 visible = reconcileSessionWarningVisibility(initialSessionWarningVisibilityState(), "session-1", warnings);
const collapsed = collapseSessionWarnings(visible); 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 changed = reconcileSessionWarningVisibility(collapsed, "session-1", [{ ...subscriptionWarning, message: "changed" }, skillWarning]);
const cleared = reconcileSessionWarningVisibility(collapsed, "session-1", []); const cleared = reconcileSessionWarningVisibility(collapsed, "session-1", []);
const returned = reconcileSessionWarningVisibility(cleared, "session-1", warnings); const returned = reconcileSessionWarningVisibility(cleared, "session-1", warnings);
expect(unavailable.collapsed).toBe(false);
expect(refreshed.collapsed).toBe(true);
expect(changed.collapsed).toBe(false); expect(changed.collapsed).toBe(false);
expect(cleared.collapsed).toBe(false); expect(cleared.collapsed).toBe(false);
expect(returned.collapsed).toBe(false); expect(returned.collapsed).toBe(false);
}); });
it("reopens when session selection changes even if the warning set is equal", () => { it("remembers collapse per session while navigating between unchanged warning sets", () => {
const visible = reconcileSessionWarningVisibility(initialSessionWarningVisibilityState(), "session-1", warnings); const firstVisible = reconcileSessionWarningVisibility(initialSessionWarningVisibilityState(), "session-1", warnings);
const collapsed = collapseSessionWarnings(visible); 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 empty = initialSessionWarningVisibilityState();
const selectedEmpty = reconcileSessionWarningVisibility(empty, "session-1", []);
const visible = reconcileSessionWarningVisibility(empty, "session-1", warnings); const visible = reconcileSessionWarningVisibility(empty, "session-1", warnings);
const collapsed = collapseSessionWarnings(visible); const collapsed = collapseSessionWarnings(visible);
const restored = restoreSessionWarnings(collapsed);
expect(collapseSessionWarnings(empty)).toBe(empty); expect(collapseSessionWarnings(empty)).toBe(empty);
expect(collapseSessionWarnings(selectedEmpty)).toBe(selectedEmpty);
expect(collapsed.collapsed).toBe(true); expect(collapsed.collapsed).toBe(true);
expect(restoreSessionWarnings(collapsed)).toEqual({ ...collapsed, collapsed: false }); expect(restored.collapsed).toBe(false);
expect(restored.collapsedWarningSets.size).toBe(0);
}); });
}); });
+35 -11
View File
@@ -1,18 +1,21 @@
import type { SessionWarning } from "./api"; import type { SessionWarning } from "./api";
export interface SessionWarningVisibilityState { export interface SessionWarningVisibilityState {
sessionId: string | undefined; selectedSessionKey: string | undefined;
warningSetSignature: string; warningSetSignature: string;
warningCount: number; warningCount: number;
collapsed: boolean; collapsed: boolean;
/** Warning-set signatures the user collapsed, keyed by machine/session identity. */
collapsedWarningSets: ReadonlyMap<string, string>;
} }
export function initialSessionWarningVisibilityState(): SessionWarningVisibilityState { export function initialSessionWarningVisibilityState(): SessionWarningVisibilityState {
return { return {
sessionId: undefined, selectedSessionKey: undefined,
warningSetSignature: sessionWarningSetSignature(undefined), warningSetSignature: sessionWarningSetSignature(undefined),
warningCount: 0, warningCount: 0,
collapsed: false, collapsed: false,
collapsedWarningSets: new Map(),
}; };
} }
@@ -30,28 +33,49 @@ export function sessionWarningSetSignature(warnings: readonly SessionWarning[] |
return JSON.stringify(warningIdentities); 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( export function reconcileSessionWarningVisibility(
current: SessionWarningVisibilityState, current: SessionWarningVisibilityState,
sessionId: string | undefined, sessionKey: string | undefined,
warnings: readonly SessionWarning[] | undefined, warnings: readonly SessionWarning[] | undefined,
): SessionWarningVisibilityState { ): SessionWarningVisibilityState {
const warningSetSignature = sessionWarningSetSignature(warnings); 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 { return {
sessionId, selectedSessionKey: sessionKey,
warningSetSignature, warningSetSignature,
warningCount: warnings?.length ?? 0, warningCount,
collapsed: false, collapsed,
collapsedWarningSets,
}; };
} }
export function collapseSessionWarnings(current: SessionWarningVisibilityState): SessionWarningVisibilityState { export function collapseSessionWarnings(current: SessionWarningVisibilityState): SessionWarningVisibilityState {
if (current.collapsed || current.warningCount === 0) return current; if (current.collapsed || current.warningCount === 0 || current.selectedSessionKey === undefined) return current;
return { ...current, collapsed: true }; const collapsedWarningSets = new Map(current.collapsedWarningSets);
collapsedWarningSets.set(current.selectedSessionKey, current.warningSetSignature);
return { ...current, collapsed: true, collapsedWarningSets };
} }
export function restoreSessionWarnings(current: SessionWarningVisibilityState): SessionWarningVisibilityState { export function restoreSessionWarnings(current: SessionWarningVisibilityState): SessionWarningVisibilityState {
if (!current.collapsed) return current; 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 };
} }