From 7014d7275276b94a5e56502acdebfcd954285919 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 21 Jul 2026 22:19:28 -0700 Subject: [PATCH] refactor(ui): unify sidebar row visibility (#112511) * refactor(ui): unify sidebar row visibility * test(ui): consolidate sidebar visibility coverage * style(ui): restore chat view line budget * chore(ui): drop superseded lint workaround --- ui/src/components/app-sidebar-session-list.ts | 21 ++- ui/src/components/app-sidebar-session-tree.ts | 3 + .../components/app-sidebar-session-types.ts | 22 +++ .../components/session-row-visibility.test.ts | 37 +++++ .../app-sidebar-cases/attention.ts | 139 +++++++---------- .../app-sidebar-cases/child-sessions-cap.ts | 141 +++++++----------- 6 files changed, 181 insertions(+), 182 deletions(-) create mode 100644 ui/src/components/session-row-visibility.test.ts diff --git a/ui/src/components/app-sidebar-session-list.ts b/ui/src/components/app-sidebar-session-list.ts index c7ae7b28cab7..f82d6a5f93be 100644 --- a/ui/src/components/app-sidebar-session-list.ts +++ b/ui/src/components/app-sidebar-session-list.ts @@ -19,8 +19,10 @@ import { AppSidebarSessionNarrationElement } from "./app-sidebar-session-narrati import { limitSidebarSessionRows, loadStoredSidebarCatalogGrouping, + rowDemandsVisibility, SIDEBAR_SESSION_PAGE_SIZE, SIDEBAR_SESSION_SEE_LESS_THRESHOLD, + RowVisibilityReason, sidebarSessionMetaId, storeSidebarCatalogGrouping, type SidebarRecentSession, @@ -258,20 +260,12 @@ export abstract class AppSidebarSessionListElement extends AppSidebarSessionNarr protected visibleSessionChildren(session: SidebarRecentSession): readonly SidebarRecentSession[] { const showAllChildren = this.fullyShownChildSessionKeys.has(session.key); - // The cap hides quiet children only: the active branch and any branch with - // live runs (runningChildCount is transitive) must stay visible, or an - // auto-expanded parent would omit its own selection or a running session. + // Active, running, and attention-bearing branches must bypass the quiet-child cap. return showAllChildren ? session.children : session.children.filter( (child, index) => - index < SIDEBAR_VISIBLE_CHILD_SESSION_LIMIT || - child.visuallyActive || - child.containsActiveDescendant || - child.hasActiveRun || - child.status === "running" || - child.runningChildCount > 0 || - child.attention.kind !== "none", + index < SIDEBAR_VISIBLE_CHILD_SESSION_LIMIT || rowDemandsVisibility(child), ); } @@ -350,9 +344,12 @@ export abstract class AppSidebarSessionListElement extends AppSidebarSessionNarr : "threads"; // Collapsed Coding still signals live runs so background work stays visible. const collapsedRunningDot = - collapsed && section.work && section.rows.some((row) => row.hasActiveRun); + collapsed && + section.work && + section.rows.some((row) => rowDemandsVisibility(row, RowVisibilityReason.ActiveRun)); const collapsedAttentionDot = - collapsed && section.rows.some((row) => row.attention.kind !== "none"); + collapsed && + section.rows.some((row) => rowDemandsVisibility(row, RowVisibilityReason.Attention)); const acceptsSessions = isPinned || (this.sessionsGrouping === "category" && (section.id === "ungrouped" || Boolean(group))); diff --git a/ui/src/components/app-sidebar-session-tree.ts b/ui/src/components/app-sidebar-session-tree.ts index f082d24b5d31..cd2ff818f909 100644 --- a/ui/src/components/app-sidebar-session-tree.ts +++ b/ui/src/components/app-sidebar-session-tree.ts @@ -2,6 +2,8 @@ import type { GatewaySessionRow } from "../api/types.ts"; import { areUiSessionKeysEquivalent } from "../lib/sessions/session-key.ts"; import { SIDEBAR_SESSION_NO_ATTENTION, + rowDemandsVisibility, + RowVisibilityReason, sidebarSessionAttentionPriority, type SidebarKnownSessionAttention, type SidebarRecentSession, @@ -112,6 +114,7 @@ export function projectSessionTree(params: { // ancestor remains actionable even when the blocked descendant is hidden. const attention = children.reduce( (current, child) => + rowDemandsVisibility(child, RowVisibilityReason.Attention) && sidebarSessionAttentionPriority(child.attention) > sidebarSessionAttentionPriority(current) ? child.attention : current, diff --git a/ui/src/components/app-sidebar-session-types.ts b/ui/src/components/app-sidebar-session-types.ts index 7666560540af..b827e0d8437e 100644 --- a/ui/src/components/app-sidebar-session-types.ts +++ b/ui/src/components/app-sidebar-session-types.ts @@ -99,6 +99,28 @@ export type SidebarRecentSession = { failedChildCount: number; }; +export const enum RowVisibilityReason { + Any, + ActiveRun, + Attention, +} + +export function rowDemandsVisibility( + row: SidebarRecentSession, + reason: RowVisibilityReason = RowVisibilityReason.Any, +) { + return reason === RowVisibilityReason.ActiveRun + ? row.hasActiveRun + : reason === RowVisibilityReason.Attention + ? row.attention.kind !== "none" + : row.visuallyActive || + row.containsActiveDescendant || + row.hasActiveRun || + row.status === "running" || + row.runningChildCount > 0 || + row.attention.kind !== "none"; +} + export type SidebarSessionMenuState = { session: SidebarRecentSession; x: number; diff --git a/ui/src/components/session-row-visibility.test.ts b/ui/src/components/session-row-visibility.test.ts new file mode 100644 index 000000000000..1c33bcdd7add --- /dev/null +++ b/ui/src/components/session-row-visibility.test.ts @@ -0,0 +1,37 @@ +import { expect, it } from "vitest"; +import { + rowDemandsVisibility, + RowVisibilityReason, + type SidebarRecentSession, +} from "./app-sidebar-session-types.ts"; + +const quietRow = { + visuallyActive: false, + containsActiveDescendant: false, + hasActiveRun: false, + runningChildCount: 0, + attention: { kind: "none" }, +} as SidebarRecentSession; + +const states = [ + ["quiet", {}, false, false, false], + ["visually active", { visuallyActive: true }, true, false, false], + ["active descendant", { containsActiveDescendant: true }, true, false, false], + ["active run", { hasActiveRun: true }, true, true, false], + ["running status", { status: "running" }, true, false, false], + ["running descendant", { runningChildCount: 1 }, true, false, false], + ["attention", { attention: { kind: "question" } }, true, false, true], +] as const; + +it.each(states)( + "keeps cap, collapsed-dot, and bubbling decisions aligned for %s", + (_name, patch, cap, runningDot, attention) => { + const row = { ...quietRow, ...patch }; + expect([ + rowDemandsVisibility(row), + rowDemandsVisibility(row, RowVisibilityReason.ActiveRun), + rowDemandsVisibility(row, RowVisibilityReason.Attention), + rowDemandsVisibility(row, RowVisibilityReason.Attention), + ]).toEqual([cap, runningDot, attention, attention]); + }, +); diff --git a/ui/src/test-helpers/app-sidebar-cases/attention.ts b/ui/src/test-helpers/app-sidebar-cases/attention.ts index 54e8c0dd8608..c0bb15151acd 100644 --- a/ui/src/test-helpers/app-sidebar-cases/attention.ts +++ b/ui/src/test-helpers/app-sidebar-cases/attention.ts @@ -242,92 +242,61 @@ describe("AppSidebar session attention", () => { expect(section?.querySelector(".sidebar-recent-session")).toBeNull(); }); - it("bubbles an unloaded child's pending question to its parent and collapsed section", async () => { + it("bubbles unloaded child attention to its parent and collapsed section", async () => { const parentKey = "agent:main:parent"; - const childKey = "agent:main:subagent:question"; - const client = { - request: vi.fn().mockResolvedValue({ questions: [] }), - } as unknown as GatewayBrowserClient; - const gatewayHarness = createGatewayHarness(client); - const sessionsHarness = createSessionsHarness("main", [parentKey]); - setRows(sessionsHarness, [ - { - key: parentKey, - kind: "direct", - label: "Parent task", - updatedAt: 1, - childSessions: [childKey], - }, - ]); - const { sidebar } = await mountSidebar(gatewayHarness.gateway, sessionsHarness.sessions); - - gatewayHarness.publishEvent("question.requested", { - id: "question-child", - agentId: "main", - sessionKey: childKey, - questions: [{ questionId: "confirm", header: "Confirm", question: "Continue?", options: [] }], - createdAtMs: Date.now(), - expiresAtMs: Date.now() + 60_000, - status: "pending", - }); - await sidebar.updateComplete; - - expect( - sidebar.querySelector( - `[data-session-key="${parentKey}"] [data-session-attention="question"]`, - ), - ).not.toBeNull(); - expect(sidebar.querySelector(`[data-session-key="${childKey}"]`)).toBeNull(); - sidebar.querySelector(".sidebar-session-group-toggle")?.click(); - await sidebar.updateComplete; - expect( - sidebar - .querySelector('[data-session-section="ungrouped"]') - ?.querySelector(".sidebar-session-group-attention"), - ).not.toBeNull(); - }); - - it("bubbles an unloaded child's pending approval to its parent and collapsed section", async () => { - const parentKey = "agent:main:parent"; - const childKey = "agent:main:subagent:approval"; - const approval = { - id: "approval-child", - kind: "exec", - request: { command: "git status", sessionKey: childKey }, - createdAtMs: Date.now(), - expiresAtMs: Date.now() + 60_000, - } satisfies ExecApprovalRequest; - const sessionsHarness = createSessionsHarness("main", [parentKey]); - setRows(sessionsHarness, [ - { - key: parentKey, - kind: "direct", - label: "Parent task", - updatedAt: 1, - childSessions: [childKey], - }, - ]); - const { sidebar } = await mountSidebar( - createGateway({} as GatewayBrowserClient), - sessionsHarness.sessions, - "panel", - null, - [approval], - ); - - expect( - sidebar.querySelector( - `[data-session-key="${parentKey}"] [data-session-attention="approval"]`, - ), - ).not.toBeNull(); - expect(sidebar.querySelector(`[data-session-key="${childKey}"]`)).toBeNull(); - sidebar.querySelector(".sidebar-session-group-toggle")?.click(); - await sidebar.updateComplete; - expect( - sidebar - .querySelector('[data-session-section="ungrouped"]') - ?.querySelector(".sidebar-session-group-attention"), - ).not.toBeNull(); + for (const kind of ["question", "approval"] as const) { + localStorage.setItem("openclaw:sidebar:sessions:collapsed-sections", "[]"); + const childKey = `agent:main:subagent:${kind}`; + const gatewayHarness = createGatewayHarness({ + request: vi.fn().mockResolvedValue({ questions: [] }), + } as unknown as GatewayBrowserClient); + const sessionsHarness = createSessionsHarness("main", [parentKey]); + setRows(sessionsHarness, [ + { key: parentKey, kind: "direct", updatedAt: 1, childSessions: [childKey] }, + ]); + const approval = { + id: "approval-child", + kind: "exec", + request: { command: "git status", sessionKey: childKey }, + createdAtMs: Date.now(), + expiresAtMs: Date.now() + 60_000, + } satisfies ExecApprovalRequest; + const { sidebar } = await mountSidebar( + gatewayHarness.gateway, + sessionsHarness.sessions, + "panel", + null, + kind === "approval" ? [approval] : [], + ); + if (kind === "question") { + gatewayHarness.publishEvent("question.requested", { + id: "question-child", + agentId: "main", + sessionKey: childKey, + questions: [ + { questionId: "confirm", header: "Confirm", question: "Continue?", options: [] }, + ], + createdAtMs: Date.now(), + expiresAtMs: Date.now() + 60_000, + status: "pending", + }); + await sidebar.updateComplete; + } + expect( + sidebar.querySelector( + `[data-session-key="${parentKey}"] [data-session-attention="${kind}"]`, + ), + ).not.toBeNull(); + expect(sidebar.querySelector(`[data-session-key="${childKey}"]`)).toBeNull(); + sidebar.querySelector(".sidebar-session-group-toggle")?.click(); + await sidebar.updateComplete; + expect( + sidebar + .querySelector('[data-session-section="ungrouped"]') + ?.querySelector(".sidebar-session-group-attention"), + ).not.toBeNull(); + sidebar.remove(); + } }); it("bubbles descendant attention and keeps its branch visible past the child cap", async () => { diff --git a/ui/src/test-helpers/app-sidebar-cases/child-sessions-cap.ts b/ui/src/test-helpers/app-sidebar-cases/child-sessions-cap.ts index 362fb14093d9..17fb73be1bf6 100644 --- a/ui/src/test-helpers/app-sidebar-cases/child-sessions-cap.ts +++ b/ui/src/test-helpers/app-sidebar-cases/child-sessions-cap.ts @@ -1,49 +1,59 @@ import { describe, expect, it } from "vitest"; import type { GatewayBrowserClient } from "../../api/gateway.ts"; +import type { GatewaySessionRow } from "../../api/types.ts"; import { createGateway, createSessionsHarness, mountSidebar } from "../app-sidebar.ts"; import { waitForFast } from "../wait-for.ts"; import "../../components/app-sidebar.ts"; -describe("AppSidebar child session cap", () => { - it("caps visible children until requested and resets the cap after collapse", async () => { - const gateway = createGateway({} as GatewayBrowserClient); - const childKeys = Array.from( - { length: 6 }, - (_, index) => `agent:main:subagent:child-${index + 1}`, - ); - const harness = createSessionsHarness("main", ["agent:main:parent"]); - harness.list.mockResolvedValue({ - ts: 100_000, - path: "", - count: childKeys.length, - defaults: { modelProvider: null, model: null, contextTokens: null }, - sessions: childKeys.map((key, index) => ({ +const parentKey = "agent:main:parent"; +const childKeys = Array.from({ length: 6 }, (_, index) => `agent:main:subagent:child-${index + 1}`); + +async function mountChildSessions(extraRows: GatewaySessionRow[] = []) { + const harness = createSessionsHarness("main", [parentKey]); + harness.list.mockResolvedValue({ + ts: 100_000, + path: "", + count: childKeys.length, + defaults: { modelProvider: null, model: null, contextTokens: null }, + sessions: [ + ...childKeys.map((key, index) => ({ key, - spawnedBy: "agent:main:parent", + spawnedBy: parentKey, kind: "direct" as const, label: `Subagent: Child ${index + 1}`, updatedAt: index + 1, })), - }); - const { sidebar } = await mountSidebar(gateway, harness.sessions); - harness.publishList({ - result: { - ts: 2, - path: "", - count: 1, - defaults: { modelProvider: null, model: null, contextTokens: null }, - sessions: [ - { - key: "agent:main:parent", - kind: "direct", - label: "Parent task", - updatedAt: 1, - childSessions: childKeys, - }, - ], - }, - }); - await sidebar.updateComplete; + ...extraRows, + ], + }); + const { sidebar } = await mountSidebar( + createGateway({} as GatewayBrowserClient), + harness.sessions, + ); + harness.publishList({ + result: { + ts: 2, + path: "", + count: 1, + defaults: { modelProvider: null, model: null, contextTokens: null }, + sessions: [ + { + key: parentKey, + kind: "direct", + label: "Parent task", + updatedAt: 1, + childSessions: childKeys, + }, + ], + }, + }); + await sidebar.updateComplete; + return sidebar; +} + +describe("AppSidebar child session cap", () => { + it("caps visible children until requested and resets the cap after collapse", async () => { + const sidebar = await mountChildSessions(); const toggle = sidebar.querySelector("[data-child-session-toggle]"); toggle?.click(); @@ -73,57 +83,18 @@ describe("AppSidebar child session cap", () => { }); it("keeps live children visible past the cap", async () => { - const gateway = createGateway({} as GatewayBrowserClient); - const childKeys = Array.from( - { length: 6 }, - (_, index) => `agent:main:subagent:child-${index + 1}`, - ); - const harness = createSessionsHarness("main", ["agent:main:parent"]); - harness.list.mockResolvedValue({ - ts: 100_000, - path: "", - count: childKeys.length, - defaults: { modelProvider: null, model: null, contextTokens: null }, - sessions: [ - ...childKeys.map((key, index) => ({ - key, - spawnedBy: "agent:main:parent", - kind: "direct" as const, - label: `Subagent: Child ${index + 1}`, - updatedAt: index + 1, - })), - // Quiet child beyond the cap with a RUNNING grandchild: the branch - // must bypass the cap via the transitive runningChildCount. - { - key: "agent:main:subagent:grandchild", - spawnedBy: "agent:main:subagent:child-6", - kind: "direct" as const, - label: "Subagent: Grandchild run", - updatedAt: 10, - status: "running" as const, - hasActiveRun: true, - }, - ], - }); - const { sidebar } = await mountSidebar(gateway, harness.sessions); - harness.publishList({ - result: { - ts: 2, - path: "", - count: 1, - defaults: { modelProvider: null, model: null, contextTokens: null }, - sessions: [ - { - key: "agent:main:parent", - kind: "direct", - label: "Parent task", - updatedAt: 1, - childSessions: childKeys, - }, - ], + // Quiet child beyond the cap with a running grandchild must bypass it. + const sidebar = await mountChildSessions([ + { + key: "agent:main:subagent:grandchild", + spawnedBy: childKeys[5], + kind: "direct", + label: "Subagent: Grandchild run", + updatedAt: 10, + status: "running", + hasActiveRun: true, }, - }); - await sidebar.updateComplete; + ]); sidebar .querySelector('[data-child-session-toggle="agent:main:parent"]')